core icon indicating copy to clipboard operation
core copied to clipboard

mqtt device trigger -> discovery_id does not work with trigger_variables?

Open AnderssonPeter opened this issue 1 year ago • 1 comments

The problem

Hi I have the following blueprint that works as expected

blueprint:
  name: 'Test'
  domain: automation
  input:
    button_sensor:
      name: Philips Hue Tap dial switch sensor entity
      description: Sensor for Philips Hue Tap dial switch to use
      selector:
        device:
          multiple: false
          integration: mqtt
          manufacturer: Philips
          model: 'Hue Tap dial switch (8719514440937/8719514440999)'
mode: restart
max_exceeded: silent
trigger:
  - platform: device
    domain: mqtt
    device_id: !input button_sensor
    type: action
    subtype: button_1_press_release
    discovery_id: '0x001788010d7d3cac action_button_1_press_release'
action:
  - service: notify.mobile_app_pixel_6_pro
    data:
      message: "Test"

But if I move the value of discovery_id into trigger_variables and use a template in discovery_id then it no longer works?

blueprint:
  name: 'Test'
  domain: automation
  input:
    button_sensor:
      name: Philips Hue Tap dial switch sensor entity
      description: Sensor for Philips Hue Tap dial switch to use
      selector:
        device:
          multiple: false
          integration: mqtt
          manufacturer: Philips
          model: 'Hue Tap dial switch (8719514440937/8719514440999)'
mode: restart
max_exceeded: silent
trigger_variables:
  action_button_1_press_release: '0x001788010d7d3cac action_button_1_press_release'
trigger:
  - platform: device
    domain: mqtt
    device_id: !input button_sensor
    type: action
    subtype: button_1_press_release
    discovery_id: '{{ action_button_1_press_release }}'
action:
  - service: notify.mobile_app_pixel_6_pro
    data:
      message: "test"

It no longer works?

What version of Home Assistant Core has the issue?

core-2023.12.3

What was the last working version of Home Assistant Core?

No response

What type of installation are you running?

Home Assistant Container

Integration causing the issue

mqtt

Link to integration documentation on our website

https://www.home-assistant.io/integrations/mqtt/

Diagnostics information

No response

Example YAML snippet

blueprint:
  name: 'Test'
  domain: automation
  input:
    button_sensor:
      name: Philips Hue Tap dial switch sensor entity
      description: Sensor for Philips Hue Tap dial switch to use
      selector:
        device:
          multiple: false
          integration: mqtt
          manufacturer: Philips
          model: 'Hue Tap dial switch (8719514440937/8719514440999)'
mode: restart
max_exceeded: silent
trigger_variables:
  action_button_1_press_release: '0x001788010d7d3cac action_button_1_press_release'
trigger:
  - platform: device
    domain: mqtt
    device_id: !input button_sensor
    type: action
    subtype: button_1_press_release
    discovery_id: '{{ action_button_1_press_release }}'
action:
  - service: notify.mobile_app_pixel_6_pro
    data:
      message: "test"

Anything in the logs that might be useful for us?

No response

Additional information

No response

AnderssonPeter avatar Dec 26 '23 14:12 AnderssonPeter

Hey there @emontnemery, @jbouwh, mind taking a look at this issue as it has been labeled with an integration (mqtt) you are listed as a code owner for? Thanks!

Code owner commands

Code owners of mqtt can trigger bot actions by commenting:

  • @home-assistant close Closes the issue.
  • @home-assistant rename Awesome new title Renames the issue.
  • @home-assistant reopen Reopen the issue.
  • @home-assistant unassign mqtt Removes the current integration label and assignees on the issue, add the integration domain after the command.
  • @home-assistant add-label needs-more-information Add a label (needs-more-information, problem in dependency, problem in custom component) to the issue.
  • @home-assistant remove-label needs-more-information Remove a label (needs-more-information, problem in dependency, problem in custom component) on the issue.

(message by CodeOwnersMention)


mqtt documentation mqtt source (message by IssueLinks)

home-assistant[bot] avatar Dec 26 '23 14:12 home-assistant[bot]

I am not sure what you are trying to do. I cannot find that discovery_id can be used in automation. This discovery id should be unique, and is used for MQTT discovery, but it cannot be used in automations.

https://www.home-assistant.io/docs/automation/trigger/#mqtt-trigger

https://www.home-assistant.io/integrations/device_trigger.mqtt/

jbouwh avatar Dec 26 '23 19:12 jbouwh

@jbouwh I have the following automation that I'm trying to convert to a blueprint

alias: test dial
description: ""
trigger:
  - platform: device
    domain: mqtt
    device_id: 159f1b5a4d1e1a848b14e72e0698ccf7
    type: action
    subtype: button_1_press_release
    discovery_id: 0x001788010d7d3cac action_button_1_press_release
condition: []
action:
  - service: notify.mobile_app_pixel_6_pro
    data:
      message: test

This was created using the GUI and works as expected and as you can see it uses the discovery_id in a automation.

AnderssonPeter avatar Dec 26 '23 20:12 AnderssonPeter

Right, The discovery ID is indeed added through the UI. It is based on the discovery topic of the config payload. I am not sure you can use templates to render the discovery ID.

jbouwh avatar Dec 26 '23 21:12 jbouwh

https://www.home-assistant.io/docs/automation/trigger/#device-triggers. Say's MQTT device triggers are set up through the UI, but templates are not support. In fact the schema only accepts a plain string for discovery_id and the other fields.

See:

https://github.com/home-assistant/core/blob/01ded7daea399cc11ee2ff65247d07d4255fa3b8/homeassistant/components/mqtt/device_trigger.py#L60-L69

So it is not supported to use templates.

jbouwh avatar Dec 26 '23 21:12 jbouwh

Yes I found that out the hard way, but is there no way to fix it in the next version of home assistant? This is a major paint point for everyone that uses zigbee2mqtt as it forces us to repeat the same automation instead of just using a blueprint?

AnderssonPeter avatar Dec 26 '23 22:12 AnderssonPeter

Well technically, this is not a fix, but a feature request, and this is not the place to register that.

You can register it here: https://community.home-assistant.io/c/feature-requests/13

Or open a PR.

jbouwh avatar Dec 26 '23 22:12 jbouwh

To come back to this: A template can be added to discovery_id, but it would be rendered only once when the the automation is loaded. This means that all referenced objects in the template must exist before the automation is loaded. You could try this you self if this would work if you want:

diff --git a/homeassistant/components/mqtt/device_trigger.py b/homeassistant/components/mqtt/device_trigger.py
index fc7528743fa..4c95b44e665 100644
--- a/homeassistant/components/mqtt/device_trigger.py
+++ b/homeassistant/components/mqtt/device_trigger.py
@@ -9,6 +9,7 @@ import attr
 import voluptuous as vol
 
 from homeassistant.components.device_automation import DEVICE_TRIGGER_BASE_SCHEMA
+from homeassistant.components.mqtt.models import MqttCommandTemplate, MqttValueTemplate
 from homeassistant.config_entries import ConfigEntry
 from homeassistant.const import (
     CONF_DEVICE,
@@ -21,6 +22,7 @@ from homeassistant.const import (
 from homeassistant.core import CALLBACK_TYPE, HomeAssistant, callback
 from homeassistant.exceptions import HomeAssistantError
 from homeassistant.helpers import config_validation as cv
+from homeassistant.helpers.template import Template
 from homeassistant.helpers.trigger import TriggerActionType, TriggerInfo
 from homeassistant.helpers.typing import ConfigType, DiscoveryInfoType
 
@@ -328,7 +330,7 @@ async def async_attach_trigger(
     """Attach a trigger."""
     mqtt_data = get_mqtt_data(hass)
     device_id = config[CONF_DEVICE_ID]
-    discovery_id = config[CONF_DISCOVERY_ID]
+    discovery_id = Template(config[CONF_DISCOVERY_ID], hass).async_render()
 
     if discovery_id not in mqtt_data.device_triggers:
         mqtt_data.device_triggers[discovery_id] = Trigger(

jbouwh avatar Dec 27 '23 12:12 jbouwh

I think I managed to get my case working with a mqtt trigger instead, but haven't tested it fully yet, if it fails I will try this patch.

AnderssonPeter avatar Dec 27 '23 17:12 AnderssonPeter

I've described my potential use case here: https://community.home-assistant.io/t/wth-why-cant-i-use-mqtt-device-triggers-in-a-blueprint/482219/22?u=misiu

the idea is to be able to build discovery_id from a variable in a blueprint.

A template can be added to discovery_id, but it would be rendered only once when the the automation is loaded. This means that all referenced objects in the template must exist before the automation is loaded.

does this mean that we should be able to create variable in blueprint, let's say:

variables:
  input_remote: !input remote
  input_discovery_id: "{{ device_entities(input_remote)[0].split('.')[1].split('_')[0] }}"

and then inside trigger we should be able to use it like this:

trigger:
  - platform: device
    domain: mqtt
    device_id: !input remote
    type: action
    subtype: "off"
    discovery_id: "{{input_discovery_id}} action_off"

EDIT: Sadly this does not work

@jbouwh what about other places like device_id? Should a similar approach work? I'll try the changes you proposed and I can try to create PR with it, but I'm afraid it can get closed because this might need an architecture decision. WDYT?

Misiu avatar Jan 11 '24 11:01 Misiu

@jbouwh what about other places like device_id? Should a similar approach work? I'll try the changes you proposed and I can try to create PR with it, but I'm afraid it can get closed because this might need an architecture decision. WDYT?

The current mqtt trigger schema does not support templating (that applies for all fields for the mqtt trigger). I do not believe it would need an architecture to support it, as using templates in a schema is a common pattern. But the question is if it would be of any use, because the templates would have to be evaluated at initialization time. This means you are limited to what can be rendered through templates at startup. If the variables you want to use are available at setup (during HA startup) then it could work.

jbouwh avatar Jan 11 '24 16:01 jbouwh

If I read https://www.home-assistant.io/docs/automation/trigger/#trigger-variables it seems that trigger variables should be supported. So let's reopen the issue and see if we can get this to work, as it it clearly not working as documented yet.

This will be the scope though: https://www.home-assistant.io/docs/configuration/templating/#limited-templates

jbouwh avatar Jan 11 '24 16:01 jbouwh

This should make it possible to use templating for discovery_id, type and sub_type. using the trigger_variables:

https://github.com/home-assistant/core/pull/107830

jbouwh avatar Jan 11 '24 22:01 jbouwh

According to https://community.home-assistant.io/t/wth-why-cant-i-use-mqtt-device-triggers-in-a-blueprint/482219/29?u=misiu the IEEE address is added to the identifiers attribute for the device, so in theory, one can use:

{{ (device_attr('d4bacfb73175d44ad4fdc88cbc3676b1', 'identifiers')|list)[0][1].split('_')[-1] }}

to get the IEEE address, but according to https://www.home-assistant.io/docs/configuration/templating/#devices the device_attr isn't supported in limited templates, so I guess this won't work in trigger_variables. Do you have any ideas on how this could be supported?

EDIT: I wanted to avoid asking questions here, but this is still related to the issue because there should be a way to get IEEE address in trigger_variables

Misiu avatar Jan 12 '24 09:01 Misiu

the device_attr isn't supported in limited templates, so I guess this won't work in trigger_variables.

Did you try it? It is just that the template is rendered once, at the time the automation is loaded. Not at run time.

jbouwh avatar Jan 12 '24 09:01 jbouwh

the device_attr isn't supported in limited templates, so I guess this won't work in trigger_variables.

Did you try it? It is just that the template is rendered once, at the time the automation is loaded. Not at run time.

I'm editing my blueprint at this moment and will get back with the results in a second

Misiu avatar Jan 12 '24 09:01 Misiu

@jbouwh when I add new trigger_variable like this:

trigger_variables:
  input_remote: !input remote
  input_cover: !input cover
  input_discovery_id: "{{ device_entities(input_remote)[0].split('.')[1].split('_')[0] }}"
  tmp: "{{ (device_attr(input_remote, 'identifiers')|list)[0][1].split('_')[-1] }}"

the automation isn't triggered at all, but when I remove the variable and call the same template in action:

action:
  - service: persistent_notification.create
    metadata: {}
    data:
      message: "{{ (device_attr(input_remote, 'identifiers')|list)[0][1].split('_')[-1] }}"

I get a persistent notification with the right value. I guess this is a limitation of limited templates.

There was a suggestion to add a new Jinja2 function that would extract the IEEE address, but I have no experience with that and also I'm not sure how to create one that will work in device triggers (limited templates)

Misiu avatar Jan 12 '24 09:01 Misiu

device_attr might not be available in the trigger_variables, but with the PR it can be used directly in a template on discovery_id.

jbouwh avatar Jan 12 '24 11:01 jbouwh

device_attr might not be available in the trigger_variables, but with the PR it can be used directly in a template on discovery_id.

Confirmed! It works when used directly 🎉

What do you think about adding a new Jinja2 function to extract the IEEE address? How to make it available inside trigger_variables?

I don't know the logic behind templates, but right now if I have 4 triggers in my blueprint the same template will get evaluated 4 times, with trigger_variables this will get evaluated once right?

Misiu avatar Jan 12 '24 11:01 Misiu

Trigger variables will be inserted for use inside the template, further all template features are available including the state of the device automation as this

jbouwh avatar Jan 12 '24 12:01 jbouwh

This alternative PR removes the need to add discovery_id. So that makes it even easier that way to use blueprints:

https://github.com/home-assistant/core/pull/108309

jbouwh avatar Jan 18 '24 17:01 jbouwh