Led on when not supposed to be

Question

So I made a program that controls two LED's when two buttons are pressed, but one of the LED's comes on when the button isn't pressed. And what's weird is that if I literally move around the breadboard the LED might turn off for a little, and if hold the button down while doing it the LED remains solid. I have done a lot of taking out and switching wires without any unexpected effects. However, if I switch the LED wires the other LED turns and stays on while the first one turns off, so I am pretty sure that it is a programming error. Here is my code:

const int ledPin1 = 8;
const int buttonPin1 = 1;
const int ledPin2 = 9;
const int buttonPin2 = 2;

int buttonState1 = 0;
int buttonState2 = 0;

void setup() 
{
  pinMode(ledPin1, OUTPUT);
  pinMode(buttonPin1, INPUT);
  pinMode(ledPin2, OUTPUT);
  pinMode(buttonPin2, INPUT);
}

void loop() 
{
buttonState1 = digitalRead(buttonPin1);
buttonState2 = digitalRead(buttonPin2);

  if (buttonState1 == LOW)
  {
    digitalWrite(ledPin1, HIGH);
  }
  else
  {
    digitalWrite(ledPin1, LOW); 
  }

  if (buttonState2 == LOW)
  {
    digitalWrite(ledPin2, HIGH);
  }
  else
  {
    digitalWrite(ledPin2, LOW); 
  }
}

Answer 1

The behavior you describe is symptomatic of a floating input.

I guess you have directly wired your buttons between input pins and +5V (or GND, that won't change the observed behavior).

The problem is when you don't push the button, the input pin is left floating and can be read as HIGH or LOW based on strange factors.

The solution to this is to use pullup (if you connect the other side of the button to GND) or pulldown (if your button is connected to +5V) resistors.

The following image, off the arduino website, shows such a circuit with a pulldown resistor, i.e. connect the switch to +5V and the resistor to GND:

An even simpler way is to connect the buttons to GND and use pinMode(pin, INPUT_PULLUP); in setup().

Answer 2

I don't think it is necessary to do the declaration of int buttonstate1 = 0; and int buttonstate2 = 0; seeing that in your loop the buttonstates gets read by you using digitalRead. Also I don't think the buttons is inverted, because you aren't using a pull up resistor if I'm correct.

Hopefully this helped.

Also something that I found helped a lot is that if you use a lastbuttonstate variable too. Than your if statement might look something like this:

if(buttonstate1 == HIGH && previousbuttonstate1 == LOW)
{
digitalWrite(ledPin1, HIGH);
previousbuttonstate1 = buttonstate1;
}

else if(buttonstate1 == HIGH && previousbuttonstate1 == HIGH)
{
digitalWrite(ledPin1, HIGH);
}

else if(buttonstate1 == LOW && previousbuttonstate1 == HIGH)
{
digitalWrite(ledPin1, LOW);
previousbuttonstate1 = buttonstate1;
}

else
{
digitalWrite(ledPin1, LOW);
}

Basically adding the previousbuttonstate1 just gives it a more accurate way of checking the button state(I'm typing under correction).

Sorry for the sloppy answer, but hopefully it helps.