Skip to content

Fix bug where isPressed returns True a second time #25

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

bboe
Copy link

@bboe bboe commented Dec 5, 2024

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.

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.
@alto777
Copy link

alto777 commented Feb 3, 2025

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

using setDebounceTime() causes that isPressed() evaluates to true two times
2 participants