Problem with millis() timing

[butchmarzee]

Hi,

I started experiencing using millis(), and I immediately ran into a problem.
The code is simple, I press a button, LED should come one for 5 sec and off. The problem is the 5 sec sometimes 3 ish, sometimes 5 ish, not 5 all the time. I can't see where the problem is in the code?
could you guys help me what I am doing wrong?

const int button = 3;
const int led = 8;

int buttonState1 = 0;

unsigned long startMillis;
unsigned long currentMillis;
const int periodled = 5000;

boolean ledState1 = false;


void setup() {

  pinMode(button, INPUT_PULLUP);
  pinMode(led, OUTPUT);
  startMillis = millis();

}

void loop() {
 
   if (ledState1 == true) {
    digitalWrite(led, HIGH);
}
  else {
    digitalWrite(led, LOW);
  }   
 
  buttonState1 = digitalRead(button);

  if (buttonState1 == LOW) {
     ledState1 = true;
     }

    if (ledState1 == true) {
     currentMillis = millis();
      
      if (currentMillis - startMillis >= periodled) {
        ledState1 = false;
        startMillis = currentMillis;
          
  }
             
   } 
  
} 

[UKHeliBob]

You aren't you setting startMillis to currentMillis when you find the button pressed

[J-M-L]

startMillis Should be set when you turn the light on. There is no need to keep turning it on. The test on millis()-startMillis will be used to turn it off.

[butchmarzee]

could you show me where to put it?

[LarryD]

void loop() 
{
  currentMillis = millis();

  buttonState1 = digitalRead(button);
  
  //was there a change in switch state ?
  if(lastButtonState1 != buttonState1)
  { 
     lastButtonState1 = buttonState1; // update to the new state
 
     if (ledState1 == false && buttonState1 == HIGH)
     {
     ledState1 = true;        //enable the TIMER
     startMillis = millis();  //reset the TIMER
     digitalWrite(led, HIGH); //turn ON the LED
     }
   }
     
   if (ledState1  == true && currentMillis - startMillis >= periodled)
      {
        ledState1 = false;         //disable the TIMER
        digitalWrite(led, LOW);    //turn OFF the LED
      } 
  
}  //END OF loop( )

[GolamMostafa]

@butchmarzee

could you show me where to put it?

1. Can you try to create a sketch where L (Built-in Led of UNO) will remain ON for 5-sec once button is found closed; where, the ON-period will be implemented by delay(5000) function?

2. Once you have achieved the above objective, then try to implement the ON-period by millis() function instead of delay() function; as a result, you will be able to figure out where to put the startMillis variable.

[Idahowalker]

Just a few comments.

@butchmarzee

startMillis = currentMillis;

Consider when did your program get the value currentMillis verses how current startMillis wouold be if you did StartMillis = millis().

@butchmarzee

buttonState1 = digitalRead(button);

  if (buttonState1 == LOW) {
     ledState1 = true;
     }

Especially if using a Uno, try to save memory use space.

No need for a variable that is used once and thrown away with each iteration end, then is created again at the next iteration.

if (digitalRead(button) == LOW) {
ledState1 = true;
}

Don't forget to attend to the suggestions above this post, that will go a long way towards solving the thingy.

@butchmarzee

if (currentMillis - startMillis >= periodled)

By the time your code gets to here that is no longer current millis, It takes 4 or 5 milliseconds just to execute/fetch millis().

if (millis()- startMillis >= periodled)

[butchmarzee]

thanks, i think now i see it

[butchmarzee]

thanks, I do it again from zero

[GolamMostafa]

@butchmarzee

thanks, I do it again from zero

Tell us if you have written the codes of Post-1 yourself or you have copied it from somewhere?

[butchmarzee]

i used the millis() idea from here:

Using millis() for timing. A beginners guide

Part 1 It is not usually long before new Arduino users discover that although the delay() function is easy to use it has side effects, the main one of which is that its stops all activity on the Arduino until the delay is finished (not quite true, I know, but that is usually how the problem presents itself). When this occurs the new user is usually directed to the BlinkWithoutDelay example sketch (BWoD) and/or the excellent thread entitled Several things at the same time. Several things at the…

the rest was mine

[UKHeliBob]

@butchmarzee

i used the millis() idea from here:

Using millis() was a good idea, but you used it incorrectly

Are you clear what you did wrong ?

[GolamMostafa]

@butchmarzee

i used the millis() idea from here:

You need to understand the working principle of millis() function, which is as follows:

1. After the uploading of a sketch in UNO, a 32-bit millisCounter is started with the ATmega328P MCU of the UNO Board. The millisCounter is updated/incremented by 1 ms time period in the background on interrupt basis. The content of the millisCounter (the elapsed time since the UNo has started) can be known at any time by executing the following code:

unsigned long prMillis = millis();   //prMillis = present millis of the millisCounter

2. The simple soltion to your problem should stand as:

#define Led 13
unsigned long prMillis;
const int button = 3;

void setup()
{
  Serial.begin(9600);
  pinMode(button, INPUT_PULLUP);
  pinMode(Led, OUTPUT);
}

void loop()
{
  if(digitalRead(button) == LOW)
  {
    prMillis = millis();
    while(millis() - prMillis < 5000)
    {
      digitalWrite(Led, HIGH);
    }
    digitalWrite(Led, LOW);  
  }
} 

3. Now you can add your advanced programming features (like checking the last states of the button and Led) with the sketch of Step-2.

4. At the very first hand, get/make something that works and then add the complexities.

[butchmarzee]

i guess because the startMillis was in the setup(), it ran once, but i kept updated it with startMillis = currentMillis;

[butchmarzee]

I think I started to understand
This one seems to be working fine:

const int button = 3;
const int led = 8;

unsigned long startMillis;
unsigned long period = 5000;

boolean ledState = false;

void setup() {
  Serial.begin(115000);
  pinMode(button, INPUT_PULLUP);
  pinMode(led, OUTPUT);

}

void loop() {

  Serial.println(ledState);
  Serial.println(millis() - startMillis);
  
  if(digitalRead(button) == LOW) {
    startMillis = millis();
    ledState = true;
  }
    if((ledState == true) && (millis() - startMillis >= period)) {
      ledState = false;
     }
    
   if(ledState == true){
    digitalWrite(led, HIGH);
  }
  else{
    digitalWrite(led, LOW);
  }
}

[GolamMostafa]

@butchmarzee

if((ledState == true) && (millis() - startMillis >= period)) {
      ledState = false;
     }

Congratulations! Your sketch works fine.

However, the above quoted codes should logically be appearing at the end (after the else clause).

[butchmarzee]

like this?

const int button = 3;
const int led = 8;

unsigned long startMillis;
unsigned long period = 5000;

boolean ledState = false;

void setup() {
  Serial.begin(115000);
  pinMode(button, INPUT_PULLUP);
  pinMode(led, OUTPUT);

}

void loop() {

  Serial.println(ledState);
  Serial.println(millis() - startMillis);
  
  if(digitalRead(button) == LOW) {
    startMillis = millis();
    ledState = true;
  }
   
   if(ledState == true){
    digitalWrite(led, HIGH);
  }
  else{
    digitalWrite(led, LOW);
  }

  if((ledState == true) && (millis() - startMillis >= period)) {
      ledState = false;
  }

}

[GolamMostafa]

@butchmarzee

like this?

Yes!

Your sketch of Post-17 and my sketch of Post-13 are doing the same job; but, there is a difference in respect of which your sketch is better. Can you tell the difference?

[butchmarzee]

great! now I gonna put more buttons and more leds. Just to experience a little bit.
and maybe put each one into separate functions.

[paulpaulson]

Hello
Take some time and study the application of data structures.
It is simple make von struct with members of the pin addresses for LED, button and assioates states. Now you need one piece of code, called method, to work with the contents of the data structure and not a many pieces of code for each LED and button.
This is called Object-oriented programming (OOP) and is well supported by the Arduino family.
There is no need to understand or make a class definition. It is more usefull to learn how array´s, enums´s and struct´s are working together to have the first C++ program. That makes fun.
Have a nice day and enjoy coding in C++.