Timer overflow not handled

[aweatherguy]

This is a nice idea. Thanks for putting it out there.

There's a problem with timer overflow. This is nothing new -- anybody who compares timers (millis or micros) in Arduino is exposed to this.

millis() overflows every 50 days or so and for micros it is about 71 minutes. The problem results from comparing two unsigned 32-bit values, like this:

if((*timerFunctionPtr)() >= timeOut)

Say that millis() returns 0xFFFFFFF0 when you call the start function with a delay of 32 (0x20). The value stored in the timeOut variable is (0xFFFFFFF0 + 0x00000020) = 0x00000010 because the 32-bit timer value has overflowed.

Now, say you test for timeout 8 timer ticks later by calling elapsed(). At this time, millis() will return 0xFFFFFFF8, and elapsed() will effectively perform this test:

if ( 0xFFFFFFF8 > 0x00000010 )

which will of course be true and you will think the time has elapsed.

This only happens every 71 minutes (micros) or 50 days (millis) -- and will create extremely hard to find bugs. Imagine you have a system that only crashes once every 71 minutes or 50 days an you have to figure out where the problem is! These are the kind of bugs that make you look for another line of work.

Here's a proposed fix -- a new function to compare two timer values. There are other ways to do this but this is perhaps less code space than some.

It compares two 32-bit timer values, and returns one of three values:

-1 if a is earlier than b
0 if and and be are equal
+1 if a is later than b

The statement in elapsed() would then look like this:

if( TimerCompare((*timerFunctionPtr)(), timeOut) > 0 )
Here's the proposed TimerCompare() function:

`
int8_t TimerCompare(uint32_t a, uint32_t b)
{
int8_t c;

__asm__ __volatile__ (
    "; form (a-b)   \n\t"
    "eor  %0,%0     \n\t"
    "sub  %A1,%A2   \n\t"
    "sbc  %B1,%B2   \n\t"
    "sbc  %C1,%C2   \n\t"
    "sbc  %D1,%D2   \n\t"
    "breq .+8       \n\t"
    "brmi .+4       \n\t"
    "ldi  %0,1      \n\t"
    "rjmp .+2       \n\t"
    "ldi %0,0xff   \n\t"
    : "=&r" (c)
    : "r" (a), "r" (b)
);

return c;

}
`

[avandalen]

Hi I was be aware of this problem but had no time to go into it. Thank you for your help! Can the TimerCompare() function: be done in c++ instead of instructionluage? Can you change this in github? kind regards Albert From: aweatherguy Sent: Thursday, October 19, 2017 4:36 AM To: avandalen/VirtualDelay Cc: Subscribed Subject: [avandalen/VirtualDelay] Timer overflow not handled (#2) This is a nice idea. Thanks for putting it out there. There's a problem with timer overflow. This is nothing new -- anybody who compares timers (millis or micros) in Arduino is exposed to this. millis() overflows every 50 days or so and for micros it is about 71 minutes. The problem results from comparing two unsigned 32-bit values, like this: if((*timerFunctionPtr)() >= timeOut) Say that millis() returns 0xFFFFFFF0 when you call the start function with a delay of 32 (0x20). The value stored in the timeOut variable is (0xFFFFFFF0 + 0x00000020) = 0x00000010 because the 32-bit timer value has overflowed. Now, say you test for timeout 8 timer ticks later by calling elapsed(). At this time, millis() will return 0xFFFFFFF8, and elapsed() will effectively perform this test: if ( 0xFFFFFFF8 > 0x00000010 ) which will of course be true and you will think the time has elapsed. This only happens every 71 minutes (micros) or 50 days (millis) -- and will create extremely hard to find bugs. Imagine you have a system that only crashes once every 71 minutes or 50 days an you have to figure out where the problem is! These are the kind of bugs that make you look for another line of work. Here's a proposed fix -- a new function to compare two timer values. There are other ways to do this but this is perhaps less code space than some. It compares two 32-bit timer values, and returns one of three values:

…

-1 if a is earlier than b 0 if and and be are equal +1 if a is later than b The statement in elapsed() would then look like this: if( TimerCompare((*timerFunctionPtr)(), timeOut) > 0 ) Here's the proposed TimerCompare() function: ` int8_t TimerCompare(uint32_t a, uint32_t b) { int8_t c;

__asm__ __volatile__ ( "; form (a-b) \n\t" "eor %0,%0 \n\t" "sub %A1,%A2 \n\t" "sbc %B1,%B2 \n\t" "sbc %C1,%C2 \n\t" "sbc %D1,%D2 \n\t" "breq .+8 \n\t" "brmi .+4 \n\t" "ldi %0,1 \n\t" "rjmp .+2 \n\t" "ldi %0,0xff \n\t" : "=&r" (c) : "r" (a), "r" (b) ); return c; } ` — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.

[edgar-bonet]

Simple fix:

--- avdweb_VirtualDelay.cpp.orig	2017-10-22 16:07:11.488732486 +0200
+++ avdweb_VirtualDelay.cpp	2017-10-22 16:09:30.880156256 +0200
@@ -37,7 +37,7 @@
 bool VirtualDelay::elapsed() 
 { bool pulse = 0;
   if(running)
-  { if((*timerFunctionPtr)() >= timeOut) 
+  { if((signed long)((*timerFunctionPtr)() - timeOut) >= 0)
     { running = 0;
       pulse = 1; // return 1 just one time  
      }      

C.f. How can I handle the millis() rollover?, first answer, section “What if I really need to compare timestamps?”

[avandalen]

thanks very much.. Albert From: Edgar Bonet Sent: Sunday, October 22, 2017 4:19 PM To: avandalen/VirtualDelay Cc: Albert van Dalen ; Comment Subject: Re: [avandalen/VirtualDelay] Timer overflow not handled (#2) Simple fix:

…

--- avdweb_VirtualDelay.cpp.orig 2017-10-22 16:07:11.488732486 +0200 +++ avdweb_VirtualDelay.cpp 2017-10-22 16:09:30.880156256 +0200 @@ -37,7 +37,7 @@ bool VirtualDelay::elapsed() { bool pulse = 0; if(running) - { if((*timerFunctionPtr)() >= timeOut) + { if((signed long)((*timerFunctionPtr)() - timeOut) >= 0) { running = 0; pulse = 1; // return 1 just one time } C.f. How can I handle the millis() rollover?, first answer, section “What if I really need to compare timestamps?” — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[avandalen]

Hi Edgar,
You helped me a lot.
I changed the library and tested to whole day. See the Test_rollover folder.
It seems to work now for delay values from 0 to 2^31-1, including timer rollover.
But, now there is a strange problem: It works only when the 2 library files (cpp, h) are in the project folder "Test_rollover". When in the library folder it doesn't work properly when the setMillis() is used.
I have never seen such an issue.

[edgar-bonet]

It works only when the 2 library files (cpp, h) are in the project folder

This is not a problem with the code. It looks more like a problem with the Arduino IDE.

I can only guess, but maybe it's still using cached object files from a previous build.

[avandalen]

Here is the article where I use your contrinution http://www.avdweb.nl/arduino/libraries/virtualdelay.html The library has to be timer-rollover proof The 32 bit millis() and micros() timers counts from 0, 1, 2 to 4294967295 (2^32-1) and than rollover to 0 again. The millis() timer is going to roll over after roughly 49.7 days, the micros() rolls over every 71.6 minutes. We have to take care about this rollover, because it may happen that we start the virtual-delay just before the rollover time. After the rollover the timestamps will be messed up. The time compare part in the VirtualDelay library is tricky, it compares 2 timestamps like this: if(millis() >= timeOut) However, this won’t work, we have to use: if((millis()-timeOut)>=0) Believe it or not, in this situation, both lines are not identical! You can read all about this in the detailed story from Edgar Bonet. Thanks Albert From: Edgar Bonet Sent: Monday, October 23, 2017 6:32 PM To: avandalen/VirtualDelay Cc: Albert van Dalen ; Comment Subject: Re: [avandalen/VirtualDelay] Timer overflow not handled (#2) It works only when the 2 library files (cpp, h) are in the project folder This is not a problem with the code. It looks more like a problem with the Arduino IDE. I can only guess, but maybe it's still using cached object files from a previous build. — You are receiving this because you commented. Reply to this email directly, view it on GitHub, or mute the thread.

[avandalen]

Hi I have solved it, see github Albert From: aweatherguy Sent: Thursday, October 19, 2017 4:36 AM To: avandalen/VirtualDelay Cc: Subscribed Subject: [avandalen/VirtualDelay] Timer overflow not handled (#2) This is a nice idea. Thanks for putting it out there. There's a problem with timer overflow. This is nothing new -- anybody who compares timers (millis or micros) in Arduino is exposed to this. millis() overflows every 50 days or so and for micros it is about 71 minutes. The problem results from comparing two unsigned 32-bit values, like this: if((*timerFunctionPtr)() >= timeOut) Say that millis() returns 0xFFFFFFF0 when you call the start function with a delay of 32 (0x20). The value stored in the timeOut variable is (0xFFFFFFF0 + 0x00000020) = 0x00000010 because the 32-bit timer value has overflowed. Now, say you test for timeout 8 timer ticks later by calling elapsed(). At this time, millis() will return 0xFFFFFFF8, and elapsed() will effectively perform this test: if ( 0xFFFFFFF8 > 0x00000010 ) which will of course be true and you will think the time has elapsed. This only happens every 71 minutes (micros) or 50 days (millis) -- and will create extremely hard to find bugs. Imagine you have a system that only crashes once every 71 minutes or 50 days an you have to figure out where the problem is! These are the kind of bugs that make you look for another line of work. Here's a proposed fix -- a new function to compare two timer values. There are other ways to do this but this is perhaps less code space than some. It compares two 32-bit timer values, and returns one of three values:

…

-1 if a is earlier than b 0 if and and be are equal +1 if a is later than b The statement in elapsed() would then look like this: if( TimerCompare((*timerFunctionPtr)(), timeOut) > 0 ) Here's the proposed TimerCompare() function: ` int8_t TimerCompare(uint32_t a, uint32_t b) { int8_t c;

__asm__ __volatile__ ( "; form (a-b) \n\t" "eor %0,%0 \n\t" "sub %A1,%A2 \n\t" "sbc %B1,%B2 \n\t" "sbc %C1,%C2 \n\t" "sbc %D1,%D2 \n\t" "breq .+8 \n\t" "brmi .+4 \n\t" "ldi %0,1 \n\t" "rjmp .+2 \n\t" "ldi %0,0xff \n\t" : "=&r" (c) : "r" (a), "r" (b) ); return c; } ` — You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub, or mute the thread.