pfeerick / elapsedMillis

Arduino 'port' of the elapsedMillis library
MIT License
84 stars 26 forks source link

elapsedSeconds doesn't seem to be rollover-safe. #16

Closed drf5n closed 3 weeks ago

drf5n commented 1 month ago

Since elapsedSeconds uses millis()/1000, I was curious about how it handles rollover so I took the example and reset the timer0_millis:

/*
  Simple demo of using all the timing helpers elapsedMillis makes available.

 Code from https://github.com/pfeerick/elapsedMillis/blob/master/examples/blinkingLeds/blinkingLeds.ino

  Modified to test the rollover of elapsedSeconds.
  Wokwi: https://wokwi.com/projects/402859003988453377

  See https://github.com/pfeerick/elapsedMillis/issues/16

  Either attach LEDs with series resistors to the indicated pins, or a
  six led / six bit 'Chartreuse' module plugged into pins 8 through GND.

  Wired up in order, the leds have a nice walking/counting effect.

  This example code is in the public domain.
*/

#include <elapsedMillis.h>  // https://github.com/pfeerick/elapsedMillis

//declare these global if you don't want them reset every time loop runs
elapsedMicros LED1micro;
elapsedMicros LED2micro;
elapsedMillis LED3millis;
elapsedMillis LED4millis;
elapsedSeconds LED5seconds;
elapsedSeconds LED6seconds;

const int LED1 = 8;
const int LED2 = 9;
const int LED3 = 10;
const int LED4 = 11;
const int LED5 = 12;
const int LED6 = 13;

// delay between blinks of the LED
unsigned long LED1_Interval = 62500;
unsigned long LED2_Interval = 125000;
unsigned long LED3_Interval = 250;
unsigned long LED4_Interval = 500;
unsigned long LED5_Interval = 1;
unsigned long LED6_Interval = 2;

extern volatile unsigned long timer0_millis;

void setup()
{
  // initialize the LED pins as outputs
  pinMode(LED1, OUTPUT);
  pinMode(LED2, OUTPUT);
  pinMode(LED3, OUTPUT);
  pinMode(LED4, OUTPUT);
  pinMode(LED5, OUTPUT);
  pinMode(LED6, OUTPUT);
  noInterrupts ();
  timer0_millis = -7200;
  interrupts ();

  LED1micro = LED2micro = LED3millis = LED4millis = LED5seconds = LED6seconds = 0;
}

void loop()
{
  if (LED1micro >= LED1_Interval)
  {
    digitalWrite(LED1, !(digitalRead(LED1))); // toggle the LED state
    LED1micro -= LED1_Interval;                           // reset the counter to 0 so the counting starts over...
  }

  if (LED2micro >= LED2_Interval)
  {
    digitalWrite(LED2, !(digitalRead(LED2))); // toggle the LED state
    LED2micro -= LED2_Interval;                            // reset the counter to 0 so the counting starts over...
  }

  if (LED3millis >= LED3_Interval)
  {
    digitalWrite(LED3, !(digitalRead(LED3))); // toggle the LED state
    LED3millis -= LED3_Interval;                           // reset the counter to 0 so the counting starts over...
  }

  if (LED4millis >= LED4_Interval)
  {
    digitalWrite(LED4, !(digitalRead(LED4))); // toggle the LED state
    LED4millis -= LED4_Interval;                           // reset the counter to 0 so the counting starts over...
  }

  if (LED5seconds >= LED5_Interval)
  {
    digitalWrite(LED5, !(digitalRead(LED5))); // toggle the LED state
    LED5seconds -= LED5_Interval;                          // reset the counter to 0 so the counting starts over...
  }

  if (LED6seconds >= LED6_Interval)
  {
    digitalWrite(LED6, !(digitalRead(LED6))); // toggle the LED state
    LED6seconds -= LED6_Interval;                         // reset the counter to 0 so the counting starts over...
  }
}

This code blinks the elapsedSeconds LEDs a couple times, and then they stick on.

Wokwi simulation:

https://wokwi.com/projects/402859003988453377

image
pfeerick commented 1 month ago

I've not tested it yet, but #17 should resolve this.

drf5n commented 1 month ago

I've not tested it yet, but #17 should resolve this.

17 does a & 0xFFFFFFFF on an unsigned long, so likely will get optimized away and do nothing.

The millis() rollover at 0xFFFFFFFF makes the seconds rollover happen with the change from 0xFFFFFFFF/100=4294967sec to 0sec at 295ms past the time seconds changed into 4294967. Second 4294967 is short by 705ms, and then a 0 - 4294967 comparison might result in unexpectedly huge numbers like 4294967000.

void setup() {
  Serial.begin(115200);

  unsigned long zero = 0, biggest = -1, second = 1000;
  unsigned long rolloverSecond = biggest / 1000;
  unsigned long lastSecondMs = rolloverSecond*1000;
  Serial.println(biggest);
  Serial.println(rolloverSecond);
  Serial.println(lastSecondMs);
  Serial.println( zero - rolloverSecond );
}

void loop() {
  // put your main code here, to run repeatedly:

}

gives these numbers:

4294967295
4294967
4294967000
4290672329

It seems complicated to handle seconds as millis()/1000. Maybe the methods would work better with elapsedSeconds base number as ms?

For example:

class elapsedSeconds
{
private:
    unsigned long ms=0;
public:
    elapsedSeconds(void) { ms = millis();}
    elapsedSeconds(unsigned long val) { ms = millis() - 1000UL*val ; }
    elapsedSeconds(const elapsedSeconds &orig) { ms = orig.ms; }
    operator unsigned long () const { return (millis()-ms)/1000; }
    elapsedSeconds & operator = (const elapsedSeconds &rhs) { ms = rhs.ms; return *this; }
    elapsedSeconds & operator = (unsigned long val) { ms = millis() - val*1000; return *this; }
    elapsedSeconds & operator -= (unsigned long val)      { ms += val*1000 ; return *this; }
    elapsedSeconds & operator += (unsigned long val)      { ms -= val*1000 ; return *this; }
    elapsedSeconds operator - (int val) const           { elapsedSeconds r(*this); r.ms += val*1000UL; return r; }
    elapsedSeconds operator - (unsigned int val) const  { elapsedSeconds r(*this); r.ms += val*1000UL; return r; }
    elapsedSeconds operator - (long val) const          { elapsedSeconds r(*this); r.ms += val*1000UL; return r; }
    elapsedSeconds operator - (unsigned long val) const { elapsedSeconds r(*this); r.ms += val*1000UL; return r; }
    elapsedSeconds operator + (int val) const           { elapsedSeconds r(*this); r.ms -= val*1000UL; return r; }
    elapsedSeconds operator + (unsigned int val) const  { elapsedSeconds r(*this); r.ms -= val*1000UL; return r; }
    elapsedSeconds operator + (long val) const          { elapsedSeconds r(*this); r.ms -= val*1000UL; return r; }
    elapsedSeconds operator + (unsigned long val) const { elapsedSeconds r(*this); r.ms -= val*1000UL; return r; }
};

elapsedSeconds secs, secs2(40);
extern volatile unsigned long timer0_millis;

void setup() {
  Serial.begin(115200);

  unsigned long zero = 0, biggest = -1, second = 1000;
  unsigned long rolloverSecond = biggest / 1000;
  unsigned long lastSecondMs = rolloverSecond*1000;
  Serial.println(biggest);
  Serial.println(rolloverSecond);
  Serial.println(lastSecondMs);
  Serial.println( zero - rolloverSecond );
  secs = 10 ; Serial.println(secs);
  secs += 10 ; Serial.println(secs);
  secs -= 5 ; Serial.println(secs);
  secs2; Serial.println(secs2);

  Serial.println();

  secs = -10;

  noInterrupts();
  timer0_millis  = zero - 10*second;
  interrupts();
}

void loop() {
  unsigned long now = millis();
  static unsigned long lastSec = secs;
  if(secs != lastSec){
    lastSec = secs;
    Serial.print(now);
    Serial.print(" ");
    Serial.print(secs);
    Serial.print(" ");
    Serial.print(secs2);
    Serial.println();
  }
  if(secs == 30){
    secs = 100;
    secs2 = secs;
  }

}