Weird math issue? Maybe timing? Tachometer coding.

I'm working on a tachometer for a lathe. I'm using an IR sensor reading a single reflective patch on the wheel. I originally saw lots of examples using an ISR to increment a counter and then taking the number of counts in a given time, generally 1 second, and using that to determine RPM. I found out quickly the limitations of using this method.

The other method I've seen is to count the time between pulses using microseconds and calculate the RPM from that. I tried this and it works but I'm having a weird issue. Here is the code:

int rpm = 0;
int rpmDigits; //number of digits in the RPM value
unsigned long rpmArray[2] = {0,0};
byte rpmindex = 0;
unsigned long interval = 0;


void setup(){
  Serial.begin(9600);
  tft.begin(); 
  attachInterrupt(digitalPinToInterrupt(21), isr, RISING); //Interrupts are called on Rise of Input
  tftPrintBackground();
  tftPrintRectangles();
}

 
void loop(){
  delay(5);
  detachInterrupt(0); //Interrupts are disabled
  //unsigned long tempArray[2]; 
  //memcpy(tempArray, rpmArray, sizeof tempArray); 
  attachInterrupt(digitalPinToInterrupt(21), isr, RISING);   //Restart the interrupt processing
  interval = abs(rpmArray[0] - rpmArray[1]);  
  rpm = (60*1000000)/interval;
  //if (rpm > 0){
    Serial.print("start=");
    Serial.print(rpmArray[0]);
    Serial.print(" end=");
    Serial.print(rpmArray[1]);
    Serial.print(" interval=");
    Serial.print(interval);
    Serial.print(" rpm=");
    Serial.println(rpm);
    tftPrintSpindleRPM(rpm);
  //}


// int x= rpm;                                //  CALCULATE NUMBER OF DIGITS IN RPM
//    while(x!=0) {
//      x = x/10;
//      rpmDigits++;
//    }   
}

void isr(){
  rpmindex ^= 1;
  rpmArray[rpmindex] = micros();
}

When I run this I'm getting odd output to the serial monitor. Most lines look as they should but I get a bunch that say...

start=11756100 end=11909416 interval=4294813980 rpm=0

If you do the math the difference between 11756100 and 11909416 is 153316, not 4294813980. Confused on why the math is wrong.

You’re subtracting a larger number from a smaller, so you’ll get a negative result. But you’re using an unsigned type, so it gets converted into the equivalent of the negative number which is apparently 4294813980.

Check which number is bigger and then do the math appropriately.

Your math is incorrect, as is the use of abs() here:

 interval = abs(rpmArray[0] - rpmArray[1]);

The following does not disable interrupts, and is not necessary.

 detachInterrupt(0); //Interrupts are disabled

Variables shared with interrupt routines must be declared volatile, and access to them must be protected from corruption by making a copy with the interrupts turned off, as follows. Later, use the copy.

noInterrupts();
long rpm_copy=rpmArray[rpmindex];
interrupts();

Appreciate you showing the proper way to start and stop interrupts. I'll try that and try making a copy of the array.

I don't however understand what you mean by the math is incorrect and it's not the right use of abs. Would seem the very spot to use it. Again, if I look at the serial output it's 20 lines that look correct with correct numbers and then an oddball line where the RPM shows zero and I get the output I posted above.

How is the math incorrect? During the ISR the code is storing the time alternately between array position zero and 1 each time it executes. So there's no way to know if the value in position 0 will be higher or lower than position one. Thus, using abs to get the absolute value.

wildbill:
You're subtracting a larger number from a smaller, so you'll get a negative result. But you're using an unsigned type, so it gets converted into the equivalent of the negative number which is apparently 4294813980.

Check which number is bigger and then do the math appropriately.

Ahhhh.. I think I see what you mean. I wasn't able to post all of the serial output because I only have a screen shot of it. I went back and looked however and see the screwy result only happens when array position zero contains a smaller number than array position 1. Seems like the easy solution is to define interval as a long instead of an unsigned long.

travisr100:
Seems like the easy solution is to define interval as a long instead of an unsigned long.

Then half the time your times will be negative. I think the correct solution is to do the math in 'unsigned long" (as you are doing) and casting to 'long' before taking the absolute value:

interval = abs((long)(rpmArray[0] - rpmArray[1]));

Note: If I recall correctly, the 'abs()' function works in 'float' so you are doing conversions from long to float and float to unsigned long.
The way I handle it is to have the ISR do the subtraction since it knows that the new time is always greater than the old time:

void isr(){
  static unsigned long prteviousMicros = 0;
  unsigned long currentMicros = micros();
  interval = currentMicros - previousMicros;
  previousMicros = currentMicros;
}

Don't forget to change the declaration to "volatile unsigned long interval;".