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

Projects
None yet
4 participants
@Koenkk
Copy link

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

This comment has been minimized.

Copy link

commented Feb 3, 2019

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 home-assistant/core as a code owner Feb 3, 2019

@ghost ghost added the in progress label Feb 3, 2019

@homeassistant homeassistant added cla-signed and removed cla-needed labels 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

This comment has been minimized.

Copy link
Contributor

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

This comment has been minimized.

Copy link
Author

commented Feb 3, 2019

I've added logging

expire_after = self._config.get(CONF_EXPIRE_AFTER)
and
self._expiration_trigger = None

expire_after: 0.001:

image

expire_after: 1:
image

This seem to trigger the value_is_expired one tick earlier. I don't know if this is the cleanest solution though.

@balloob

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Member

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

This comment has been minimized.

Copy link
Author

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

This comment has been minimized.

Copy link
Contributor

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
You can’t perform that action at this time.