Momentary button as ON/OFF Switch

This code does not work, what could be the reason ?

const byte buttonpin1 = 2;
const byte buttonpin2 = 3;
const byte buttonpin3 = 4;
const byte buttonpin4 = 5;
const byte buttonpin5 = 6;
const byte ledpin1 = 8;
const byte ledpin2= 9;
const byte ledpin3= 10;
const byte ledpin4= 11;
const byte ledpin5 = 12;
byte state = 0;
void setup()
{ 
   
pinMode(buttonpin1, INPUT);
pinMode(buttonpin2, INPUT);
pinMode(buttonpin3, INPUT);
pinMode(buttonpin4, INPUT);
pinMode(buttonpin5, INPUT);
pinMode(ledpin1, OUTPUT);
pinMode(ledpin2, OUTPUT);
pinMode(ledpin3, OUTPUT);
pinMode(ledpin4, OUTPUT);
pinMode(ledpin5, OUTPUT);
}
void loop()
{
    if (digitalRead(buttonpin1) == HIGH)
 {
        state = !state;  // toggle state
        digitalWrite(ledpin1, state);
        delay(10);  // debounce
        while (digitalRead(buttonpin1) == HIGH) {}
        delay(10);  // debounce
    }
    if (digitalRead(buttonpin2) == HIGH)
 {
        state = !state;  // toggle state
        digitalWrite(ledpin2, state);
        delay(10);  // debounce
        while (digitalRead(buttonpin2) == HIGH) {}
        delay(10);  // debounce
    }
    if (digitalRead(buttonpin3) == HIGH)
 {
        state = !state;  // toggle state
        digitalWrite(ledpin3, state);
        delay(10);  // debounce
        while (digitalRead(buttonpin3) == HIGH) {}
        delay(10);  // debounce
    }
    if (digitalRead(buttonpin4) == HIGH)
 {
        state = !state;  // toggle state
        digitalWrite(ledpin4, state);
        delay(10);  // debounce
        while (digitalRead(buttonpin4) == HIGH) {}
        delay(10);  // debounce
    }
    if (digitalRead(buttonpin5) == HIGH)
 {
        state = !state;  // toggle state
        digitalWrite(ledpin5, state);
        delay(10);  // debounce
        while (digitalRead(buttonpin5) == HIGH) {}
        delay(10);  // debounce
    }
}

First we need to determine if you have properly wired up your button switches. I see in your code you are not enabling internal pull-ups so that means you need to have wired external pull-up or pull down resistors for each switch and wired the switches correctly to either ground or +5vdc depending on which type of resistor pull (up or down) you are using.

The simplest way is to enable internal pull-ups for the button pins and wire the switch between the digital pin and ground.

Lefty

Switches are connected

o +5V
|
|
/
\ 10K
/
| 100 Ohm
o----///----o To pin
|
o
o Switch
|
GND

LEDs are connected with pullup resistor of 560 Ohms

Your pull-up resistors ensure that your inputs are high if the buttons are not pressed. Your if statements should be testing for low

You are sharing the state variable among the five leds, you need five state variables, or an array

Your code is very repetitive, consider using a function and passing the input pin, output pin & a pointer to state.

Thanks Mr. Wildbill.
I tested the code for single led. It is working OK.
You are right that I am sharing the state variables among the 5 leds, this is the main cause of the malfunction.
How do I share the state variables among 5 leds in this particular sketch to make it working ?

Easiest way, in keeping with how you've handled the other multiples is to have five state variables - state1, state2 etc. Better would be to look at arrays, but you would need to change your program rather more to get any benefit.

Trying to change the above mentioned program to Arrays but it gives the following error and does not compile.
Void loop () {
A-function definition is not allowed here before “{” token
Please help to resove this issue.

const byte buttons[]={2,3,4,5,6};
const byte leds[] = {8,9,10,11,12};
byte state[]={1,2,3,4,5};

void setup()
{ 
byte i;
for (i = 0; i < 5; i = i + 1) {

pinMode(buttons[i], INPUT);
pinMode(leds[i],OUTPUT);
}
void loop(){
 byte i;
for (i = 0; i < 5; i = i + 1) {
 
{
    if (digitalRead(buttons[i]) == LOW)
 {
        state[i] = !state[i]  // toggle state
        digitalWrite(leds[i], state[i]);
        delay(10);  // debounce
        while (digitalRead(buttons[i]) == LOW) {}
        delay(10);  // debounce
    }
   
    }
}
const byte buttons[]={2,3,4,5,6};
const byte leds[] = {8,9,10,11,12};
byte state[]={1,2,3,4,5};

void setup()
{ 
byte i;
for (i = 0; i < 5; i = i + 1) {

pinMode(buttons[i], INPUT);
pinMode(leds[i],OUTPUT);
} }
void loop(){
 byte i;
for (i = 0; i < 5; i = i + 1) {
 
{
    if (digitalRead(buttons[i]) == LOW)
 {
        state[i] = !state[i];  // toggle state
        digitalWrite(leds[i], state[i]);
        delay(10);  // debounce
        while (digitalRead(buttons[i]) == LOW) {}
        delay(10);  // debounce
    }
   
    }
} 
}

You were missing a } at the end of both setup() and loop() also there was a missing ;

There is no need for the 100R series resistor on the input line, in fact it makes it slightly less reliable.

Thanks Mike.
Intially when the board is powered or reset, leds does not work with single push, it required two pushes
to make it working, afterwards it works with single push. What could be the problem with this code ?

I think the resistor is there to protect the chip if you plug in a pre-programmed chip, whose pin was set to output high, and a switch shorts it to ground ( before reprogramming the chip)

it required two pushes
to make it working,

Yes that's how you wrote it, you set up this array.

byte state[]={1,2,3,4,5};

I don't know why you put those numbers in it as you only need it to be high or low. I would change it to:-

byte state[]={0,0,0,0,0};

Thanks all for enlightening me. The sketch is working OK
How to implement logical operators ie. AND OR in above mentioned code ?
The state of five buttons & leds are being monitored, when the button is pressed coressponding led comes ON.
Suppose we want ledpin6 to be ON when ledpin3 and ledpin4 comes ON
i.e. digitalRead(ledpin3==LOW && ledpin4==LOW); digitalWrite(ledpin6==LOW)

Trying to switch ON led6 when the state of button 1 & 2 is LOW
but it does not work. Please guide me What is the correct way to make it working ?

if ((state1 =!state1) && (state2 =!state2))
    {
    digitalWrite(ledpin6,LOW);
    }else{
    digitalWrite(ledpin6,HIGH);
    {

Hard to say without seeing your current sketch, but this should get you closer:

if ((state1 ==LOW) && (state2 ==LOW))
  {
  digitalWrite(ledpin6,LOW);
  }
else
  {
  digitalWrite(ledpin6,HIGH);
  }

Thanks Mr. Wildbill

I tried the following code, which is also working but I think my approach was wrong

if  ((state1!=1)&&(state2!=1)) {
   digitalWrite(ledpin6,LOW); 
} else 
{
digitalWrite(ledpin6,LOW); 
}

I wish to store the status of leds in EEPROM i…e. led1,led2,led3,led4 and led6, In case of power failure, it should show the actual status which was prior to the power failure. I tried the following code but it does not work.Please help me to resolve this issue.

#include <EEPROM.h>
const byte buttonpin1 = 2;
const byte buttonpin2 = 3;
const byte buttonpin3 = 4;
const byte buttonpin4 = 5;
const byte buttonpin5 = 6;
const byte ledpin1 = 8;
const byte ledpin2= 9;
const byte ledpin3= 10;
const byte ledpin4= 11;
const byte ledpin5 = 12;
const byte ledpin6 = 13;
byte state1 = 0;
byte state2 = 0;
byte state3 = 0;
byte state4 = 0;
byte state5 = 0;
int add = 0;
void setup()
{ 
   
pinMode(buttonpin1, INPUT);
pinMode(buttonpin2, INPUT);
pinMode(buttonpin3, INPUT);
pinMode(buttonpin4, INPUT);
pinMode(buttonpin5, INPUT);
pinMode(ledpin1, OUTPUT);
pinMode(ledpin2, OUTPUT);
pinMode(ledpin3, OUTPUT);
pinMode(ledpin4, OUTPUT);
pinMode(ledpin5, OUTPUT);
}
void loop()
{
    if (digitalRead(buttonpin1) == LOW)
 {
        state1 = !state1;  // toggle state
        digitalWrite(ledpin1, state1);
        delay(100);  // debounce
        while (digitalRead(buttonpin1) == LOW) {}
        delay(100);  // debounce
    }
    if (digitalRead(buttonpin2) == LOW)
 {
        state2 = !state2;  // toggle state
        digitalWrite(ledpin2, state2);
        delay(100);  // debounce
        while (digitalRead(buttonpin2) == LOW) {}
        delay(100);  // debounce
    }
    if (digitalRead(buttonpin3) == LOW)
 {
        state3 = !state3;  // toggle state
        digitalWrite(ledpin3, state3);
        delay(100);  // debounce
        while (digitalRead(buttonpin3) == LOW) {}
        delay(100);  // debounce
    }
    
    if (digitalRead(buttonpin4) == LOW)
 {
        state4 = !state4;  // toggle state
        digitalWrite(ledpin4, state4);
        delay(100);  // debounce
        while (digitalRead(buttonpin4) == LOW) {}
        delay(100);  // debounce
    }
    if (digitalRead(buttonpin5) == LOW)
 {
        state5 = !state5;  // toggle state
        digitalWrite(ledpin5, state5);
        delay(100);  // debounce
        while (digitalRead(buttonpin5) == LOW) {}
        delay(100);  // debounce
    }
   
    if ((state1 ==LOW) && (state2 ==LOW))
  {
  digitalWrite(ledpin6,LOW);
  }
else
  {
  digitalWrite(ledpin6,HIGH);
  }
  
  EEPROM.write(0, state1);
  EEPROM.write(0, state2);
  EEPROM.write(0, state3);
  EEPROM.write(0, state4);
  EEPROM.write(0, state5);
    
}

Hello there !
I am having the following problem, when I am writing to EEPROM

Power ON all leds are ON. (OK)
When button 1 is pressed led1 goes OFF, When Reset led1 remains OFF. (remembers the state of the button1)
but When button 2 is pressed led2 goes OFF, When Reset, led2 remains OFF but led1 goes ON (remembers the state of button2 but
looses the state of button2) and so on with other buttons
Note; It remembers only one state of the button at a time.

Problem no. 2 when i insert contiondition (state1 ==LOW) && (state2 ==LOW)
it does not behave properly.
Please help me !!
The code I am using is

#include <EEPROM.h>
const byte buttonpin1 = 2;
const byte buttonpin2 = 3;
const byte buttonpin3 = 4;
const byte buttonpin4 = 5;


const byte ledpin1 = 8;
const byte ledpin2= 9;
const byte ledpin3= 10;
const byte ledpin4= 11;
const byte ledpin6= 12;

byte state1 = 0;
byte state2 = 0;
byte state3 = 0;
byte state4 = 0;

int add = 0;
void setup()
{ 
   
pinMode(buttonpin1, INPUT);
pinMode(buttonpin2, INPUT);
pinMode(buttonpin3, INPUT);
pinMode(buttonpin4, INPUT);

pinMode(ledpin1, OUTPUT);
pinMode(ledpin2, OUTPUT);
pinMode(ledpin3, OUTPUT);
pinMode(ledpin4, OUTPUT);
pinMode(ledpin6, OUTPUT);

}
void loop()
{
    if (digitalRead(buttonpin1) == LOW)
 {
        state1 = !state1;  // toggle state
        digitalWrite(ledpin1, state1);
        delay(100);  // debounce
        while (digitalRead(buttonpin1) == LOW) {}
        int state1= digitalRead(buttonpin1);
        
        delay(500);  // debounce
    }
    if (digitalRead(buttonpin2) == LOW)
 {
        state2 = !state2;  // toggle state
        digitalWrite(ledpin2, state2);
        delay(100);  // debounce
        while (digitalRead(buttonpin2) == LOW) {}
         int state2= digitalRead(buttonpin2);
        delay(500);  // debounce
    }
    if (digitalRead(buttonpin3) == LOW)
 {
        state3 = !state3;  // toggle state
        digitalWrite(ledpin3, state3);
        delay(100);  // debounce
        while (digitalRead(buttonpin3) == LOW) {}
         int state3= digitalRead(buttonpin3);
        delay(500);  // debounce
    }
    
    if (digitalRead(buttonpin4) == LOW)
 {
        state4 = !state4;  // toggle state
        digitalWrite(ledpin4, state4);
        delay(100);  // debounce
        while (digitalRead(buttonpin4) == LOW) {}
         int state4= digitalRead(buttonpin4);
        delay(500);  // debounce
    }
    
   if ((state1 ==LOW) && (state2 ==LOW))
  {
  digitalWrite(ledpin6,LOW);
  }
else
  {
  digitalWrite(ledpin6,HIGH);
  }
 
 
  EEPROM.write(0, state1);
  EEPROM.write(0, state2);
  EEPROM.write(0, state3);
  EEPROM.write(0, state4);
  
  add=add+1;
  if( add=512 )
   add=0;

}

The setup() function should have a routine in it to read the EEPROM and restore the LED states. I can't see one.

Thanks Mike

The setup() function should have a routine in it to read the EEPROM and restore the LED states.

I wrote this routine for setup but no success.

//write a 0 to all 512 bytes of the EEPROM
for (int i = 0; i < 512; i++)
EEPROM.write(i, 0);

No a function to read the EEPROM and set the state of your program accordingly. With that you are just overwriting anything you previously wrote to it.