Traffic light using millis() problem

Hi.

I am trying to make a trafficlight system using the millis() commands instead of the delay() commands.

Im completely new to arduinos and their programing so it might be really bad and unefficient.

The problem I am experiencing right now is that some of the lights are not turing on/off when i want them to even if the code is basically the same as the code for the working parts. The parts not working are the lights off on pin 12 and the lights on on pin 11. The part when the light on pin 12 turns on for the second time also seems to not work, but that might be because it never got turned off on the first part.

Here is my terrible and unfinished code which I am using for now to see if it even works. I am going to change the intervals so they work more than one time. Right now I just need help with finding the problem i cant seem to find.

unsigned long previousMillis1 = 0;
unsigned long previousMillis2 = 0;
unsigned long previousMillis3 = 0;
unsigned long previousMillis4 = 0;
unsigned long previousMillis5 = 0;
unsigned long previousMillis6 = 0;

int intervalstart1 = 16000;  // green, car: 5 sec
int intervalstart2 = 5000;   // yellow.1, car: 3 sec
int intervalstart3 = 8000;   // red, car: 5 sec
int intervalstart4 = 13000;  // green, human: 5 sec
int intervalstart5 = 8000;   // red, human: 8 sec
int intervalstart6 = 13000;  // yellow.2, car: 3 sec

int intervalstop1 = 5000;    // grönt, car: 5 sec
int intervalstop2 = 8000;    // gul.1, car: 3 sec
int intervalstop3 = 13000;   // röt, car: 5 sec
int intervalstop4 = 8000;    // grönt, human: 5 sec
int intervalstop5 = 13000;   // röt, human: 8 sec
int intervalstop6 = 13000;   // gul.2, car: 3 sec

void setup()
{
  pinMode(13, OUTPUT);  // Red, car            3
  pinMode(12, OUTPUT);  // yellow, car        2
  pinMode(11, OUTPUT);  // green, car         1
  pinMode(10, OUTPUT);  // red, human       4
  pinMode(9, OUTPUT);   // green, human    5
  
  // start by having these leds on
  digitalWrite(11, HIGH);
  digitalWrite(10, HIGH);
}

void loop()
{
  unsigned long currentMillis1 = millis();
  unsigned long currentMillis2 = millis();
  unsigned long currentMillis3 = millis();
  unsigned long currentMillis4 = millis();
  unsigned long currentMillis5 = millis();
  unsigned long currentMillis6 = millis();
  
  // start leds
  if (currentMillis1 - previousMillis1 >= intervalstart1) {
    previousMillis1 = currentMillis1;
    digitalWrite(11, HIGH);
  }
  if (currentMillis2 - previousMillis2 >= intervalstart2) {
    previousMillis2 = currentMillis2;
    digitalWrite(12, HIGH);
  }
  if (currentMillis3 - previousMillis3 >= intervalstart3) {
    previousMillis3 = currentMillis3;
    digitalWrite(13, HIGH);
  }
  if (currentMillis4 - previousMillis4 >= intervalstart4) {
    previousMillis4 = currentMillis4;
    digitalWrite(10, HIGH);
  }
  if (currentMillis5 - previousMillis5 >= intervalstart5) {
    previousMillis5 = currentMillis5;
    digitalWrite(9, HIGH);
  }
  if (currentMillis6 - previousMillis6 >= intervalstart6) {
    previousMillis6 = currentMillis6;
    digitalWrite(12, HIGH);
  }
  
  // stop leds
  if (currentMillis1 - previousMillis1 >= intervalstop1) {
    previousMillis1 = currentMillis6;
    digitalWrite(11, LOW);
  }
  if (currentMillis2 - previousMillis2 >= intervalstop2) {
    previousMillis2 = currentMillis2;
    digitalWrite(12, LOW);
  }
  if (currentMillis3 - previousMillis3 >= intervalstop3) {
    previousMillis3 = currentMillis3;
    digitalWrite(13, LOW);
  }
  if (currentMillis4 - previousMillis4 >= intervalstop4) {
    previousMillis4 = currentMillis4;
    digitalWrite(10, LOW);
  }
  if (currentMillis5 - previousMillis5 >= intervalstop5) {
    previousMillis5 = currentMillis5;
    digitalWrite(9, LOW);
  }
  if (currentMillis6 - previousMillis6 >= intervalstop6) {
    previousMillis6 = currentMillis6;
    digitalWrite(12, LOW);
  }

}

Hope someone will see the problem other than how bad it is made. Also, i didnt use a function to repeat the same code so there is a long stretch of code instead. Sorry.

Your intervals run completely concurrent - when you are examining for Led Start, you are also examining for Led Stop. You could add a Boolean for which state you are in now so as to determine which state you are timing out. Likely, when you are turning the LED on, you are also turning it OFF.

You don’t really need currentMillis-1, 2, …6 since currentMillis is, well, current (not a big deal).

I’m also curious why you are examining for timeout for pins 11, 12, 13, 10, 9, and 12 again? Maybe you just haven’t noticed yet.

So, maybe read up on state machine (or finite state machine) - you essentially have these LED’s in one of three states - Red, Yellow, or Green. Using a state machine may make it easier for you to program. Your mileage may vary.

Thank you for the reply.

I have listened to your advise and looked up a tutorial on how to make a state machine. I also changed it a bit so now the pedestrain crossing only goes green if a button is pressed, otherwise they get a red light even if it is red for the cars.

// Pins
 int red = 13, yellow = 12, green = 11;
 int p_red = 10, p_green = 9;
 int button = 7;
 
 // System variables
 byte state = 0; // initial state
 unsigned int i = 1; // system counter
 unsigned int del = 100; // system delay legnth
 boolean flag = false;
 
void setup() {
  pinMode(red, OUTPUT);
  pinMode(yellow, OUTPUT);
  pinMode(green, OUTPUT);
  pinMode(p_red, OUTPUT);
  pinMode(p_green, OUTPUT);
  pinMode(button, INPUT_PULLUP);
}
 
void loop() {
  // Check button
  if(digitalRead(button) == 0) flag = true;
  switch(state){
    case 0:
      digitalWrite(red, HIGH);
      digitalWrite(yellow, LOW);
      digitalWrite(green, LOW);
      if(flag){
        digitalWrite(p_red, LOW);
        digitalWrite(p_green, HIGH);
      }else{
        digitalWrite(p_red, HIGH);
        digitalWrite(p_green, LOW);
      }
      if((i%100)==0){
        flag = false;
        state = 1;
        i = 0;
      }
    break;
 
    case 1:
      digitalWrite(red, HIGH);
      digitalWrite(yellow, HIGH);
      digitalWrite(green, LOW);
      digitalWrite(p_red, HIGH);
      digitalWrite(p_green, LOW);
      if((i%30)==0){
        state = 2;
        i = 0;
      }
    break;
 
    case 2:
      digitalWrite(red, LOW);
      digitalWrite(yellow, LOW);
      digitalWrite(green, HIGH);
      digitalWrite(p_red, HIGH);
      digitalWrite(p_green, LOW);
      if((i%80)==0){
        state = 3;
        i = 0;
      }
    break;
 
    case 3:
      digitalWrite(red, LOW);
      digitalWrite(yellow, HIGH);
      digitalWrite(green, LOW);
      digitalWrite(p_red, HIGH);
      digitalWrite(p_green, LOW);
      if((i%20)==0){
        state = 0;
        i = 0;
      }
    break;
    default:
    break;
  }
  i++;
  delay(del);
}

Honestly couldn’t have done it without any help. Thanks!