Variable updated in ISR is miscounting

Question

I'm using a timer to fire a interrupt in configurable time periods. In that ISR, a pin is toggled, and a uint32_t variable is incremented.

In the main program, a while loop waits until that variable has reached a certain threshold - to then disable the timer via its clock source selection.

while(stepcounter <= threshold) {
}
disablecounter();

Sometimes - depending on optimization level, whether I put a small delay in the while loop, etc., this check seems to fail - advancing to disablecounter() even though stepcounter has not reached threshold.

If I stack the while loop like this:

while(stepcounter <= threshold) {
}
while(stepcounter <= threshold) {
}
disablecounter();

it runs just fine. This also works:

while(stepcounter <= threshold) {
  while(stepcounter <= threshold) {
  }
}
disablecounter();

I'm assuming there's some odd behaviour when comparing rapidly changing 32-bit numbers against a threshold. This issue also occurs when using a uint16_t for stepcounter, but after a higher number of cycles.

For analysis, I'm simply running this loop over and over again; setting stepcounter back to 0 and restarting the timer. With uint32_t, it usually fails after the 5th or so loop. For uint16_t, it fails after around 25 loops.

Using a tempstepcounter like this also seems to work, but only when comparing against both counters at the same time:

while(tempstepcounter <= threshold || stepcounter <= threshold) {
  tempstepcounter = stepcounter;
}
disablecounter();

Any ideas on this? It this because 32-bit numbers take longer to read from the registers than the time from ISR to ISR? ISR frequency should be around 32 kHz during this.

ISR code:

ISR(TIMER1_COMPA_vect) {
  PORTF ^= (1 << PORTF0));
  stepcounter++;
}  

Answer 1

You need to declare it volatile in both places and shut off interrupts while reading it. The mainline has to read it byte-by-byte, and an interrupt could otherwise occur between any two byte-reads, rendering the result of the 4-byte read as garbage.

'volatile' informs the compiler that the variable might not contain what it did last time (which the compiler might otherwise expect if the code being compiled doesn't change it. Turning off the interrupts protects the variable being changed while it's being read.

An even better way to protect the reads than turning off interrupts, is for the ISR to update the counter, then set a single-byte flag to indicate that there's a new count. The mainline would monitor that flag and read the counter only when the flag gets set, then clear the flag. This can work as long as the mainline checks the flag frequently enough that it will never not notice the flag until just before the next update by the ISR. But if you can keep to that constraint, this is a better idea than turning off interrupts.

If too many sub-tasks resort to turning off interrupts, it can have a negative effect on response time or even cause missed interrupts in some cases. This is less likely to be a problem for the relatively simple programs most of us write and run, most of the time. But when you get to tracking and controlling more complex processes and make heavier use of the interrupt system, bad habits have a way of coming back to bite you.