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

Pin debouncer for buttons rather than gamepad debouncer + improved logic #857

Merged
merged 8 commits into from
Feb 26, 2024

Conversation

bsstephan
Copy link
Contributor

@bsstephan bsstephan commented Feb 23, 2024

A couple classes had their own debouncer or reference to the gamepad debouncer. this seemed inefficient and caused potential for incongruent behavior, thinking that instead we should just debounce the GPIO pins (for buttons). this does that.

Meanwhile, Feral questioned the logic of our gamepad debouncer as of 0.7.6, which was a plausible explanation for reports of double inputs cropping up lately. Feral's bugfix has been incorporated into this refactor and we're reaching out to see if it addresses those issues.

a couple classes had their own debouncer or reference to the gamepad
debouncer. this seemed inefficient and caused potential for incongruent
behavior, thinking that instead we should just debounce the GPIO pins
(for buttons). this does that. this doesn't yet clean up all the code
(specifically the old debouncer) but it does get everything on the new
debouncer.

things appear to work as before, maybe they solve some debouncing
problems for folks, work to be done
this fixes boot time shortcuts that were broken by my refactor
@bsstephan bsstephan marked this pull request as ready for review February 23, 2024 19:39
@bsstephan bsstephan changed the title WIP: Pin debouncer for buttons rather than gamepad debouncer Pin debouncer for buttons rather than gamepad debouncer Feb 23, 2024
the previous debouncer logic would try to only wait for the
debounceDelay on button depresses, IIRC in an attempt to get the real
button press immediately, but the more I look at it, the more I come to
the same conclusion as Feral that such logic is flawed, and will
register flapping switches because it's eagerly not debouncing them.

with this change, button presses only register if it's been
debounceDelay (default 5ms) since the last change. buttons will still
register immediately as long as the delay has been met, so in practice
this may only affect a latency tester and some kind of superhuman who
mashes buttons at sub-5ms speeds.

this applies Feral's logic (from the button debouncer) to the refactored
pin debouncer.

Co-authored-by: FeralAI <13342258+FeralAI@users.noreply.github.com>
@FeralAI FeralAI mentioned this pull request Feb 23, 2024
@bsstephan bsstephan changed the title Pin debouncer for buttons rather than gamepad debouncer Pin debouncer for buttons rather than gamepad debouncer + improved logic Feb 24, 2024
@FeralAI FeralAI mentioned this pull request Feb 24, 2024
Copy link
Contributor

@arntsonl arntsonl left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Fantastic work, thank you @bsstephan . This is so much cleaner, one centralized place to handle our debouncing logic.

@arntsonl arntsonl merged commit 5e82068 into OpenStickCommunity:main Feb 26, 2024
30 checks passed
@strusic
Copy link

strusic commented Mar 1, 2024

Ive tried this fw, and debouncing works fantastic, but on xbox mode X button wont respond, sometimes I got shutdown menu on my xbox. On oled screen it show that button is pressed.

@arntsonl
Copy link
Contributor

arntsonl commented Mar 1, 2024

@strusic can you make an issue for that? I'll look into it, its probably something I didn't catch with the logic I'm using to do xbox one.

https://github.com/OpenStickCommunity/GP2040-CE/issues

@strusic
Copy link

strusic commented Mar 1, 2024

@arntsonl done, as you requested. Looking forward to new pull that fix this ;)

This pull request was closed.
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.

3 participants