Just as a heads up, you are accessing variables in your ISR and main loop without any regard for their volatile nature. Yes, you have them declared volatile, but you have a race condition. Technically, the 'pulseTimex' variables can change midway through their use in the loop. This is especially true since these are long values and not likely to be anywhere near atomic operations.
I would suggest you create a new variable which signals the ISR has run. In the main loop, test if the variable indicates the ISR has fired. If so, do the calculations there and then. That way, your ISR only looks something like:
volatile bool fired = false ;
volatile unsigned long errorCount = 0UL ;
.
.
.
void halltrigger()
{
HallTimeOld = HallTimeNew;
HallTimeNew = micros();
if( !fired )
{
fired = true ;
} else {
errorCount++ ;
}
}
Then the loop can have the following, whereby 'pulseTimex' no longer needs to be volatile.
void loop()
{
.
.
.
if( fired )
{
pulseTime4 = pulseTime3; //keeping track of last 4 signal times
pulseTime3 = pulseTime2;
pulseTime2 = pulseTime1;
// Make sure we don't have a race condition - disable interrupts while
// accessing our shared variables.
cli() ;
pulseTime1 = HallTimeNew - HallTimeOld;
fired = false ;
sei() ;
}
.
.
.
}
Also, since you're CPU constrained right now, you're obviously going to need some serious optimization. For starters, add to your loop:
void loop()
{
while( true )
{
// Place your existing loop here
}
}
As for the pulseTime value shifts, you might consider using an array and using memcpy. I've been meaning to test performance to see which is faster. As such, I'm not sure it will be faster but it would be worth a try. The code would look something like this:
volatile unsigned long pulseTime[4] ;
.
.
.
if( fired )
{
memcpy( &pulseTime[0], &pulseTime[1], 3*sizeof(unsigned long) ) ;
// Make sure we don't have a race condition - disable interrupts while
.
.
.
}
Now that you have an 'errorCount' variable, wouldn't hurt to periodically test to see if any errors have occurred. If you're seeing a lot of them, meaning past some threshold, to shut things down because time is off and undefined. Having any type of delay in your main loop should give you pause. But at least you can now detect some timing errors.
Lastly, as for a signal generator, would this be a good use for something like a 555 tied to a pot?
Edits: Fixed potential race condition. Optimized ISR. Realized much later I used memcpy to shift the wrong direction. Fixed.