Koenkk / zigbee2mqtt

Zigbee 🐝 to MQTT bridge 🌉, get rid of your proprietary Zigbee bridges 🔨
https://www.zigbee2mqtt.io
GNU General Public License v3.0
12.16k stars 1.68k forks source link

base_topic is not used everywhere #22559

Open waclaw66 opened 6 months ago

waclaw66 commented 6 months ago

What happened?

I'm running two instances of Z2M with unique base_topic connected to a single mqtt broker and single HomeAssistant with mqtt integration. The problem is that entities like connection_state from https://github.com/Koenkk/zigbee2mqtt/blob/ee5cf5a901ea6fb3426bd534d4f3d44a8ce5c28f/lib/extension/homeassistant.ts#L1844 method are not using configured base_topic, but always zigbee2mqtt prefix. That leads to that entities have ambiguous names and HomeAssistant gives them additional index suffix like: binary_sensor.zigbee2mqtt_bridge_connection_state binary_sensor.zigbee2mqtt_bridge_connection_state_2

What did you expect to happen?

I would expect that every entity shall have an unique id based on configured base_topic like: binary_sensor.zigbee2mqtt_first_bridge_connection_state binary_sensor.zigbee2mqtt_second_bridge_connection_state

How to reproduce it (minimal and precise)

Two instances of Z2M with unique base_topic.

Zigbee2MQTT version

1.37.0

Adapter firmware version

20230507

Adapter

SONOFF Zigbee 3.0 USB Dongle Plus ZBDongle-P

Debug log

No response

waclaw66 commented 6 months ago

I think object_id is to blame, an example from mqtt integration diag data...

          "discovery_data": {
            "topic": "homeassistant/binary_sensor/122105103981011015010911311611695107116_0x00124b0021b7800d/connection_state/config",
            "payload": {
              "device": {
                "configuration_url": "http://10.9.0.33:8036/#/settings",
                "hw_version": "zStack3x0 20230507",
                "identifiers": [
                  "zigbee2mqtt_bridge_0x00124b0021b7800d"
                ],
                "manufacturer": "Zigbee2MQTT",
                "model": "Bridge",
                "name": "Zigbee2MQTT Bridge",
                "sw_version": "1.37.0"
              },
              "device_class": "connectivity",
              "entity_category": "diagnostic",
              "name": "Connection state",
              "object_id": "zigbee2mqtt_bridge_connection_state",
              "origin": {
                "name": "Zigbee2MQTT",
                "sw_version": "1.37.0",
                "support_url": "https://www.zigbee2mqtt.io"
              },
              "payload_off": "offline",
              "payload_on": "online",
              "state_topic": "zigbee2mqtt_kt/bridge/state",
              "unique_id": "bridge_0x00124b0021b7800d_connection_state_zigbee2mqtt_kt",
              "value_template": "{{ value_json.state }}",
              "platform": "mqtt"
            }

object_id doesn't use the configured basic_topic of value zigbee2mqtt_kt and entity_id is derived from object_id

There is comparison of connection_state entity mqtt data of both z2m instances: 1.

device:
  configuration_url: http://10.9.0.31:8036/#/settings
  hw_version: zStack3x0 20230507
  identifiers:
    - zigbee2mqtt_bridge_0x00124b0021b7800c
  manufacturer: Zigbee2MQTT
  model: Bridge
  name: Zigbee2MQTT Bridge
  sw_version: 1.37.0
device_class: connectivity
entity_category: diagnostic
name: Connection state
object_id: zigbee2mqtt_bridge_connection_state
origin:
  name: Zigbee2MQTT
  sw_version: 1.37.0
  support_url: https://www.zigbee2mqtt.io
payload_off: offline
payload_on: online
state_topic: zigbee2mqtt/bridge/state
unique_id: bridge_0x00124b0021b7800c_connection_state_zigbee2mqtt
value_template: '{{ value }}'
platform: mqtt

2.

device:
  configuration_url: http://10.9.0.33:8036/#/settings
  hw_version: zStack3x0 20230507
  identifiers:
    - zigbee2mqtt_bridge_0x00124b0021b7800d
  manufacturer: Zigbee2MQTT
  model: Bridge
  name: Zigbee2MQTT Bridge
  sw_version: 1.37.0
device_class: connectivity
entity_category: diagnostic
name: Connection state
object_id: zigbee2mqtt_bridge_connection_state
origin:
  name: Zigbee2MQTT
  sw_version: 1.37.0
  support_url: https://www.zigbee2mqtt.io
payload_off: offline
payload_on: online
state_topic: zigbee2mqtt_kt/bridge/state
unique_id: bridge_0x00124b0021b7800d_connection_state_zigbee2mqtt_kt
value_template: '{{ value_json.state }}'
platform: mqtt

unique_ids and state_topics are different, but object_ids not

waclaw66 commented 6 months ago

How to fix it? object_id has to be unique across all z2m instances... https://www.home-assistant.io/integrations/mqtt/#naming-of-mqtt-entities _If the object_id option is set, then this will be used to generate the entity_id. If, for example, we have configured a sensor, and we have set object_id to test, then Home Assistant will try to assign sensor.test as entity_id, but if this entity_id already exits it will append it with a suffix to make it unique, for example, sensor.test2. @mundschenk-at can you please check that?

mundschenk-at commented 6 months ago

The HA entity ID is constructed from the device name and the object_id. Duplicate entity IDs are automatically suffixed by HA and can be renamed manually (either by renaming the device or individually). I don't think there is anything to fix here conceptually.

A possible enhancement would be an option to add something like a friendly name for the network and/or the coordinator, but I don't know whether @Koenkk wants to have the added complexity for a relatively niche use case.

waclaw66 commented 6 months ago

Adding friendly name doesn't help, the main problem is that z2m doesn't use base_topic for object_id, similary as for unique_id or state_topic, then two instances of z2m provide bridge state autodiscovery entities (bridge_connection_state, bridge_version, ...) with the same name. So the fix is quite simple, use base_topic instead of fixed "zigbee2mqtt" prefix to generate unique object_id across z2m instances. See yamls above, object_id of 2. instance should be zigbee2mqtt_kt_bridge_connection_state instead of zigbee2mqtt_bridge_connection_state which is duplicate of 1. instance.

  1. instance uses base_topic "zigbee2mqtt"
  2. instance uses base_topic "zigbee2mqtt_kt"
mundschenk-at commented 6 months ago

Sorry, it's been some time that I've worked on that code. It is not HA but Z2M that adds the device name. That logic is used for all discovery topics. The other points still stand.

waclaw66 commented 6 months ago

I'm not sure whether you understand the problem I'm trying to explain. Currently every z2m instance produces entities like switch.zigbee2mqtt_bridge_permit_join with the exactly same name. This fact makes usage of multiple z2m instances together difficult with a single HA. Permit join (and other "zigbee2mqttbridge*") entities of those z2m instances doesn't have a unique name and are suffixed with (_2, _3, ...) by HA in unpredictable way. It works fine for sensor entities with unique IEEE name as a part of entity name, but not for those zigbee2mqtt_bridge_ entities. I can understand how the current build logic of object_id works, however I kindly request to make bridge entities names unique across multiple z2m instances.

mundschenk-at commented 6 months ago

I do understand the problem you are describing, I just don't think its impact is very severe. After all, you can rename both entities and devices in Home Assistant very easily.

Edit: Also I'm not against making the bridge devices more unique per se, but using the discovery topic instead of the device name for the object ID is not the right way. Personally, I'd rather add the IEEE address of the coordinator to the device name (i.e. Zigbee2MQTT Bridge (0x12345679abc)) and let you rename it to whatever you want in HA. Or, as suggested earlier, add a Friendly Name setting for the coordinator/network.

waclaw66 commented 6 months ago

you can rename both entities and devices in Home Assistant very easily.

Sure, I did it already. It's easy when one knows what's happening, but it can be confusing for others.

I used to use custom entities for those bridge states from the historic template - that works fine, however a friend of mine started to use two instances of z2m like me (home/cottage - quite common usecase) with current template and was quite confused why unique base_topic is not taken into account to distiguish those bridge entities. Therefore I reported the issue.

I'd rather add the IEEE address of the coordinator to the device name

That's also an option, but unique base_topic is more tempting. Also the friend of mine expected that. However IEEE address within object_id is also fine zigbee2mqtt_bridge_0x12345679abc_connection_state

mundschenk-at commented 6 months ago

Adding the IEEE address to the generated friendly name would be more in line with general HA usage. I would not like to have special logic just for the bridge discovery and adding the base topic to the name would result in rather awkward entity names (sensor.zigbee2mqtt_bridge_zigbee2mqtt_cottage_connection_state). Unfortunate, the base topic by itself can't really be used to replace Zigbee2MQTT in the friendly name, as that would look even more awkward.

github-actions[bot] commented 1 week ago

This issue is stale because it has been open 180 days with no activity. Remove stale label or comment or this will be closed in 30 days

mundschenk-at commented 1 week ago

I'll make a PR.