wildbill:
There's nothing wrong with your buttons - look up debouncing. Using a small delay as you did is a common solution.
Not so much nothing wrong with the button, as nothing unusual. Switch contacts in general do bounce for a number of milliseconds, and digital devices are so fast that they tend to see every part of this bounce as a separate event.
You may be interested in my code framework for performing differently timed events in concert and robustly debouncing switch inputs. Detection of a prolonged press can easily be integrated by using your time comparison (for HOLD_DELAY) against the single button state flag that I provide.
// Blink without "delay()" - multi!
const int led1Pin = 13; // LED pin number
const int led2Pin = 10;
const int led3Pin = 11;
const int button1 = 4;
int led1State = LOW; // initialise the LED
int led2State = LOW;
int led3State = LOW;
char bstate1 = 0;
unsigned long count1 = 0; // will store last time LED was updated
unsigned long count2 = 0;
unsigned long count3 = 0;
unsigned long bcount1 = 0;
// Have we completed the specified interval since last confirmed event?
// "marker" chooses which counter to check
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.
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(led2Pin, OUTPUT);
pinMode(led3Pin, 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);
}
// Act if the latter time (ms) has now passed on this particular counter,
if (timeout(&count2, 300UL )) {
if (led2State == LOW) {
led2State = HIGH;
}
else {
led2State = LOW;
}
digitalWrite(led2Pin, led2State);
}
if (timeout(&count3, 77UL )) {
if (led3State == LOW) {
led3State = HIGH;
}
else {
led3State = LOW;
}
digitalWrite(led3Pin, led3State);
}
}