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

Fix virtual double press event for deconz #402

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

Conversation

richardkerr
Copy link

@richardkerr richardkerr commented Sep 1, 2022

Breaking change

The helper_last_controller_event "a" attribute is now forced to be written as a string, may break some integrations if a number or other type is expected.

Proposed change*

This change modifies all helper_last_controller_event updates to force the trigger_action to be written (and appropriately quoted) as a string.

There are a collection of regexes that test the contents of this json before parsing it and expect this value to be quoted. The existing implementation means that Virtual Double Press events were never generated in my use cases (deconz + ikea).

A better change might be to update the regexes to allow all types of variables or rework completely how that validation is applied, given that it is evidently fragile, but I settled on this approach as it seemed less impactful.

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.
This change modifies all helper_last_controller_event updates to force
the trigger_action to be written (and appropriately quoted) as a string.

There are a collection of regexes that test the contents of this json
and expect this value to be quoted.  A better change might be to update
the regexes but this seems safer.
@github-actions
Copy link
Contributor

github-actions bot commented Sep 1, 2022

Hey @richardkerr, 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!

@maartenpaul
Copy link

I am not sure I am writing in the right place, but I also ran into problems that my virtual double press events using deconz were not properly detected, recently. After some troubleshooting I figured out that this is the problem, because the deconz event ID is a number the regular expression used in the blueprint is not properly working.
Manually applying the suggested fix here seems to work until the fix is approved into the main branch.

@richardkerr
Copy link
Author

Glad you found it helpful @maartenpaul, but I don't hold much hope for this being merged given how many open PRs there are here.

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