millis() fails to increment when interrupts are disabled. [imported]

[cmaglie]

This is Issue 187 moved from a Google Code project.
Added by 2010-01-15T23:00:14.000Z by [email protected].
Please review that bug for more context and additional comments, but update this bug.

Original labels: Type-Defect, Priority-Medium, Component-Core

Original description

When IRQs are disabled at the time when timer0 overflows, micros() will
return wrong results.

What steps will reproduce the problem?

1. call t1=micros() just before a overflow of timer0
2. disable IRQ
3. wait after overflow of timer0 and after TCNT0 > 0
4. call t2=micros()
5. enable IRQ
6. call t3=micros()

now t2 is smaller than t1 because the ISR TIMER0_OVF_vect in wiring.c has
not been called and the overflow handling in micros() is wrong:

wrong, because it only works when t==0:
if ((TIFR0 & _BV(TOV0)) && (t == 0))
t = 256;

much better, but doesn't handle longer times with disabled IRQs:
if (TIFR0 & _BV(TOV0))
t += 256;

best: see attachment of patch

What is the expected output?
t1=2010104
t2=2010116
t3=2010124
ovl1=0 ovl2=1 novl2=0
t2-t1=12 t3-t1=20 t3-t2=8

What do you see instead?
t1=2010104
t2=2009092
t3=2010124
ovl1=0 ovl2=1 novl2=1
t2-t1=-1012 t3-t1=20 t3-t2=1032

What version of the Arduino software are you using? On what operating
system? Which Arduino board are you using?
I'm using Arduino-17 with Windows XP+SP3 and Arduino Duemilanove-328.

Please provide any additional information below.
The attachment "MicrosTimerOverflowTest.pde" contains a test case which
reproduces the problem. t2 must always be greater than t1.

The patch "wiring.patch" contains a fix which corrects the problem and also
solves the missing timer overflows problem when IRQs are disabled for a
longer time, as long as micros() is called at least once between two timer
overflows (around 1 microsecond on a ATmega 16Mhz).

[matthijskooijman]

The first part of this issue (micros() sometimes returning a too low value in an ISR directly) seems to be fixed already, I think it now properly corrects for a pending overflow interrupt.

The second part of this issue, micros() returning a too low value when interrupts are disabled for more than 1024us-2048us (at 16Mhz) is still present. What happens then is that the overflow flag is already set when a second overflow happens, which makes the timer value 0 again. Since there is no second flag to compensate for this overflow, the micros() return value becomes lower and incorrect.

I had a quick look at the linked patch, which seems bigger than needed to me (and it probably is out of date too). It might serve as inspiration, though.

I guess the minimum fix for this issue is to simply call the overflow ISR from micros() (and also millis()) when the flag is set and interrupts are disabled. The attached patch moves the ISR code into a separate function that is called by the ISR too, but I actually think that you can just call an ISR directly as well (that might be slightly slower because it will save more registers than strictly required, but the ISR itself will be a lot faster because it does not have to save call-clobbered registers).

I might have a stab at fixing this. Some other releated issues which could be useful to keep in mind would be #151, #57, #48, #320 and arduino/Arduino#1442.