Jump to content
43oh

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.

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?

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

Link to post
Share on other sites

Join the conversation

You can post now and register later. If you have an account, sign in now to post with your account.

Guest
Reply to this topic...

×   Pasted as rich text.   Paste as plain text instead

  Only 75 emoji are allowed.

×   Your link has been automatically embedded.   Display as a link instead

×   Your previous content has been restored.   Clear editor

×   You cannot paste images directly. Upload or insert images from URL.

×
×
  • Create New...