openhab / openhab-addons

Add-ons for openHAB
https://www.openhab.org/
Eclipse Public License 2.0
1.86k stars 3.56k forks source link

[mqtt.homeassistant] HA Discovery for Tasmota Device assigns every Component a separate Thing #8305

Closed inanimateobjectsaretryingtokillme closed 3 years ago

inanimateobjectsaretryingtokillme commented 3 years ago

Expected Behavior

Components should be grouped into Things by Device. Tested 2 of the plugs in Home Assistant: They are discovered correctly as two Devices with 13 Entities each.

Current Behavior

In Openhab2, the 2 devices are discovered as 26 Things with one channel each.

If I had to guess, I'd say openhab might be using the topic or the json/uniq_id (both of which can be different for different Components of the same Thing?) as the Group for the Thing when it should be grouping by the json/dev/ids or something like that (which appears to be the same for all Components of each Thing)?

Here's some json from the topics homeassistant/sensor/395267_ANALOG_A0 and homeassistant/sensor/395267_ENERGY_TotalStartTime which should be Discovered as 2 of 13 channels of Thing "AWP07L_395267" (:

{"name":"AWP07L_395267 ANALOG A0","stat_t":"tele/AWP07L_395267/SENSOR","avty_t":"tele/AWP07L_395267/LWT","pl_avail":"Online","pl_not_avail":"Offline","uniq_id":"395267_ANALOG_A0","dev":{"ids":["395267"]},"unit_of_meas":" ","ic":"mdi:eye","frc_upd":true,"val_tpl":"{{value_json['ANALOG']['A0']}}"}
{"name":"AWP07L_395267 ENERGY TotalStartTime","stat_t":"tele/AWP07L_395267/SENSOR","avty_t":"tele/AWP07L_395267/LWT","pl_avail":"Online","pl_not_avail":"Offline","uniq_id":"395267_ENERGY_TotalStartTime","dev":{"ids":["395267"]},"unit_of_meas":" ","ic":"mdi:progress-clock","frc_upd":true,"val_tpl":"{{value_json['ENERGY']['TotalStartTime']}}"}

and two from a different device:

{"name":"AWP07L_2EE1AD ANALOG A0","stat_t":"tele/AWP07L_2EE1AD/SENSOR","avty_t":"tele/AWP07L_2EE1AD/LWT","pl_avail":"Online","pl_not_avail":"Offline","uniq_id":"2EE1AD_ANALOG_A0","dev":{"ids":["2EE1AD"]},"unit_of_meas":" ","ic":"mdi:eye","frc_upd":true,"val_tpl":"{{value_json['ANALOG']['A0']}}"}
{"name":"AWP07L_2EE1AD ENERGY TotalStartTime","stat_t":"tele/AWP07L_2EE1AD/SENSOR","avty_t":"tele/AWP07L_2EE1AD/LWT","pl_avail":"Online","pl_not_avail":"Offline","uniq_id":"2EE1AD_ENERGY_TotalStartTime","dev":{"ids":["2EE1AD"]},"unit_of_meas":" ","ic":"mdi:progress-clock","frc_upd":true,"val_tpl":"{{value_json['ENERGY']['TotalStartTime']}}"}

And the discoveries according to /srv/openhab2-logs/events.log:

[home.event.InboxAddedEvent] - Discovery Result with UID 'mqtt:homeassistant_2EE1AD_5FENERGY_5FTotalStartTime:b73c55d3:2EE1AD_5FENERGY_5FTotalStartTime' has been added.
[home.event.InboxAddedEvent] - Discovery Result with UID 'mqtt:homeassistant_2EE1AD_5FENERGY_5FVoltage:b73c55d3:2EE1AD_5FENERGY_5FVoltage' has been added.
[home.event.InboxAddedEvent] - Discovery Result with UID 'mqtt:homeassistant_2EE1AD_5FENERGY_5FYesterday:b73c55d3:2EE1AD_5FENERGY_5FYesterday' has been added.
[home.event.InboxAddedEvent] - Discovery Result with UID 'mqtt:homeassistant_2EE1AD_5FRL_5F1:b73c55d3:2EE1AD_5FRL_5F1' has been added.
[home.event.InboxAddedEvent] - Discovery Result with UID 'mqtt:homeassistant_2EE1AD_5Fstatus:b73c55d3:2EE1AD_5Fstatus' has been added.
[and more]

Couldn't find a group ID or device ID or anything like that in the logs or anywhere in the paper json db. Looks like the mqtt topic is used as UID. Where are those "5F" coming from? Double parsing / htmlencode for "_" as "%5f" => "_5f" somewhere?

Steps to Reproduce

Installed openhab, mosquitto, mqtt(2) on Raspie. Installed Tasmota, "setopt19 1" on smart plug ( like that: https://community.openhab.org/t/how-am-i-supposed-to-install-an-mqttv2-non-system-broker/103670/7 )

I tried adding one of the discovered "HomeAssistant MQTT Component" as a Thing in the hopes that it would get grouped with the other Discoveries somehow later:

You are about to add a new thing AWP07L_395267 AWP07L_395267 (Switch) (mqtt:homeassistant_395267_5FRL_5F1:b73c55d3:395267_5FRL_5F1) from the inbox.

Waited a while in case channels are supposed to get grouped dynamically triggered by mqtt. Nothing happened, so I added another Component of the same Device:

You are about to add a new thing AWP07L_395267 ENERGY Voltage (Sensor) (mqtt:homeassistant_395267_5FENERGY_5FVoltage:b73c55d3:395267_5FENERGY_5FVoltage) from the inbox.

Did not find a way in the paper UI to get them grouped together. If I'm supposed to combine them somehow, it is unclear how.

Switched to testing, Deleted [paper ui Discoveries, openhab cache, mosquitto db], rebooted, "setopt19 1" on Tasmota to trigger rediscovery, same results. Switched to Snapshop 2.5.8~S200-1, Deleted [paper ui Discoveries, openhab cache, mosquitto db], rebooted, "setopt19 1" on Tasmota to trigger rediscovery, same results.

Your Environment

Openhabian v1.6-779 Release = Raspbian GNU/Linux 10 (buster) Platform = Raspberry Pi 4 Model B Rev 1.2 openhab2: stable,now 2.5.7-1, unstable, 2.5.8~S200-1 binding-mqtt - 2.5.8.SNAPSHOT Tasmota 8.4.0 AWP07L smart plug

Some random links because I have no idea what I'm doing

That looks like it could be the commit that implemented the feature originally: https://github.com/openhab/openhab-addons/commit/57aa2232909e77a379fe5a2c031926b4ff10451d

Part of the corresponding HA implementation could be there somewhere: https://github.com/home-assistant/core/search?q=dev_ids

Not sure if that's the right place to start messing with stuff, it just looks like it might be: https://github.com/openhab/openhab-addons/blob/127f05d7cbb865ae2f7a939c761707f0479d5143/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/BaseChannelConfiguration.java#L130

That is what components should be grouped by? https://github.com/openhab/openhab-addons/blob/127f05d7cbb865ae2f7a939c761707f0479d5143/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/BaseChannelConfiguration.java#L107

AAaaaaAA where am I now!? https://github.com/openhab/openhab-addons/blob/7628135323df6f07e3c2993d7b816a16e334d849/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/ListOrStringDeserializer.java#L32

The json parsing part, possibly? https://github.com/openhab/openhab-addons/blob/127f05d7cbb865ae2f7a939c761707f0479d5143/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/MappingJsonReader.java#L156

Wait, what? I feel like I have looked too far into code I don't understand: https://github.com/openhab/openhab-addons/blob/127f05d7cbb865ae2f7a939c761707f0479d5143/bundles/org.openhab.binding.mqtt.homeassistant/src/main/java/org/openhab/binding/mqtt/homeassistant/internal/HaID.java#L186 OK, I give up.

bodiroga commented 3 years ago

Hi @inanimateobjectsaretryingtokillme! (awesome nickname :rofl:)

Thanks for your bug report! I was also seeing it, but I thought that the problem was more complex and I did not dare to get into it. But I have seen that the solution is trivial, so I will provide a PR soon :wink:

inanimateobjectsaretryingtokillme commented 3 years ago

Great, that was fast!

Discovery worked right away with those changes. Getting one thing with all 13 channels now and they look OK in Paper UI. Tested a switch and one sensor: both working.

bodiroga commented 3 years ago

Fantastic, thanks for the feedback @inanimateobjectsaretryingtokillme! :+1: