Improve MQTT startup behavior to disallow Home Assistant from setting new light state during startup

icanos / hassio-plejd

Hass.io add-on for Plejd home automation devices

Apache License 2.0

126 stars 37 forks source link

Improve MQTT startup behavior to disallow Home Assistant from setting new light state during startup #297

Closed SweVictor closed 9 months ago

SweVictor commented 9 months ago

Startup behavior and switching state on Plejd lights have been issues for a long time.

After a lot of investigation and several discussions in multiple tickets, the issue is related to SET STATE messages that are set as RETAIN by Home Assistant in MQTT and therefore sent to this addon on every startup.

This PR fixes that by forcefully removing those messages during startup. It also cleans up the startup flow and does cleanup and discovery both immediately and when Home Assistant sends a birth message over MQTT.

Resolves #218

Replaces the PR #268

Does not address fetching the state from the Plejd mesh that #260 suggests.

tomjaimz commented 7 months ago

Hey @SweVictor - pretty excited about this one as a release. :-) Do you know how long until a 0.12.0 will come out with this functionality?

SweVictor commented 7 months ago

@tomjaimz I was kinda hoping that at least one more person would try out the build in develop before releasing publicly.

That said I have had this running for quite some time myself so I'm guessing it will work... Thoughts?

tomjaimz commented 6 months ago

@SweVictor Started testing this now. :-)

So far so good. Had it running happily for about four hours and then my Zigbee2MQTT bridge failed and everything MQTT related died - hard to say if it's related to the Plejd add on or not, I have been messing more generally with my Zigbee network recently. So I will continue to test.

Also saw this in the logs just before everything failed, but I don't believe it's related, possible just needing to refresh the token or something:

24-02-11 17:20:04 WARNING (MainThread) [supervisor.api.middleware.security] local_plejd missing API permission for /host/info 24-02-11 17:20:04 ERROR (MainThread) [supervisor.api.middleware.security] Invalid token for access /host/info

I will let you know if anything in particular persists.

tomjaimz commented 6 months ago

@SweVictor I can replicate the "becoming available" bug for this release. If I perform a core restart, either via a ha core restart command from the terminal, or by using the RESTART option in Developer tools, then when their Add on loads, all of the Plejd devices become "Unavailable". If I then go in and restart the Plejd Add-on, all my devices are available again.

SweVictor commented 6 months ago

@SweVictor I can replicate the "becoming available" bug for this release. If I perform a core restart, then when their Add on loads, all of the Plejd devices become "Unavailable". If I then go in and restart the Plejd Add-on, all my devices are available again.

That's very interesting and also a bit strange. Strange as in the Plejd addon should start very late to avoid this type of behavior. That said, it's not unreasonable that this change could cause it since we remove mqtt caching of old messages. Could it be the case that there is some race condition between when the mqtt broker starts and the Plejd addon starts? Could you see this in HA logs?

Anyone else seen this?