Strange millis() behaviour

[soccou 1]

I am trying to get a simple program working that just takes some A/D readings and then every few seconds prints the values over serial.

 

It works fine for about 37.2 hours, and then just stops. The chip appears to freeze. If I reset it, it will go back to work.

 

Once millis is roughly larger than 134214583, nothing. I took the exact same code (changed i/o pin names) and ran it on an arduino and it does not have this problem, so I suspect a bug in the Energia libraries.

 

Anyone have any ideas?

 

Thanks,

Scott

[chicken 630]

Can you share the code-snippet where you use millis?

 

Note, that millis() at some point restarts at zero. The Arduino documentation says, that this happens after about 50 days. I guess that unlike Arduino, Energia does rollover before the full 32bit is reached, i.e. the rollover depends on timer resolution and CPU clock. It looks like the unpredictability of the rollover was fixed in Arduino 1.0.

 

For reliable long-term operation your code needs to handle the overflow situation, even for Arduino. This blog post might help: http://www.faludi.com/2007/12/18/arduino-millis-rollover-handling/

 

If you use millis() to wait for a certain time period, try using delay() instead.

[soccou 1]

@@chicken,

 

thanks for the quick reply. Initially I was attempting a simple conditional:

#define SEND_DELAY 5000
volatile unsigned long transmitTime;
void setup() {
    transmitTime = millis() + SEND_DELAY;
}

void loop() {

    if (millis() > transmitTime) {
        processIO();
        transmitTime = millis() + SEND_DELAY;
    }

}

after that didn't work on energia, i tried the normal library i have used on many projects on the Arduino:

http://playground.arduino.cc/code/timer

 

but that failed too. I will look through your link and try some of those suggestions.

[chicken 630]

I see that the linked library has the same issue

 

https://github.com/JChristensen/Timer/blob/master/Event.cpp

unsigned long now = millis();
if (now - lastEventTime >= period)

At some point in time, whether it's 9 hours with the LaunchPad or 50 days with the Arduino, now will be smaller than lastEventTime as the milliseconds exceed the maximum and restart at 0.

 

The correct code should include the special case where now is smaller than lastEventTime. Something like:

unsigned long now = millis();
if ((now > lastEventTime && (now - lastEventTime) >= period) 
    || (now < lastEventTime && (MAXMILLISECONDS - lastEventTime + now) >= period))

Unfortunately MAXMILLISECONDS, i.e. the time when millis() rolls over, is not known with the current version of Energia. It seems that Arduino fixed that a while back by making sure that millis() counts to 2^32-1 before restarting at 0.

 

PS: actually, the Timer library might work with Arduino thanks to 32 bit overflow magic with unsigned long and negative results

 

Correction: If the timer rolls over after 2^32 milliseconds, i.e. uses all 32 bits, then the first comparison works. No need to check for MAXMILLISECONDS.

[grahamf72 169]

The error stems from wiring.c. The variable wdtcounter is an unsigned long which is incrementend every time the WDT interrupt is called, about 32000 times per second on 16Mhz devices. Therefore millis() can only count to (2^32-1)/32000 before overflow. The delay() function is less effected because it accesses the wdtcounter directly, so it's own test variable will also overflow - if delay is called for a duration that covers the overflow time it will give a shorter delay than expected, but will not lock.

 

One partial workaround would be to modify wiring.c, and increment wdtcounter from 0 to 32, then increment a millis variable every time wdtcounter reaches 32. This would allow millis to be accurate for about 50 days like Arduino.

 

Another option would be to use a variation of the code I posted on the following thread: http://forum.43oh.com/topic/3447-sleep-mode-during-a-fixed-duration/

By using the timer, and resetting it at the start of each duration, you can guarantee you won't have any overflow issues.

[chicken 630]

I ported the latest Arduino implementation of millis, micros and delay into Energia.

 

Attached the modified wiring.c and a small test program. No intense testing yet, so use at your own risk. Particularly the microsecond part looks suspicious

 

The main difference to Arduino is, that Energia uses the WDT which does not allow to read the current timer reading. This means, that the resolution is 512/F_CPU and therefore maximum precision for microseconds is +/- 256/F_CPU, e.g. +/-256us at 1MHz or +/-16us at 16MHz.

 

PS: My implementation is 20 bytes larger than the original

TimerFixes.zip

[chicken 630]

Found an error in my micros() implementation, double-counting some microseconds.

 

By coincidence, it now compiles to exactly the same size as the old wiring.c.

 

I did some basic verification with MSP430G2553. As this is a rather central piece of code, it would be great if others could verify with different MSP430 variants before I send a pull request to the Energia project.

 

For testing, replace /energia/hardware/msp430/cores/msp430/wiring.c with the version in the attached archive.

TimerFixes v2.zip

[chicken 630]

I verified, that with my fix works for

now = millis();
if(now - starttime > period)

Note that this works due to fixed point math. If now is smaller than starttime, the result in unsigned integer is still the correct period:

starttime = 0xFFFFFFFF
now = 0x00000001
now-starttime = 1 - (-1) = 2

However you still have to be careful when defining the condition. This will not work reliably:

now = millis();
if(now > starttime + period)

In the case where starttime + period crosses the rollover threshold, now is always greater until it also rolls over.

starttime = 0xFFFFFFF0
period = 0x00000020
starttime + period = 0x00000010
now = 0xFFFFFFFn > 0x00000010

[soccou 1]

@@chicken,

 

wow, I really appreciate your support. Thank you for your time on this one.

 

I will try to test this tomorrow.

 

Scott

[chicken 630]

@@chicken,

 

I will try to test this tomorrow.

 

Scott

Any chance to test it yet?

[soccou 1]

Any chance to test it yet?

 

Hi, yes i have been trying all week, sorry for not getting back sooner.

However, I think I may have complications from another source.

 

It is time consuming to try a change then wait 37hours to see if it works. Do you know any way to preset the millis counter for easier testing?

[chicken 630]

It is time consuming to try a change then wait 37hours to see if it works. Do you know any way to preset the millis counter for easier testing?

 

For test purposes, you could set the initial milliseconds value to something else than 0 in line 48 of wiring.c:

 

 

volatile unsigned long wdt_millis = 0;

 

e.g. 0xffff0000 should give you about a minute before roll over.

[soccou 1]

For test purposes, you could set the initial milliseconds value to something else than 0 in line 48 of wiring.c:

volatile unsigned long wdt_millis = 0;

e.g. 0xffff0000 should give you about a minute before roll over.

 

Thanks for the suggestion, however it didn't seem to work.

I changed:

//volatile unsigned long wdt_millis = 0;
volatile unsigned long wdt_millis = 0xffff0000;

but it still starts the count at zero.

[chicken 630]

I checked the source, and there's no other place assigning 0 to wdt_millis.

 

Did you verify that the change is actually compiled into your project? E.g by intentionally adding an error and see if the compiler barfs at you.

[soccou 1]

Yes, I just tried renaming wdt_millis to a bad value, and the compiler does barf on compile.

I also searched and couldn't find any other references. So I am puzzled why it still starts at zero.