-2

i wanted to change LEDs with only one button. First click on the Button - Red Led turns on, Second - Red turns off and green turns on, third - Green turns off and yellow turns on, fourth - starts again with Red...

First i tried to turn on Red with one click and then turn off...

int red = 8;
int button = 13;
int buttonstate = 0;
bool redOn = false;


void setup(){
  pinMode(red,OUTPUT);
  pinMode(button,INPUT);
}

void loop(){
  
 buttonstate = digitalRead(button);
    

if (buttonstate == 1){
  if (redOn == false){
    digitalWrite(red,HIGH); 
    redOn = true;
        }
  else{
    digitalWrite(red,LOW);
    redOn = false;
    }
}
}

That works. Then I tried to add the other two LEDs..

int red = 8;
int yellow = 3;
int green = 6;
int button = 13;
int buttonstate = 0;
bool redOn = false;
bool yellowOn = false;
bool greenOn = false;


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

void loop(){
  
 buttonstate = digitalRead(button);
    

if (buttonstate == 1){
  if (redOn == false){
    digitalWrite(red,HIGH); 
    redOn = true;
        }
  else if (redOn == true && greenOn == false && yellowOn == false) {
    digitalWrite(red,LOW);
    redOn = false;
    digitalWrite(green,HIGH);
    greenOn = true;
    }
  else if (redOn == false && greenOn == true && yellowOn == false) {
    digitalWrite(green,LOW);
    greenOn = false;
    digitalWrite(yellow,HIGH);
    yellowOn = true;
    }
   else if (redOn == false && greenOn == false && yellowOn == true) {
    digitalWrite(yellow,LOW);
    yellowOn = false;
    }
  
  else{
      redOn = false;
      greenOn = false;
      yellowOn = false;
}
}
}

That doesn't work. Has someone an idea how to realize this?

K4M41D
  • 23
  • 1
  • 2
  • Where have you declared redOn, greenOn, and yellowOn? – Naseef Chowdhury Feb 02 '22 at 17:13
  • Another thing you can do is to test all the LEDs like you did test Red LED in the first code snippet. This will give you an idea if the integer values for LEDs are correct. Later you might want to add log prints on all the if/else conditions. – Naseef Chowdhury Feb 02 '22 at 17:16
  • @NaseefChowdhury `redOn`, etc. are declared in the code shown here. – Drew Dormann Feb 02 '22 at 17:17
  • 2
    Proper formatting and indentation would make the code much more readable... – Aconcagua Feb 02 '22 at 17:17
  • The `if` statements in your second program seem excessively convoluted. If your intention is to cycle through three states, consider using a single `enum{ red, green, blue }` instead of three `bool`s. – Drew Dormann Feb 02 '22 at 17:19
  • `if(!redOn) {} else if(redOn) {}` is a more natural way to check booleans (and pointers), apart from if `redOn` is `false` within first `if` then you *cannot* get to the else any more, so if you get there, `redOn` *must* be true and doesn't need to be checked once more... – Aconcagua Feb 02 '22 at 17:20
  • 1
    Whenever red is off, it will be switched on; otherwise: If both others are off, then green will be switched on, red off (and afterwards on again immediately), afterwards all leds are switched off – the other two if's cannot get met as (see my comment before) `redOn` cannot be `false` then – this case has been caught with very first if! Note that yellow cannot ever get switched on. So what is the blinking pattern you actually intend? – Aconcagua Feb 02 '22 at 17:40
  • Current code is equivalent to `if(!redOn) { red(high); } else if(!greenOn) { red(low); green(high); } else { red(low); green(low); }`... – Aconcagua Feb 02 '22 at 17:49

2 Answers2

0

There are two things to put into consideration:

First, the processor of the Arduino is way faster than the human, meaning if you press the button even for short time, your code will get it as many rapid times. You solve that by either adding a small delay at the end of the loop() function like:

delay(1000)

Or by setting a variable that contains the previous state of the button and act according to change of state (from 0 to 1)

The second thing is that you are starting to check the red light alone first:

if (redOn == false){
    digitalWrite(red,HIGH); 
    redOn = true;
        }

This if statement will be valid when the green light is on as you are setting redOn to false there

else if (redOn == true && greenOn == false && yellowOn == false) {
    digitalWrite(red,LOW);
    **redOn = false;**
    digitalWrite(green,HIGH);
    greenOn = true;
    }

So, after you turn off the red light and turn on the green and you press the button, the red light will turn on and the green light will stay on since you will be executing the first if statement where you do nothing to the green and yellow lights:

if (redOn == false){
    digitalWrite(red,HIGH); 
    redOn = true;
        }

In addition, you will never reach the state where the yellow light turns on. So you can modify the first if statement to be more specific like the other if statements by replacing

if (redOn == false){
    digitalWrite(red,HIGH); 
    redOn = true;
        }

With

if (!greenOn && !yellowOn){
    digitalWrite(red,HIGH); 
    redOn = true;
        }

Remove the last else since you don't want them to turn off all at once again

else{
      redOn = false;
      greenOn = false;
      yellowOn = false;
}

Side Note: When using boolean variables in if statement you don't have to use the '==' comparator, just use the boolean variables

Use:

if(redOn)

Instead of:

if(redOn == true)

And use:

if(!redOn)

Instead of:

if(redOn == false)

I hope that my answer is helpful :)

Best Regards.

Mrabcom
  • 26
  • 1
-1

You should at first detect if the button changed before running into the light switching stuff:

int buttonState = 0;
void loop()
{
    int bs = digitalRead(button);
    if(bs != buttonState)
    {
        // the button state changed!
        buttonState = bs;

        if(bs == 1)
        {
            // ... AND has been pressed
            // (if you want to switch on releasing, compare against 0)
            // -> switch now your LED outputs appropriately
        }
    }
}

From your code I'm not fully sure how the desired LED lighting pattern should look like, though, so skipping this part (until you provide a precise description).

Edit according to your comment:

There are several ways to achieve this blink pattern; with your boolean variables you could do like this (only considering the booleans, signals need to be set accordingly):

if(redOn)
{
    redOn = false;
    greenOn = true;
}
else if(greenOn)
{
    greenOn = false;
    yellowOn = true;
}
else if(yellowOn)
{
    yellowOn = false;
}
else
{
    redOn = true;
}

Above code would leave a dark phase after yellow being on; if you don't want one, you can simply have:

else if(greenOn)
{
    // ...
}
else
{
    yellowOn = false; // actually don't need the variable, just set the GPIO
    redOn = true;
}

Another variant is having a counter:

unsigned int ledState = 0;
void loop()
{
    if(buttonDetected) // see above
    {
        digitalWrite(red,    ledState == 0 ? HIGH : LOW);
        digitalWrite(green,  ledState == 1 ? HIGH : LOW);
        digitalWrite(yellow, ledState == 2 ? HIGH : LOW);
        ledState = (ledState + 1) % 3; // or 4, if you want a dark phase added!
    }
}
Aconcagua
  • 24,880
  • 4
  • 34
  • 59
  • With the first click on the Button the red LED should turn on. With the next click the red LED should turn of and the green LED should turn on. Then the green should turn off and the yellow should turn on. And then it should start again with the red. Is that what you meant? – K4M41D Feb 02 '22 at 18:33