Change the sequence of LEDs with button press

Question

I am new to arduino. I have created a circuit , here is the image:

I want something that as I press the button the sequence of LEDs will change and as I press the button again , the sequence will change again.

I programmed code as following but it don't works correctly:

int led1 = 11;
int led2 = 10;

int output = 2;
int input = 4;
int mode = 0;

void setup() {
  pinMode(led1,OUTPUT);
  pinMode(led2,OUTPUT);
  pinMode(output,OUTPUT);
  pinMode(input,INPUT_PULLUP);
  digitalWrite(output,LOW);

}

void loop() {
  while(mode == 0 && digitalRead(input) == 1){
    digitalWrite(led1,HIGH);
    digitalWrite(led2,HIGH);
    delay(100);
    digitalWrite(led1,LOW);
    delay(100);
    digitalWrite(led1,HIGH);
    delay(100);
    digitalWrite(led2,LOW);
    delay(100);
    digitalWrite(led2,HIGH);
  }
  if(digitalRead(output) == 0){
    mode++;
    if(mode >= 2){mode = 0;}
  }
  while(mode == 1 && digitalRead(input) == 1){
    analogWrite(led1,255);
    delay(400);
    analogWrite(led1,150);
    delay(400);
    analogWrite(led2,255);
    delay(400);
    analogWrite(led2,150);
  }

}

According to the above code , two leds led1 and led2 are connected with arduino. When the button is off the both LEDs turns on then led1 turns off then turn on again , next time led2 turns off and turns on again.As the button will be pressed , brightness of led1 will be less than the brightness of led2 and then will again be equal to the brightness of led2, then brightness of led2 will be less than the brightness of led1 and then will be equal to the brightness of led1. But as I press the button , nothing happens but the led1 and led2 keeps turning on and off . However sometimes when I press the button then it works.

What is the problem with my code and how to solve it?

Answer 1

There are a few issues with your wiring.

• As already stated in comments, you should not connect the battery to the 5V input of the Arduino. Use the “Vin” input instead.
• You should put a current-limiting resistor in series with each of your LEDs. Any value in the range 330 Ω – 1 kΩ should be fine. Your Arduino (and your LEDs) will last longer if you stop abusing it!
• There is no point in using two digital IO pins to read a button. Just connect the button between a digital input and GND. Alternatively, you can wire it like in Holmez's answer, but that requires an extra wire and a resistor.

And there are two main issues with your code:

• You forgot to debounce the button. Mechanical button are bouncy. The easiest way to debounce them is to use a ready-made library like, e.g., Bounce2.
• You are using delay(), which means you cannot respond to user input while your Arduino is busy delaying. You should manage your timings with milis() instead, as explained in the Blink Without Delay Arduino tutorial.

Here is my take at your problem. This is based on the finite state machine programming pattern, with a state defined by two variables. There is a mode variable used to keep track of whether we are in “digital” or in “analog” mode. And there is a “phase” variable used to keep track of the phase along the blinking pattern. Phases 0 through 3 correspond to (LED1/LED2) in the states: (HIGH/HIGH), (LOW/HIGH), (HIGH/HIGH), (HIGH/LOW). Note that, for simplicity, the LEDs are controlled with analogWrite(), even in “digital” mode.

#include <Bounce2.h>

const uint8_t BUTTON_PIN = 4;
const uint8_t LED1 = 11;
const uint8_t LED2 = 10;

Bounce button;

void setup()
{
    button.attach(BUTTON_PIN, INPUT_PULLUP);
    pinMode(LED1, OUTPUT);
    pinMode(LED2, OUTPUT);
    analogWrite(LED1, 255);
    analogWrite(LED2, 255);
}

/*
 * Vary the LED brightnesses along the following pattern:
 *
 *  phase:   0   1   2   3   0
 *          ___     ___________
 *  LED1:      \___/
 *          ___________     ___
 *  LED2:              \___/
 */
void loop()
{
    static enum { DIGITAL, ANALOG } mode = DIGITAL;
    static uint8_t phase = 0;
    static uint16_t phase_duration = 100;
    static uint8_t low_level = 0;
    static uint16_t last_change = 0;

    // Switch modes on button presses.
    button.update();
    if (button.fell()) {
        switch (mode) {
            case DIGITAL:
                mode = ANALOG;
                low_level = 150;
                phase_duration = 400;
                break;
            case ANALOG:
                mode = DIGITAL;
                low_level = 0;
                phase_duration = 100;
                break;
        }

        // Set initial phase.
        phase = 0;
        last_change = millis();
        analogWrite(LED1, 255);
        analogWrite(LED2, 255);
    }

    // Switch phase after phase_duration elapsed.
    if (millis() - last_change >= phase_duration) {
        last_change += phase_duration;
        phase = (phase + 1) % 4;
        switch (phase) {
            case 0:
                analogWrite(LED2, 255);
                break;
            case 1:
                analogWrite(LED1, low_level);
                break;
            case 2:
                analogWrite(LED1, 255);
                break;
            case 3:
                analogWrite(LED2, low_level);
                break;
        }
    }
}

Answer 2

Use a boolean:

boolean blink = false;
if(digitalRead(output) == 1){
    blink = !blink;
}

if(blink == true){
    digitalWrite(led1,HIGH);
    digitalWrite(led2,HIGH);
    delay(100);
    digitalWrite(led1,LOW);
    delay(100);
    digitalWrite(led1,HIGH);
    delay(100);
    digitalWrite(led2,LOW);
    delay(100);
    digitalWrite(led2,HIGH); 
}else{
    analogWrite(led1,255);
    delay(400);
    analogWrite(led1,150);
    delay(400);
    analogWrite(led2,255);
    delay(400);
    analogWrite(led2,150);
}

Answer 3

There are several issues that I can see. To start with, you have the button wired to 2 pins, one configured as an input and one as an output. Starting out, you should connect the button as in the example below.

This will read LOW until the button is pressed.

I suspect in your case that because your using in INPUT_PULLUP, your button will read HIGH when the button isn't pressed, but will continue to read HIGH even after the button is pressed - which leads to the second issue of your first while loop - because mode is initialised to 0, and the button is always reading HIGH, the condition for your while loop will always be true, consequently, the loop will never exist.

The next issue (if you change the code so the first while loop does exit) is that you're trying to do a digitalRead on an OUTPUT pin - as an OUTPUT pin, it will never detect the state of the button.

Another issue will be that when you press the button and the if statement is reached (because digitalRead(input) == 0), even if the button is pressed for the shortest time, the loop function will be run several times, and the if statement will be run several times which will effectively result in the mode containing either 0 or 1 almost at random.

While the best way to fix this is with an interrupt (another lesson for another day), a simple way is something like this:

    int buttonPressedProcessed = 0; // an indicator so we know if the button press has been processed.

void setup() {
    ... 
}

void loop() {
  while(mode == 0 && digitalRead(input) == 1){
    buttonPressedProcessed = 0; // reset button pressed and processed indicator
    ...
  }
        // check also the button pressed and processed indicator
  if(digitalRead(input) == 0 && buttonPressedProcessed == 0){
    buttonPressedProcessed = 1; // set button pressed and processed indicator
    mode++;
    if(mode >= 2){mode = 0;}
  }
  while(mode == 1 && digitalRead(input) == 1){
    buttonPressedProcessed = 0; // reset button pressed and processed indicator
    ...
  }
}