Jump to content
Bernie

Energia bug in Blink without delay tutorial

Recommended Posts

Hi

 

I've joined the forum to post a bug (I believe) in a tutorial on the Energia website. Specifically, the blink without delay tutorial. The following line of code is used to determine whether to toggle the state of the LED:

if(currentMillis - previousMillis > interval)

The documentation for the millis method states:

 

 

This number will overflow (go back to zero), after approximately 50 days.

 

As far as I can tell, this means that after approximately 50 days the example given will cease to blink and remain in the last state set.

 

Before someone shoots me down saying that it's just an example and not intended to run for longer, please bear in mind that these tutorials are used by us beginners and we assume they are examples of good practice.

Share this post


Link to post
Share on other sites

If you edit the first post, you can edit the title.  Make sure to click the "use full editor" button.

 

That's an excellent point.  I'd hate for someone to start with the blink example and write something that always stopped working after 50 days.  That could be horribly frustrating.  

The concern I would have is that adding extra code to something as basic as a blink example to address the issue might confuse a beginner.  I did some quick googling, and apparently there are a number of posts about this issue for Arduino as well - not surprising considering Energia is a fork of Arduino and many of the code samples come from the Arduino (including the blink examle).  

 

I'd suggest either adding a more complicated blink example that addresses the issue and/or commenting the basic blink example with a warning about the 50 day problem.  

 

Thoughts?

Share this post


Link to post
Share on other sites

The best solution would be to use a class similar to boost::chrono that will abstract away this stuff from the user. It would allow for a user to just do the same, but these would be more intelligent classes, not plain 32 bit integers.

Share this post


Link to post
Share on other sites

You don't have to worry about this. The unsigned 32 bit wraps around and the math works properly. Take a look at the code below followed by the results. This is just some straight c code you can compile on your desktop pc and run.
 

#include <stdio.h>
#include <stdint.h>

const uint32_t intervalMs = 10000;
uint32_t currMs = 0;
uint32_t prevMs = 0;

void check(check) {
  uint32_t diff = currMs-prevMs;

  if ( diff > intervalMs ) {
    printf("currMs=%u-prevMs=%u diff(%u) > interval=(%u) == %s\n",
	   currMs, prevMs, diff, intervalMs,
	   ((diff) > intervalMs ? "T":"F") );
    prevMs += intervalMs;
  }

}

void main() {
  do {
    check();
    currMs++;
  } while(1);
}
currMs=10001-prevMs=0 diff(10001) > interval=(10000) == T
currMs=20001-prevMs=10000 diff(10001) > interval=(10000) == T

... many lines later near the wrap ...

currMs=4294960001-prevMs=4294950000 diff(10001) > interval=(10000) == T

... and the first value after currMs wraps around ..

currMs=2705-prevMs=4294960000 diff(10001) > interval=(10000) == T
currMs=12705-prevMs=2704 diff(10001) > interval=(10000) == T

... and on it goes ..

2705-4294960000=10001 when they are both unsigned 32 bit integers.

The real bug is using a long instead of an unsigned long for the value previousMillis and interval.

[edit]
I just checked over at the arduino github and it looks like someone fixed it here:

https://github.com/arduino/Arduino/commit/83ef1814cfda808f94cd80a70d7cee8725e24a13

[/edit]


-rick

Share this post


Link to post
Share on other sites

Create an account or sign in to comment

You need to be a member in order to leave a comment

Create an account

Sign up for a new account in our community. It's easy!

Register a new account

Sign in

Already have an account? Sign in here.

Sign In Now

×