arduino traffic light, 1st time poster

Hello! New arduino enthusiast here! I am making a traffic light project with a traffic light and a pedestrian light.

what i want to happen:

button1 is for the pedestrians to press when wanting to cross the street. The program should constantly check to see if the button has been pressed. If it has been pressed, the boolean variable pedwait1 will become true and will guide the program to the particular if statement that would turn the pedestrian light green along with the traffic light. else would be the case that pedwait1 is false and that would guide the program to the else statement that would STILL turn the traffic light green, but the pedestrian light would remain red.

what is happening:

pedwait1 is seemingly always true without ever pressing the button. the pedestrian light is always green when the traffic light is green. The program starts with the traffic light A and pedestrian light A both green. It is unlikely I have a short in my circuit, so please help me look for bad code.

needless to say: i’m having the same problem with pedwait2, and button2 on the B side of the intersection but for clarity’s sake I left that out of my question. thanks for your time!!!

  // Lights {GREEN, YELLOW, RED, GREEN, YELLOW, RED}
int light [10]= {2,3,4,5,6,7,8,9,10,11};
int turngreen = 5000;
int turnyellow = 12000;
int turnred = 5000;
int turnyellow2 = 12000;
int button1 = 12;
int button2 = 13;
unsigned long currentmillis = millis();
unsigned long previousmillis = 0;
boolean pedwait1 = false;
boolean pedwait2 = false;
// SETUP
void setup() {
 

  for(int x = 0; x < 10; x++){
  pinMode(light[x], OUTPUT);
  }
  
  pinMode(button1, INPUT);
  pinMode(button2, INPUT);  
    
  //digitalWrite(light[7],HIGH);
  //digitalWrite(light[11],HIGH);
}

// RUN
  void loop() {
 // Update Time

        // 2-green 3-yellow 4-red  5-green 6-yellow 7-red   8-lgreen 9-lred    10-lgreen 11-lred 
        // [0]     [1]      [2]    [3]     [4]      [5]     [6]      [7]       [8]       [9]


    for(previousmillis = currentmillis; currentmillis - previousmillis < turngreen; currentmillis = millis()){
        
      if(digitalRead(button1 == HIGH)){
          
        pedwait1 = true;
      }
      
    }
    
    //******************************************************************
   
    //This section is just side A turning green for cars, while side B turns red for cars and pedestrians. 
    
    if(pedwait1 == true){
   
      digitalWrite(light[2],LOW);
      
      digitalWrite(light[0],HIGH);
      
      digitalWrite(light[5],HIGH);
      
      digitalWrite(light[4],LOW);
      
      digitalWrite(light[9],HIGH);
      
      digitalWrite(light[8],LOW);
      
      //******************************************************************
      
      //This just says side A also turns green for pedestrians because the button is being pressed.
      
      digitalWrite(light[7],LOW);
      
      digitalWrite(light[6],HIGH);
    
      //pedwait1 did its job. time to turn it back to false
      
      pedwait1 = false;
        
    }
 
    else{
        
      
        //for(previousmillis = currentmillis; currentmillis - previousmillis < turngreen; currentmillis = millis()){
        //}
        
        
        //********************************************************************
        
        //This section just says exactly the same thing as the last section, but it doesn't have the two lines of code of
        //that make side A turn green for pedestrians.
                
        digitalWrite(light[2],LOW);
        
        digitalWrite(light[0],HIGH);
        
        digitalWrite(light[5],HIGH);
        
        digitalWrite(light[4],LOW);
        
        digitalWrite(light[9],HIGH);
        
        digitalWrite(light[8],LOW);
    }
 
 
    for(unsigned long previousmillis2 = currentmillis; currentmillis - previousmillis2 < turnyellow; currentmillis = millis()){
      
      if(digitalRead(button2 == HIGH)){
          
        pedwait2 = true;
      }
     
    }  
    
      //************************************************************
     
      //only change is side A going from green to yellow
      
      digitalWrite(light[0],LOW);
      
      digitalWrite(light[1],HIGH);
    
    

    
    for(unsigned long previousmillis3 = currentmillis; currentmillis - previousmillis3 < turnred; currentmillis = millis()){
    
      if(digitalRead(button2 == HIGH)){
          
        pedwait2 = true;
      }
   
    }  
    

    if(pedwait2 == true){    
       //*************************************************************************
       
       //This section is the inverse of the first section so it's
       //just side B turning green for cars, while side A turns red for cars and pedestrians.  
       
        digitalWrite(light[1],LOW);
        
        digitalWrite(light[2],HIGH);
        
        digitalWrite(light[3],HIGH);    
         
        digitalWrite(light[5],LOW);
        
        digitalWrite(light[7],HIGH);
        
        digitalWrite(light[6],LOW);
        
       //***************************************************************************
       
       //This just says side B also turns green for pedestrians because the button is being pressed.
       
        digitalWrite(light[8],HIGH);
        
        digitalWrite(light[9],LOW);
        
        pedwait2 = false;
        
    }

    else{  
       
        //************************************************************************
       
        //This section just says exactly the same thing as the last time we have 6 changes happening, but it doesn't have the two lines of code of
        //that make side B turn green for pedestrians.
       
          
        digitalWrite(light[1],LOW);
        
        digitalWrite(light[2],HIGH);
        
        digitalWrite(light[3],HIGH);    
         
        digitalWrite(light[5],LOW);
        
        digitalWrite(light[7],HIGH);
        
        digitalWrite(light[6],LOW);
          
    }    
        
    for(unsigned long previousmillis4 = currentmillis; currentmillis - previousmillis4 < turnyellow2; currentmillis = millis()){
      
      if(digitalRead(button1 == HIGH)){
                
              pedwait1 = true;
      }
     
    }  
    
    //*******************************************************************
   
    //only change is side B going from green to yellow
    
    digitalWrite(light[3],LOW);
    
    digitalWrite(light[4],HIGH);
    
     
    
  }

        // 2-green 3-yellow 4-red  5-green 6-yellow 7-red   8-lgreen 9-lred    10-lgreen 11-lred 
        // [0]     [1]      [2]    [3]     [4]      [5]     [6]      [7]       [8]       [9]

Pin 13 is the pin for the system led. The older version of the Arduino Uno board could have troubles when connecting a button to that pin.

The way you use currentmillis and previousmillis is like a bowl of moondust under a mushroom. I mean, it makes no sense to me. Did you see such a for-loop with millis somewhere else ?

I think you want to keep the loop() running without using delays. So you can split it into two parts. First part is collect the input (the buttons), second part is to act upon the current state.

Can you make a drawing with the traffic lights and the buttons ? I think that programming the code is easier when you add an extra layer. For example, a certain traffic light can be green, yellow or red. You could make a function and set the color: TrafficLight ( RED );

Look at the demo several things at a time for the usual way to use millis() to manage timing.

Using millis() in a FOR loop is pretty much the same as using the delay() function. Don't do it.

...R

claphands: ... pedwait1 is seemingly always true without ever pressing the button.

Have a look at your if construct.

if(digitalRead(button1 == high)) {}

You are passing the result of a logical operation to the digitalRead function, which will equate to passing either a 1 (true) or a 0 (false).

You should still try to sort out that for loop though.

This is off the top of my head but it might give you some ideas.

const uint16_t debounce = 500;  //button must be pressed for 1/2 second to register

uint16_t buttonStart =0;
boolean pedestrianWaiting = false;

loop() {
  uint16_t now = millis();
  
  if( ! pedestrianWaiting ) {
    if( digitalRead(button1) == HIGH ) {
      buttonStart = now;
    }
    else {
      buttonStart = 0;
    }
    //button must be pressed longer than debounce time
    pedestrianWaiting = (buttonStart > 0) && (now - buttonStart > debounce);
  }

  if( pedestrianWaiting ) {
    // do lights sequence 
    // remember to set pedestrianWaiting to false when you are done
  }
}