8 timer with LCD Screen

Ok i have progressed my LCD menu and i am nearly there. Everything seems to work ok until the last part.

Project : 8 timer with LCD display and clock

Function: we select a timer and input 2 parameters
1/ how many minutes we want on in a 24hr period
2/ how many cycles must the above minutes be split up in the 24hr period

Once these parameters have been set the state machine should move to the last state which should be the 8 timers screens appearing for s period of time going round in a loop.
this where i have the problem. Once i set the parameters and press my ENTER button it goes to the start of the screens loop and freezzes, it does not progress to the next screen.

I have attached the section of the code and also below uploaded the entire code.
The part of the code which is not working as it should starts with case LOOP1 near the end.

Could someone please have a look and advise where i am going wrong

void loop()
{
  static unsigned long ts;
  ButtonStateOld = ButtonPush;
  ButtonPush = ReadKeypad();             //Check for button activity and allow for debounce
  if (ButtonStateOld != ButtonPush)
  {

//code cut short 
  
      case TIMERSETUP:
      TimerSetUp();
      state = SWITCH5;
      break;
      
      case SWITCH5:
      if (ButtonPush == ENTER)
      {
        state = TIM1;
      }
      else if (ButtonPush == BACK)
      {
        state = SETUP;
      }
      break;
      
      case TIM1:
      T1();
      state = SW1;
      break;
      
      case SW1:
      if (ButtonPush == DOWN)
      {
        state = TIM2;
      }
      else if (ButtonPush == UP)
      {
        state = TIM8;
      }
      else if (ButtonPush == BACK)
      {
        state = TIMERSETUP;
      }
      else if (ButtonPush == ENTER)
      {
        state = SUBTIM1;
      }
      break;
      
      //case TIM2 - TIM8 are same

      
      case SW8:
      if (ButtonPush == DOWN)
      {
        state = TIM1;
      }
      else if (ButtonPush == UP)
      {
        state = TIM7;
      }
      else if (ButtonPush == BACK)
      {
        state = TIMERSETUP;
      }
      else if (ButtonPush == ENTER)
      {
        state = SUBTIM8;
      }
      break;
      
      case SUBTIM1:
      T1TMD();
      state = SW9;
      break;
      
      case SW9:
      if (ButtonPush == ENTER)
      {
        state = SUBSUBTIM1;
      }
      else if (ButtonPush == BACK)
      {
        state = TIM1;
      }
      break;
      
      case SUBSUBTIM1:
      T1CD();
      state = SW10;
      break;
      
      case SW10:
      if (ButtonPush == ENTER)
      {
        state = LOOP1;
      }
      else if (ButtonPush == BACK)
      {
        state = SUBTIM1;
      }
      break;
      
      case SUBTIM2:
      T2TMD();
      state = SW11;
      break;
      
      case SW11:
      if (ButtonPush == ENTER)
      {
        state = SUBSUBTIM2;
      }
      else if (ButtonPush == BACK)
      {
        state = TIM2;
      }
      break;
      
//SUBTIM2 - 8 & SUBSUBTIM2 - 8 are same
      
//Once gone through set up and settings this section is the screen loop where each timer will be displayed for a period until futher activity from user input.
      
      case LOOP1:  // timer 1
      ScreenLoop1();
      ts = millis();
      state = SWAP1;
      break;
      
      case SWAP1:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP2;
      }
      break;
      
      case LOOP2:  // timer 2
      ScreenLoop2();
      ts = millis();
      state = SWAP2;
      break;
      
      case SWAP2:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP3;
      }
      break;
      
      case LOOP3:  //timer3
      ScreenLoop3();
      ts = millis();
      state = SWAP3;
      break;
      
      case SWAP3:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP4;
      }
      break;
      
      case LOOP4:  //timer4
      ScreenLoop4();
      ts = millis();
      state = SWAP4;
      break;
      
      case SWAP4:  
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP5;
      }
      break;
      
      case LOOP5:  //timer5
      ScreenLoop5();
      ts = millis();
      state = SWAP5;
      break;
      
      case SWAP5:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP6;
      }
      break;
      
      case LOOP6:  //timer6
      ScreenLoop6();
      ts = millis();
      state = SWAP6;
      break;
      
      case SWAP6:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP7;
      }
      break;
      
      case LOOP7:  //timer7
      ScreenLoop7();
      ts = millis();
      state = SWAP7;
      break;
      
      case SWAP7:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP8;
      }
      break;
      
      case LOOP8:  //timer8
      ScreenLoop8();
      ts = millis();
      state = SWAP8;
      break;
      
      case SWAP8:
      //ButtonState = analogRead(ButtonPin);
      //ButtonPush = ButtonState;
      if ((millis() > ts + 500) && (ButtonPush == NONE))
      {
        state = LOOP1;
      }
      break;
    }
  }
}

AquArt_FTm_C_temp.ino (15.1 KB)

Void_Functions.ino (4.85 KB)

What does the ScreenLoop1() function do ?

ScreenLoop1 is general screen which shows the stats of the timer in this case timer 1. So ScreeLoop2, 3, 4, 5, 6, 7 & 8 are identical just the number of the timer changes.

void ScreenLoop1()
{
  lcd.clear();
  MainTitle();
  lcd.print("Timer 1    /  ");
}

Why take 8 functions "into the shower" when you could do it with one ?

Well i asked about this on a previuos time and all i got was negative feedback. I would be most greatfull if you could expalin how. I am a neebie trying to learn

If you use a function that takes a parameter, something like this

void ScreenLoop(int timerNo)
{
  lcd.clear();
  MainTitle();
  lcd.print("Timer ");
  lcd.print(timerNo);
  lcd.print("    /  ");
}

and call it with the timer number as a parameter

ScreenLoop(1);
ScreenLoop(2);
//etc

or with a variable as the parameter

Ah ok, so that gets rid of all the ScreenLoop's from 2-8 that i have. And will that solve the problem in the main loop that currently they are not flipping to the next timer. At moment all that shows up is effectively the ScreenLoop1 but does not go to the next ScreenLoop2 and so on, even though i am telling it to via the SWAP state

will that solve the problem in the main loop that currently they are not flipping to the next timer

Quite possibly not but the code will certainly be neater and the real problem may show up easier.

Through some trial and error i found where the problem lays. It seems that the button function is stopping the state machine or should i say is restricting the state machine to button presses only

void loop()
{
  t = rtc.getTime();
  ButtonStateOld = ButtonPush;
  ButtonPush = ReadKeypad();             //Check for button activity and allow for debounce
  {
 [color=red]   if (ButtonStateOld != ButtonPush)[/color] //This is the offending line
    {
      static unsigned long ts;
      static int state = SETUP;
      switch (state)
      {
        case SETUP:
        Screen1();
        state = SWITCH1;
        break;
  
        case SWITCH1:
        if (ButtonPush == ENTER)
        {
          state = CLOCKSETUP;
        }
        else if (ButtonPush == BACK)
        {
          state = TIMERSETUP;
        }
        break;
  
        case CLOCKSETUP:
        ClockSetUp();
        state = SWITCH2;
        break;
  
        case SWITCH2:
        if (ButtonPush == ENTER)
        {
          state = SETHR;
        }
        else if (ButtonPush == BACK)
        {
          state = SETUP;
        }
        break;

But i need that line to make the buttons work properly. Any ideas anybody?

is restricting the state machine to button presses only

What else should cause a state change ? Can you OR the other condition with the change of button state ?

if ( (ButtonStateOld != ButtonPush) || (someOtherCondition == true) )

I will try that thanks