Unintended reaction: Sketch makes LEDs turn on and of randomly/LEDs don't react

Question

Update: This piece of code does not do the same as the one in the right answer, it also reacts when one lets go of the button.

int button_prev = 0;
int button = 0;

void loop(){

    button_prev = button;

    button = digitalRead(12);

    if (button_prev != button) {

      digitalWrite(act_pin[j], HIGH);

      j = (j+1) % 5;

    }
}

I printed out j and it seems like, instead of counting up +1 everytime the button is pressed once, it turns +1 and then +1 again which makes two LEDs at a time turn on. But I don't see where the behavior of j comes from...

[Update: There was a mistake in the code which caused the random behavior, but now, nothing happens when I push the button.]

[I wrote a sketch which is supposed to make the Arduino turn one of five LEDs on one after another every time I press the switch.

But instead of turning one LED on after another, directly after upload it just makes them turn on and of at random. I don't push the button. Serial.println as well just spits out one character, similiar looking to a d.]

Can anyone point out my mistake to me?

This is the circuit

and this is the old code of the sketch

int act_pin[6] = {A0, A1, A2, A3, A4};

int j = 0;

void setup(){

  Serial.begin(9600);

  for (int k = 0; k < 5; k++) {

     pinMode(act_pin[k], OUTPUT); //sets analog pins A0 - A4 as output

     digitalWrite(act_pin[k], LOW); //turns LEDs connected to A0 - A4 off

     pinMode(12, INPUT); //sets pin connected to button as input

}

}


void loop(){

  char button = {digitalRead(12)}; //stores if the button was pressed

  Serial.println(button);

  if(button == HIGH){ 

    digitalWrite(act_pin[j], HIGH); //turns LEDs on A0 - A4 on one after one with every button press 

    j = (j+1) % 5; //makes digitalWrite jump to the next LED next time the button is pressed and limits it to 4

    button = LOW; //turns the button's pin low again, so nothing will happen until the button is pressed again
}

}

Answer 1

I think you're looking for the action (in pseudo-code):

ONCE (Button Pressed):
    Light LED j;
    j++

But you have an IF(button) in a Loop{}. Something like:

WHILE (Button Pressed):
    Light LED j;
    j++

So when you press the button j will increment every iteration of Loop {} (very fast) and you won't be able to see the LEDs changing. Your 'button = LOW' is not effective because 'button' gets set again the next time if the button is still pressed.

Try printing j instead of button at the line:

Serial.println(j);

and that should highlight the problem.

I would implement a ONCE(Button Pressed) function by observing the previous button state and checking it has changed from LOW to HIGH.

    button_prev = button;
    button = digitalRead(12);
// Check if button changed from LOW to HIGH
    if (!button_prev && button) {
    ...code
    }

EDIT: Dealing with Contact Bounce

Switches are mechanical so they don't switch cleanly. This is called contact bounce and means the switch switches on/off very rapidly for about 10ms or so after the contact has been pressed or released.

See Measuring Switch Bounce

You can deal with it quite simply in the code as follows (pseudo code) just by waiting for the bounce to dissapear and checking the button is still pressed:

ONCE (Button Pressed):
   WAIT(20ms)              // wait for the bounce to stop
   IF(Button Pressed):     // Ensure the button is still pressed
    ...code...

Try the following Arduino Code:

    button_prev = button;
    button = digitalRead(12);
// Check if button changed from LOW to HIGH
    if (!button_prev && button) {
       delay(20);                 // Wait for debounce
       if(digitalRead(12))  {     // Make sure the button is still pressed
        ...code
       }
    }

Answer 2

Your code has a serious bug here:

if(button == HIGH){ 

    digitalWrite(act_pin[j], HIGH);

    j = j+1;

    button = LOW;
}

You increment j without any boundary so it will have the following cycling values:

0, 1, 2, 3 ... 32767, -32768, -32767 ... -1, 0 ...

Hence, quickly act_pin[j] will return garbage...

The solution is easy, you must limit j to [0,1,2,3,4], just replace the j = j + 1 line as follows:

j = (j + 1) % 5;

% is the modulo integral operation that will give you the reminder of dividing j by 5, which is always strictly less than 5, and it cycles as you want.

Answer 3

if(button = HIGH)

Oops.

if (button == HIGH)

And see the Debounce example sketch to learn how to debounce inputs.