Help with looping a function

Hello all,

I am new to the world of Arduino and wanted to try a simple project to start off.
Currently what I am trying to do is have a board show LED "animations" and change to the next one after a button is pressed.
I have code that is able to cleanly get button presses, but when I add in a function to turn the LEDs on and off, they either get stuck in a loop or end up running once then wait for the next button press. Does anyone know of a good way to keep the line(); function to keep going until the button is pressed again? I have been racking my brain but I've been unable to come up with a solution using both while or do...while loops.

Here is the skeleton code that I am running:

// Pin number for the LEDs and Button
const int red = 11;
const int yel = 10;
const int gre = 9;
const int btn = 2;


// Button variable for opperation
int buttonState;
int lastButtonState = HIGH;
int btnCounter = 0;

int reading;

// the following variables are unsigned longs because the time, measured in
// milliseconds, will quickly become a bigger number than can be stored in an int.
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

void setup() {
  pinMode(btn, INPUT_PULLUP);
  pinMode(red, OUTPUT);
  pinMode(yel, OUTPUT);
  pinMode(gre, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  // Read the button's state
  reading = digitalRead(btn);

  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        btnCounter++;
      
        switch(btnCounter){  
          case 1 :
            Serial.println("Case 1");
            break;
          case 2: 
            Serial.println("Case 2");
            break;
          case 3:
            Serial.println("Case 3");
            break;
          default: 
            Serial.println("Reset");
            btnCounter = 0;
        }
        
      }
      
    }
    
  }
  
  lastButtonState = reading;
}

void line(){
  digitalWrite(red, HIGH);
  delay(100);
  digitalWrite(yel, HIGH);
  delay(100);
  digitalWrite(gre, HIGH);
  delay(100);
  digitalWrite(red, LOW);
  delay(100);
  digitalWrite(yel, LOW);
  delay(100);
  digitalWrite(gre, LOW);
  delay(100);
}

Step 1 is to look at where your switch case is in the code. It is inside an if statement so it ONLY runs when that button is first pressed. You probably want that out where it runs on every loop and not inside that if statement.

Step 2 is going to be to learn to handle that line() function without using delay. But that’s he next big that’s going to bite.

Yeah, all the if statements in the void loop() section is there to help with the button's input. To my understanding, it makes sure that a button press is not registered milliseconds after the button is actually pressed. When I didn't have that, one press would be registered as two and would mess up. That is why I have that there, is there another way to stop it from registering right away like that?

As for the line() function, is there a way to turn on and off LEDs without using some sort of timer or delay?

And to clarify, I am trying to loop the digitalWrite and delays until the button is pressed again. I tried using do...while loops not too long ago, but what the code segment would only loop while the button is down.

Thank you very much for the time and consideration,
-Miguel :smiley:

anytime you use delay() the sketch freezes and does nothing else. so when you are delaying you cant detect the button push. you can setup another timer just like you did with your lastDebounceTime variable and use it for the lights. this way the lights wont effect the rest of your code. If you are unfamiliar with arrays and loops you should really really read up. they make things so much eaiser and make things possible that wouldnt normaly be.

at the top i would start by declairing the animation arrays... one array for the pins and one array for the state of the pin. then you can easily move through your animation with a timer.

this is untested code but should start and stop your animation when your button is pressed

 // Pin number for the LEDs and Button
const int red = 11;
const int yel = 10;
const int gre = 9;
const int btn = 2;

//here is your animation
int pins []={11,10,9,11,10,9};
int states[]={HIGH,HIGH,HIGH,LOW,LOW,LOW};

int index;

// Button variable for opperation
int buttonState;
int lastButtonState = HIGH;
int btnCounter = 0;
int reading;
unsigned long last;
bool playing;
// the following variables are unsigned longs because the time, measured in
// milliseconds, will quickly become a bigger number than can be stored in an int.
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

void setup() {
  pinMode(btn, INPUT_PULLUP);
  pinMode(red, OUTPUT);
  pinMode(yel, OUTPUT);
  pinMode(gre, OUTPUT);
  Serial.begin(9600);
}

void loop() {
    if(playing){
   if (millis() - 500 > last) {
     last = millis();
     digitalWrite(pins[index],states[index] );
     index++;if(index>5){index=0;}
     } }
     
  reading = digitalRead(btn);

  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        // toggle the animation on and off;
        playing=!playing;
        btnCounter++;
      
        switch(btnCounter){  
          case 1 :
            Serial.println("Case 1");
            break;
          case 2: 
            Serial.println("Case 2");
            break;
          case 3:
            Serial.println("Case 3");
            break;
          default: 
            Serial.println("Reset");
            btnCounter = 0;
        }
        
      }
      
    }
    
  }
  
  lastButtonState = reading;
}

once you understand what is going on with the above code. you can use an array of arrays to list out several different animations. you allready have a variable called btnCounter that tracks the number of times the button was pressed. you can use it to tell the code which animation it should be using.

BTW, i replaced your switch statement with something shorter that does the same thing.

//here is your animations
int pins  [][6]={
  {11,10,9,11,10,9},
  {11,10,9,9,10,9},
  {9,10,11,9,10,11}
};
int states[][6]={
  {HIGH,HIGH,HIGH,LOW,LOW,LOW},
  {HIGH,LOW,HIGH,LOW,HIGH,LOW},
  {LOW,HIGH,HIGH,LOW,HIGH,LOW}
  
  };



int index;

// Button variable for opperation
int buttonState;
int lastButtonState = HIGH;
int btnCounter = 0;
int reading;
unsigned long last;

// the following variables are unsigned longs because the time, measured in
// milliseconds, will quickly become a bigger number than can be stored in an int.
unsigned long lastDebounceTime = 0;  // the last time the output pin was toggled
unsigned long debounceDelay = 50;    // the debounce time; increase if the output flickers

void setup() {

  pinMode(btn, INPUT_PULLUP);
  pinMode(red, OUTPUT);
  pinMode(yel, OUTPUT);
  pinMode(gre, OUTPUT);
  Serial.begin(9600);
}

void loop() {
  
   if (millis() - 500 > last) {
     last = millis();
  
     digitalWrite(pins[btnCounter][index],states[btnCounter][index] );
     index++;if(index>5){index=0;}
     } 
     
  reading = digitalRead(btn);

  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        // move to next animation
        index=0;
        btnCounter++;if( btnCounter>2){ btnCounter=0;}
        Serial.println("Case "+String( btnCounter));
      
        
      }
      
    }
    
  }
  
  lastButtonState = reading;
}

MA0145:
Yeah, all the if statements in the void loop() section is there to help with the button's input. To my understanding, it makes sure that a button press is not registered milliseconds after the button is actually pressed. When I didn't have that, one press would be registered as two and would mess up. That is why I have that there, is there another way to stop it from registering right away like that?

That's not what I'm talking about. You still need that debouncing code and you should keep it. But the switch case part that checks the btnCounter needs to be outside of it.

It needs to be, when the button is debounced and pressed, increase the button count by 1. ALWAYS run the switch and do an action based on the button count.

Right now you have, if the button is debounced and pressed, add one to the button count and run the switch and do an action once but not again.

The loop function is repeating over and over really fast. Or it should be, if you don't use delay. Let it do that and let it call your switch case over and over. That way your case statements run over and over in a loop. See how the loop function works now :wink:

So you have to look at the brackets and figure out what blocks of code belong to what if statements. Move the switch case part out where it's not controlled by those if statements.

void loop() {
  // Read the button's state
  reading = digitalRead(btn);

  if (reading != lastButtonState) {
    // reset the debouncing timer
    lastDebounceTime = millis();
  }

  if ((millis() - lastDebounceTime) > debounceDelay) {
    // whatever the reading is at, it's been there for longer than the debounce
    // delay, so take it as the actual current state:

    // if the button state has changed:
    if (reading != buttonState) {
      buttonState = reading;

      // only toggle the LED if the new button state is HIGH
      if (buttonState == HIGH) {
        btnCounter++;
      }

    }

  }
  switch (btnCounter) {
    case 1 :
      Serial.println("Case 1");
      break;
    case 2:
      Serial.println("Case 2");
      break;
    case 3:
      Serial.println("Case 3");
      break;
    default:
      Serial.println("Reset");
      btnCounter = 0;
  }


  lastButtonState = reading;
}

MA0145:
As for the line() function, is there a way to turn on and off LEDs without using some sort of timer or delay?

Once we have things running over and over like I said in the last reply, you can write the line function to use the "Blink Without Delay" method of timing. There's an example in the IDE, and tons and tons of tutorials on the topic. Just google "Blink Without Delay" and read until you get it. Instead of waiting to turn the light off, you let the function get called over and over and over really fast and constantly checking to see if it is time to turn it off. Most of the time your function does nothing and just returns. But every now and then it's time to toggle a light and it does that and then returns.