Stalling LED sequence on push button

hey guys,

I am having an issue with the second push button on the code included.

The first push button works as i want and returns to the }else{ to run the constant function.

My problem is the second push button… when i press it, it runs the function to the end then stalls on the last point, not running for 10secs as i intend. The first button can be pressed again and runs, but then returns to a stalled position from the end of chase 2.

What i’d really/hope for and dream of is it to run the chase2 to for 10secs and return to the else condition, as it does for the chase1

Still pretty new to this and cant seem to figure what i am missing or have done wrong.

Any advice or direction would be much appriciated.

thanks in advance :slight_smile:

const int led1 = 2;
const int led2 = 3;
const int led3 = 4;
const int led4 = 5;
const int led5 = 10;
const int led6 = 7;

const int pushButton1 = 9;
const int pushButton2 = 8;
int buttonState1 = LOW;
int buttonState2 = LOW;

unsigned long currentMillis;
unsigned long currentMillis2;
unsigned long currentMillis3;
unsigned long interval = 500;
unsigned long interval2 = 250;
unsigned long interval3 = 750;
unsigned long previousMillis = 0;       
unsigned long previousMillis2 = 0;
unsigned long previousMillis3 = 0;
unsigned long duration1 = 0;
unsigned long duration2 = 0;

int ledState = LOW;
int ledState2 = LOW;
int ledState3 = LOW;
int ledState4 = LOW;
int ledState5 = LOW;

int i = 0;
int j = 0;

void setup() {

    pinMode(led1, OUTPUT);
    pinMode(led2, OUTPUT);
    pinMode(led3, OUTPUT);
    pinMode(led4, OUTPUT);
    pinMode(led5, OUTPUT);
    pinMode(led6, OUTPUT);

    pinMode(pushButton1, INPUT_PULLUP);
    pinMode(pushButton2, INPUT_PULLUP);
  
Serial.begin(9600);
}

void loop() 
{
  Serial.println("read PB1&2");
  buttonState1 = digitalRead(pushButton1);
    buttonState2 = digitalRead(pushButton2);
    Serial.println(buttonState1);
    Serial.println(buttonState2);
if (buttonState1 == HIGH)
{
  duration1 = millis() + 10000;
   Serial.println("create duration1");
   delay(150);
}
Serial.println("read mills1");
  if(millis() <= duration1)
  {
chase();

}
Serial.println("read PB2");

if (buttonState2 == HIGH)
{
  duration2 = millis() + 10000;
Serial.println("create duration2");
delay(150);
}
Serial.println("read millis2");
  if(millis <= duration2)
{
  chase2();
  
}else{
  Serial.println("read else");
 twoByTwo();
}
}
//end of main loop

  void chase()
  {
  currentMillis = millis();   
  if(currentMillis - previousMillis >= interval) {   
    previousMillis = previousMillis + interval; 
digitalWrite (led6, HIGH);
Serial.println("chase");
    i = i+1;       
    if (i==5){
      i = 0;
    }
    switch(i){
    case 0:
      digitalWrite (led1, HIGH);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, LOW); 
      digitalWrite (led5, LOW);  
      Serial.println("chasePt1");
      break;
    case 1:
      digitalWrite (led1, LOW);
      digitalWrite (led2, HIGH);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, LOW);  
      digitalWrite (led5, LOW); 
      Serial.println("chasePt2");
      break;
    case 2:
      digitalWrite (led1, LOW);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, HIGH);
      digitalWrite (led4, LOW);  
      digitalWrite (led5, LOW); 
      Serial.println("chasePt3");
      break;
       case 3:
      digitalWrite (led1, LOW);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, HIGH);  
      digitalWrite (led5, LOW); 
      Serial.println("chasePt4");
      break;
        case 4:
      digitalWrite (led1, LOW);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, LOW);  
      digitalWrite (led5, HIGH);
      Serial.println("chasePt5"); 
      break;
    }
  }
  }
//end chase

   void chase2()
  {
  currentMillis2 = millis();   
  if(currentMillis2 - previousMillis2 >= interval2) {   
    previousMillis2 = previousMillis2 + interval2; 
digitalWrite (led6, HIGH);
Serial.println("chase2");
    j = j+1;       //i is either 0, 1 or 2
    if (i==5){
      j = 0;
    }
    switch(j){
    case 0:
      digitalWrite (led1, HIGH);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, LOW);
      digitalWrite (led5, HIGH); 
      Serial.println("chase2pt1"); 
      break;
    case 1:
      digitalWrite (led1, HIGH);
      digitalWrite (led2, HIGH);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, LOW);
      digitalWrite (led5, LOW);  
      Serial.println("chase2pt2");
      break;
    case 2:
      digitalWrite (led1, LOW);
      digitalWrite (led2, HIGH);   
      digitalWrite (led3, HIGH);
      digitalWrite (led4, LOW);
      digitalWrite (led5, LOW);
      Serial.println("chase2pt3");  
      break;
       case 3:
      digitalWrite (led1, LOW);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, HIGH);
      digitalWrite (led4, HIGH); 
      digitalWrite (led5, LOW);
      Serial.println("chase2pt4"); 
      break;
       case 4:
      digitalWrite (led1, LOW);
      digitalWrite (led2, LOW);   
      digitalWrite (led3, LOW);
      digitalWrite (led4, HIGH); 
      digitalWrite (led5, HIGH); 
      Serial.println("chase2pt5");
      break;
    }
  }
  }

  ///end chase2

 void twoByTwo(){
 unsigned long currentMillis = millis();
Serial.println("2b2onTime");
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;

    
    if (ledState == LOW) {
      ledState = HIGH;
      ledState2 = HIGH;
      ledState3 = HIGH;
      ledState4 = HIGH;
       ledState5 = HIGH;
    } else {
      ledState = LOW;
      ledState2 = LOW;
      ledState3 = LOW;
      ledState4 = LOW;
       ledState5 = LOW;
    }

    digitalWrite(led1, !ledState);
    digitalWrite(led2, ledState2);
    digitalWrite(led3, !ledState3);
    digitalWrite(led4, ledState4);
    digitalWrite(led5, !ledState5);
    digitalWrite(led6, LOW);
  }
}

This:

if (millis <= duration2)

is taking the pointer to the millis() function and comparing that pointer to a unsigned long variable. The compiler does not like that, but it only gives a warning, not an error.

Please press Ctrl+T, because the text layout is a mess.

The next part is good. If you use millis() always like that, then millis() is working.

void twoByTwo()
{
  unsigned long currentMillis = millis();
  if (currentMillis - previousMillis >= interval)
  {
    previousMillis = currentMillis;

But you use also this:

void chase2()
{
  currentMillis2 = millis();
  if (currentMillis2 - previousMillis2 >= interval2)
  {
    previousMillis2 = previousMillis2 + interval2;

Use that only when you need it, it is safer this way:

previousMillis2 = currentMillis2;

Avoid these:

duration2 = millis() + 10000;

if (millis <= duration2)

That will cause a rollover problem.

I prefer to declare local variables only locally. You have now a lot of global variables.

I also prefer to have the millis() timers running in the loop() or in a function that is called from the loop(). I use ‘bool’ variables to tell other parts of the sketch in what state the sketch is in.

Now that I mentioned the ‘state’ of a sketch, have you heard of a Finite State Machine before ?
https://majenko.co.uk/blog/finite-state-machine

I do not fully understand what you want to do. Can you give a good description of what should happen in every situation.

When you want to have fun with examples with millis(), ‘bool’ variable and a Finite State Machine, you could try a few examples I made.

oh wow.. thank-you for a speedy response!

The project is for a lighting/sound rig for a game-show, i will connect it to a relay board that will control 180 lights (linked in sets of 5) in a circle as well as 2 siren spinney/flashy light things.
I want the lights to flicker when idle then have two different chase patterns, 1 at a time (chase) or 2 at a time (chase2). i am also looking to link in a adafruit soundboard for a 'WINNER' and 'LOOSER'.
so effectively 2 buttons one win, one loose..

Pretty simple idea, but the deeper i go the more complicated, yet exciting this becomes, haha.

i have to go to the countryside and will have no wifi, but i have loaded up all the links/sketches you sent and am very greatful.

and thanks for the ctrl+T pointer, i will use this in future :slight_smile:

So there are three different states for the sketch: idle, chase, chase2.
You don't have to make a Finite State Machine, you can use two 'bool' variables or use the state of the buttons as you have now.

When the button is pressed, then that pattern should run, and when the button is release it will return to the idle pattern ?
Where do those 10 seconds come from ?

Apologies for the late response but i have been away for a few days.

the ten seconds is going to be adapted to the side show sirens and sound bites. I need it to come on for a set length of time when the button is pressed and run for that time, then return to the running/idle state.

i sorted out with your suggestion and the program worked perfectly, exactly how i need it to.

however, i have just sat down to add in a little more and the buttons are just being read as closed/HIGH/1 constantly, i though it could be my wiring but i ran the code on a arduino with no wires attached and still Serial.prints the same with no wires attached. Am i missing something here, pretty sure i haven’t messed the code up since saturday.

const int led1 = 2;
const int led2 = 3;
const int led3 = 4;
const int led4 = 5;
const int led5 = 7;
const int led6 = 10;



const int pushButton1 = 9;
const int pushButton2 = 8;
int buttonState1 = LOW;
int buttonState2 = LOW;



unsigned long currentMillis;
unsigned long currentMillis1;
unsigned long currentMillis2;
unsigned long interval = 500;
unsigned long interval2 = 250;
unsigned long interval3 = 750;
unsigned long previousMillis = 0;

unsigned long duration1 = 0;
unsigned long duration2 = 0;

int ledState = LOW;
int ledState2 = LOW;
int ledState3 = LOW;
int ledState4 = LOW;
int ledState5 = LOW;

int i = 0;
int j = 0;

void setup() {

  pinMode(led1, OUTPUT);
  pinMode(led2, OUTPUT);
  pinMode(led3, OUTPUT);
  pinMode(led4, OUTPUT);
  pinMode(led5, OUTPUT);
  pinMode(led6, OUTPUT);

  pinMode(pushButton1, INPUT_PULLUP);
  pinMode(pushButton2, INPUT_PULLUP);

  Serial.begin(9600);
}

void loop()
{
  Serial.println("read PB1&2");
  buttonState1 = digitalRead(pushButton1);
  buttonState2 = digitalRead(pushButton2);
  Serial.println(buttonState1);
  Serial.println(buttonState2);
  if (buttonState1 == HIGH)
  {
    currentMillis1 = millis();
    duration1 = currentMillis1 + 5000;
    Serial.println("create duration1");
    delay(150);
  }
  Serial.println("read mills1");
  if (currentMillis <= duration1)
  {
    chase2();

  }
  Serial.println("read PB2");

  if (buttonState2 == HIGH)
  {
    currentMillis2 = millis();
    duration2 = currentMillis2 + 5000;
    Serial.println("create duration2");
    delay(150);
  }
  
  Serial.println("read millis2");
  if (currentMillis <= duration2)
  {
    chase();

  } else {
    Serial.println("read else");
    twoByTwo();
  }
}
//end of main loop

void chase()
{
  currentMillis = millis();
  if (currentMillis - previousMillis >= interval) {
    previousMillis = previousMillis + interval;
    digitalWrite (led6, HIGH);
    Serial.println("chase");
    i = i + 1;
    if (i == 5) {
      i = 0;
    }
    switch (i) {
      case 0:
        digitalWrite (led1, HIGH);
        digitalWrite (led2, LOW);
        digitalWrite (led3, LOW);
        digitalWrite (led4, LOW);
        digitalWrite (led5, LOW);
        Serial.println("chasePt1");
        break;
      case 1:
        digitalWrite (led1, LOW);
        digitalWrite (led2, HIGH);
        digitalWrite (led3, LOW);
        digitalWrite (led4, LOW);
        digitalWrite (led5, LOW);
        Serial.println("chasePt2");
        break;
      case 2:
        digitalWrite (led1, LOW);
        digitalWrite (led2, LOW);
        digitalWrite (led3, HIGH);
        digitalWrite (led4, LOW);
        digitalWrite (led5, LOW);
        Serial.println("chasePt3");
        break;
      case 3:
        digitalWrite (led1, LOW);
        digitalWrite (led2, LOW);
        digitalWrite (led3, LOW);
        digitalWrite (led4, HIGH);
        digitalWrite (led5, LOW);
        Serial.println("chasePt4");
        break;
      case 4:
        digitalWrite (led1, LOW);
        digitalWrite (led2, LOW);
        digitalWrite (led3, LOW);
        digitalWrite (led4, LOW);
        digitalWrite (led5, HIGH);
        Serial.println("chasePt5");
        break;
    }
  }
}
//end chase

void chase2()
{
  currentMillis = millis();
  if (currentMillis - previousMillis >= interval2) {
    previousMillis = previousMillis + interval2;
    digitalWrite (led6, HIGH);
    Serial.println("chase2");
    j = j + 1;     //i is either 0, 1 or 2
    if (j == 5) {
      j = 0;
    }
    switch (j) {
      case 0:
        digitalWrite (led1, HIGH);
        digitalWrite (led2, LOW);
        digitalWrite (led3, LOW);
        digitalWrite (led4, LOW);
        digitalWrite (led5, HIGH);
        Serial.println("chase2pt1");
        break;
      case 1:
        digitalWrite (led1, HIGH);
        digitalWrite (led2, HIGH);
        digitalWrite (led3, LOW);
        digitalWrite (led4, LOW);
        digitalWrite (led5, LOW);
        Serial.println("chase2pt2");
        break;
      case 2:
        digitalWrite (led1, LOW);
        digitalWrite (led2, HIGH);
        digitalWrite (led3, HIGH);
        digitalWrite (led4, LOW);
        digitalWrite (led5, LOW);
        Serial.println("chase2pt3");
        break;
      case 3:
        digitalWrite (led1, LOW);
        digitalWrite (led2, LOW);
        digitalWrite (led3, HIGH);
        digitalWrite (led4, HIGH);
        digitalWrite (led5, LOW);
        Serial.println("chase2pt4");
        break;
      case 4:
        digitalWrite (led1, LOW);
        digitalWrite (led2, LOW);
        digitalWrite (led3, LOW);
        digitalWrite (led4, HIGH);
        digitalWrite (led5, HIGH);
        Serial.println("chase2pt5");
        break;
    }
  }
}

///end chase2

void twoByTwo() {
  unsigned long currentMillis = millis();
  Serial.println("2b2onTime");
  if (currentMillis - previousMillis >= interval) {
    previousMillis = currentMillis;


    if (ledState == LOW) {
      ledState = HIGH;
      ledState2 = HIGH;
      ledState3 = HIGH;
      ledState4 = HIGH;
      ledState5 = HIGH;
    } else {
      ledState = LOW;
      ledState2 = LOW;
      ledState3 = LOW;
      ledState4 = LOW;
      ledState5 = LOW;
    }

    digitalWrite(led1, !ledState);
    digitalWrite(led2, ledState2);
    digitalWrite(led3, !ledState3);
    digitalWrite(led4, ledState4);
    digitalWrite(led5, !ledState5);
    digitalWrite(led6, LOW);
  }
}

ignore that.. sorted it :slight_smile:

thanks for you help!

sorry if this is off topic but the long switch statement are difficult to read. if you learn loops and arrays you can do all kinds of cool stuff.

you could replace your entire switch statements with these lines!!!!

int mypins []={2,3,4,5,7,10};

//replace first switch
int n = 6;
while(n>0){n--;
if(i==n){ digitalWrite (mypins[n], HIGH);}
else{digitalWrite (mypins[n], LOW);}

}


//replace your chase pattern

int n2;
int n = 6;
if(i==0){n2=5;}else{n2=i-1;}
while(n>0){n--;
if(i==n||n2==n){ digitalWrite (mypins[n], HIGH);}
else{digitalWrite (mypins[n], LOW);}

}

then you could eisily expand to however many lights and do fancier patterns through a bit of math if you ever want to add more to your project. and you wouldnt have to write it all out!