Stuck in "if" statement?

So I am pretty new to Arduino and I am trying to make a traffic light that will have several modes with millis() function. I have noticed that the code gets stuck in one of the "if" statements. Am I doing something wrong?

Here is my incomplete code, hope it's clear to you:

const int RED = 3;
const int YELLOW = 4;
const int GREEN = 6;
const int SWITCH1 = A0;
const int SWITCH2 = A1;
const int SWITCH3 = A2;
const unsigned long aaa = 10;
const unsigned long bbb = 100;
const unsigned long ccc = 250;
const unsigned long ddd = 1000;
const unsigned long eee = 2000;
const unsigned long fff = 5000;
unsigned long previousMillis1 = 0;
unsigned long previousMillis2 = 0;
unsigned long currentMillis = millis();
int ledState1 = LOW;
int ledState2 = LOW;
int ledState3 = LOW;

void setup() {
  // put your setup code here, to run once:
  Serial.begin(9600);
  pinMode(RED, OUTPUT);
  pinMode(YELLOW, OUTPUT);
  pinMode(GREEN, OUTPUT);
  pinMode(SWITCH1, INPUT);
  pinMode(SWITCH2, INPUT);
  pinMode(SWITCH3, INPUT);
}

void loop() {
  unsigned long currentMillis = millis();
  Serial.println(millis());
  if (digitalRead(SWITCH1) == HIGH) {

    ledState2 = LOW;
    ledState3 = LOW;
    if (ledState1 == LOW) {

      if ((millis() - previousMillis1) >= ddd) {
        Serial.println("YELLOWBLINK");
        ledState1 = HIGH;
        previousMillis1 = millis();
      }
    } else if ((millis() - previousMillis1) >= ccc) {
      ledState1 = LOW;
      previousMillis1 = millis();
    }

    digitalWrite(YELLOW, ledState1);
    digitalWrite(RED, ledState2);
    digitalWrite(GREEN, ledState3);
  }
  if (digitalRead(SWITCH2) == HIGH) {

     //gets stuck here 
    if (currentMillis - previousMillis2 >= bbb){
      Serial.println("RED");
      previousMillis2 = currentMillis;
      ledState2 = HIGH;
      ledState3 = LOW;
      ledState1 = LOW;
    }
    if (currentMillis - previousMillis2 >= fff) {
      Serial.println("TURNINGGREEN");
      previousMillis2 = currentMillis;
      ledState1 = HIGH;
    }
    if (currentMillis - previousMillis2 >= ddd) {
      Serial.println("GREEN");
      previousMillis2 = currentMillis;
      ledState1 = LOW;
      ledState2 = LOW;
      ledState3 = HIGH;
    }
    digitalWrite(YELLOW, ledState1);
    digitalWrite(RED, ledState2);
    digitalWrite(GREEN, ledState3);
  }
}

Welcome to the forum

Which if condition so you think is causing the problem ?

Thanks.

So the problem is that it gets stuck at the red light. The "yellow blink" does work even after that though. One thing I know is that when it gets "stuck", the serial monitor does print out the word "RED" repeatedly. So my guess is that the problem somewhere is under that tag (//gets stuck here).

Do you know how to use the State Machine programming technique ?

//gets stuck here 
    if (currentMillis - previousMillis2 >= bbb){
      Serial.println("RED");
      previousMillis2 = currentMillis;
      ledState2 = HIGH;
      ledState3 = LOW;
      ledState1 = LOW;
    }

const unsigned long bbb = 100;

This will cause the printing to repeat 10 times per second. What do you want to happen?

Look at the above if blocks closely. bbb = 100, fff = 5000, ddd = 1000. The last two if statements will never become true! This is because currentMillis - previousMillis2 >= bbb will become true before the last 2 if statements and you will reset previousMillis2.

here is a traffic-light code that is using a state-machine

// MACRO-START * MACRO-START * MACRO-START * MACRO-START * MACRO-START * MACRO-START *
// Take it for granted at the moment scroll down to void setup
// start of macros dbg and dbgi
#define dbg(myFixedText, variableName) \
  Serial.print( F(#myFixedText " "  #variableName"=") ); \
  Serial.println(variableName);
// usage: dbg("1:my fixed text",myVariable);
// myVariable can be any variable or expression that is defined in scope

#define dbgi(myFixedText, variableName,timeInterval) \
  do { \
    static unsigned long intervalStartTime; \
    if ( millis() - intervalStartTime >= timeInterval ){ \
      intervalStartTime = millis(); \
      Serial.print( F(#myFixedText " "  #variableName"=") ); \
      Serial.println(variableName); \
    } \
  } while (false);
// usage: dbgi("2:my fixed text",myVariable,1000);
// myVariable can be any variable or expression that is defined in scope
// third parameter is the time in milliseconds that must pass by until the next time a
// Serial.print is executed
// end of macros dbg and dbgi
// print only once when value has changed
#define dbgc(myFixedText, variableName) \
  do { \
    static long lastState; \
    if ( lastState != variableName ){ \
      Serial.print( F(#myFixedText " "  #variableName" changed from ") ); \
      Serial.print(lastState); \
      Serial.print( F(" to ") ); \
      Serial.println(variableName); \
      lastState = variableName; \
    } \
  } while (false);
// MACRO-END * MACRO-END * MACRO-END * MACRO-END * MACRO-END * MACRO-END * MACRO-END *

const byte RedLED_Pin    = 4;
const byte YellowLED_Pin = 5;
const byte GreenLED_Pin  = 6;


const byte sm_RedOn           = 1;
const byte sm_WaitRedOn       = 2;
const byte sm_RedYellowOn     = 3;
const byte sm_WaitRedYellowOn = 4;
const byte sm_GreenOn         = 5;
const byte sm_WaitGreenOn     = 6;
const byte sm_YellowOn        = 7;
const byte sm_WaitYellowOn    = 8;

byte myState;

const byte RedOn  = HIGH;
const byte RedOff = LOW;

const byte YellowOn  = HIGH;
const byte YellowOff = LOW;

const byte GreenOn  = HIGH;
const byte GreenOff = LOW;

unsigned long myWaitTimer; // variables that deal with millis() MUST be unsigned long


void SwitchTrafficLights(byte Red, byte Yellow, byte Green){
  digitalWrite(RedLED_Pin,   Red);
  digitalWrite(YellowLED_Pin,Yellow);
  digitalWrite(GreenLED_Pin, Green);
}

  
void myTrafficLightStateMachine() {

  switch (myState)  {

    case sm_RedOn:
      SwitchTrafficLights(RedOn, YellowOff, GreenOff);
      myWaitTimer = millis(); // setup/initialise timer (with actual snapshot if time)
      myState = sm_WaitRedOn;
      break; // immidiately jump down to END-OF-SWITCH


    case sm_WaitRedOn:
      if ( TimePeriodIsOver(myWaitTimer, 5000) ) { // check if 5000 milliseconds have passed by
        // if REALLY 5000 milliseconds have passed by
        myState = sm_RedYellowOn;
      }
      break; // immidiately jump down to END-OF-SWITCH


    case sm_RedYellowOn:
      SwitchTrafficLights(RedOn, YellowOn, GreenOff);
      myWaitTimer = millis(); // setup/initialise timer (with actual snapshot if time)
      myState = sm_WaitRedYellowOn;
      break; // immidiately jump down to END-OF-SWITCH


    case sm_WaitRedYellowOn:
      if ( TimePeriodIsOver(myWaitTimer, 2000) ) { // check if 5000 milliseconds have passed by
        // if REALLY 2000 milliseconds have passed by
        myState = sm_GreenOn;
      }
      break; // immidiately jump down to END-OF-SWITCH


    case sm_GreenOn:
      SwitchTrafficLights(RedOff, YellowOff, GreenOn);
      digitalWrite(RedLED_Pin   , LOW);
      digitalWrite(YellowLED_Pin, LOW);
      digitalWrite(GreenLED_Pin , HIGH);
      myWaitTimer = millis(); // setup/initialise timer (with actual snapshot if time)
      myState = sm_WaitGreenOn;
      break; // immidiately jump down to END-OF-SWITCH


    case sm_WaitGreenOn:
      if ( TimePeriodIsOver(myWaitTimer, 10000) ) { // check if 5000 milliseconds have passed by
        // if REALLY 10000 milliseconds have passed by
        myState = sm_YellowOn;
      }
      break; // immidiately jump down to END-OF-SWITCH


    case sm_YellowOn:
      SwitchTrafficLights(RedOff, YellowOn, GreenOff);
      digitalWrite(RedLED_Pin   , LOW);
      digitalWrite(YellowLED_Pin, HIGH);
      digitalWrite(GreenLED_Pin , LOW);
      myWaitTimer = millis(); // setup/initialise timer (with actual snapshot if time)
      myState = sm_WaitYellowOn;
      break; // immidiately jump down to END-OF-SWITCH


    case sm_WaitYellowOn:
      if ( TimePeriodIsOver(myWaitTimer, 2000) ) { // check if 5000 milliseconds have passed by
        // if REALLY 2000 milliseconds have passed by
        myState = sm_RedOn;
      }
      break; // immidiately jump down to END-OF-SWITCH
  } // END-OF-SWITCH

}


void PrintFileNameDateTime() {
  Serial.println( F("Code running comes from file ") );
  Serial.println( F(__FILE__) );
  Serial.print( F("  compiled ") );
  Serial.print( F(__DATE__) );
  Serial.print( F(" ") );
  Serial.println( F(__TIME__) );
}


// easy to use helper-function for non-blocking timing
boolean TimePeriodIsOver (unsigned long &startOfPeriod, unsigned long TimePeriod) {
  unsigned long currentMillis  = millis();
  if ( currentMillis - startOfPeriod >= TimePeriod ) {
    // more time than TimePeriod has elapsed since last time if-condition was true
    startOfPeriod = currentMillis; // a new period starts right here so set new starttime
    return true;
  }
  else return false;            // actual TimePeriod is NOT yet over
}

unsigned long MyTestTimer = 0;                   // Timer-variables MUST be of type unsigned long
const byte    OnBoard_LED = 13;


void BlinkHeartBeatLED(int IO_Pin, int BlinkPeriod) {
  static unsigned long MyBlinkTimer;
  pinMode(IO_Pin, OUTPUT);

  if ( TimePeriodIsOver(MyBlinkTimer, BlinkPeriod) ) {
    digitalWrite(IO_Pin, !digitalRead(IO_Pin) );
  }
}


void PrintDebugInfos() {
  /*
  dbgc("1", digitalRead(RedLED_Pin) );
  dbgc("2", digitalRead(YellowLED_Pin) );
  dbgc("3", digitalRead(GreenLED_Pin) );
  */
  PrintStateNameIfChanged(myState);
  //dbgc("PDI",myState);
}

void PrintStateNameIfChanged(byte p_State) {
  // array indexes start at 0. This is the reason why an 
  // entry "dummy" is inserted at first
  char states [] [20] = {"dummy",           // index 0 
                         "RedON",           // index 1
                         "WaitRedOn",       // index 2
                         "RedYellowOn",     // index 3
                         "WaitRedYellowOn", // index 4
                         "GreenOn",         // index 5
                         "WaitGreenOn",     // index 6
                         "YellowOn",        // index 7
                         "WaitYellowOn"     // index 8
                        };

  static byte lastState;

  if (lastState == 0) {
    lastState = sm_RedOn;
  }
  
  if (lastState != p_State) { // check if state has changed since last time
    // only in case the state HAS changed
    Serial.print("change from state ");
    Serial.print(states[lastState]);
    Serial.print(" to ");
    Serial.println(states[p_State]);
    lastState = p_State;
  }
}


void setup() {
  Serial.begin(115200);
  Serial.println("Setup-Start");
  PrintFileNameDateTime();

  digitalWrite(RedLED_Pin   , LOW);
  digitalWrite(YellowLED_Pin, LOW);
  digitalWrite(GreenLED_Pin , LOW);
  pinMode(RedLED_Pin,   OUTPUT);
  pinMode(YellowLED_Pin, OUTPUT);
  pinMode(GreenLED_Pin, OUTPUT);
  myState = sm_RedOn;
}


void loop() {
  BlinkHeartBeatLED(OnBoard_LED, 250);
  myTrafficLightStateMachine();
  PrintDebugInfos();
}

and as a WokSIm

best regards Stefan

As @LarryD alluded to you will need to use states to keep up with the light changing from red to yellow to green.

Three push buttons.

Thanks of the help, but I think I got it working myself.
Here is one part from the final code:

  if (digitalRead(SWITCH2) == HIGH) {


    if (currentMillis - previousMillis1 >= 0) {
      Serial.println("RED");
      ledState2 = HIGH;
      ledState3 = LOW;
      ledState1 = LOW;
    }
    if (currentMillis - previousMillis1 >= 5000) {
      Serial.println("TURNINGGREEN");
      ledState1 = HIGH;
      ledState2 = HIGH;
      ledState3 = LOW;
    }
    if (currentMillis - previousMillis1 >= 6000) {
      Serial.println("GREEN");
      ledState1 = LOW;
      ledState2 = LOW;
      ledState3 = HIGH;
    }
    if (currentMillis - previousMillis1 >= 11000) {
      Serial.println("TURNINGRED");
      ledState1 = HIGH;
      ledState2 = LOW;
      ledState3 = LOW;
    }
    if (currentMillis - previousMillis1 >= 13000) {
      Serial.println("RED");
      ledState1 = LOW;
      ledState2 = HIGH;
      ledState3 = LOW;
      previousMillis1 = currentMillis;
    }
    digitalWrite(YELLOW, ledState1);
    digitalWrite(RED, ledState2);
    digitalWrite(GREEN, ledState3);
  }

Hello mrlamp

It´s recommended to post the complete sketch to help other forum Members too.

This topic was automatically closed 180 days after the last reply. New replies are no longer allowed.