How do I make LEDs lights up one by one according to how long the button was pressed and hold

Question

In my project, I am trying to create a timer display with LEDs base on how long a button was pressed and hold.

There are 8 LED lights in total, when the toggle button was pressed, 1st LED lights up; keep holding/ pressing the button, the 2nd LED lights up after 1sec, the 3rd LED lights up after 2sec and so on. So 7s after the button was pressed and hold, all 8 LEDs shall light up. When the button was released at anytime, all LEDs would be off.

Something is wrong with my code which the timer seems not working (saving the time when the button was pressed) and calculate the time difference ("current time" - "time when button pressed") in order to lights up the LED.

My codes:

 int current;
 long millis_held;      
 long prev_secs_held;
 unsigned long firstTime;     
 byte previous = LOW;    

 const int buttonPin = 13;

 void setup() {
 pinMode(buttonPin, INPUT);
 pinMode(2,OUTPUT) ;
 pinMode(3,OUTPUT) ;
 pinMode(4,OUTPUT) ;
 }

 void loop() {
 current = digitalRead(buttonPin);

 if (current == HIGH && previous == LOW) { 
 firstTime = millis();
 digitalWrite(2, HIGH);

 millis_held = (millis() - firstTime);

 if (millis_held >= 1000 && firstTime <2000){
 digitalWrite(3, HIGH);
 }

  if (millis_held >= 2000){
  digitalWrite(4, HIGH);
 }
 }

 else {
 digitalWrite(2, LOW);
 digitalWrite(3, LOW);
 digitalWrite(4, LOW);
 }
 }

Hope you guys can help, thanks a lot.

Answer 1

There are multiple errors in your code.

In the loop, the first if might not have the use you intended it to have. You are checking for previous == low, but you never set previous to high. Therefore this check can be removed entirely, or you could restructure your code a bit and add a previous = high.

Secondly, this if is also flawed:

if (millis_held >= 1000 && firstTime <2000){
    digitalWrite(3, HIGH);
}

You are checking for firstTime < 2000 which means you need to press the button within 2 seconds of the power-up of the Arduino or else this code will not run.

Lastly, for this to run you probably only want to set firstTime = millis() once when you press the button, and not every time the loop() runs thereafter while you keep the button pressed. You would need to move the bracket on line 32 to line 22, that is from just above the else to the line above millis_held = (millis() - firstTime); and then add previous = HIGH; in the first if.

I am leaving it to you to fix your code, since that would be good practice to get some feeling for it. Happy coding!

Lastly you still need to add the last 5 leds, but once the first few work you should be fine.

Answer 2

Great thanks to Ananas_hoi, I was finally able to get my code work accordingly!! The following code only show the 4 LED to be lights up one by one every second when the button was pressed and hold:

int current;
long millis_held;      
unsigned long firstTime; 
byte previous = LOW;         

const int buttonPin = 13;

void setup() {
  pinMode(buttonPin, INPUT);
  pinMode(2,OUTPUT) ;  
  pinMode(3,OUTPUT) ;
  pinMode(4,OUTPUT) ;
  pinMode(5,OUTPUT) ; 
}

void loop() {
  current = digitalRead(buttonPin); 

  if (current == HIGH && previous == LOW) { 
     firstTime = millis();
     digitalWrite(2, HIGH);
     previous = HIGH;
  }
 
  millis_held = (millis() - firstTime);

  if (millis_held >= 1000 && previous == HIGH) {
      digitalWrite(3, HIGH);
     }
 
  if (millis_held >= 2000 && previous == HIGH) {
      digitalWrite(4, HIGH);
     }
 
  if (millis_held >= 3000 && previous == HIGH) {
      digitalWrite(5, HIGH);
     }


  if (current == LOW) {
  digitalWrite(2, LOW);
  digitalWrite(3, LOW);
  digitalWrite(4, LOW);
  digitalWrite(5, LOW);
  previous = LOW;
 }
}