I'm amazed I got a menu selection and value handling working at all after four days of looking at examples and trial-and-error.
I have four menu items, three of which should easily be switched between (button press on encoder, value change by rotating said encoder) and one service mode which won't be hidden, but shouldn't be changed during regular use, so for a good UX, I don't want to make the end user click past it each time. I have 50 ms debounce for switching between settings in the menu and 1000 ms for a long press to go to the forth menu option. This I can not get to work.
Code:
void loop() {
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState){ //Button is pressed, start counting for debounce
if ( (millis() - lastDebounceTime) > debounceDelay) { //button pressed long enough
if (buttonState == LOW){
if (menuItem == 0){
menuItem = 1;
Serial.println("menuItem was 0, set to 1");
}
else if (menuItem == 1){
menuItem = 2;
Serial.println("menuItem was 1, set to 2");
}
else{
menuItem = 0;
Serial.println("menuItem was 2, set to 0");
}
Serial.println((String)"menuItem " + menuItem);
if ( (millis() - lastDebounceTime) > longpressDelay) { //Listen for long press
menuItem=3;
Serial.println("menuItem set to 3");
}
}
lastDebounceTime = millis(); //Reset debounce
lastButtonState = buttonState;
DisplayUpdate();
}
}
All Serial.println is for debugging purposes.
The last if statement is fulfilled even for letting go of the button and waiting for 1000 ms. It should only execute on button press, which I hoped would be taken care of by the if (buttonState == LOW){. Should I use a while-loop while the buttonState== LOW? If yes, would that halt code execution if the button is stuck? Is there a more elegant way to do it?
Here is a minimum example to show the problem at hand:
#include <Encoder.h>
#define ENCODER_PIN_A 3
#define ENCODER_PIN_B 2
#define SW_PIN 4
const int buttonPin = SW_PIN;
int menuItem = 0;
int buttonState = HIGH;
int lastButtonState = HIGH;
int menu0Value = 0;
int menu1Value = 0;
int menu2Value = 0;
int menu3Value = 0;
unsigned long lastDebounceTime = 0;
unsigned long debounceDelay = 50;
unsigned long longpressDelay = 1000;
void setup() {
uint16_t time = millis();
time = millis() - time;
Serial.begin(9600);
Serial.println("Basic Encoder Test:");
pinMode(buttonPin, INPUT_PULLUP);
}
void loop() {
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState){ //Button is pressed, start counting for debounce
if ( (millis() - lastDebounceTime) > debounceDelay) { //button pressed long enough
if (buttonState == LOW){
if (menuItem == 0){
menuItem = 1;
Serial.println("menuItem was 0, set to 1");
}
else if (menuItem == 1){
menuItem = 2;
Serial.println("menuItem was 1, set to 2");
}
else{
menuItem = 0;
Serial.println("menuItem was 2, set to 0");
}
Serial.println((String)"menuItem " + menuItem);
if ( (millis() - lastDebounceTime) > longpressDelay) { //Listen for long press
menuItem=3;
Serial.println("menuItem set to 3");
}
}
lastDebounceTime = millis(); //Reset debounce
lastButtonState = buttonState;
}
}
}
It waits happily for 1000 ms from the button being released and goes to menuItem=3, so I need to keep clicking it to switch between the other items. The opposite of what I want. If I move the if (buttonState == LOW){ to go before the if ( (millis() - lastDebounceTime) > debounceDelay) { , it's never satisfied.
When debugging it is sometimes helpful to collapse code blocks. I collapsed the code block under if (buttonState == LOW)). Look closely at what happens. If you are holding the button down then as soon as the debounceDelay is exceeded lastButtonState will be set equal to buttonState and you will not enter the if (buttonState != lastButtonState) conditional again until the button is released! Therefore the if ( (millis() - lastDebounceTime) > longpressDelay) conditional will never be satisfied!
void loop() {
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState) { //Button is pressed, start counting for debounce
if ( (millis() - lastDebounceTime) > debounceDelay) { //button pressed long enough
if (buttonState == LOW)
{
// other stuff
}
lastDebounceTime = millis(); //Reset debounce
lastButtonState = buttonState;
}
}
}
If you have a long period of HIGH, and push the button into LOW, then if ( (millis() - lastDebounceTime) > longpressDelay) { triggers.
You only want a long LOW to kick you into menu 3.
To do that you have to remember when it last turned LOW, and check on it long after the initial debounce time. The check for a long press can't be inside all of the conditionals.
Thanks! I should pay more attention to this. Is it done manual by real programmers, or they just don't cut-paste things back and forth from inside to outside of a if-statement to do trial and errors?
Here is where I am at the moment. I'm not proud of it in any way, I'm just trying to learn. It does almost do what I expect from a UX. For example, the laptop I'm writing this on, if I press and hold down the power button, I wait until the computer shuts off. It would be very bad UX to wait for that minimum time and only have it shut down upon release of the button. I have the opposite, it waits for the button to be released before going to menuItem 3. I tried changing to if (buttonReleaseState == LOW){ but it gives very strange behavior.
#include <Encoder.h>
#define ENCODER_PIN_A 3
#define ENCODER_PIN_B 2
#define SW_PIN 4
const int buttonPin = SW_PIN;
int menuItem = 0;
int buttonState = HIGH;
int lastButtonState = HIGH;
int buttonLongState = HIGH;
int lastLongButtonState = HIGH;
int buttonReleaseState = HIGH;
int menu0Value = 0;
int menu1Value = 0;
int menu2Value = 0;
int menu3Value = 0;
unsigned long lastLongDebounceTime = 0;
unsigned long lastDebounceTime = 0;
unsigned long debounceDelay = 50;
unsigned long longpressDelay = 1000;
void setup() {
uint16_t time = millis();
time = millis() - time;
Serial.begin(9600);
Serial.println("Basic Encoder Test:");
pinMode(buttonPin, INPUT_PULLUP);
}
void loop() {
buttonState = digitalRead(buttonPin);
if (buttonState != lastButtonState){ //Button is pressed, start counting for debounce
if (( (millis() - lastDebounceTime) > debounceDelay)) { //button pressed long enough
if (buttonState == LOW){
if (menuItem == 0){
menuItem = 1;
Serial.println("menuItem was 0, set to 1");
}
else if (menuItem == 1){
menuItem = 2;
Serial.println("menuItem was 1, set to 2");
}
else{
menuItem = 0;
Serial.println("menuItem was 2, set to 0");
}
Serial.println((String)"menuItem " + menuItem);
}
lastDebounceTime = millis(); //Reset debounce
lastButtonState = buttonState; //Reset buttonState
// DisplayUpdate();
}
}
buttonLongState = digitalRead(buttonPin);
if (buttonLongState != lastLongButtonState){
Serial.println("buttonLongState has changed");
Serial.println((String)"buttonLongState is " + buttonLongState);
if (((millis() - lastLongDebounceTime) > longpressDelay)) { //Listen for long press
Serial.println("button pressed long enough to trigger long press");
buttonReleaseState = digitalRead(buttonPin);
Serial.println((String)"buttonReleaseState is " + buttonReleaseState);
if (buttonReleaseState == HIGH){
menuItem=3;
Serial.println((String)"buttonState is " + buttonState);
Serial.println((String)"millis is " + millis());
Serial.println((String)"lastLongDebounceTime is " + lastLongDebounceTime);
Serial.println((String)"lastDebounceTime is " + lastDebounceTime);
Serial.println("menuItem set to 3");
}
}
lastLongDebounceTime = millis(); //Reset long debounce
lastLongButtonState = buttonState;
}
}
All suggestions how to give the same behavior in a more structured and proficient manner is appreciated!
Understandable. My mechanical tutor does something very similar and complains how I work in SolidWorks which is not really suited for trying out things, going back and forth...