So, I have this project I'm working on, think alarm clock, where I have two buttons. The first button navigates the menu when pressed and allows the user to set the values of the menu (i.e. change the time), and the other button acts as the snooze.
However, there is another menu. An administrative menu, so to speak, that is accessed by holding both buttons down for a few seconds. Currently, everything works like a charm, except for one scenario which causes all presses to register instantaneously as holds. If the user holds down both buttons for double the allotted time, everything goes berserk. It also appears to happen after a long delay, after a couple minutes it seems, every input is detected as a hold. It's incredibly frustrating.
Anyways, here's the code. Any chance anyone could help me either A) resolve this bug or B) revamp the code so that it functions properly. Any input or help is appreciated. Thanks so much!
//Subroutine to read button1 presses and record whether it is a hold or a short press
void buttonRead()
{
//Read the state of the button1
buttonVal1 = digitalRead(buttonPin1);
//Test for button1 and button2 held down
if (buttonVal1 == LOW && (millis() - buttonDownTime1) > 4500
&& buttonVal2 == LOW && (millis() - buttonDownTime2) > 4500)
{
buttonCombo = true;
ignoreUp1 = true;
ignoreUp2 = true;
}
//Test for button1 pressed and store the down time
if (buttonVal1 == LOW && buttonLast1 == HIGH && (millis() - buttonUpTime1) > debounce)
{
ignoreUp1 = false;
buttonDownTime1 = millis();
}
//Test for button1 release and store the up time
if (buttonVal1 == HIGH && buttonLast1 == LOW && (millis() - buttonDownTime1) > debounce)
{
if (ignoreUp1 == false) buttonPress1 = true;
buttonUpTime1 = millis();
}
//Test for button1 held down for longer than the hold time
if (buttonVal1 == LOW && (millis() - buttonDownTime1) > holdTime)
{
buttonHold1 = true;
ignoreUp1 = true;
buttonDownTime1 = millis();
}
buttonLast1 = buttonVal1;
//Read the state of the button2
buttonVal2 = digitalRead(buttonPin2);
//Test for button2 pressed and store the down time
if (buttonVal2 == LOW && buttonLast2 == HIGH && (millis() - buttonUpTime2) > debounce)
{
ignoreUp2 = false;
buttonDownTime2 = millis();
}
//Test for button2 release and store the up time
if (buttonVal2 == HIGH && buttonLast2 == LOW && (millis() - buttonDownTime2) > debounce)
{
if (ignoreUp2 == false) buttonPress2 = true;
buttonUpTime2 = millis();
}
//Test for button2 held down for longer than the hold time
if (buttonVal2 == LOW && (millis() - buttonDownTime2) > holdTime)
{
buttonHold2 = true;
ignoreUp2 = true;
buttonDownTime2 = millis();
}
buttonLast2 = buttonVal2;
}
//Framework for testing/debugging and using button input
void buttonAction()
{
buttonRead();
Serial.print(buttonLast1);
Serial.print(" ");
Serial.print(buttonLast2);
Serial.print(" ");
Serial.print(buttonVal1);
Serial.print(" ");
Serial.print(buttonVal2);
Serial.println();
if (buttonPress1)
{
Serial.print("Button 1 Pressed");
Serial.println();
buttonPress1 = false;
}
if (buttonHold1)
{
Serial.print("Button 1 Held");
Serial.println();
buttonHold1 = false;
}
if (buttonPress2)
{
Serial.print("Button 2 Pressed");
Serial.println();
buttonPress2 = false;
}
if (buttonHold2)
{
Serial.print("Button 2 Held");
Serial.println();
buttonHold2 = false;
}
if (buttonCombo)
{
Serial.print("Button Combo");
Serial.println();
buttonCombo = false;
buttonHold1 = false;
buttonHold2 = false;
}
}
Okay, I've fixed the problem, but I admit that quite a bit of the code was copied from other places, which I then tested in my application and adjusted, so I was wondering if anyone could explain to me a few logical issues that I'm having trouble understanding. First off, the new code, including variable declarations this time.
//Button1 Variables
int buttonPin1 = 8;
int buttonVal1 = 0;
int buttonLast1 = 0;
unsigned long buttonDownTime1;
unsigned long buttonUpTime1;
boolean ignoreUp1 = false;
boolean buttonPress1 = false;
boolean buttonHold1 = false;
//Button2 Variables + Debounce and Hold Time
int buttonPin2 = 9;
int buttonVal2 = 0;
int buttonLast2 = 0;
unsigned long buttonDownTime2;
unsigned long buttonUpTime2;
boolean ignoreUp2 = false;
boolean buttonPress2 = false;
boolean buttonHold2 = false;
//ButtonCombo variables
boolean buttonCombo = false;
boolean ignoreHold1 = false;
boolean ignoreHold2 = false;
//Button use variables
int debounce = 20;
const long holdTime = 5000;
//Subroutine to read button presses and record whether it is a hold or a short press
void buttonRead()
{
//Read the state of the button1
buttonVal1 = digitalRead(buttonPin1);
//Test for button1 and button2 held down
if (buttonVal1 == LOW && (millis() - buttonDownTime1) > 4500
&& buttonVal2 == LOW && (millis() - buttonDownTime2) > 4500)
{
buttonCombo = true;
ignoreUp1 = true;
ignoreUp2 = true;
ignoreHold1 = true;
ignoreHold2 = true;
buttonDownTime1 = millis();
buttonDownTime2 = millis();
}
//Test for button1 pressed and store the down time
if (buttonVal1 == LOW && buttonLast1 == HIGH && (millis() - buttonUpTime1) > debounce)
{
ignoreUp1 = false;
ignoreHold1 = false;
buttonDownTime1 = millis();
}
//Test for button1 release and store the up time
if (buttonVal1 == HIGH && buttonLast1 == LOW && (millis() - buttonDownTime1) > debounce)
{
if (ignoreUp1 == false) buttonPress1 = true;
buttonUpTime1 = millis();
}
//Test for button1 held down for longer than the hold time
if (buttonVal1 == LOW && (millis() - buttonDownTime1) > holdTime)
{
ignoreUp1 = true;
if (ignoreHold1 == false) buttonHold1 = true;
buttonDownTime1 = millis();
}
buttonLast1 = buttonVal1;
//Read the state of the button2
buttonVal2 = digitalRead(buttonPin2);
//Test for button2 pressed and store the down time
if (buttonVal2 == LOW && buttonLast2 == HIGH && (millis() - buttonUpTime2) > debounce)
{
ignoreUp2 = false;
ignoreHold2 = false;
buttonDownTime2 = millis();
}
//Test for button2 release and store the up time
if (buttonVal2 == HIGH && buttonLast2 == LOW && (millis() - buttonDownTime2) > debounce)
{
if (ignoreUp2 == false) buttonPress2 = true;
buttonUpTime2 = millis();
}
//Test for button2 held down for longer than the hold time
if (buttonVal2 == LOW && (millis() - buttonDownTime2) > holdTime)
{
ignoreUp2 = true;
if (ignoreHold2 == false) buttonHold2 = true;
buttonDownTime2 = millis();
}
buttonLast2 = buttonVal2;
}
//Framework for testing/debugging and using button input
void buttonAction()
{
buttonRead();
Serial.print(buttonLast1);
Serial.print(" ");
Serial.print(buttonLast2);
Serial.print(" ");
Serial.print(buttonVal1);
Serial.print(" ");
Serial.print(buttonVal2);
Serial.println();
if (buttonPress1)
{
Serial.print("Button 1 Pressed");
Serial.println();
buttonPress1 = false;
}
if (buttonHold1)
{
Serial.print("Button 1 Held");
Serial.println();
buttonHold1 = false;
}
if (buttonPress2)
{
Serial.print("Button 2 Pressed");
Serial.println();
buttonPress2 = false;
}
if (buttonHold2)
{
Serial.print("Button 2 Held");
Serial.println();
buttonHold2 = false;
}
if (buttonCombo)
{
Serial.print("Button Combo");
Serial.println();
buttonCombo = false;
buttonHold1 = false;
buttonHold2 = false;
}
}
I added a couple of ignoreButtonHold variables, to prevent the buttonHolds from triggering after buttonCombo has triggered. I also added redeclaration of the buttonDownTime variables to restart the clock, so to speak, on the hold.
So here are my questions.
In all of the buttonHold scenarios, there is required a redeclaration of the buttonDownTime variables. This restarts the timer to determine how long the button has been held down, so I understand the reasoning, but I can't help but feel like there must be a better way to do this. Some way to prevent the program from running until the button is released. I think I can issue another if statement to check for buttonLast = HIGH. That sounds right. Just typing it out seems to help get everything in order, and maybe this post can help others.
This method of declaring and storing button information seems terribly inefficient to me. It just seems like a lot of code for something so simple. Does anyone have any ideas on a better way to do this? Reading button inputs, reading how long the buttons have been held, and relaying that information in a useable way?
Right now, I'm overwriting buttonHold with buttonCombo but making the holdTime on buttonCombo half a second shorter than holdTime for buttonHold. It makes sense, and it works, but, again, it seems inefficient. It's simple logic, but more complex logic might make the situation easier to manage.
Thanks for your help everyone! Hopefully the answers will help other people out with similar questions!
void buttonRead()
{
//Read the state of the button1
buttonVal1 = digitalRead(buttonPin1);
//Test for button1 and button2 held down
if (buttonVal1 == LOW && (millis() - buttonDownTime1) > 4500
&& buttonVal2 == LOW && (millis() - buttonDownTime2) > 4500)
{
might be a good idea to read buttonVal2 before testing it as otherwise it will not reflect the true state