Help with code having millis overflow issue

Using Arduino Programming Questions
#1

Dear all,

I am currently doing a small project involving a push button, relay board, buzzer, arduinomega

Basically after 40minutes a buzzer is switched ON.

If the user does not press the button in 5minutes the arduino gives a short (relay).

My code is very basic (since I have not touched in a long time).

Can you please help regarding the overflow. After 49 days the timer misbehaves and goes haywire.

I have tried doing the 'hack' but the timer goes crazy.

Below is the code:

const byte button=6;
const byte buttonreset=5; //da definire
const int outputIndicator=2;
const int outputBuzzer=3;
const int outputPanel=4;
int interval = 1000; //to find what value i have to give
extern volatile unsigned long timer0_millis; //timer hack

int buttonState=0;
int buttonresetState=0; //da definire
unsigned long timer1;
//unsigned long currentMillis = millis();
unsigned long previousMillis=0;

void setup() {
pinMode(button, INPUT); //reset
pinMode(buttonreset, INPUT); //da definire
pinMode(outputIndicator, OUTPUT);
pinMode(outputBuzzer, OUTPUT);
pinMode(outputPanel, OUTPUT);
Serial.begin(9600);

}

void loop()
{
Serial.print(timer1);
buttonState = digitalRead (button);
buttonresetState = digitalRead (buttonreset);//da definire

if (buttonState == HIGH )
{
digitalWrite (outputIndicator, HIGH);
timer1=millis();
digitalWrite (outputBuzzer, LOW);
digitalWrite (outputPanel, LOW);

noInterrupts (); //hack to reset timer
timer0_millis = 0;
interrupts ();

}

else
{
digitalWrite (outputIndicator, LOW);
if (millis()-timer1>2400000 ) //40min
{
digitalWrite (outputBuzzer, HIGH);
// delay(3000);
}

else if (millis()-timer1>2430000 ) //42min
{
digitalWrite (outputPanel, HIGH);
// delay(500);
}

}
//}
}

#2

After 49 days the timer misbehaves and goes haywire.

Used properly, millis() overflow is not a problem. What do you mean by “haywire” ?

Please follow the advice on posting code given in Read this before posting a programming question

In particular note the advice to Auto format code in the IDE and to use code tags when posting code here as it prevents some combinations of characters in code being interpreted as HTML commands such as italics, bold or a smiley character, all of which render the code useless

If the code exceeds the 9000 character inline limit then attach it to a post

#3 
    digitalWrite (outputIndicator, LOW);
    if (millis() - timer1 > 2400000   )      //40min
    {
      digitalWrite (outputBuzzer, HIGH);
    }
    else if (millis() - timer1 > 2430000 )      //42min
    {
      digitalWrite (outputPanel, HIGH);
    }

The 'else' is a problem. It's only going to look for an interval > 40.5 minutes (not 42) when it is NOT TRUE that the interval is greater than 40 minutes. If the interval is not >40 it is certainly not >40.5. Remove the 'else' to check both conditions, or keep the 'else' and put the longer interval first (it will check for >40 when not >40.5).

#4

The millis() timer hack is really bad. Many sketches and libraries will no longer work of you do that.
Did you find that somewhere ? There should be someone explaining how wrong it is.

#5

Since you are working at a time granularity of minutes, the easiest solution is to keep a 32bit minutes counter running. Every 60,000 milliseconds, increment it. Use only the minutes counter in your time calculation.
The minutes counter will also overflow, but only every 60,000 lots of 49 days which should, for all practical purposes, be long enough.

#6

Duplicate topics merged

Why did you start a second one ?

#7

Koepel:
The millis() timer hack is really bad. Many sketches and libraries will no longer work of you do that.
Did you find that somewhere ? There should be someone explaining how wrong it is.

Yes found this on the web and it stated that it should not be used. I just tried it after leaving the arduino on for 50 days and then I tried this to check what would happen...

UKHeliBob:
Duplicate topics merged

Why did you start a second one ?

Sorry I did not see any replies. I will remove now.

UKHeliBob:
Used properly, millis() overflow is not a problem. What do you mean by "haywire" ?

Please follow the advice on posting code given in Read this before posting a programming question

In particular note the advice to Auto format code in the IDE and to use code tags when posting code here as it prevents some combinations of characters in code being interpreted as HTML commands such as italics, bold or a smiley character, all of which render the code useless

If the code exceeds the 9000 character inline limit then attach it to a post

Haywire meaning it does not work after 40minutes than but it was working after 5 minutes then 2min then 40minutes.

#8

I will remove now

The topics have been merged

#9

johnwasser:

    digitalWrite (outputIndicator, LOW);

if (millis() - timer1 > 2400000   )      //40min
   {
     digitalWrite (outputBuzzer, HIGH);
   }
   else if (millis() - timer1 > 2430000 )      //42min
   {
     digitalWrite (outputPanel, HIGH);
   }



The 'else' is a problem. It's only going to look for an interval > 40.5 minutes (not 42) when it is NOT TRUE that the interval is greater than 40 minutes. If the interval is not >40 it is certainly not >40.5. Remove the 'else' to check both conditions, or keep the 'else' and put the longer interval first (it will check for >40 when not >40.5).

I do not know if I understood you correctly.
My logic is this
After 40minutes
Buzzer is triggered
If buzzer is triggered for more than 2 minutes & the push button is not pressed
Relay is ON.
Then i would want to reset the timer after the push button is pressed.
Is this possible using millis?

#10

Then i would want to reset the timer after the push button is pressed

When using millis() for timing you do not "reset the timer", instead you save the value of millis() when an event occurs and make all timing comparisons relative to that using subtraction of the start time from the current time

#11

UKHeliBob:
When using millis() for timing you do not "reset the timer", instead you save the value of millis() when an event occurs and make all timing comparisons relative to that using subtraction of the start time from the current time

But this will be a problem since after 49 days the timer will overflow.
The arduino must be left powered on

#12

But this will be a problem since after 49 days the timer will overflow.

No it won't

Using unsigned values and subtraction for the comparison avoids the problem with overflows. To make it simple let's try an example using byte sized variables which are easier to visualise than unsigned longs

Suppose that you have saved a value of 100 as the start time and the time is now 120. 120 - 100 gives an elapsed time of 20 as you would expect. OK so far

Suppose that you have saved a value of 245 as the start time, that the timer has overflowed at 255 and its value is now 15. 15 - 245 gives an elapsed time of -230 but we are dealing with unsigned values so the result of the subtraction is actually 255 - (15 - 240) which gives an elapsed time of 25, which is correct

#13

This is how to test for a timer expiring:

// repeating timer
unsigned long my_time_variable = 0L ;
#define TIME_DELAY ......

if (millis() - my_time_variable >= TIME_DELAY)
{
  my_time_variable += TIME_DELAY ; // setup for next time
  ...
}

// single-shot timing:
bool my_timer_enabled = false ;

if (my_timer_enabled && my_time_variable >= TIME_DELAY)
{
  my_timer_enabled = false ; // cancel timer
  .....
}

// triggering single shot timer:
{
  my_time_variable = millis() ;
  my_timer_enabled = true ;
}

Note the subtraction in the test is essential to avoid wrap-around issues, this works due to the
wonders of modular arithmetic, so long as the TIME_DELAY is less than 2^32 milliseconds.

Note that for a single-shot timer you must have a state variable to cancel the timer or it will
repeatedly trigger once the timeout is reached - your code didn't have this.

You were trying to piggy back two single-shot timers, which requires two boolean state
variables or equivalently a 3-state integer state variable to record the three states.

And to reiterate: that timer0 hack is bad bad bad...