roopesh / ad-qolsys

AppDaemon app for Qolsys IQ Panel 2
MIT License
22 stars 10 forks source link

MQTT retain flag should be True #22

Open jsb5151 opened 2 years ago

jsb5151 commented 2 years ago

I've experienced a few situations where discovery is failing because the topics simply don't show up in Mosquitto after a reconnect of the MQTT client, and this seems to be due to the fact that most messages sent by ad-qolsys have the Retain flag set to false. I discovered this by trying to "fix" the other issue I flagged (#21) by manually pushing a discovery config, but it would still show up as Unavailable in the HA GUI.

Most if not all other MQTT integrations use Retain=True, why is this one not doing that? I see Retain=true in some lines of the code but not sure why it doesn't apply to all of them.

roopesh commented 2 years ago

This was a conscious decision that I'm unlikely to reverse (anyone can fork this and run their own version). I decided this is meant to reflect the current state of the security system. I don't want to assume state. I want it to be accurate or nothing. However, maybe I missed something. How can you guarantee accuracy with retention?

On Mon, Oct 25, 2021 at 1:14 PM jsb5151 @.***> wrote:

I've experienced a few situations where discovery is failing because the topics simply don't show up in Mosquitto after a reconnect of the MQTT client, and this seems to be due to the fact that most messages sent by ad-qolsys have the Retain flag set to false. I discovered this by trying to "fix" the other issue I flagged (device_class empty value) by manually pushing a discovery config, but it would still show up as Unavailable in the HA GUI.

Most if not all other MQTT integrations use Retain=True, why is this one not doing that? I see Retain=true in some lines of the code but not sure why it doesn't apply to all of them.

— You are receiving this because you are subscribed to this thread. Reply to this email directly, view it on GitHub https://github.com/roopesh/ad-qolsys/issues/22, or unsubscribe https://github.com/notifications/unsubscribe-auth/AABXEKBRAWQ4G7OM57CQDJTUIW3EFANCNFSM5GWDZYUQ . Triage notifications on the go with GitHub Mobile for iOS https://apps.apple.com/app/apple-store/id1477376905?ct=notification-email&mt=8&pt=524675 or Android https://play.google.com/store/apps/details?id=com.github.android&referrer=utm_campaign%3Dnotification-email%26utm_medium%3Demail%26utm_source%3Dgithub.

jsb5151 commented 2 years ago

Typically, integrations don't use Retain=False as a fallback mechanism. This creates unwanted behaviour where the sensors show as offline if the client application monitoring the topics have connected to the broker after that message was sent. Whilst I was testing this integration, I connected using MQTT Explorer to diagnose why some sensors wouldn't show up, and MQTT Explorer would not report the last mqtt-states at all, and mqtt-availability was showing "offline" for all my sensors... even though they were updating just fine in Home Assistant!

IMHO, the best way to do this is to use LWT: Last Will & Testament messages. Basically, a client or device would connect to the broker and publish a message to a "device/status" topic with payload "online", but the last will message of that client is "offline". So, if the client device disconnects from the broker for whatever reason (voluntarily or not), the broker updates that topic with the "offline" message. This is a widely used mechanism with many MQTT clients (paho, etc). More info: https://www.hivemq.com/blog/mqtt-essentials-part-9-last-will-and-testament/

AppDaemon implements this natively - the "appdaemon/status" topic shows online, but if I kill the addon, Mosquitto reverts it to its last will message which is "offline". And yes, the Retain flag is True on either one of them. :)

Pablo1 commented 2 years ago

@roopesh - take a look a the pull request I submitted. I experienced the same symptoms and went down the same path but it was not related to persistence.

@jsb5151 - the qolsys panel is quite chatty and repeats the zone_info events quite frequently and IIRC they include all of the zone info so there is no need to maintain state independently. If you set the debug flag you an see the messages coming

jsb5151 commented 2 years ago

That's true - I just created an automation that sends an INFO request to the panel, which fixes the states. Not the most elegant solution but it works...

roopesh commented 2 years ago

@jsb5151 I do use a Will that makes the sensor unavailable. That is a more accurate status.