Well, firstly you have it wired wrong if you are taking the pushbuttons to 5V and using pull-downs, as it is much safer and more reliable to use pull-ups - yes, the internal pull-ups - and the switches to ground. This avoids bringing your 5V line out from the board and a potential, however slight, of accidentally shorting the 5V line to ground.
Secondly, your problem when buttons are pressed suggests that you do not understand the problem of bounce as it relates to all kinds of mechanical switches.
Here is an example code of debouncing to make a toggle function reliably work:
// Button toggle with extreme reliability!
const int led1Pin = 13; // LED pin number
const int button1 = 10;
int led1State = LOW; // initialise the LED
char bstate1 = 0;
unsigned long bcount1 = 0; // button debounce timer. Replicate as necessary.
// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
// Routines by Paul__B of Arduino Forum
boolean timeout(unsigned long *marker, unsigned long interval) {
if (millis() - *marker >= interval) {
*marker += interval; // move on ready for next interval
return true;
}
else return false;
}
// Deal with a button read; true if button pressed and debounced is a new event
// Uses reading of button input, debounce store, state store and debounce interval.
// Routines by Paul__B of Arduino Forum
boolean butndown(char button, unsigned long *marker, char *butnstate, unsigned long interval) {
switch (*butnstate) { // Odd states if was pressed, >= 2 if debounce in progress
case 0: // Button up so far,
if (button == HIGH) return false; // Nothing happening!
else {
*butnstate = 2; // record that is now pressed
*marker = millis(); // note when was pressed
return false; // and move on
}
case 1: // Button down so far,
if (button == LOW) return false; // Nothing happening!
else {
*butnstate = 3; // record that is now released
*marker = millis(); // note when was released
return false; // and move on
}
case 2: // Button was up, now down.
if (button == HIGH) {
*butnstate = 0; // no, not debounced; revert the state
return false; // False alarm!
}
else {
if (millis() - *marker >= interval) {
*butnstate = 1; // jackpot! update the state
return true; // because we have the desired event!
}
else
return false; // not done yet; just move on
}
case 3: // Button was down, now up.
if (button == LOW) {
*butnstate = 1; // no, not debounced; revert the state
return false; // False alarm!
}
else {
if (millis() - *marker >= interval) {
*butnstate = 0; // Debounced; update the state
return false; // but it is not the event we want
}
else
return false; // not done yet; just move on
}
default: // Error; recover anyway
{
*butnstate = 0;
return false; // Definitely false!
}
}
}
void setup() {
pinMode(led1Pin, OUTPUT);
pinMode(button1, INPUT);
digitalWrite(button1,HIGH); // internal pullup all versions
}
void loop() {
// Toggle LED if button debounced
if (butndown(digitalRead(button1), &bcount1, &bstate1, 10UL )) {
if (led1State == LOW) {
led1State = HIGH;
}
else {
led1State = LOW;
}
digitalWrite(led1Pin, led1State);
}
}
In fact, a quite simple variant of this code can be used to debounce eight buttons concurrently. And of course, though this code is configured to respond specifically to a new "make" operation, it is equally relevant to conveying a reliable "on-off" determination.
