Go Down

Topic: BlinkWithoutDelay Change (Read 901 times) previous topic - next topic

Coding Badly

I suggest making two changes to the Blink Without Delay example...

http://arduino.cc/en/Tutorial/BlinkWithoutDelay

The program has a small "drift" in the time interval; the blink rate is slightly longer than 1000ms.  For an example like Blink Without Delay this isn't a problem.  But, for long running intervals, it can create serious problems.

Code: [Select]
long previousMillis = 0;        // will store last time LED was updated
[glow] long thisMillis;                     // value from millis() this time through the loop[/glow]


Code: [Select]
[glow]   thisMillis = millis();[/glow]
  if (millis() - [glow]thisMillis[/glow] > interval) {
    previousMillis = [glow]thisMillis[/glow];


previousMillis should be initialized in setup...

Code: [Select]
void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
  [glow]previousMillis = millis();[/glow]
}

BenF

Or you could change this:

Code: [Select]
  if (millis() - previousMillis > interval) {
    // save the last time you blinked the LED
    previousMillis = millis();  


to this:

Code: [Select]
  if (millis() - previousMillis > interval) {
    // save the last time you blinked the LED
    previousMillis += interval;

Coding Badly

While the two appear to be interchangeable, there is a big difference between the two when the rest of application takes more than interval to execute.  There are advantages and disadvantages to each and it's really application dependent which is the better choice.  I respectively suggest that, for a blinking LED, the code I posted is the better choice.

BenF

#3
Oct 18, 2009, 01:45 am Last Edit: Oct 18, 2009, 01:47 am by borref Reason: 1
As I see the tutorial - "Show how to blink without using delay" - it is pretty clear as is and as such a rewrite is not really needed. When I read "serious problems" I'm expecting my board to catch fire or worse.

What caught my interest is more academic (e.g. can I learn something, is there some other aspect to this tutorial that may be useful).

You state:

Quote
The program has a small "drift" in the time interval; the blink rate is slightly longer than 1000ms.


This is true as millis() is called twice and quite possibly there will be an interval between the two calls that accumulate. For the skecth as such it is (subjectively) irrelevant and the change does not add value unless you also point out the difference. The fix you proposed and what I suggested as an alternative will correct this issue.

Then you post the additional requirement as follows:

Quote
... when the rest of application takes more than interval to execute.


If this is the case, your proposed change will accumulate a "drift" every time "the rest of the application" runs longer than interval. This is in conflict with what you set out to fix in your first post. What I proposed will not, but the time between loops may occasionally be shorter whenever "the rest of the application" runs longer than interval. If the loop always takes longer than interval to run, either variation will drift.

In my opinion the "better" code is the sample that best illustrate the text - so unles you change both nothing is gained.

Or .. Is there something I missed?

Coding Badly

A few things I've realized about the code in BlinkWithoutDelay...


  • The presence of a drift is dependent on the timing of the rest of the application.  In the case of BlinkWithoutDelay, there is (almost) no other code so the interval will always be 1000 ms.
  • When the code is put into another application, it is possibly that the actual interval is consistently 1001 ms.
  • Even in other applications, the interval is much more likely to be 1000 ms.
  • In some applications, for a given pass through loop, there's no way to predict if the interval will be 1000 ms or 1001 ms.



Back to the discussion...

Quote
a rewrite is not really needed


I didn't suggest rewriting.  I suggested fixing the buggy code.

Quote
I'm expecting my board to catch fire or worse


I say the problem is serious because people new to the Arduino are being taught a flawed way to code an interval timer.  

Quote
The fix you proposed and what I suggested as an alternative will correct this issue


I now believe your suggestion is the better choice for BlinkWithoutDelay.  Ideally, the web page would include both with a brief description of the pros and cons.

shelleycat

I'm not sure I have understood the problem.

Is it that the original example is calling millis() twice, so there is a risk
that the millisecond has moved on between the first and second call?

Coding Badly


Go Up