Refactor unique id handling throughout the addon #179
Conversation
SweVictor
changed the title
|
Work in DeviceRegistry (central repository for all entities in the addon) is started as of now. Main changes include
Currently the code is not intended to run, it's just posted here for code review purposes. |
…kup during BLE startup
|
Version ready for initial dev testing. Expected to work mostly, but not well-tested. Scenes are moved to HA scenes rather than switches so currently they can't be listened to. |
- Minor updates to other packages - Relates to #181
I use Plejd scenes in my automations, what does this comment actually mean for me to be able to continue using those? |
That is a very good question :) Currently I don't see the scenes anywhere in HA, I can see them in MQTT Explorer. The idea is to move them from mqtt "switches" to mqtt "scenes", but for some reason they're not showing up. Any ideas as to why? Mqtt screenshot for overall topic structure Light (working) Scene (not working) |
|
Alright - so - I reached out to the HA community at https://community.home-assistant.io/t/mqtt-scene-config-discovery-from-addon-not-working/297211 Possibly the wrong sub-forum, but either way - no replies for over a week. Since I can't get scenes over mqtt to work at all I'll try reverting to using switches, hopefully in the next few days, unless I get some help from someone how to move forward. |
|
@JohnPhoto and others - scenes should now work as before (registered as switches in mqtt) |
|
Noteworthy evolution: With high probability, DAL-01 will be launched tomorrow. Both IOS and Android Apps are updated to support it since yesterday, and suppliers seems prepared. |
Hopefully the basic structure is similar (API-wise), BLE-wise the current limit of 255 devices seems like a very blocking thing... Either way I think we should finish this PR first unless someone can check it with DAL-01 VERY quickly and report back that some changes are needed. |
Tried to understand this. From my layman point of view, MQTT scenes are what HA performs TOWARDS a MQTT device that is able to carry out routines. Also, I think scenes are not the right way forward for "Plejd Scenes". Also, I think the Switch is not right. Instead, my vote is on the device_trigger that you catch in an automation. If I get this right, first you register your device_trigger in the registry: What you publish, you also create an automation for it. I'll se if I can test this hypothesis with |
Yes - that's exactly what I tried to implement. The problem is that if I replace "switch" with "scene" in mqtt config message (from addon to HA) I can't see anything showing up in HA. No switch, no scene, no nothing. And I don't know how to debug that. The revert that makes switches show up again is here c646bc5 /Victor |
I think you got it wrong. I have it working now with 1.) Register the Device Trigger in the Device Registry: 2.) Create an automation (I did it from the HA UI): 3.) Fire away Thats it. When I publish "execute" my dimmer lights up, all according to the automation created. NOTE: A few screeshots attached: 2.) Information about the object in the device registry 3.) The MQTT info in the registry BR, Marcus |
|
...and...this is how the WPH-01 also should be implemented. As a device trigger. |
|
NOTE!!! This may not be the way to go for scenes. If we use device tracker for Plejd Scenes, you cannot runt the Plejd Scene from the Home Assistant UI. We don't want to loose that. I would rather keep the Plejd Scenes as HA Switches and use Device Tracker for WPH-01. |
|
...and while you poke in this... Please add "unique_id" when Plejd Scenes are registered with /Marcus |
|
Reg WPH-01 integration, did som more digging (sneak peaking at zigbee2mqtt). The following MQTT works in the desired way. I.e. having all the buttons connected to ene entity in Home Assistant, with different actions. A few IMPORTANT notes: |
Yes, exactly that. I would prefer
The problem is that I can't get the mqtt scene part to create any devices in HA. If you can make that work I'd be happy to try again. |
Thanks. But in order to understand what causing this, I'd need to see the result of a silly log during startup. In particular, the response from Plejd API that starts like this: Before you publish this online you want to remove some information like From the info provided I have good and bad news, The bad news is that we will not be able to capture turning the WRT-01 knob. The good news is that WRT-01 behaves just like the WPH-01. This message In order to be able to implement support for WRT-01 as device_automation I need to see (at minimum) I would assume the inputAddress[] to look like this: In the And in order to decode it, I need the |
|
Here is the complete log from the startup. |
Some vital part are missing. There must be information before the first row of your post. I'm missing Please include from this line: Another humble request. Either post the log as a an attached file or use Ctrl-e (Insert code). The indentations helps readability. Sorry for pushing for more and more; but what was provided so far will not allow me to add support fot WRT-01 or identify why your device appears as Light instead of relay. /Marcus |
|
Thanks @vBrolin . Very many discussions in parallel now. As stated, I will "soon" merge this, so
|
|
REL-02.txt |
Perfect. Thanks. On the RELAY/LIGHT issue, the plugin does what it is supposed to do. Plejd are for some strange reason reporting When looking at Frikyla Nedervåning Frikyla Övervåning I think this is a bug on Plejds side. The only thing I can think of is to remove the device from Plejd and re-install it again. /Marcus |
|
It's probably as you say, some sort of bug. I readded the device and it imports correct now. |
|
Alright! Thanks both - I'm calling it and merging this to develop so we can move on to the next PR! |
|
As for updating instructions, you can just go to Integrations->MQTT-Devices, Go into each device and remove it and it will remove the entities to, it might be a quicker and easier way to do it for most |
|
Hey, Though this PR is closed, I found one thing that may require fixing. Maybe it is like this by design, but I do have an issue with mu installation that may be related to this implementation. Not sure. I think It may be my installation, but nowadays only on of the DIM-02 outpus appaer in the list: Still, both of them are available. And, as seen in the following pictures - they are identified as related: |
|
Hmm... My thinking is that I guess you're saying that since you mostly "think" about loads, it might make sense to add individual devices instead. However, from a physical perspective it's reasonable it's one device (physical) with two entities (outputs). Do you feel the same way given a future where we can get info about commands sent as well? From my perspective it makes sense to have one DIM_02 with two Light entities, one device_automation and possibly something else... |
I'm not sure about this one. Your reasoning makes sense. But, when you add area into the equation, it becomes problematic. The different outputs from a DIM-02/REL-02 can be in different rooms. And a room is proposed (in a coming PR) to be connected to an area in Home Assistant. When I send suggested_areas with different rooms, one of the DIM-02 devices looks to be corrupt. When sending this: The implemetation in Home Assistan incorrectly assigns Dimmer_2_2 to the same one as Dimmer_2_1. I suspect this is because they share the same device.identifiers. I will test this hypothesis. |
|
...and a picture of that... |
|
Yup, can confirm this as a required change. When instead using My Ui now defaults to this: And Dimmer_2_2 appeared again: Will you fix, or shall I add the change to the coming PR for suggested_area? This change will be a prerequisite for that PR. |
|
Well. Device key in mqtt discovery does not support unique_id as far as I can see. And the device (physical) is actually the same, so I still think it's reasonable to group under the same device in HA (unless convinced otherwise). Rooms I don't think is good enough of a reason. HA already supports adding entities to other rooms than the device they belong to:
|
|
Ok, this will finally be a matter of opinion then. This effectively means than hassio-plejd will not support That unlike Bond, Hue, Hunter Douglas PowerView, Lutron Caséta, Netatmo, Nexia, NuHeat, Roku, Sonos, Tado, Universal Devices ISY994. Ref. https://www.home-assistant.io/blog/2021/03/03/release-20213/#suggested-areas Trust me that a few people I supported lately are confused when all devices with the same name does not show up and will be equally confused when N devices of the same name appears. I do believe that |
|
One more thing: I consider that the standpoint you have is not fully compatible with the information model from Plejd. This is why you have multiple entries in It is not possible to name devices in Plejd. It is only possible to name outputs. A DIM-02 and a REL-02 will therefore have two names. Which is the right one to use in HA? Right now it is randomly picked. First come, first served. |
|
So, I revered back to Devloper branch to document the current as-is behavior. In my humble opinion, this needs a change. In summaryWorks as expected (+): 3 Enough evidence has been provided to consider a remedy. See details. Details
|
Work in progress regarding changing unique id handling throughout the addon. Will affect internals, lookups, mqtt, break existing devices in HA and more.
Lots of discussion in #102