Skip to content

Correct double-click detection logic #676

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

Merged

Conversation

danleongjy
Copy link
Contributor

Thank you for taking the time to work on a Pull Request. Your contribution is really appreciated! 🎉
Please don't delete any part of the template, since keeping the provided structure will help maintainers to review your work more rapidly.

Sections marked as * are required and need to be filled in.

Breaking change

  • Corrects logic for detecting virtual double-presses, so that it will only register double-presses if the same button was pressed twice within the helper_double_press_delay. The button's behaviour may change from what end-users previously expected.
  • Previously, any double-press writes "a": "double_press" to helper_last_controller_event. This PR adjusts helper_last_controller_event to identify which button was double-pressed (eg. "a": "double_press_up"). Users implementing automations that look for "a": "double_press" in the state of helper_last_controller_event need to adjust their automations.

Proposed change*

There is an issue with the logic for checking for virtual double-presses. First, the old state of the helper_last_controller_event is saved to variable last_controller_event, then helper_last_controller_event is updated to the new trigger_action. Finally, the check for double-press uses the condition trigger_action | string in states(helper_last_controller_event). This effectively checks that trigger_action is in itself. As a result, if any two buttons are pressed within helper_double_press_delay, a double-press action is detected, even if they are different buttons. Which double-press action is fired depends on which is the first one exposed. Changing the condition to trigger_action | string == last_controller_event reflects the intended logic correctly and results in the intended behaviour.

To enable better tracking of which button was double-pressed, the value pushed to helper_last_controller_event is also adjusted to be button-specific.

Tested for E1810, E1743, E1766, E1812 and E2001/2002

Closes #643

Checklist*

  • I followed sections of the Contribution Guidelines relevant to changes I'm proposing.
  • I properly tested proposed changes on my system and confirm that they are working as expected.
  • I formatted files with Prettier using the command npm run format before submitting my Pull Request.

Copy link
Contributor

Hey @danleongjy, thank you so much for your contribution! 🚀

🔄 We're currently running a few checks to make sure that everything is great with your contribution.
If further actions need to be performed before your contribution can be reviewed, additional guidance will be provided to you in the next comment.

Results are coming soon, stay tuned!

Copy link
Contributor

github-actions bot commented Mar 30, 2025

Hey @EPMatt, thank you so much for your contribution! 🚀

🔄 We're currently running a few checks to make sure that everything is great with your contribution.
If further actions need to be performed before your contribution can be reviewed, additional guidance will be provided to you in the next comment.

Results are coming soon, stay tuned!

Copy link
Contributor

github-actions bot commented Mar 30, 2025

@EPMatt
Copy link
Owner

EPMatt commented Mar 30, 2025

Thank you so much for your contribution @danleongjy! 🚀

Tested on my system, and confirmed your changes are working with Z2M. We could easily replicate the same logic on all the controller blueprints that implement virtual double press events. Would you be up to work on this? Happy to help as I can with updating docs + version number as well.
I am currently working on improving the Contributor experience, so I'd be glad to get your feedback on the process of contributing to this project.

Thanks again! 🔥

@EPMatt EPMatt linked an issue Mar 30, 2025 that may be closed by this pull request
1 task
@EPMatt EPMatt mentioned this pull request Mar 30, 2025
1 task
@danleongjy
Copy link
Contributor Author

That's great to know! Sure, I will go ahead and propose changes to the other controller blueprints to use the new logic for double-press checks. Do you prefer to have 1 PR per controller or it's ok to batch all of them into 1 fresh PR?

To be honest, this is my first GitHub PR, so all I can say is that the instructions and guides were quite clear to follow!

@EPMatt
Copy link
Owner

EPMatt commented Apr 1, 2025

Hey @danleongjy,
thanks - glad to hear that you are having a great experience with this project!

A single PR with changes for all controllers sounds good to me. I'll proceed with adding the missing pieces to this PR - version bump and a Changelog entry in the docs, so you can also mirror such edits as you contribute to the other blueprints in your new PR.

Thanks again for your help! 🔥

@EPMatt EPMatt merged commit 0f0a549 into EPMatt:main Apr 1, 2025
5 checks passed
Copy link
Contributor

github-actions bot commented Apr 1, 2025

github-actions bot pushed a commit that referenced this pull request Apr 1, 2025
…#676)

* Correct double-click detection logic

* feat(blueprints): bump version to 2025.04.01

* docs(blueprints): add changelog entry

---------

Co-authored-by: Matteo Agnoletto <[email protected]>
Co-authored-by: EPMatt <[email protected]>
@danleongjy danleongjy deleted the ikea_e1743_e1766_e1524_e1810_e2001_e2002 branch April 1, 2025 09:06
Copy link
Contributor

github-actions bot commented May 2, 2025

Hi there,

🔒 This pull request has been automatically locked since there has not been any recent activity after it was closed. Please open a new issue for related bugs. If you want to submit a contribution to the project, you can open a new PR.

Thanks!

@github-actions github-actions bot locked and limited conversation to collaborators May 2, 2025
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
2 participants