Pages: [1]   Go Down
Author Topic: BlinkWithoutDelay Change  (Read 703 times)
0 Members and 1 Guest are viewing this topic.
Global Moderator
Dallas
Online Online
Shannon Member
*****
Karma: 178
Posts: 12288
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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:
long previousMillis = 0;        // will store last time LED was updated
[glow] long thisMillis;                     // value from millis() this time through the loop[/glow]

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

previousMillis should be initialized in setup...

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

Offline Offline
Edison Member
*
Karma: 3
Posts: 1001
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

Or you could change this:

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

to this:

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

Global Moderator
Dallas
Online Online
Shannon Member
*****
Karma: 178
Posts: 12288
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

Offline Offline
Edison Member
*
Karma: 3
Posts: 1001
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
« Last Edit: October 17, 2009, 06:47:59 pm by borref » Logged

Global Moderator
Dallas
Online Online
Shannon Member
*****
Karma: 178
Posts: 12288
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

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.
Logged

UK, Southwest
Offline Offline
Full Member
***
Karma: 5
Posts: 138
Arduino rocks
View Profile
 Bigger Bigger  Smaller Smaller  Reset Reset

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?
Logged

Global Moderator
Dallas
Online Online
Shannon Member
*****
Karma: 178
Posts: 12288
View Profile
WWW
 Bigger Bigger  Smaller Smaller  Reset Reset

Yes.
Logged

Pages: [1]   Go Up
Jump to: