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

What is the correct way to utilise auto-discovered buttons? #959

Closed
mihalski opened this issue Jan 29, 2019 · 65 comments
Closed

What is the correct way to utilise auto-discovered buttons? #959

mihalski opened this issue Jan 29, 2019 · 65 comments

Comments

@mihalski
Copy link

mihalski commented Jan 29, 2019

The example shows the use of MQTT triggers but shouldn't that be redundant for auto-discovered devices?

Previously this format of automation worked for me but sometime recently that has changed.

- alias: Family Room TV Subtitles
  trigger:
    platform: state
    entity_id: sensor.family_room_button
  condition:
      condition: state
      entity_id: sensor.family_room_button
      state: 'long'
  action:
    service: remote.send_command
    entity_id: remote.harmony_hub
    data:
      command:
        - Subtitle
      device: 38539492

Now all button automations triggered by state trigger twice. I considered using to: in state but this fails when the same button presses are triggered as previously.

Am I doing something wrong? Has Home Assistant changed something about the way it handles these automations?

I am using Home Assistant 0.86.3 and zigbee2mqtt 1.0.1.

@Koenkk
Copy link
Owner

Koenkk commented Jan 29, 2019

I indeed would like to get rid of the current MQTT triggers and just use the corresponding HA entities. Could you try playing with force_update and expire_after (https://www.home-assistant.io/components/sensor.mqtt/). This should fix the issue with: I considered using to: in state but this fails when the same button presses are triggered as previously.

@mihalski
Copy link
Author

That would require shifting to manually specified MQTT topic based triggers no?
The interesting thing is that this affects the Xiaomi buttons but no the motion or door/window sensors. I don't know if this is because they are sensors instead of binary_sensors but that's the only difference I can see.

@Koenkk
Copy link
Owner

Koenkk commented Jan 30, 2019

@mihalski the problem with the buttons is that the state doesn't change (single -> single)

@mihalski
Copy link
Author

mihalski commented Jan 30, 2019

In my testing that is only an issue when you don't use a condition that checks the state and instead use to: 'single' under the state trigger. When your trigger is the state a (single -> single) "change" triggers a check of the condition and it works as intended.

Problem I am having is that the condition triggers TWICE from a single press (and single MQTT publish).

screen shot 2019-01-30 at 4 56 27 pm

@subzero79
Copy link

This is correct, i don't know why this is happening. Yesterday my whole network went down so i had to reflash and repair all my devices. After upgrading the zigbee2mqtt to the latest dev commit i realised the click automatons in my double and single wireless wall xiaomi switches (first gen, no long or double hw click) weren't working.

I've recently implemented a nodered flow to have single, double and triple click on those wall switches by using the ha click sensors pushed by zigbee2mqtt discovery. When i started debugging in the state:change node i can see two msg's per click, even if the mqtt sub shows one line topic-payload. At the beginning i thought this was an error in nodered, but connecting to the ha websocket stream api also showed two events per click .

ata: {"event_type": "state_changed", "data": {"entity_id": "sensor.button_desktop_8072_click", "old_state": {"entity_id": "sensor.button_desktop_8072_click", "state": "single", "attributes": {"battery": 100, "voltage": 3062, "linkquality": 76, "click": "single", "friendly_name": "button_desktop_8072_click", "icon": "mdi:toggle-switch"}, "last_changed": "2019-01-30T10:10:50.266755+00:00", "last_updated": "2019-01-30T10:10:50.266755+00:00", "context": {"id": "343c917569c44e10a0cbb5e27fe0dbd6", "user_id": null}}, "new_state": {"entity_id": "sensor.button_desktop_8072_click", "state": "single", "attributes": {"battery": 100, "voltage": 3062, "linkquality": 76, "click": "single", "friendly_name": "button_desktop_8072_click", "icon": "mdi:toggle-switch"}, "last_changed": "2019-01-30T10:10:53.342483+00:00", "last_updated": "2019-01-30T10:10:53.342483+00:00", "context": {"id": "458451d2237b4590b2cf7b41f9fa3acb", "user_id": null}}}, "origin": "LOCAL", "time_fired": "2019-01-30T10:10:53.342522+00:00", "context": {"id": "458451d2237b4590b2cf7b41f9fa3acb", "user_id": null}}

data: {"event_type": "state_changed", "data": {"entity_id": "sensor.button_desktop_8072_click", "old_state": {"entity_id": "sensor.button_desktop_8072_click", "state": "single", "attributes": {"battery": 100, "voltage": 3062, "linkquality": 76, "click": "single", "friendly_name": "button_desktop_8072_click", "icon": "mdi:toggle-switch"}, "last_changed": "2019-01-30T10:10:53.342483+00:00", "last_updated": "2019-01-30T10:10:53.342483+00:00", "context": {"id": "458451d2237b4590b2cf7b41f9fa3acb", "user_id": null}}, "new_state": {"entity_id": "sensor.button_desktop_8072_click", "state": "single", "attributes": {"battery": 100, "voltage": 3062, "linkquality": 76, "click": "single", "friendly_name": "button_desktop_8072_click", "icon": "mdi:toggle-switch"}, "last_changed": "2019-01-30T10:10:53.344018+00:00", "last_updated": "2019-01-30T10:10:53.344018+00:00", "context": {"id": "788961d7d46d456b9efe180e0ce79f7b", "user_id": null}}}, "origin": "LOCAL", "time_fired": "2019-01-30T10:10:53.344044+00:00", "context": {"id": "788961d7d46d456b9efe180e0ce79f7b", "user_id": null}}

Looks like something changed in ha mqtt

@subzero79
Copy link

The problem here is the json_attributes_topic being the same as the state topic. HA docs says should be different, just deleting the directive causes the sensors go back into to normal behaviour.

@Koenkk
Copy link
Owner

Koenkk commented Jan 30, 2019

I'm wondering if the automation of the OP is correct at all. Isn't this also triggered when e.g. a new battery percentage is being send?

@mihalski
Copy link
Author

To be honest I don't know as that has never happened since I started using zigbee2mqtt. The battery levels remain at 100. And I don't get triggers when there is a regular update that does not include a click.

@subzero79
Copy link

This commit 6912f81

should be reverted IMO until is properly implemented

@Koenkk
Copy link
Owner

Koenkk commented Jan 31, 2019

@emontnemery can you confirm this?

@Koenkk
Copy link
Owner

Koenkk commented Feb 3, 2019

@subzero79 refactoring json_attributes_topic to a different topic will only partially solve this problem. The above automation is also trigger on a zigbee2mqtt restart because the sensor goes from unavailable to online again.

@emontnemery
Copy link

emontnemery commented Feb 3, 2019

@Koenkk any update of a device, including updating attributes, will cause automation to be evaluated. Hence, to not have issues, the automation condition must check state change. I hope this answers the question..

OPs automation trigger is not correct, there should be a from and to state in the trigger to not trigger on attribute change. https://www.home-assistant.io/docs/automation/trigger/#state-trigger

@Koenkk
Copy link
Owner

Koenkk commented Feb 3, 2019

@emontnemery thanks for clarifying.

I've submitted a PR to Home Assistant (home-assistant/core#20716). Once this is accepted we can use the following syntax to respond to button clicks.

- alias: Resond to button click
  trigger:
    platform: state
    entity_id: sensor.my_switch_click
    to: 'single'
  action:
    entity_id: light.my_bulb_light
    service: light.toggle

@emontnemery
Copy link

emontnemery commented Feb 3, 2019

The trigger might still not be correct, it might fire if there is for example a change of battery level within the 0.4 s. I think you need to specify a from: state also. I'm not totally sure about this though. Maybe you can test with a long expire_after.

This was wrong, see below.

@emontnemery
Copy link

To be clear, to make sure you can have non ambiguous automation triggers, I think it's better to fire a state update when the button is no longer pressed (if possible).

@subzero79
Copy link

Yes, partially but the second case is likely not to happen as much as normally pressing the button, and according how users have their automations, lights will go on, then off on single click when it shouldn’t. As for now (same as when the project started) solution is to use the mqtt topic.

@h4nc
Copy link

h4nc commented Feb 3, 2019

The trigger might still not be correct, it might fire if there is for example a change of battery level within the 0.4 s. I think you need to specify a from: state also. I'm not totally sure about this though. Maybe you can test with a long expire_after.

I‘m also not sure about this. But I think setting a certain from state will cause other issues.
It could come from several states. E.g. the Aqara double switch has 9 different states. So this would mean setting up 9 different triggers, right? Could come from every of those to „left“. This would make the code confusing and long.

And if you set up 9 of those automations than you will have to add those lines to every automation. So 9 triggers per automation. Would make the code incredibly and unnecessarily long.

@emontnemery
Copy link

emontnemery commented Feb 4, 2019

@h4nc I double checked the code, specifying to: in the state trigger is enough, sorry for giving wrong information. https://github.com/home-assistant/home-assistant/blob/1fe67fb1b0ceb7859be8192bfe9a29fca3f6eb30/homeassistant/components/automation/state.py#L54-L57

@mihalski
Copy link
Author

mihalski commented Feb 5, 2019

Is some sort of solution going to be possible so that an automation is only triggered once as it had been before support for json_attributes_topic was added? And I mean by using state rather than mqtt.

@Koenkk
Copy link
Owner

Koenkk commented Feb 5, 2019

@mihalski probably yes, I'm going to try @emontnemery solution which he mentioned here: home-assistant/core#20716 (comment).

This should make an automation in the form of

- alias: Resond to button click
  trigger:
    platform: state
    entity_id: sensor.my_switch_click
    to: 'single'
  action:
    entity_id: light.my_bulb_light
    service: light.toggle

possible. Until then, use the MQTT trigger.

@mihalski
Copy link
Author

mihalski commented Feb 5, 2019

Interesting discussion. Will the solution still cause a double trigger from an automation such as this?

- alias: Resond to button click
  trigger:
    platform: state
    entity_id: sensor.my_switch_click
  condition:
      condition: state
      entity_id: sensor.my_switch_click
      state: 'single'
  action:
    entity_id: light.my_bulb_light
    service: light.toggle

Admittedly just having to: and no condition: is much cleaner but this solution doesn't care what the previous state was, it just checks the current state. If we can't have the cleaner solution I'd settle for the next best thing.

@emontnemery
Copy link

@mihalski the to state trigger will only trigger on change of the sensor state, and not trigger if there is an attribute change.

@h4nc
Copy link

h4nc commented Feb 5, 2019

But this wouldn’t trigger if you use your button to toggle something, right?

Because in this case it would „change“ from single to single.
I think this won’t trigger the automation again.

@Koenkk
Copy link
Owner

Koenkk commented Feb 5, 2019

@h4nc that's why zigbee2mqtt needs to send a {"click": "idle"} after a {"click": "single"}. (like @emontnemery mentioned here: home-assistant/core#20716 (comment)

@mihalski
Copy link
Author

mihalski commented Feb 5, 2019

So technically it SEEMS like it would be best to trigger idle instantly after whatever click was made? Home Assistant wouldn't miss the initial trigger would it?

@emontnemery
Copy link

@mihalski right, Hass will act on each state change regardless on if they happen back to back or with some delay

@Koenkk
Copy link
Owner

Koenkk commented Feb 5, 2019

Latest dev branch allows automations in the form of:

- alias: Resond to button click
  trigger:
    platform: state
    entity_id: sensor.my_switch_click
    to: 'single'
  action:
    entity_id: light.my_bulb_light
    service: light.toggle

thanks @emontnemery for helping with this!

@h4nc
Copy link

h4nc commented Feb 5, 2019

Thanks!

Could we inform each other in this thread when this is released in the master version too?

I for myself use hassio, so it while take a little.

However basically there is no difference between this solutuon and the mqtt solution, right?
It's just a cosmetical issue no?

@emontnemery
Copy link

Btw, this should make the instant sensor state changes 'idle' -> 'single' -> 'idle' work reliably: home-assistant/core#21590

@Koenkk
Copy link
Owner

Koenkk commented Mar 2, 2019

@emontnemery many thanks again, reopening this.

@Koenkk Koenkk reopened this Mar 2, 2019
@emontnemery
Copy link

@Koenkk home-assistant/core#21590 landed in home-assistant 0.90 which was released a few weeks back. It should now be perfectly fine to implement buttons by sending e.g. a "click" state immediately followed by an "off" state.
Would you mind giving that a try again?

Meanwhile, the named events is discussed here: home-assistant/architecture#178

@Koenkk
Copy link
Owner

Koenkk commented Apr 7, 2019

@emontnemery implemented, thanks again!

All: can you please test if it works? Example can be found here: http://www.zigbee2mqtt.io/integration/home_assistant.html#via-home-assistant-entity, the limitation about the 1 - 2 seconds trigger frequency can be ignored now.

@eithe
Copy link

eithe commented Apr 13, 2019

Great stuff @Koenkk, it works perfectly here on my various xiaomi switches. Very responsive on subsequent click, and we haven't experienced any "lost" clicks like before. I've been running it for about 4 days.

@mihalski
Copy link
Author

@eithe What about double clicks? I loose more than go through. Especially when close together..

Prior to my original post I could double click, pause for 1 second and double click again and it would always work.

I'll try all my switches this week (I'll be on holidays) and document their responsiveness along the entire path between zigbee2mqtt and Home Assistant.

Would be great if there was a native Home Assistant API alongside MQTT (like what esphomelib has). Would remove any issues created by how MQTT <-> HA interact.

@eithe
Copy link

eithe commented Apr 13, 2019

@mihalski I'll test and report back,

@eithe
Copy link

eithe commented Apr 13, 2019

It seems to register my double clicks fine, but now that I looked into it in more details, there is something wonky where I get the click state changed events twice. High chance it's something on my end, will report back if not.

@mihalski
Copy link
Author

That sounds like the behaviour I used to get previously. This is why when I had long click toggling subtitles it would turn them on and then off again on a single long click. I don't think it's doing that anymore but I'll have to check when I get the chance.

@eithe
Copy link

eithe commented Apr 13, 2019

Yeah, I experienced the exact same thing before as well.

@emontnemery
Copy link

@eithe there will be two state change events, once set to 'click' and the second where it's cleared ''

@eithe
Copy link

eithe commented Apr 23, 2019

Yeah, it works for me now, I made sure I actually ran the latest version of the software and firmware.

One thing I’ve noticed is with a Xiaomi double wall switch I have where if first click the left button and then click the right button just after (seems to before the state goes back to unknown in HA), it fires the left click event twice:

«Click left»
Event: unknown
Event: left
«Click right»
Event: left
Event: right
Event: unknown

Anyone seeing the same?

@emontnemery
Copy link

For the error where 'left' fires twice, I think it would be helpful if you log MQTT traffic to find if it's a bug in Hass (no duplicate in MQTT messages, Hass generates the duplicate) or in zigbee2mqtt.

@Koenkk
Copy link
Owner

Koenkk commented May 12, 2019

I assume this issue is solved now. Feel free to re-open if it's not the case.

@Koenkk Koenkk closed this as completed May 12, 2019
@Koenkk Koenkk mentioned this issue May 14, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
wilmardo pushed a commit to wilmardo/zigbee2mqtt that referenced this issue Sep 26, 2019
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

7 participants