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

Allow MQTT sensor expire_after to be a float value. #20716

Closed
wants to merge 1 commit into from

Conversation

Koenkk
Copy link
Contributor

@Koenkk Koenkk commented Feb 3, 2019

Description:

Currently when setting expire_after of a MQTT sensor to e.g. 0.4 the expire_after mechanism is not triggered. This is because the expire_after value is rounded to 0 resulting in the expire_after mechanism to be disabled.

This PR fixes that problem by interpreting the expire_after as a float value.

Related issue (if applicable): not applicable

Pull request in home-assistant.io with documentation (if applicable): not applicable

Checklist:

  • The code change is tested and works locally.
  • Local tests pass with tox. Your PR cannot be merged unless tests pass
  • There is no commented out code in this PR.

If user exposed functionality or configuration variables are added/changed: not applicable

If the code communicates with devices, web services, or third-party tools: not applicable

If the code does not interact with devices: not applicable

@homeassistant
Copy link
Contributor

Hi @Koenkk,

It seems you haven't yet signed a CLA. Please do so here.

Once you do that we will be able to review and accept this pull request.

Thanks!

@Koenkk Koenkk requested a review from a team as a code owner February 3, 2019 15:30
@ghost ghost added the in progress label Feb 3, 2019
@Koenkk Koenkk changed the title Allow expire_after to be a float value. Allow MQTT sensor expire_after to be a float value. Feb 3, 2019
@emontnemery
Copy link
Contributor

emontnemery commented Feb 3, 2019

I think Hass is working with an integer second precision internal timer, are you sure this works as intended? Please see discussion in #16993

@Koenkk
Copy link
Contributor Author

Koenkk commented Feb 3, 2019

@balloob
Copy link
Member

balloob commented Feb 3, 2019

@emontnemery is right, our internal clock is based on 1 second so allowing floats to be entered would be incorrect as we can't deliver.

@Koenkk
Copy link
Contributor Author

Koenkk commented Feb 4, 2019

@balloob I agree, perhaps what we need is something like a MQTT event trigger sensor.

Background information
Zigbee2mqtt discovers stateless buttons (e.g. Xiaomi WXKG01LM) as an MQTT sensor. e.g.

sensor:
  - platform: mqtt
    value_template: '{{ value_json.click }}'
    state_topic: 'zigbee2mqtt/my_button'
    name: my_button_click

A button can have multiple click types, e.g. single, double, long, etc. When the button is pressed zigbee2mqtt produces a MQTT message like zigbee2mqtt/my_button with payload {"click": "single"}.

To respond to these button clicks, we wan't an automation in the form of:

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

This works for the first button click as the state changes from uknown -> single. However when the button is pressed another time, the automation isn't triggered because the state doesn't change (single -> single).

By using the expire_after: 1 the state of the sensor resets after the 1 second, but this gives the limitation that the automation cannot be triggered multiple times a second.

Another limitation of the expire_after: 1 is that the it doesn't expire after 1 second, but rather between 1 and 2 seconds. See #20716 (comment)

What would be a good solution to this?

@balloob
Copy link
Member

balloob commented Feb 4, 2019

The good solution here is to follow our policy of not trying to store button clicks in the state machine but fire events instead. This is how every other platform does it.

Will close this PR as this won't solve it.

@balloob balloob closed this Feb 4, 2019
@ghost ghost removed the in progress label Feb 4, 2019
@balloob
Copy link
Member

balloob commented Feb 4, 2019

Or I guess in the case of zigbee2mqtt, just have an automation with an MQTT trigger listen to the correct topic.

@Koenkk
Copy link
Contributor Author

Koenkk commented Feb 4, 2019

@balloob this is indeed how it is currently done, however it would be a cleaner solution if the home assistant entity IDs could be used in the automations.

@emontnemery
Copy link
Contributor

emontnemery commented Feb 5, 2019

This works for the first button click as the state changes from unknown -> single. However when the button is pressed another time, the automation isn't triggered because the state doesn't change (single -> single).

I still think the solution is to send an MQTT event from zigbee2mqtt changing the state back single -> idle after either of:

  • immediately after sending the single state update
  • after some cooldown timer has expired if you want debouncing
  • (if possible) when the button is no longer pressed.

MQTT guarantees message order, so in this way no need to worry about the cooldown period being affected by low resolution timers in HomeAssistant or by delays caused by the MQTT broker.

Btw, if you implement the button as a binary_sensor there's always this:
https://www.home-assistant.io/components/binary_sensor.mqtt/#toggle-the-binary-sensor-each-time-a-message-is-received-on-state_topic

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

Successfully merging this pull request may close these issues.

4 participants