Help with crossing on traffic light project

Hi i'm new to all of this. i have written a programme for my uno but for some reason it doesn't work as i expected. I started with a simple traffic light simulation which worked fine but decided to go a step further and incorporate a crossing however when i press the button it carries on with the standard traffic light loop instead of stopping the loop on red and running the crossing. i know its a problem with the code but not sure where i'm going wrong. This is what code i have got.

int red = 13;
int yellow = 12;
int green = 11;
int redsmall = 10;
int greensmall = 9;
int button = 8;

void setup(){
pinMode (red, OUTPUT);
pinMode (yellow, OUTPUT);
pinMode (green, OUTPUT);
pinMode (redsmall, OUTPUT);
pinMode (greensmall, OUTPUT);
pinMode(button, INPUT);

}

void loop() {
changeLights();
delay(3000);

}

void changeLights(){
digitalWrite (redsmall, HIGH);
digitalWrite (yellow, LOW);
digitalWrite (red, HIGH);
delay (5000);

digitalWrite (yellow, HIGH);
delay (2000);

digitalWrite (yellow, LOW);
digitalWrite (green, HIGH);
digitalWrite (red, LOW);
delay (5000);

digitalWrite (yellow, HIGH);
digitalWrite (green, LOW);
delay (1000);

}

void crossing(){
if (digitalRead) (button = HIGH);
digitalWrite (yellow, HIGH);
digitalWrite (yellow, LOW);
digitalWrite (red, HIGH);
delay (1500);
digitalWrite (redsmall, LOW);
digitalWrite (greensmall, HIGH);
delay (6000);
digitalWrite (greensmall, LOW);
}

You're reading the button inside crossing() but crossing doesn't get called anywhere.

I think you want to read the button in loop() and call crossing from there if button is pushed?

ah, that makes sense. cheers. i will alter and upload the new code and see what happens :slight_smile:

Right i believe i changed it but it still doesn't work :frowning:

int red = 13;
int yellow = 12;
int green = 11;
int redsmall = 10;
int greensmall = 9;
int button = 8;

void setup(){
pinMode (red, OUTPUT);
pinMode (yellow, OUTPUT);
pinMode (green, OUTPUT);
pinMode (redsmall, OUTPUT);
pinMode (greensmall, OUTPUT);
pinMode(button, INPUT);

}

void loop() {
changeLights();
delay(3000);

}

void changeLights(){
digitalWrite (redsmall, HIGH);
digitalWrite (yellow, LOW);
digitalWrite (red, HIGH);
delay (5000);

digitalWrite (yellow, HIGH);
delay (2000);

digitalWrite (yellow, LOW);
digitalWrite (green, HIGH);
digitalWrite (red, LOW);
delay (5000);

digitalWrite (yellow, HIGH);
digitalWrite (green, LOW);
delay (1000);
if (digitalRead) (button = HIGH);
{
void crossing();
}

}

void crossing(){

digitalWrite (yellow, HIGH);
digitalWrite (yellow, LOW);
digitalWrite (red, HIGH);
delay (1500);
digitalWrite (redsmall, LOW);
digitalWrite (greensmall, HIGH);
delay (6000);
digitalWrite (greensmall, LOW);
}

The button's only going to be read at the end of changeLights(), so if it's a momentary button you might not have held it down long enough.

If you want it to be read at any time, you'll need to read up on interrupts.

PS... please put your code in code tags so it looks like this: select the code and hit the # button above the smilies.

ADDED: Oh and btw the syntax for your digitalread is wrong... it firstly needs a == not a single =, and the brackets are a bit wonky...should look like this:

if (digitalRead(button) == HIGH)....

That has now completely confused me :astonished: i am getting an error

traffic_lights_crossing.ino: In function 'void setup()':
traffic_lights_crossing:16: error: invalid conversion from 'int' to 'void ()()'
traffic_lights_crossing:16: error: initializing argument 2 of 'void attachInterrupt(uint8_t, void (
)(), int)'

This is what i have put for the code

int red = 13;
int yellow = 12;
int green = 11;
int redsmall = 10;
int greensmall = 9;
int button = 3;


void setup(){
    pinMode(red, OUTPUT);
    pinMode(yellow, OUTPUT);
    pinMode(green, OUTPUT);
    pinMode(redsmall, OUTPUT);
    pinMode(greensmall, OUTPUT);
    pinMode(button, INPUT);
    attachInterrupt(0, 3, CHANGE);
    
}

void loop() {
    changeLights();
    delay(3000);
        
}

void changeLights(){
  digitalWrite (redsmall, HIGH);
   digitalWrite (yellow, LOW);
  digitalWrite (red, HIGH);
  delay (5000);
  
  digitalWrite (yellow, HIGH);
  delay (2000);
  
  digitalWrite (yellow, LOW);
  digitalWrite (green, HIGH);
  digitalWrite (red, LOW);
  delay (5000);
  
  digitalWrite (yellow, HIGH);
  digitalWrite (green, LOW);
  delay (1000);
  if (digitalRead(button) == HIGH)
  {
    void crossing();
     }
    
 
}

void crossing(){
  
  digitalWrite (yellow, HIGH);
  digitalWrite (yellow, LOW);
  digitalWrite (red, HIGH);
  delay (1500);
  digitalWrite (redsmall, LOW);
  digitalWrite (greensmall, HIGH);
  delay (6000);
  digitalWrite (greensmall, LOW);
 }

Take the interrupt stuff out until you have it working the easy way, even though the "easy" way is not perfect since you have to wait till the end of a complete cycle for the button to get a look in

Interrupt stuff is gone. i have had a look at the code and gone through it and as far as i can see there is no problems but it still doesn't work :frowning:

Thanks for your help by the way, it is appreciated :slight_smile:

int red = 13;
int yellow = 12;
int green = 11;
int redsmall = 10;
int greensmall = 9;
int button = 8;

void setup(){
    pinMode (red, OUTPUT);
    pinMode (yellow, OUTPUT);
    pinMode (green, OUTPUT);
    pinMode (redsmall, OUTPUT);
    pinMode (greensmall, OUTPUT);
    pinMode(button, INPUT);
    
}

void loop() {
    changeLights();
    delay(3000);
        
}

void changeLights(){
  digitalWrite (redsmall, HIGH);
   digitalWrite (yellow, LOW);
  digitalWrite (red, HIGH);
  delay (5000);
  
  digitalWrite (yellow, HIGH);
  delay (2000);
  
  digitalWrite (yellow, LOW);
  digitalWrite (red, LOW);
  digitalWrite (green, HIGH);
      
  delay (5000);
 
  if (digitalRead(button) == HIGH);
  {
    void crossing();
     } 
     
  digitalWrite (yellow, HIGH);
  digitalWrite (green, LOW);
  delay (1000);
  
    
 
}

void crossing(){
  
  digitalWrite (yellow, HIGH);
  digitalWrite (yellow, LOW);
  digitalWrite (red, HIGH);
  delay (1500);
  digitalWrite (redsmall, LOW);
  digitalWrite (greensmall, HIGH);
  delay (7000);
  digitalWrite (greensmall, LOW);
  digitalWrite (redsmall, HIGH);
 }

commandosqueak:
That has now completely confused me :astonished: i am getting an error

traffic_lights_crossing.ino: In function 'void setup()':
traffic_lights_crossing:16: error: invalid conversion from 'int' to 'void ()()'
traffic_lights_crossing:16: error: initializing argument 2 of 'void attachInterrupt(uint8_t, void (
)(), int)'

This is what i have put for the code

Your problem with that fault is that you didn't structure your interrupt correctly. The attachinterrupt command requires a function to point to. You put in "3" as your function. This is an int, but the function would be a void. So it is bitching about that.

Also, I wouldn't do it on change as you may get multiple triggers. Your button should be pulled up and then connected to ground when pressed. So you would change that to LOW.You also need to put the button on an actual interrupt line. On the Uno, that is pin 2 and 3.So, assuming you put the button on three, your new line in setup() would be:

attachInterrupt(3, crossing, LOW);

Now, when the button is pressed, it will interrupt whatever it is doing and run crossing().

General point on which I tend to harp but remains important: Do not try to use interrupts unless there is a need for them.

They are basically never needed for a "HID" interface and in particular should never be used for pushbuttons where there is a need to de-bounce. Interrupts are for things that need a fast response, meaning computer-fast, that is, microseconds.

Paul__B:
Do not try to use interrupts unless there is a need for them.

And even then, try your damnedest to weasel out of using them.

Anyone notice that he is not calling the function correctly and he has a semicolon at the end of the IF statement.

if (digitalRead(button) == HIGH);
{
void crossing();
}

Void should not be included there, just have "crossing();" without the void and NO semicolon after the IF statement.