E1812 Debouncing and optimized double press routine.

[vandalon]

A proper pull request this time :)

Added debouncing
Maximum loop option to prevent endless loops
Optimized double press routine
Some other small optimizations.

Closes #22.
Closes #45.

[EPMatt]

Hi @vandalon, thank you for the contribution. :)

I've just tested this version of the blueprint. IKEA E1812 with ZHA, so we have a quite similar setup. Unfortunately I don't get a consistent behaviour across multiple runs: sometimes debouncing works and a single short press is triggered, sometimes the automation runs multiple times.

Increasing the value of helper_debounce_delay seems only to partially solve the issue (went up to 1000 but still the automation was triggered 5 times in a row).

[vandalon]

Hmmm, that is strange. do you then also see multiple ahb_controller_events on one press?

[vandalon]

I have my E1812 connected to deconz, will test with ZHA tonight to see if I can reproduce you're issue.

[vandalon]

I understand why this happens. and we can not do much about it. The debouching still works but the condition never gets matched because we use the helper which only gets filled after the debounce period. Since we also need this helper for the double press we can not fill it before the debounce period because that will mess with the double press timings.

Even though, I will test a bit more with writing to the helper before the delay and see if I can get that working.

Now debouncing still works, when zha sends 5 events, the first 4 restart the automation but because of the debounce delay nothing else happens. only the 5th and last event will go trough.

So maybe we can still release this so people can use it, and then we can try and make it work in such a manner that also to automation is not restarted 5 times.

[EPMatt]

Hmmm, that is strange. do you then also see multiple ahb_controller_events on one press?

Yeah, it's not only the automation which gets triggered multiple times. The ahb_controller_event is fired and custom action gets executed as well.

I'm also continuing testing different debouncing approaches, but still didn't get to a fully working solution. 👍🏻

[vandalon]

That is very strange, do you notice that the delay is running at all?
Can you check the automation trace that HA introduced in the latest version?

[vandalon]

I did a small change just now, to make sure that the debounce_delay is an int. Can you see if that helps?

[vandalon]

@EPMatt could you check if you still have double ahb_controller_events?
For me it seems to work correctly now.

[vandalon]

Hi again.

I've moved the delay till after update of the helper. This should help prevent automation restarts and does not seem to have a negative impact.
Changed the double press routine to update the helper after a double press and also check on this to make sure no button_short happens right after.

[vandalon]

I have an idea to prevent unnecessary automation restart, so bear with me :P

[vandalon]

So here it is, i don't see any automation restarts anymore except when releasing after a long press with "loop until release" enabled. Which makes sense because that automation takes a while and on release the debounce and double_press timeouts have expired. And we need that trigger for the "on release" event :)

[vandalon]

And to make you a bit more happy, i moved the debounce delay now to the double press routine, because in all other cases it's handled by the primary automation condition.

So how debouncing now works:
1 press, 5 events.

1: event fires automation,
2-5: events are stopped by the primary automation condition. so first press automation keeps running. (you where right to try it like this).

On a double press we still need to do a debounce timeout because otherwise one of the events following the first will trigger the wait for trigger instantly.

[EPMatt]

Hi @vandalon,
I've just tested the latest version of this blueprint. Really nice job! :)

Just as a note, even after increasing the helper_debounce_delay to 1000 I'm still randomly getting some duplicate short events, but I think this varies between different HA installations, and it might also depend on the mode: restart issue we discovered in HA Core. Anyway, definitely better than the previous version. :)

[EPMatt]

@vandalon I've just performed a few additional tests on the blueprint, but unfortunately I encountered a few issues with the double press action. This is the automation config:

use_blueprint:
    path: EPMatt/ikea_e1812.yaml
    input:
      integration: ZHA
      controller_device: <my_controller_id>
      action_button_short:
        - service: persistent_notification.create
          data:
            message: button short
      action_button_long:
        - service: persistent_notification.create
          data:
            message: button long
      action_button_release:
        - service: persistent_notification.create
          data:
            message: button release
      action_button_double:
        - service: persistent_notification.create
          data:
            message: button double
      button_double_press: true
      helper_last_controller_event: input_text.<my_input_text>
      helper_debounce_delay: 1000
      helper_double_press_delay: 5000

Short press seems to work quite well with helper_debounce_delay: 1000. I increased the helper_double_press_delay to 5000 to make sure the second press can be retrieved without being caught by the debouncing mechanism.

With this configuration I'm rarely able to trigger the double press action, and when it happens, the action gets executed 5 times, exactly the number of zha_event events received by HA. I can confirm ZHA events are triggered for both presses. I'm also making sure to wait a little bit between one press and the other one, since the double press delay is set to 5 seconds.

I'll also try to investigate this issue but I suspect there might be something wrong with the wait_for_trigger step (not your fault but something with the HA Automation integration itself).

P.S.: Sorry again for not being that active these days, unfortunately I've recently been really busy. But I want to underline that I truly appreciate your effort, time and energy you're putting into this and your other contributions as well. 👍🏻

[vandalon]

If you have set the debounce delay 2 one 2nd the double press has to have a second between first and 2nd press. Is this the case in your test?

[vandalon]

Will test tomorrow:)

[EPMatt]

If you have set the debounce delay 2 one 2nd the double press has to have a second between first and 2nd press. Is this the case in your test?

Yeah, I waited 1+ seconds before pressing the button for the second time. Again, ZHA events were properly fired (I tested it in real time with HA Developer Tools).

[EPMatt]

Will test tomorrow:)

Thank you! Let me know if you need me to perform some additional tests. :)

[vandalon]

I can not reproduce your issue, for your automation just works :)

Can you check the output of the automation trace in the HA UI?

[EPMatt]

Hi @vandalon,

Interestingly, today also single presses are not working as expected: I wasn't able to trigger one. 😅
This is the trace for the 5 automations which get triggered by a single press (posted only one since they are all the same):

As you can see debouncing is not working. Instead all automations are run, but after the wait_for_trigger step no match is available for the conditions provided in the choose step, hence the automation reaches the end of the sequence and stops, without even firing the single press.

By the way, some progress has been made into debugging the mode: restart issue: a developer seems to have found a valid solution. Maybe the fix will also mitigate a few of the issues we are having here.

Thanks again for your time. :)

[vandalon]

By the way, some progress has been made into debugging the mode: restart issue: a developer seems to have found a valid solution. Maybe the fix will also mitigate a few of the issues we are having here.

That would be awesome :)

Can you make sure there is nothing else interfering? Maybe multiple automation with the same device or something else?
I can't get the same behaviour you are having somehow.

[EPMatt]

That would be awesome :)

Definitely agree. I'm quite sure many of the problems we're dealing with are related to it. :)

Can you make sure there is nothing else interfering? Maybe multiple automation with the same device or something else?
I can't get the same behaviour you are having somehow.

Just checked and I can confirm nothing is interfering with this automation. I have only a single automation set up for that device, and as a safe measure I didn't setup any Hook with it.

This is only my opinion, but maybe we can wait the mode:restart issue get solved, then come back here. This way we can be sure that HA Core is working as expected, and the issue is only with our blueprint. :)
What do you think about it?

[EPMatt]

Hi @eloo, thank you for reporting your tests here.

I've just pushed a fix in this PR which solves the long press action issue, and simplifies the sequences for debouncing and double press events.

Could you please check if everything works as expected with the latest version of this blueprint?

Thank you so much for your valuable time and feedback! :)

[vandalon]

Why only debounce with delay if we can catch it in the primary conditions like you initially wanted? :)
Also with removing the wait for trigger makes that the 2nd press is also debounced. With wait_for_trigger the 2nd press was instantly processed.

[eloo]

@EPMatt
now there seems to be a total different problem :D

[EPMatt]

Why only debounce with delay if we can catch it in the primary conditions like you initially wanted? :)

Good point @vandalon. :) I discovered that in HA automations conditions do not act as a filter before a run is executed. Instead, a new run is started (thus stopping any other run if mode: restart is configured), then conditions are validated. If conditions are not met, action is simply not executed.

For this reason and due to mode:restart, the automation which completes execution is the last one in the row of runs triggered by a non-debounced controller event. Hence, it's much simpler to wait a certain amount of time before executing the controller action. Only the last run will see the delay expire, and will be able to complete execution.

Also with removing the wait for trigger makes that the 2nd press is also debounced. With wait_for_trigger the 2nd press was instantly processed.

I also deeply thought about this issue and found that assuming that a wait_of_trigger block allows to not stop the script execution is not in line with the Home Assistant specification.

As stated in the Script Syntax regarding the wait_for_trigger block:

WAIT FOR TRIGGER
This action can use the same triggers that are available in an automation’s trigger section. See Automation Trigger. The script will continue whenever any of the triggers fires. All previously defined trigger_variables, variables and script variables are passed to the trigger.

On the other hand, as stated in the Scripts documentation regarding script modes:

restart -> Start a new run after first stopping previous run.

This means that even if the trigger is received in the wait_for_trigger block, further execution should not continue. Instead a new run should be triggered, due to mode:restart, and the previous script should be stopped.

I don't know whether this behaviour was caused by the HA Core automation bug we filed last week. By the way it will require further investigation, since it might be another issue with Home Assistant.

I hope my explanation makes sense. Let me know what are your thoughts about it. :)

[EPMatt]

@eloo thank you for testing. :)

I think the issue is with the content of the text input helper.
Could you please reset it to an empty text and try again executing the automation?

Thanks!

[eloo]

@EPMatt
good catch.

but it was indeed empty.. so it seems to be a problem when its empty.
with an input_text set its going a bit further.

now i'm running into #42 but the with the fix i've suggested in #44 its working 👍

[EPMatt]

@eloo thank you so much for your feedback! Happy to hear that the blueprint is working. 🎉

but it was indeed empty.. so it seems to be a problem when its empty.

Right, you now have always to provide a valid entity_id for the helper_last_controller_event. We should also update documentation before merging this PR, stating that the text helper is mandatory. 👍🏻

We could also consider limiting the usage of the text helper only when necessary (eg. when debouncing is enabled and other conditions), but I think this might create a little bit of confusion among users: many reported that they weren't able to understand when the text helper was required, even after reading the docs.

I guess it's not a big issue to ask users to provide a text helper for each controller automation they're setting, even if not strictly required; moreover this might be a good future-proof choice, since we might need it later to implement more advanced features in this blueprint.

Just as a feedback, @eloo @vandalon what is your opinion about it?

Thank you so much for your time. :D

[eloo]

@EPMatt
for me it sounds reasonable to mark it as mandatory.

Also when you are going to set a double press action you can easily forget that you need to set this as well.
So for me its sounds like a good trade-off to make it mandatory.

PS: Thanks a lot for your great project. I really like the controller -> hooks pattern.

[EPMatt]

for me it sounds reasonable to mark it as mandatory.

Also when you are going to set a double press action you can easily forget that you need to set this as well.
So for me its sounds like a good trade-off to make it mandatory.

@eloo totally agree with you, it's better to make the blueprint configuration as straightforward as possible. Thank you for the suggestion. 👍🏻

PS: Thanks a lot for your great project. I really like the controller -> hooks pattern.

Thank you so much for your feedback! I'm really happy these blueprints are useful for you. 🚀

[EPMatt]

Hi @eloo @vandalon,

I've fixed #45 for this blueprint, as well as updated docs reflecting changes we are introducing here, and applied a few additional tweaks.

Could you please test that the generated automation is properly working on your HA instances?
If everything runs great then I think we're done here, thus we can merge the PR and make our changes available to the public. 🎉

Thank you very much for your precious help!

[vandalon]

I will do some testing after the weekend. I’m having a relax-weekend :P

[EPMatt]

I will do some testing after the weekend. I’m having a relax-weekend :P

Hi @vandalon, don't worry, have a nice weekend. :D

[EPMatt]

@vandalon the same goes with this PR. Since we received positive feedback from users regarding changes included here, I'm now updating changelog and merging this PR into main. :)

Thank you for all your precious time and effort you put into this great contribution. 🚀

[github-actions]

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!