millis() goes haywire!

From a much longer sketch that controls two electric gates I have cut the timer related code and pasted it into the attached sketch.
It can be compiled and uploaded for the UNO, the serial monitor shows the seconds count to 30, then a 10 sec. delay and the counting continues as planned.

In the original sketch the following happens:

After an elapsed time of approx. 10 seconds the 1th digit in the lcd display suddenly increments from 1X to 2X and 3X. Then the time-out situation is reached after approx. 13 seconds and all operations are terminated for 10 seconds!
It seems to me both timer fields suddenly receive 10.000 instead of 1000 count/second.

Is there something wrong with my coding or can there be an external factor like relay switching?

test.ino (1.78 KB)

Just post the code - don’t make people download it.

/*
  Excerpt from a long sketch controlling electric gates.
  This is the relevant code that displays seconds on the LCD and
  terminates a process in case of mechanical failure.
 */
 #include <LiquidCrystal_I2C.h>
#include <Wire.h>
LiquidCrystal_I2C lcd(0x27, 2, 1, 0, 4, 5, 6, 7, 3, POSITIVE);  // Set the LCD I2C address

  unsigned long timer; // timer field
  unsigned long startTime;
  int elapsedTime = 30000;//gates must open/close within 30 sec.
  boolean timedOut = false; // set to true when timer fired
  unsigned long previousMillis = 0;
  const long interval = 1000; 
  byte gateClosing = LOW;
  byte gateOpening = LOW;
int displaytimer = 0;

void setup() {
  // initialize serial communication at 9600 bits per second:
  Serial.begin(9600);
  
}


void loop() {
  
  
  unsigned long currentMillis = millis();
  if(currentMillis - previousMillis >= interval) {
   
    previousMillis = currentMillis; 
  displaytimer = displaytimer +1;
  if (displaytimer == 30)
  {displaytimer = 0;
 lcd.noBacklight(); }//reduce power consumption
  Serial.print ("timer  ");
  Serial.println (displaytimer);
 }
 timer = millis();
 lcd.setCursor(7,1);
 lcd.print(displaytimer);
  /* Here is what happens in case for any reason a timed out situation
 is reached while the gates are still moving. This will only happen when
 the battery voltage is too low for normal operation or in case of a
 major mechanical failure, requiring abortion of any previous command*/ 
    
 if ((!timedOut) && (startTime + elapsedTime)<= timer) 
 {
    timedOut = true; // end of timing cycle
    //irrelevant code removed
    gateOpening = LOW; //gate stopped opening
    
    gateClosing = LOW; //gate stopped closing
    lcd.setCursor(8,0);
    lcd.print("Timeout");
    delay(10000); //allow the user to figure out what went wrong
 }
 

}

all operations are terminated for 10 seconds!

As instructed?

delay(10000); //allow the user to figure out what went wrong

Yes, except that the point is reached after 13 seconds instead of 30.

So, you have posted code that works OK but not the code that doesn't. I suspect that we are not going to get very far like this.

the 1th digit in the lcd display suddenly increments from 1X to 2X and 3X.

I cannot make any sense of this but a common problem with LCDs is not clearing the data previously printed leaving it visible on the screen when new data is output. Of course, this does not happen with the Serial monitor.

This looks as if written by several people.
First you have a correct usage of a millis() based timing sequence
set up as a 1 second repeating timer when it looks like you really wanted a 30 second timer, but OK

unsigned long currentMillis = millis();
  if(currentMillis - previousMillis >= interval) {
   
    previousMillis = currentMillis;
  displaytimer = displaytimer +1;
  if (displaytimer == 30)
  {displaytimer = 0;
 lcd.noBacklight(); }//reduce power consumption
  Serial.print ("timer  ");
  Serial.println (displaytimer);
 }

Then you have a incorrect usage of millis() timing

if ((!timedOut) && (startTime + elapsedTime)<= timer)
 {
    timedOut = true; // end of timing cycle
    //irrelevant code removed
    gateOpening = LOW; //gate stopped opening
   
    gateClosing = LOW; //gate stopped closing
    lcd.setCursor(8,0);
    lcd.print("Timeout");
    delay(10000); //allow the user to figure out what went wrong
 }

Incorrect because you should never use addition when dealing with millis() only subtration.
Since you don’t show what startTime’s value is I can’t tell when it fires off, but I bet this is where the problem lies.

A delay() inside of a timing sequence??? :roll_eyes:

Nothing obviously wrong.

But,

    lcd.setCursor(7,1);
    lcd.print(displaytimer);

should really only be run when you want something on the LCD to change. i.e.: inside the "if" when the displaytimer number changes, otherwise you are writing out to the LCD every time you go through the loop.

And to be sure the LCD only shows what you want it to, you could clear the LCD for now, or append a space after the number, or make sure you always print a specific number of digits for the displaytimer.

For now, put it inside the "if" when you change the displaytimer, and make it:

    lcd.clear();
    lcd.setCursor(7,1);
    lcd.print(displaytimer);

@Hutkikz

Since you don't show what startTime's value is I can't tell when it fires off, but I bet this is where the problem lies.

startTime = 0 so the limit is currently 0 + 30 seconds.

@Hutkikz

A delay() inside of a timing sequence???

Sure. Why not? It's there for when something has failed, and he wants to pause everything while he makes sure the user reads the error message.

arduinodlb:
startTime = 0 so the limit is currently 0 + 30 seconds.

unsigned long startTime;

That does not = 0

Hutkikz:

unsigned long startTime;

That does not = 0

yes it does.

Hutkikz:

unsigned long startTime;

That does not = 0

It does by the time setup runs.

Ok I thought unless it is initialized it held whatever was left over in that memory address.

I still can't find where it was initialized to 0.

Hutkikz:
This looks as if written by several people.
First you have a correct usage of a millis() based timing sequence
set up as a 1 second repeating timer when it looks like you really wanted a 30 second timer, but OK

unsigned long currentMillis = millis();

if(currentMillis - previousMillis >= interval) {
 
   previousMillis = currentMillis;
 displaytimer = displaytimer +1;
 if (displaytimer == 30)
 {displaytimer = 0;
lcd.noBacklight(); }//reduce power consumption
 Serial.print ("timer  ");
 Serial.println (displaytimer);
}



Then you have a incorrect usage of millis() timing


if ((!timedOut) && (startTime + elapsedTime)<= timer)
{
   timedOut = true; // end of timing cycle
   //irrelevant code removed
   gateOpening = LOW; //gate stopped opening
 
   gateClosing = LOW; //gate stopped closing
   lcd.setCursor(8,0);
   lcd.print(“Timeout”);
   delay(10000); //allow the user to figure out what went wrong
}



Incorrect because you should never use addition when dealing with millis() only subtration.
Since you don't show what startTime's value is I can't tell when it fires off, but I bet this is where the problem lies.

A delay() inside of a timing sequence??? :roll_eyes:

Cutting and pasting from another sketch clearly wasn’t a good idea.
In the actual sketch startTime is set to “timer” when a button is pressed (Vehicle, Pedestrian, Close). Any of the actions that follow must be completed within 30 sec.
To omit addition with millis() I will rewrite the time-out code and use only seconds from the 1th part.

The delay at the end serves an important purpose: users can press any button but nothing will happen for 10 seconds. A timeout situation normally should never occur, if it does, something is definitely wrong.

Hutkikz:
Ok I thought unless it is initialized it held whatever was left over in that memory address.

I still can't find where it was initialized to 0.

crt0

@CKD1
Ok that makes sense

@AWOL
Ok I never heard of that. All the sources I've read (on the Internet) stated that uninitialized variables held garbage. So it is not necessary to initialize prior to set up then?

Hutkikz:
All the sources I've read (on the Internet) stated that uninitialized variables held garbage. So it is not necessary to initialize prior to set up then?

Links?

All the sources I've read (on the Internet) stated that uninitialized variables held garbage.

No, only local variables (aka automatic variables) that are not qualified "static" are uninitialised.
Any global or static variable, that is not explicitly initialised to some other value, is set to zero before "main()" runs by crt0.
This includes arrays, strings etc.

@AWOL
Good to know Thank You.

@ CKD1
Sorry for the hi-jack

Hutkikz:
@AWOL
Good to know Thank You.

@ CKD1
Sorry for the hi-jack

No problem, it's all part of my learning process.
In the meantime I have rewritten the sketch and wanted to present it here in complete form, but the forum script refused it as being more than 9000 characters (?)
I do not think that is correct, they probably also count spaces.

If it’s too big, attach it to a post, though it’s hard to imagine why it has gone from <2k to >9k.

The complete code as it runs now is in the attachment.
The lcd is still not counting the way it should, but the timeout part works as planned, i.e. the gates reach their end position before the limit set.
Another small issue (probably not related) is the left gate that doesn’t stop at halfOpen but goes all the way.

sketch_gate.ino (11.5 KB)