Subtle timing error in "BlinkWithoutDelay"

What I did: I had the Arduino do something special every time it experienced a timing error greater than 1 millisecond. What I noticed is that it accumulates 4 errors in total, then after that, no more errrors. Each of the 4 errors seems to happen after a different number of cycles, but each number of cycles seems to reproduce exactly every time I reset it. I'm assuming the phenomenon is reproducible to other people, what I'm hoping for is help in understanding why it is doing this.

I noticed there is a >= clause in the comparison:

if (currentMillis - previousMillis >= interval) {

I've seen this before. The Arduino is counting milliseconds from the last time it flipped the LED, as soon as the difference equals the expected interval, then flip the LED. However it isn't asking "equal" to the interval, it is asking "greater than or equal". The greater than is a safety, just in case it took more than 1 millisecond since the last check.

So for fun, I changed interval from a const to a long, and whenever the Arduino detected an error, I had it double the interval so that the flashing becomes noticeably longer every time this error happens. I also started the program with an interval at 25 milliseconds rather than 1000.

I noticed the program can run all night long and it never has more than 4 timing errors, which all happen within just a few seconds. So I set it up with a pushbutton on pin 2 to reset the interval to 25 milliseconds so that I can observe this phenomenon over and over. It's action is perfectly reproducible on my Arduino Uno R3, the genuine one from Italy.

So here is the modified code, I wonder if anybody has any idea how to explain what is happening? First of all, can someone confirm it is behaving the same for you as for me?

Thanks much!!
Barkfin

/* Blink without Delay

 Turns on and off a light emitting diode (LED) connected to a digital
 pin, without using the delay() function.  This means that other code
 can run at the same time without being interrupted by the LED code.

 The circuit:
 * LED attached from pin 13 to ground.
 * Note: on most Arduinos, there is already an LED on the board
 that's attached to pin 13, so no hardware is needed for this example.

 created 2005
 by David A. Mellis
 modified 8 Feb 2010
 by Paul Stoffregen
 modified 11 Nov 2013
 by Scott Fitzgerald
 modified 15 Jan 2015
 by Barkfin


 This example code is in the public domain.

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

// constants won't change. Used here to set a pin number :
const int ledPin =  13;      // the number of the LED pin
const int Switch1 = 2;      // the number of switch 1

// Variables will change :
int ledState = LOW;             // ledState used to set the LED

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

// variables are able to change :
long interval = 25;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
  pinMode(Switch1, INPUT);
}

void loop() {
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis = millis();

  if (currentMillis - previousMillis >= interval) {

    // if timing error, double the interval
    if (currentMillis - previousMillis > interval) {interval = 2*interval;}

    //  save the last time you blinked the LED
    previousMillis = currentMillis;
    
    // if the LED is off turn it on and vice-versa:
    ledState = !ledState;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);


   // check if the pushbutton was pressed to reset back to 25 milliseconds interval
    if (! digitalRead(Switch1)) {interval = 25;}
  }
}

In your code loop executes many times a millisecond. Your code will easily catch any change in value.

Carefully walk through your code assuming that, every once in a while, millis does not increment by 1 but rather by 2.

interval should be unsigned long but OK.

Coding Badly is right, the millis() function sometimes makes steps of 2.

Very well explained here

The reason I call this a timing error is that the code is written just a little bit sloppy: if the > condition is the trigger, then the cycle necessarily adds that 1 millisecond to the flash.

I heard once that North American power is controlled pretty tightly to 60 Hz. In a day there are 24 x 3600 = 86,400 seconds. If there are 60 power cycles per second, that becomes 86,400 x 60 = 5,184,000 power cycles in a day. I heard that the power operators would monitor this total and if it was a bit slow or a bit fast by the end of the day, they would adjust the frequency a skotch to force it to add up to 5,184,000 power cycles, each and every day. So if you had a clock that kept time by counting power cycles, it would never be that far out.

So anyway, I modified the code to eliminate the error accumulating. Instead of this:

if (currentMillis - previousMillis >= interval) {

    // if timing error, double the interval
    if (currentMillis - previousMillis > interval) {interval = 2*interval;}

    //  save the last time you blinked the LED
    previousMillis = currentMillis;

I made it do this:

if (currentMillis - previousMillis >= interval) {

    // increment precisely by interval
    previousMillis = previousMillis + interval;

    // if timing error, double the interval
    if (currentMillis > previousMillis) {interval = 2*interval;}

The reasoning is: the next LED flash is supposed to be precisely "interval" milliseconds after the previous one. But if the previous one didn't happen at the exact right millisecond, we don't let this error accumulate. We instead set a target to be "interval" milliseconds after the time when the last LED flash was supposed to happen. Whether it had or hadn't is immaterial. The sequence of flashes will occur without error buildup.

However, with this code modification, the Arduino experiences an unlimited number of timing errors. In the previous example, it would experience 4 timing errors only. In this example, which is just about exactly the same, and should be the same, it experiences an unlimited number of errors.

Again, anybody care to compile this and confirm if their Arduino Uno R3 is experiencing the same phenomenon?

/* Blink without Delay

 Turns on and off a light emitting diode (LED) connected to a digital
 pin, without using the delay() function.  This means that other code
 can run at the same time without being interrupted by the LED code.

 The circuit:
 * LED attached from pin 13 to ground.
 * Note: on most Arduinos, there is already an LED on the board
 that's attached to pin 13, so no hardware is needed for this example.

 created 2005
 by David A. Mellis
 modified 8 Feb 2010
 by Paul Stoffregen
 modified 11 Nov 2013
 by Scott Fitzgerald
 modified 15 Jan 2015
 by Barkfin


 This example code is in the public domain.

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

// constants won't change. Used here to set a pin number :
const int ledPin =  13;      // the number of the LED pin
const int Switch1 = 2;      // the number of switch 1

// Variables will change :
int ledState = LOW;             // ledState used to set the LED

// Generally, you should use "unsigned long" for variables that hold time
// The value will quickly become too large for an int to store
unsigned long previousMillis = 0;        // will store last time LED was updated

// variables are able to change :
long interval = 25;           // interval at which to blink (milliseconds)

void setup() {
  // set the digital pin as output:
  pinMode(ledPin, OUTPUT);
  pinMode(Switch1, INPUT);
}

void loop() {
  // here is where you'd put code that needs to be running all the time.

  // check to see if it's time to blink the LED; that is, if the
  // difference between the current time and last time you blinked
  // the LED is bigger than the interval at which you want to
  // blink the LED.
  unsigned long currentMillis = millis();

 if (currentMillis - previousMillis >= interval) {

    // increment precisely by interval
    previousMillis = previousMillis + interval;

    // if timing error, double the interval
    if (currentMillis > previousMillis) {interval = 2*interval;}
   
    // if the LED is off turn it on and vice-versa:
    ledState = !ledState;

    // set the LED with the ledState of the variable:
    digitalWrite(ledPin, ledState);


   // check if the pushbutton was pressed to reset back to 25 milliseconds interval
    if (! digitalRead(Switch1)) {interval = 25;}
  }
}

Please use code tags.

Read this before posting a programming question

Barkfin:
I made it do this:

if (currentMillis - previousMillis >= interval) {

// increment precisely by interval
    previousMillis = previousMillis + interval;

This issue is extensively discussed in Several Things at a Time which is a Sticky at the top of the Project Guidance section.

...R

So if you had a clock that kept time by counting power cycles, it would never be that far out.

In fact you are saying, use an external time source. Makes sense

Can you show how the code would change with the external 60Hz timing?
last time I looked my UNO worked on 5V DC ... :wink:

Slightly OT: 50 Hz

Barkfin:
I wonder if anybody has any idea how to explain what is happening?

The millis() counter is NOT counting in equally timed steps of 1/1000 second, but it is counting in not-equally steps of 1/1024 seconds.

So sometimes the millis() counter does not advance by one, but by two in a single step. This leads to the result, that some values of millis() will never be returned by the function, but they will be skipped.

AWOL:
Slightly OT: 50 Hz

Wow!

As long as the CPU clock is synchronized only to a ceramic resonator, millis() is only useful for short term timing.

See Gammon Forum : Electronics : Microprocessors : millis() overflow ... a bad thing? - millis() will jump from time to time.

You can avoid this by using your own timer (eg. Timer 1) and configuring it to tick exactly every ms, not every 1.024 ms.