Skip to content
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

MultiButton - ClickUp Debounce not handled? #10

Open
sergeyken opened this issue Mar 20, 2023 · 4 comments
Open

MultiButton - ClickUp Debounce not handled? #10

sergeyken opened this issue Mar 20, 2023 · 4 comments

Comments

@sergeyken
Copy link

Hello,

After looking into the source of MultiButton.h I'm under impression, that debouncing is done only for pressing button, but not for its releasing, isn't it?
At first moment the button is !pressed, after _state==StatePressed there is a change: _state=StateClickUp; _new=true;
The call to isReleased() returns (_new && _state == StateClickUp) which is true at this moment.
No check for DEBOUNCE_DELAY is performed at this situation.

Please, correct me if I am wrong?

@poelstra
Copy link
Owner

@sergeyken You're correct. When a switch is released, it immediately indicates isReleased().

Note that any bounces that follow (during the 'turn off') are debounced, but they will be debounced as being part of a 'potential turn on' (which will subsequently be 'dismissed' as being a glitch).

'Graphical' example:

__#_#_########_#_#_#__________#_#_########
  | |   |     || | |          | |   |
  | |   |     || | |          | |   +-> isClick B
  | |   |     || | |          +-+-> discarded as glitches
  | |   |     |+-+-> discarded as 'turn on glitches' too
  | |   |     +-> isReleased A
  | |   +-> isClick A
  +-+-> discarded as glitches

This behavior leads to faster response times when using 'normal' mechanical switches, which was my original use-case and why I implemented it like this.

For the use-case where turning off might in itself be glitchy, i.e. where a short 'off glitch' should not be treated as turning off, I agree that the current implementation is not ideal.

In practice, I've also used the library with a capacitive touch system (which is a bit glitchy like that) and it worked just fine for me, but YMMV.

@sergeyken
Copy link
Author

sergeyken commented Mar 21, 2023

OK, thank you for this explanation.

Maybe, this happens rarely, but the situation like this one:
........########.########............
would be handled as two separate clicks for those who checked only isClick(), and/or isReleased().
Also isDoubleClick() is detected in this situation, rather than isLongClick() as one could expect.

@sergeyken
Copy link
Author

sergeyken commented Mar 21, 2023

I've used this MultiButton code to teach my grandkid who's learning C++, and Arduino. That's why I've noticed this possible misbehavior. I recommended him to split the original MultiButton into two separate classes:

  1. base class SureButton, to handle only debouncing situations, and to report only isClick(), and isReleased()
  2. derived class ClueButton (as based on SureButton), to check real click/release from its base class SureButton, and to convert them (if needed) into isSingleClick(), isDoubleClick(), and isLongClick(); it makes the whole stuff easier to understand.

@poelstra
Copy link
Owner

Nice! Indeed, splitting them like that makes perfect sense. Especially as a learning experience, it's interesting to build something like this yourself :)

I'll leave this issue open for now, in case others run into something similar. I'll perhaps fix it myself some day.

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

No branches or pull requests

2 participants