icanos / hassio-plejd

Hass.io add-on for Plejd home automation devices
Apache License 2.0
126 stars 37 forks source link

Fix for issue discussed in #198. #199

Closed faanskit closed 3 years ago

faanskit commented 3 years ago

Scene device triggers were not unique from Scenes and device_trigger /config topic was too long

faanskit commented 3 years ago

Yea, agree. But empirical tests were conclusive. Guess a few more tests would be needed to fully conclude. Possibly ask the devs of HA.

SweVictor commented 3 years ago

Yeah, I understand, still strange! With the working solution - can you see the device triggers somewhere in HA? Do they show up in the Logbook? Can you see a device with entity connected?

faanskit commented 3 years ago

Really very strange that these changes. The device part should not have to be unique (it IS the same device), and mqtt topic length also seems strange as a limitation...

Maybe not super strange. From homeassistant/components/scene/__init__.py:

class Scene(Entity):   
 """A scene is a group of entities and the states we want them to be."""

With the working solution - can you see the device triggers somewhere in HA?

Yes, see screenshots below. Note "Model" is named Scene by you ;-)

Do they show up in the Logbook?

EDIT: You can see automations triggered in the logbook. See screenshot. History does not show this entity.

Device enheter enhetsinformation enhetsinfo_mqtt

Entity entiteter entitetsinformation

History history

Log logg

Logentry logentry

SweVictor commented 3 years ago

With the current develop (without this PR) I can see similar info under MQTT info under the scene which shows up under entities.

image

I also see the MQTT message come through, but no automation is triggered. Maybe it is as you say that while you can combine sensors and outputs in one device you cant combine scenes and other stuff (device triggers) in one device...

faanskit commented 3 years ago

I also see the MQTT message come through, but no automation is triggered. Maybe it is as you say that while you can combine sensors and outputs in one device you cant combine scenes and other stuff (device triggers) in one device...

I had strange and random behavior without the patch.

Example: Scenario: Two Plejd Scenarios (LeftOn/LeftOff), without patch:

Example: Scenario: Two Plejd Scenarios (LeftOn/LeftOff), only with unique-id patch:

As I said, strange and random behavior.

faanskit commented 3 years ago

Ok, I think I made a [small] mistake when I created this PR. Every commit/pull to my development branch will now be added to this PR. Sorry for that.

Anyway; the latest push is relevant for this PR. Please check it out. It's the feature that was not a bug wrt. Plejd Scenes from HA not being reported back to HA for automations...

faanskit commented 3 years ago

Been looking into this trying to recreate the issues again aiming to bring the length issue to the devs of HA. But, with a clean system, I cannot reproduce the "length" issue as a standalone issue.

My suspicion is that the "id" issue leaves some weird things in device registry causing these strange behaviors.

Nevertheless, I recommend the leave the fixes as proposed in PR as they appear to work and that I have seen weirdness with the length; even though I cannot reproduce it now.

SweVictor commented 3 years ago

Alright, merging this then. Thanks for the fix!

masfak97 commented 3 years ago

If I understand it correctly, the 0.8.0-beta should make device triggers work? However, even with the 0.8.0-beta installed I can't get my automation in Home Assistant to trigger repeatedly. When the plejd add-on is started, both my scenarios are created and visible in MQTT Explorer as a retained message with QoS 0 even though the payload says QoS 1. The automation gets triggered.

Every following time the plejd scenario gets called (double click physical switch) the original message fails to be replaced but it seems as if a message goes through on the parent topic (homeassistant/device_automation/plejd/XXXX_trig/ instead of homeassistant/device_automation/plejd/XXXX_trig/config). Sometimes the message is retained but still can't be replaced.

If I delete the entire message in MQTT explorer and then double-click, an empty message is created with topic homeassistant/device_automation/plejd/XXXX_trig/state. After that, for each double-click, the empty message is deleted and created.

If I restart the plejd add-on, the messages are recreated and the automation triggers once.

Do you have any ideas on what to do? I would really like to be able to trigger HA automations with Plejd scenes.

masfak97 commented 3 years ago

Found another thread discussing this and will answer my own question. Triggering an automation by an MQTT PLEJD automation trigger in HA isn't done by reacting to the MQTT message, rather using the created device with the same name as the scenario in the PLEJD app as a trigger. This works fine for me.