4 Buttons 4 Lights

Hi Everyone,

I am new and this is my first post to the forum.arduino.cc family. I enjoy reading through your posts on the many complexities Arduino Uno offers and would like to ask a question of my own.

This question may seem simple for some to answer, here it goes...

I have 4 buttons and 4 lights on my breadboard that I am trying to make work independently of one another. For example, when button 1 (pin2) is pressed I would like light 1 (pin6) to illuminate, and the same for button 2 (pin3) / light 2 (pin4), and so on...

I have verified the code to be correct and get no errors when verified/uploaded to the Arduino Uno. But it just doesn't seem to illuminate the lights properly when each button is pressed. Could one of your please verify my code is type correctly? I will post the code below.

Thank you,

Daniel

/*
 // 4 Lights, 4 Buttons
 
*/

int switchstate = 0;

void setup(){
  
// declare the LED pins as outputs 
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  

  // declare the switch pins as an input   
  pinMode(2,INPUT);
  pinMode(3,INPUT);
  pinMode(4,INPUT);
  pinMode(5,INPUT);
}

void loop(){

  // read the value of the switch
  // digitalRead() checks to see if there is voltage
  // on the pin or not  
  switchstate = digitalRead(2);
  switchstate = digitalRead(3);
  switchstate = digitalRead(4);
  switchstate = digitalRead(5);
  
  // if the button is not pressed
  // blink the red LEDs  
  if (switchstate == LOW) {
    digitalWrite(6, LOW);  // turn the red LED on pin 6 off
    digitalWrite(7, LOW);  // turn the red LED on pin 7 off
    digitalWrite(8, LOW);  // turn the red LED on pin 8 off
    digitalWrite(9, LOW);  // turn the red LED on pin 9 off
  }
  // this else is part of the above if() statement. 
  // if the switch is not LOW (the button is pressed)
  // the code below will run  
  else {
    digitalWrite(6, HIGH);  // turn the red LED on pin 6 off
    digitalWrite(7, HIGH);  // turn the red LED on pin 7 off
    digitalWrite(8, HIGH);  // turn the red LED on pin 8 off
    digitalWrite(9, HIGH);  // turn the red LED on pin 9 off
    // wait for a quarter second before changing the light
    delay(2000);
    
    
  }
}

Welcome to the forum.

You need indepent variables here

switchstate = digitalRead(2);
switchstate = digitalRead(3);
switchstate = digitalRead(4);
switchstate = digitalRead(5);

and then in the if statements. Otherwise, everything is based on the last digitalRead performed.

Use code tags when posting code (the # button)

Thank you CrossRoads,

I will add your suggested lines to the code and see what happens.

Also, I just noticed I did not add a code dialogue box in my original post, all fixed now.

Daniel

Looks much better 8)

When you say I need to add independent variables, I am not really understanding, maybe I put it in the wrong place because it still is not working correctly. What variable should I be adding exactly?

 switchstate2 = digitalRead(2);
  switchstate3 = digitalRead(3);
  switchstate4 = digitalRead(4);
  switchstate5 = digitalRead(5);

  if (switchstate2 == LOW) {
    digitalWrite(6, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(6, HIGH);  // turn the red LED on pin 6 off
}

Repeat for other switches & LEDs

Add this after all 4 if-elses
// wait for a quarter second before changing the light
delay(2000); // This is 2 seconds, 1/4 second would be 250

Also, do you have external pullups on the inputs?
Make this change also if you don't:

  pinMode(2,INPUT);
digitalWrite (2, HIGH); // enable internal pullup

  pinMode(3,INPUT);
digitalWrite (3, HIGH); // enable internal pullup

  pinMode(4,INPUT);
digitalWrite (4, HIGH); // enable internal pullup

  pinMode(5,INPUT);
digitalWrite (5, HIGH); // enable internal pullup

Okay, great. I'm getting somewhere I think. However, it is still not working and is saying "switchstate2 not declared in this scope".

/*
 // 4 Lights, 4 Buttons
 
*/

int switchstate = 0;

void setup(){
  
// declare the LED pins as outputs 
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);
  

  // declare the switch pins as an input   
  pinMode(2,INPUT);
  pinMode(3,INPUT);
  pinMode(4,INPUT);
  pinMode(5,INPUT);
}

void loop(){

  // read the value of the switch
  // digitalRead() checks to see if there is voltage
  // on the pin or not  
  switchstate2 = digitalRead(2);
  switchstate3 = digitalRead(3);
  switchstate4 = digitalRead(4);
  switchstate5 = digitalRead(5);
  
  // if the button is not pressed
  // blink the red LEDs  
  if (switchstate2 == LOW) {
    digitalWrite(6, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(6, HIGH);  // turn the red LED on pin 6 on
}
    delay(2000);
    
   if (switchstate3 == LOW) {
    digitalWrite(7, LOW);  // turn the red LED on pin 7 off
  }
  else {
    digitalWrite(7, HIGH);  // turn the red LED on pin 7 onf
}
    delay(2000);
    if (switchstate4 == LOW) {
    digitalWrite(8, LOW);  // turn the red LED on pin 8 off
  }
  else {
    digitalWrite(8, HIGH);  // turn the red LED on pin 8 on
}
    delay(2000);
    if (switchstate5 == LOW) {
    digitalWrite(9, LOW);  // turn the red LED on pin 9 off
  }
  else {
    digitalWrite(9, HIGH);  // turn the red LED on pin 9 on
}
    pinMode(2,INPUT);
digitalWrite (2, HIGH); // enable internal pullup

  pinMode(3,INPUT);
digitalWrite (3, HIGH); // enable internal pullup

  pinMode(4,INPUT);
digitalWrite (4, HIGH); // enable internal pullup

  pinMode(5,INPUT);
digitalWrite (5, HIGH); // enable internal pullup
   
    delay(2000);   
  }
int switchstate = 0;

This is a variable declaration. Its required for any variable.

  switchstate2 = digitalRead(2);
  switchstate3 = digitalRead(3);
  switchstate4 = digitalRead(4);
  switchstate5 = digitalRead(5);

These are variable assignments. variables have to be declared before they are assigned a value.

I have those lines in my code; posted above. Did I put them in the wrong lines?

Read this:

http://www.cplusplus.com/doc/tutorial/variables/

Particularly the part about declarations.

/*
 // 4 Lights, 4 Buttons
 
 */

byte switchstate2 = 0;
byte switchstate3 = 0;
byte switchstate4 = 0;
byte switchstate5 = 0;
void setup(){

  // declare the LED pins as outputs 
  pinMode(6, OUTPUT);
  pinMode(7, OUTPUT);
  pinMode(8, OUTPUT);
  pinMode(9, OUTPUT);


  // declare the switch pins as an input   
  pinMode(2,INPUT_PULLUP);
  pinMode(3,INPUT_PULLUP);
  pinMode(4,INPUT_PULLUP);
  pinMode(5,INPUT_PULLUP);
}

void loop(){

  // read the value of the switch
  // digitalRead() checks to see if there is voltage
  // on the pin or not  
  switchstate2 = digitalRead(2);
  switchstate3 = digitalRead(3);
  switchstate4 = digitalRead(4);
  switchstate5 = digitalRead(5);

  // if the button is not pressed
  // blink the red LEDs  
  if (switchstate2 == LOW) {
    digitalWrite(6, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(6, HIGH);  // turn the red LED on pin 6 off
  }
  if (switchstate3 == LOW) {
    digitalWrite(7, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(7, HIGH);  // turn the red LED on pin 6 off
  }
  if (switchstate4 == LOW) {
    digitalWrite(8, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(8, HIGH);  // turn the red LED on pin 6 off
  }
  if (switchstate5 == LOW) {
    digitalWrite(9, LOW);  // turn the red LED on pin 6 off
  }
  else {
    digitalWrite(9, HIGH);  // turn the red LED on pin 6 off
  }
  // this else is part of the above if() statement. 
  // if the switch is not LOW (the button is pressed)
  // the code below will run  

  // wait for a quarter second before changing the light
  delay(250);


}

Not as readable perhaps, but you don't really need to store the state...

digitalWrite(6, digitalRead(2));
digitalWrite(7, digitalRead(3));
digitalWrite(8, digitalRead(4));
digitalWrite(9, digitalRead(5));

That format is good for simple thing like this if that is all it will do. Doesn't lend itself to expansion if you want to start expanding the code into doing more tho, and then the first thing you do is rewrite it using if-else or switch:case or something.

Why don't you take the next step and learn about functions?

Create a function that takes two pin numbers - one for the switch pin and one for the LED pin. In that function, read the switch state and set the LED state.

void readSwitchAndSetState(byte swiPin, byte ledPin)
{
   byte state = digitalRead(swiPin);
   digitalWrite(ledPin, state);
}

Then, call that function 4 times:

void loop()
{
   readSwitchAndSetState(2, 6);
   readSwitchAndSetState(3, 7);
   readSwitchAndSetState(4, 8);
   readSwitchAndSetState(5, 9);
}

The stuff you are doing for one set of pins in loop() now is exactly the same. Move that to a function, as shown here, to reduce the amount of code you have to write/modify/maintain/understand.

If you find that there is a problem with one set of input and output pins, it is likely that the problem affects the other three sets, too. Then, you have to fix the code 4 times. I only need to fix it once.

If you find that the wrong LED lights up when you press a given switch, then it is easy to see that it is the call to the function that is wrong, so it's much easier to see what needs to be fixed.

Oh, and when you get around to discovering that you want to press the switch once to turn the LED on and once to turn it off, fixing one set of code is going to be much easier.

Similarly, when you need to debounce the switches, doing it once is easier than doing it 4 times.

I keep telling myself that I am writing for speed, and jumping back & forth to functions takes more time vs inline code.
So I don't use them.
Hardware designer hangup ...

I also really dislike have to jump all around in someone's code trying to figure out what the heck is going on. Late at night, I just glaze over trying to follow some undocumented code with functions all over the place.

Functions and variables with meaningful names are your friend. In the Arduino IDE putting functions on different tabs makes it easy to switch to them for reference. Some editors support code folding so that sections can be temporarily collapsed to aid navigation around the code, but each to their own.

Presumably you use functions when the same code is used in various places in the same program.

Nope. All written inline. I have been coding Arduino since Fall 2010, have not written a single function.
I use stuff on the Arduino Reference page, which my wife tells me is a mix of macros and functions.
I'll use a couple of libraries - SPI, I2C, Serial, Keypad, VirtualWire. Sleep on a remote.
(Is writing an ISR a function? I've used interrupts twice.)

I do write stuff that is sort of function like.
For example, I will have code in multiple tabs that loop cycles thru:
I check if an RF message is received, update some variables, set some flags.
I check if a serial message is received, update some variables, set some flags.
I check if a certain amount of time has passed, update some variables, set some flags.
In the display update tab, if the display update flag is set I'll update the outputs & clear the flag, send a serial message out.
So sort of function like, and sort of not.

The real advantage, in my opinion, to having functions is that you can test each function, independently. Then, use a function KNOWING that it works. The digitalRead() function was not tested as part of a script that had a loop function that was 4000 lines long. That function was possibly developed as part of a larger script, but when it worked, it was made into a function.

Imagine having to replicate all the code in digitalRead() every time you needed to read a pin. Imagine doing that to support ALL the Arduino boards and clones.

Hey it works!

Thanks guys.