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

Single press latency if both double press and long press are disabled. #122

Open
borovsky-d opened this issue Sep 11, 2023 · 4 comments
Open
Labels
enhancement New feature or request

Comments

@borovsky-d
Copy link

borovsky-d commented Sep 11, 2023

Is your feature request related to a problem? Please describe:
Hi, thanks a lot for adding 22a20b2, it noticeably reduces latency! I think it may be further reduced so filing this feature request.

Describe the solution you'd like:
Does the plugin really need to wait "Release" event if long and double press are disabled? it seems like "Release" would happen anyways so it seems like waiting it just delays the action.

Describe alternatives you've considered:
I don't see other alternatives.

Additional context:
If this idea sounds reasonable to you, I can submit a pull request with the change. Something like:

index 7bd7ea9..dbd70d5 100644
--- a/src/ButtonState.ts
+++ b/src/ButtonState.ts
@@ -148,12 +148,16 @@ export class ButtonTracker {
             case ButtonState.IDLE: {
                 if (action === 'Press') {
                     this.state = ButtonState.DOWN;
+                    this.log.debug(`btrk ${this.href} now in state DOWN`);
                     if (this.longPressDisabled) {
-                        this.log.info(`button ${this.href} long press disabled. suppressing.`);
+                        if (this.doublePressDisabled) {
+                            this.shortPressCB();
+                        } else {
+                            this.log.info(`button ${this.href} long press disabled. suppressing.`);
+                        }
                     } else {
                         this.timer = setTimeout(longPressTimeoutHandler, this.longPressTimeout);
                     }
-                    this.log.debug(`btrk ${this.href} now in state DOWN`);
                 } else {
                     // no-op
                     this.log.debug(`btrk ${this.href} no-op IDLE action ${action}`);

Thanks for considering it!

@borovsky-d borovsky-d added the enhancement New feature or request label Sep 11, 2023
@act9999
Copy link

act9999 commented Sep 11, 2023

This! I’m locked on an old version before double and long were implemented because I need responsiveness from something that’s replacing a physical switch.

@borovsky-d borovsky-d changed the title Single press latency if both doubdouble press and long press are disbled. Single press latency if both double press and long press are disabled. Sep 11, 2023
@thenewwazoo
Copy link
Owner

My time is so limited to work on this (or any other OSS project) that I greatly appreciate PRs, and chances are good that if you pinky-swear it works for you, I'll merge it and ship it straight to prod. 😆

@thenewwazoo
Copy link
Owner

@borovsky-d, I just looked at your changes in the -fast version and they look correct. Once you've tested, I'd greatly appreciate (and eagerly merge) a PR. :)

@dominick-han
Copy link

Hello, interested in this feature as well

Just tested it locally, seems like this triggers 1 short press event on DOWN and another 1 short press event on UP

I'll see if I can fix it and make PR accordingly

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

No branches or pull requests

4 participants