EPMatt / awesome-ha-blueprints

A curated collection of automation blueprints for Home Assistant.
https://epmatt.github.io/awesome-ha-blueprints
GNU General Public License v3.0
811 stars 237 forks source link

Bug - Fix_multiple bugs incontroller blueprint for philips_324131092621 on Zigbee2MQTT #597

Open nomike opened 5 months ago

nomike commented 5 months ago

Thank you for taking the time to work on a Pull Request. Your contribution is really appreciated! :tada: 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.

Proposed change*

There are several bugs in the blueprint which make it unsuable when using Zigbee2MQTT. I debugged the blueprint on my system and prepared this MR. Before submitting, I stumbled upon a few other PRs which address some of the issues exactly like I did, but not all of them.

Thus I still decided to go forward, so you can fix all issues with one single PR.

Merging this PR will solve the following issues:

216

406

425

457

496

541

558

576

and will make the following PRs obsolete:

406

542

577

The fixes in more detail

Event mapping for zigbee2mqtt is wrong

There are two issues with that mapping. First of all, the events are sent with underscores, whereas the blueprint expects dashes. This prevents the script from working at all.

Secondly the event to listen for on short presses should be "on_press_release" and not "on_press". Listening to "on_press" triggers the script too early and too often. This causes all sorts of weird behaviour and a lot of race conditions. Listening to "on_press_release" solves this issue.

trigger_delta calculation is not working due to the regex not matching the helper value

The helper value looks like this:

{"a":"on_press_release","t":1706926380.358825}

The blueprint however, expects this:

{"a" :"on_press_release", "t" :1706926380.358825}

I've modified the regular expression to make whitespaces optional and also accept other types of whitespace (tab, newline) and multiple consecutive whitespaces. This should also make it future proof.

Automation should not run on "on_press" events

As the event mapping is now using "on_press_release" events, the whole automation should not run for "on_press" events anymore. This also causes race conditions and weird unpredictable behaviour where double presses are not recognized for example.

I've changed the trigger condition to be false if such an event is received.

Checklist*

github-actions[bot] commented 5 months ago

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