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

[RF][BREAKING] Improve Home Assistant auto discoverability #2057

Merged
merged 7 commits into from
Nov 7, 2024

Conversation

JamiePhonic
Copy link
Contributor

@JamiePhonic JamiePhonic commented Sep 13, 2024

Description:

This is a proposed change to improve Home Assistant auto discoverability for devices using the "RF" (RCSwitch) library

Pass "received" as the type argument and switchRF[0] as the subtype argument to announceDeviceTrigger() which will cause received codes to be picked up by home assistant as triggers, i.e. When setting up automations in Home Assistant, selecting a device trigger and an OMG device will allow the user to select a code picked up during the autoDiscover window as the trigger for the automation, for example "1394004" received. In order to support this, the '-DvalueAsATopic=true' argument must be added to all environments using the "RF" Library because the changes modify the payload sent to the devices 433toMQTT/[Value] topic to include an action value that gets set to "received" and then back to "" as this appears to be how home assistant recognises the trigger.

Also change the getUniqueId() call to remove the leading '-' since the getUniqueId() function as defined in ZmqttDiscovery.ino already suffixes a "-" between the MAC address and the name parameter which will prevent MQTT topics having '--' in them. This doesnt provide any real benefit but makes things look nicer if using a tool like MQTT Explorer to inspect the raw messages as they come through.

I acknowledge that this my be a potentially breaking change for existing users and so this may not be accepted on this basis.

Mitigation Guide for people not using HA, or using a manual HA yaml configuration

The changes proposed in this PR will break existing configurations for people not using HA, or using a manual HA yaml configuration.
Thankfully for simple configurations where you are looking for a single value, the resolution is fairly straight forward. You simply need to append the "code" you are checking for to the end of the MQTT topic.

An existing manual HA MQTT configuration may look like this:

mqtt:
  binary_sensor:
    - name: "doorbell"
      state_topic: "home/OpenMQTTGateway/433toMQTT"
      value_template: >- 
        {% if value_json.value == '14163857' %}
          {{'ON'}}
        {% else %}
          {{states('binary_sensor.doorbell') | upper}}
        {% endif %}
      off_delay: 30
      device_class: 'sound'

To update this configuration to work with the new implementation:

mqtt:
  binary_sensor:
    - name: "doorbell"
      state_topic: "home/OpenMQTTGateway/433toMQTT/14163857"
      force_update: true  # Sends update events even if the value hasn’t changed (which will always be the case)
      value_template: >- 
        {% if value_json.value == '14163857' %}
          {{'ON'}}
        {% else %}
          {{states('binary_sensor.doorbell') | upper}}
        {% endif %}
      off_delay: 30
      device_class: 'sound'

For more complex implementations, like Door Sensors or other scenarios where you are checking for 2 different codes, you will need to create seperate sensors for each "code" and then check which one was most recently updated.
In Home Assistant this will require the use of a binary template sensor to compare the last_updated of the 2 binary sensors to determine the current state based on which one was most recently triggered.

An existing manual HA MQTT configuration may look like this:

mqtt:
  binary_sensor:
    - name: "test"
      state_topic: "home/OpenMQTTGateway/433toMQTT"
      value_template: >-
        {% if value_json.value == 7821834 %}
          {{'ON'}}
        {% elif value_json.value == 7821838 %}
          {{'OFF'}}
        {% else %}
          {{states('binary_sensor.test') | upper}}
        {% endif %}
      qos: 0
      device_class: opening

The new configuration will look like this:

mqtt:
  binary_sensor:
    - name: "front_door_open"
      unique_id: front_door_open  # must be set in order to 'hide' this sensor in the front end (if you wish)
      force_update: true  # Sends update events even if the value hasn’t changed (which will always be the case)
      state_topic: "home/OpenMQTTGateway/433toMQTT/7821834"
      value_template: >-
        {% if value_json.value == 7821834 %}
          {{'ON'}}
        {% endif %}

    - name: "front_door_closed"
      unique_id: front_door_closed  # must be set in order to 'hide' this sensor in the front end (if you wish)
      force_update: true  # Sends update events even if the value hasn’t changed (which will always be the case)
      state_topic: "home/OpenMQTTGateway/433toMQTT/7821838"
      value_template: >-
        {% if value_json.value == 7821838 %}
          {{'ON'}}
        {% endif %}

binary_sensor:
  - platform: template
    sensors:
      front_door:
        friendly_name: "Front Door"
        value_template: >-
          {% if states.binary_sensor.front_door_open.last_changed > states.binary_sensor.front_door_closed.last_changed %}
            {{'ON'}}
          {% else %}
            {{'OFF'}}
          {% endif %}
        device_class: opening

Alternativley, You could create an automation with multiple triggers (1 trigger for each code) which then updates a binary helper depending on which trigger started the automation using "Trigger ID's"

Checklist:

  • The pull request is done against the latest development branch
  • Only one feature/fix was added per PR and the code change compiles without warnings
  • I accept the DCO.

James Carey added 2 commits September 13, 2024 17:38
add '-DvalueAsATopic=true' to RF based environments. Required to support changes in zgatewayRF.ino for Home Assistant discoverability
Pass "recieved" as the type argument and switchRF[0] as the subtype argument to announceDeviceTrigger() which will cause recieved codes to be picked up by home assistant as triggers, i.e. When setting up automations in Home Assistant, selecting a `device` trigger and an OMG device will allow the user to select a code picked up during the autoDiscover window as the trigger for the automation, for example "1394004" recieved. In order to support this, the `'-DvalueAsATopic=true'` argument must be added to all environments using the RF Library

Also change getUniqueId() call to remove leading '-' which will prevent MQTT topics having '--' in them
@JamiePhonic JamiePhonic changed the title Modify gatewayRF to improve Home Assistant auto discoverability [RF] Improve Home Assistant auto discoverability Sep 16, 2024
@1technophile
Copy link
Owner

Could you point me to the documentation of Home Assistant auto-discovery for this please?
I would like to understand the different possibilities.

Also is it currently working with HA, I see a typo here recieved

Fix Type-O's in ZgatewayRF.ino

"recieved" => "received"
@JamiePhonic
Copy link
Contributor Author

JamiePhonic commented Sep 18, 2024

Fixed the Type-O's.

I wasn't actually able to find any definitive documentation from Home Assistant on how this works, Instead i looked at the MQTT messages coming from Zigbee2MQTT and replicated what it was doing.

The closest documentation i can find is here: https://www.home-assistant.io/docs/automation/trigger/#mqtt-trigger

It does definitely work. I have a FlipperZero and have been using it to sent a ton of different 433Mhz payloads and verifying that a test automation i setup to send a notification only triggers for the specifically selected payload.

@1technophile
Copy link
Owner

@Odyno as you added the device trigger discovery here #1090 do you mind taking a look at this PR ?

@Odyno
Copy link
Contributor

Odyno commented Sep 19, 2024

Hi @JamiePhonic, excuse me, but I would need more information on this, there are some things I do not understand.

Let me try an example, given an RF Binary message ‘101010000101010110000110’ according to the current implementation we will have 2 messages:
The Discovery Message (Message A1) on homeassistant/device_automation/XXXXXXXXX--11031942-RF/config

{
  "automation_type": "trigger",
  "type": "button_short_press",
  "subtype": "turn_on",
  "topic": "home/OMG2_BTRF/433toMQTT/11031942",
  "device": {
    "identifiers": [
      "XXXXXXXXX"
    ],
    "via_device": "OMG2_BTRF"
  }
}

The real message (Message A2) on home/OMG2_BTRF/433toMQTT/11031942

{
"value": 11031942,
"protocol": 1,
"length": 24,
"delay": 413,
"tre_state": "-",
"binary": "101010000101010110000110",
"raw": "12814,1265,398,431,1224,1254,405,422,1234,1248,411,418,1234,421,1234,427,1227,424,1232,1251,409,417,1236,1246,413,41
7,1237,1243,417,413,1240,1242,416,1237,421,408,1245,413,1240,417,1238,417,1237,1247,412,1242,416,410,1243,"
}

and if the ‘repeatRFwMQTT’ option is set to true only the Message A2 is re-sent.

With the proposed implementation we should have the following messages (correct me if I am wrong)
The Discovery Message (Message B1) on homeassistant/device_automation/XXXXXXXXX-11031942-RF/config

{
    "automation_type": "trigger",
    "type": "recevied",                    //<-----------------------------------
    "subtype": "11031942",           //<-----------------------------------
    "topic": "home/OMG2_BTRF/433toMQTT/11031942",
    "device": {
        "identifiers": [
        "XXXXXXXXX"
        ],
        "via_device": "OMG2_BTRF"
    }
}

and 2 Messages (Message B2 and B3) on home/OMG2_BTRF/433toMQTT/11031942

  {
    "action": "received",       //<-----------------------------------
    "value": 11031942,
    "protocol": 1,
    "length": 24,
    "delay": 413,
    "tre_state": "-",
    "binary": "101010000101010110000110",
    "raw": "12814,1265,398,431,1224,1254,405,422,1234,1248,411,418,1234,421,1234,427,1227,424,1232,1251,409,417,1236,1246,413,417,1237,1243,417,413,1240,1242,416,1237,421,408,1245,413,1240,417,1238,417,1237,1247,412,1242,416,410,1243,"
}
 
{
    "action": "",                  //<-----------------------------------
    "value": 11031942,
    "protocol": 1,
    "length": 24,
    "delay": 413,
    "tre_state": "-",
    "binary": "101010000101010110000110",
    "raw": "12814,1265,398,431,1224,1254,405,422,1234,1248,411,418,1234,421,1234,427,1227,424,1232,1251,409,417,1236,1246,413,417,1237,1243,417,413,1240,1242,416,1237,421,408,1245,413,1240,417,1238,417,1237,1247,412,1242,416,410,1243,"
}

and if the ‘repeatRFwMQTT’ option is set to true only the Message B3 is re-sent.

My questions are as follows:

Could you tell me why this makes it better?

Finally, you mentioned the HA MQTT Trigger (https://www.home-assistant.io/docs/automation/trigger/#mqtt-trigger), but if I'm not mistaken that object is unrelated to this context, in fact it can be used on any topic regardless of how it was created or discovered... am I wrong?

Sorry for the long message, but as I said, I am not getting things right beyond the code and would like to understand the logic first, thank you for your understanding

@JamiePhonic
Copy link
Contributor Author

JamiePhonic commented Sep 20, 2024

Hi @Odyno,

I'll do my best to answer your questions and explain my reasoning:

In the case of the discovery message (from A1 to B1) for the MQTT Device trigger (https://www.home-assistant.io/integrations/device_trigger.mqtt) you have abandoned the default to put two unsupported custom as type (https://www.home-assistant.io/integrations/device_trigger.mqtt/#type) and subtype (https://www.home-assistant.io/integrations/device_trigger.mqtt/#subtype)... Why? Is there some library on HA that I am missing that generates devices on HA via these typings?

Based on the documentation you have referenced above, i took advantage of the following behaviour:

If set to an unsupported value, will render as subtype type

This causes the automation discovery to register received values in Home Assistant as "{code}" received
For example:
image
This makes creating an automation based on a received code significantly faster as you would no longer need to add an additonal condition to check the value of the gatewayRF sensor matches the code you wished to trigger on before proceeding with the automation.

The message is sent twice with only the difference of the action field.... this helps it how?

I discovered while testing this implementation that automations would only trigger once becasue the MQTT message then didn't change significantly for subsequent re-transmissions of the same code. I was not able to find any definitive documentation that explains what is required by Home Assistant to count an MQTT message as a trigger, so i did the only thing i could think of and looked at what a known working implementation did. Since i also run Zigbee2MQTT, i looked at the MQTT payloads it was sending when i pressed a zigbee switch i have.

I observed the following:

Topic: zigbee2mqtt/Aqara Button
Payload:
{
  "action": "single",
  "battery": 100,
  "device_temperature": 26,
  "linkquality": 65,
  "power_outage_count": 28,
  "voltage": 3015
}

Followed immediatley by:

Topic: zigbee2mqtt/Aqara Button
Payload:
{
  "action": "",
  "battery": 100,
  "device_temperature": 26,
  "linkquality": 65,
  "power_outage_count": 28,
  "voltage": 3015
}

I decided to test applying this methodology and it appeared to work. It seems Home Assistant simply needs to see the MQTT message change significantly enough in order to count it as a trigger.

I did test this without adding the "action" element, but instead compiling a build with the '-Dmessage_UTCtimestamp=true' build flag but this did not appear to work.

I hope this answers your questions.

Edit:

With regards to

The message is sent twice with only the difference of the action field.... this helps it how?

I decided to roll back this change and test again just to verify my hypothesis and got different results.
Without the additional "action" field, Home Assistant now registers the trigger correctly, i presume because the "raw" field changes significantly enough to cause the trigger to fire?

I'm not sure why this didn't occur during my initial testing. Perhaps there was something weird happening with my Home Assistant instance that was causing the trigger not to fire correctly?

In any case, i'm happy to push an updated commit rolling this change back since everything seems to function correctly without it, However it may be beneficial to keep it in, in the off chance that 2 subsequent RF signals have the same raw value? (i'm not sure how likely this is to occur)

@JamiePhonic
Copy link
Contributor Author

@Odyno I have rolled back the addition of "action" object in MQTT Payload in this PR.
Would you mind taking another look and let me know your thoughts?

@Odyno
Copy link
Contributor

Odyno commented Oct 17, 2024

Hi, I'm testing your code locally, sorry for a long time, but the new computer got me in trouble with USB configuration and other personal reasons stopped me from testing it.
I'll be back soon

@Odyno
Copy link
Contributor

Odyno commented Oct 17, 2024

Ok for me @1technophile

@1technophile
Copy link
Owner

Thanks @Odyno.

@JamiePhonic this is going to break existing setups for people not using HA, or having a manual HA yaml configuration, could you write a guide in the PR description about how one could easily migrate between the previous API and the new one.

@JamiePhonic
Copy link
Contributor Author

@1technophile This has now been done.
I tried to give a general explantion of how to migrate to the proposed API and given specific examples for Home Assistant as that's all im familiar with. I've used Node-Red before so i could quickly spin up an instance to create some examples for that if needs be.

@1technophile
Copy link
Owner

Great, thanks

@1technophile 1technophile merged commit f5bca00 into 1technophile:development Nov 7, 2024
77 checks passed
@1technophile 1technophile changed the title [RF] Improve Home Assistant auto discoverability [RF][BREAKING] Improve Home Assistant auto discoverability Nov 7, 2024
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

Successfully merging this pull request may close these issues.

3 participants