Arduino timer with set/reset buttons

Question

i'm building an automatic hay feeder for my horse,something like this but with just two floors

every 24hrs the plate in the middle should fall and let the hay fall down for the horse to eat,the interface should be very simple,3 buttons one to start the timer, one to stop it and deploy the hay and one to reset the servo in his initial position to lock the plate again, i'm having some problems with the timer,it starts when i press the green button but after it finishes to count it stops and i have to press the green button again, instead it should go endlessly unless i press the red button to reset it

const int greenB = 2; 
const int redB = 3;        

int inAction = 0;
int greenState = 0;
int redState = 0;         
void setup() {

Serial.begin(9600);
pinMode(greenB, INPUT);
pinMode(redB, INPUT);
}

void loop() {

  greenState = digitalRead(greenB);
  redState = digitalRead(redB);

  if(greenState == HIGH){
    inAction = 1;
    while(inAction == 1){
      for(int i = 0; i<10;i++){
        if(redState == HIGH){
          Serial.println("timer stopped");
          goto stopTimer;        
        }
        if(i == 9){
          Serial.println("Cycle completed");
        }
         
        Serial.println("10 seconds timer");
        delay(1000);  
      }
      stopTimer: inAction = 0; 
      
    }
  }
}

Answer 1

Because you used goto, you caused a problem. Once the code reaches the end of the for loop it will then reach the line labelled stopTimer: and set inAction to 0. That stops everything until you press the green button again.

There is no need to use goto, ever in C++. If you ever find that you want to use goto then you are most likely doing it wrong. Try this instead:

void loop() {

  greenState = digitalRead(greenB);
  redState = digitalRead(redB);

  if(greenState == HIGH){
    inAction = 1;
    while(inAction == 1){
      for(int i = 0; i<10;i++){
        if(redState == HIGH){
          Serial.println("timer stopped");
          inAction = 0;
          break;   // Ends the for loop right here:       
        }
        if(i == 9){
          Serial.println("Cycle completed");
        }
         
        Serial.println("10 seconds timer");
        delay(1000);  
      }      
    }
  }
}

When you press the red button, the break statement ends the for loop. It exits right there. It's also the last thing in loop so loop then exits. Next time through loop inAction is set to 0 and that will stop it. But otherwise it just keeps running your 10 second countdown over and over.

There is a lot more you can do to improve this code. Writing it with the delay function means that you may have to hold the button down for up to a second while that line completes before it registers that you pressed it. But that may be a lesson for another day. This will at least show you how goto is bad and why you had the unwanted behavior.

One big improvement would be to let the loop function be your loop and actually make use of your state variable inAction. See if you understannd why this is better: (What if you had some more code that you wanted to run while inAction was 0?)

void loop() {

  greenState = digitalRead(greenB);
  redState = digitalRead(redB);

  if(greenState == HIGH){
    inAction = 1;
  }
  if(inAction == 1){
    for(int i = 0; i<10;i++){
      if(redState == HIGH){
        Serial.println("timer stopped");
        inAction = 0;
        break;   // Ends the for loop right here:       
      }
      if(i == 9){
        Serial.println("Cycle completed");
      }
         
      Serial.println("10 seconds timer");
      delay(1000);  
      }      
    }
  }
}