Using switch instead of a bunch of while loops

Hi there!

I’m quite new to the world of arduino and I came across a little question while making a project.
The project I’m talking about is a simple project where I use an arduino, a relay board and a little remote control to control a traffic light.

The code I have that works is as follows:

// declaration of pins
int redLight = 12;
int yellowLight = 11;
int greenLight = 10;
int redButton = 9;
int yellowButton = 8;
int greenButton = 7;
int cycleButton = 6;

// declaration of timer
unsigned long Timer;

// setup code, runs once:
void setup() {
  // digital output pins
  pinMode(redLight, OUTPUT);
  pinMode(yellowLight, OUTPUT);
  pinMode(greenLight, OUTPUT);

  // digital input pins
  pinMode(redButton, INPUT);
  pinMode(yellowButton, INPUT);
  pinMode(greenButton, INPUT);
  pinMode(cycleButton, INPUT);
}

int var = 8; // variable used to check what needs to happen

// main code, runs repeatedly:
void loop() {
  // set initial value of light as "off"
  digitalWrite(greenLight, HIGH);
  digitalWrite(yellowLight, HIGH);
  digitalWrite(redLight, HIGH);

  // check for input
  check_input();

  while (var == 0) { // cycle mode
    if (digitalRead(greenLight) == HIGH && digitalRead(yellowLight) == HIGH && digitalRead(redLight) == HIGH) { // turns green on if no other color is on yet
      change_light(greenLight);
      Timer = millis(); // set a time to compare to
    }
    // cycle like traffic light
    if (digitalRead(greenLight) == LOW && millis() - Timer >= 20000UL) { // change green to yellow after 20 seconds
      change_light(yellowLight);
      Timer = millis(); // set a time to compare to
    }
    if (digitalRead(yellowLight) == LOW && (millis() - Timer) >= 4000UL) { // change yellow to red after 4 seconds
      change_light(redLight);
      Timer = millis(); // set a time to compare to
    }
    if (digitalRead(redLight) == LOW && (millis() - Timer) >= 20000UL) { // change red to green after 20 seconds
      change_light(greenLight);
      Timer = millis(); // set a time to compare to
    }
    check_input();
  }

  while (var == 1) { // green light
    change_light(greenLight);
    check_input();
  }

  while (var == 2) { // yellow light
    change_light(yellowLight);
    check_input();
  }

  while (var == 3) { // red light
    change_light(redLight);
    check_input();
  }
}

// check input
void check_input() {
  if (digitalRead(cycleButton) == HIGH) {
    var = 0;
    Timer = millis(); // set a time to compare to
  }
  if (digitalRead(greenButton) == HIGH) {
    var = 1;
  }
  if (digitalRead(yellowButton) == HIGH) {
    var = 2;
  }
  if (digitalRead(redButton) == HIGH) {
    var = 3;
  }
}

// change light
void change_light(int light) {
  if (digitalRead(light) == HIGH) {
    // turn other lights off
    digitalWrite(redLight, HIGH);
    digitalWrite(yellowLight, HIGH);
    digitalWrite(greenLight, HIGH);

    // turn chosen light on
    digitalWrite(light, LOW);
  }
}

The input of the receiver of my remote can be read like a normal push button and the light changes (or cycles) according to the input.

Now my question is: why can I not replace the while loops with one switch with cases. I have been testing this out with no results yet. I think it will look a little neater and make my code a little bit shorter and possibly easier to understand.

The code I have to replace the while loop is as follows:

// check for input
  check_input;

  switch (var) {
    case 0: // cycle mode
      if (digitalRead(greenLight) == HIGH && digitalRead(yellowLight) == HIGH && digitalRead(redLight) == HIGH) { // turns green on if no other color is on yet
        change_light(greenLight);
        Timer = millis(); // set a time to compare to
      }
      // cycle like traffic light
      if (digitalRead(greenLight) == LOW && millis() - Timer >= 20000UL) { // change green to yellow after 20 seconds
        change_light(yellowLight);
        Timer = millis(); // set a time to compare to
      }
      if (digitalRead(yellowLight) == LOW && (millis() - Timer) >= 4000UL) { // change yellow to red after 4 seconds
        change_light(redLight);
        Timer = millis(); // set a time to compare to
      }
      if (digitalRead(redLight) == LOW && (millis() - Timer) >= 20000UL) { // change red to green after 20 seconds
        change_light(greenLight);
        Timer = millis(); // set a time to compare to
      }
      break;
    case 1: // turn green light on
      change_light(greenLight);
      break;
    case 2: // turn yellow light on
      change_light(yellowLight);
      break;
    case 3: // turn red light on
      change_light(redLight);
      break;
  }

I have also added the working version and the version with the switch as an attachment to this thread if anyone would like to play around with my code.

If anyone would be able to answer my question I would very much appreciate it :slight_smile:

Thanks in advance!

Edit: I have changed the comments in the example codeblocks that said “set a timer” to “set a time to compare to” because, as PaulS pointed out, that is not an accurate comment.

trafficlight.zip (2.12 KB)

Now my question is: why can I not replace the while loops with one switch with cases.

You can, if you do it right. Keep in mind that your current code is saying "stay here, doing this, until the condition changes". Your new code is saying "if the condition is this, do that". It does not wait anywhere for the condition to change.

        Timer = millis(); // set a timer

That is NOT what that code does. The millis() function returns a time, not a timer. There is a huge difference between a time and a timer.

There is NO reason to read the state of output pins. You made the pin have some state. Remember the state of that pin.

Thank you very much for your answer :slight_smile: I will keep your comments in mind when workingon future projects.