Odd issue with 32 bit math

Hi all

I have a strange problem which should be really simple to solve yet I can't see it!

Very briefly, I'm measuring the pulse width and period of a signal by recording the transitions in an interrupt routine; on a rising or falling edge, the value of micros() is stored to work out the duty cycle.

Below is by interrupt routine:

void sensor_interrupt(void)
{
  if ((PIND & 0b00001000) == 0b00001000) // If pin is high
  {
    if (pulseStarted == false)   // If we're starting a new pulse...
    {
      pulseStart = micros();
      pulseStarted = true;
    }      
    else
    {
      pulseEnd = micros();   // Otherwise this is the end of the pulse
      pulseStarted = false;
    }
  }
  else
    pulsePeriod = micros();
}

And here I am printing out the value of micros() at the various points:

    Serial.print(pulseStart);
    Serial.print(",");
    Serial.print(pulseEnd);
    Serial.print(",");
    Serial.print(pulsePeriod);
    Serial.print(",");
    Serial.println(pulseEnd - pulseStart);

What is really strange (to me) is that the 'pulseEnd - pulseStart' calculation usually gives the right answer, but sometimes doesn't! Here's the serial output:

15057568,15058564,15058500,996
16058808,16059800,16059740,992
17060044,17061040,17060976,4294966300
18061284,18062276,18063208,4294966300
19062520,19063516,19064448,996
20063756,20064752,20065684,992

Lines 1 and 2 are correct, 3 and 4 are not and 5 and 6 are!

Since I'm dealing with large numbers I assume there's a math issue somewhere (all variables are unsigned longs) but I can't for the life of me see it!

Because an interrupt occurs mid calculation, or mid read of pulseEnd or pulseStart.
Most Arduinos are 8 bit processors. They do 32-bit maths 8 bits at a time.
What do you think happens if you do “half” of a calculation with one value and the other “half” after an interrupt has updated the new value?

noInterrupts();
uint32_t copyPulseStart = pulseStart;
uint32_t copyPulseEnd = pulseEnd;
uint32_t copyPulsePeriod = pulsePeriod;
interrupts();

Now do all your display and calculations in main with the copies.

Edit: Also, if you output your readings AFTER you capture pulseStart but BEFORE you capture pulseEnd and/or pulsePeriod you also have a problem.

But as you don’t say when “various points” is, or provide us with a complete sketch, we can’t say if that is an issue or not.

Best practice is....
a) Do simple calculations (like add and subtract) in the interrupt routine or at the very least maintain a consistent set of variables which are the last complete set of results. i.e. capture pulseStart, pulseEnd, pulsePeriod in the interrupt and when you have ALL THREE copy them to some shared output variables.
b) Never read from shared variables in loop that are updated in an interrupt, or vice versa, without disabling interrupts in loop first. Remember to always reenable them afterwards. NEVER disable or enable interrupts from interrupt code.
c) Make sure mainline code takes copies of all variables it needs, and doesn’t perhaps allow an interrupt to occur between reading or writing related variables such that you have an inconsistent set of variables.
d) Mark shared variables between main and interrupt routines with the “volatile” keyword.

Many thanks pcbbc - you're quite right. I'd not thought about how often the interrupt occurs (quite often) and it didn't occur to me that the values might be changing during the calculation.

Appreciate the help!