Fix bug where isPressed returns True a second time
As currently implemented, if the button is pressed and immediately released before the next loop call, e.g., due to a blocking action like a delay call, the subsequent loop call will still consider the button as pressed. The same behavior occurs with unpressed when holding the button, and only momentarily releasing it.
To fix this issue, we need to do away with the previousSteadyState variable so that it does not require two calls to loop in order to change the state.
Here is an example program that demonstrates the issue:
#include <ezButton.h>
ezButton button(14);
int count = 0;
void setup() {
Serial.begin(9600);
button.setDebounceTime(50);
}
void loop() {
button.loop();
if(button.isPressed()) {
Serial.print(++count);
Serial.print(" pressed");
delay(500);
}
else if(button.isReleased()) {
Serial.println(" released");
delay(500);
}
}
The output produced looks like:
23:52:08.656 -> 1 pressed2 pressed released
23:52:11.361 -> 3 pressed4 pressed released
23:52:13.805 -> 5 pressed6 pressed released
23:52:16.183 -> 7 pressed8 pressed released
With this fix in place the output looks as expected:
23:54:25.883 -> 1 pressed released
23:54:27.468 -> 2 pressed released
23:54:28.688 -> 3 pressed released
23:54:29.877 -> 4 pressed released
23:54:30.996 -> 5 pressed released
I believe this pull request may resolve #10, #12, and possibly #8.
I should have looked first! I have been vexed by this flaw. I wrote a method that I added to a local copy of ezButton to handle the updating of the previousSteadyState and lastSteadyState correctly all circumstances. I left out the counting stuff and had to add a few variables to the class. Otherwise it leaves intact all the functions.To use it, I just call dsLoop(). David Smith's loop.
// C++, so this is the loop.
// to the class I add
// byte state = is; // assumed stable, say if that isn't good for you, what to do?
// bool isOne = false; // needs initialisation for she is pressing already? Yes, modify constructor
// unsigned long lastTime;
void ezButton::dsLoop() {
bool reading = digitalRead(btnPin) == HIGH; // exactly.
unsigned long now = millis(); // argh! no global now
previousSteadyState = lastSteadyState;
switch (state) {
case is :
if (reading != isOne) {
lastTime = now;
state = may;
}
break;
case may :
if (reading == isOne)
state = is;
else {
if (now - lastTime > debounceTime) { // use ezB's debounce setting. if it is zero, what?
state = is;
isOne = !isOne; // only toggle of internal stable state
lastSteadyState = isOne;
}
}
break;
}
}
It stabilises the reading and only stable readings get pushed into the P<-L history. In state is we are stable. Transitions move to state may. If that obtains for the debounce period, the stable state is toggled. Otherwise, it returns to the current state move back to is.
The original loop looked plausible but does indeed need to be called in time to "see" some things that it doesn't really need to see to do its job.
Either way this issue gets solved for everyone will be nice. I would just replace the loop method, but I can imagine fixing it that way would break some existing code. Actually, I can't imagine that, but you know.
alto777