roopesh / ad-qolsys

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

Update qolsys_requests.py #7

Closed smwoodward closed 3 years ago

smwoodward commented 3 years ago

Added retain=true for mqtt publishing to maintain entities in HA

Idea behind this is to maintain the entities in HA during a restart of AD or other event where the entities might be automatically removed from HA. A situation I noticed was when I restarted AD during an update to this script, the entities disappeared and were recreated. I also use HomeKit to pass things over to Apple Home and when this happened it required the devices to be recreated and moved back into the correct rooms.

I'm not 100% sure that I added the correct keyword argument.

roopesh commented 3 years ago

It would be better if these were configurable. Retain also has negative side effects (e.g. false status) that some people may not want. I'm not sure yet if/when to use a retained message vs waiting for a real message.

smwoodward commented 3 years ago

How would appdaemon handle last will of mqtt items in this situation? That would likely be the best want to handle it is to retain and have a last will of "offline" or another status that would allow HA or what ever HA is also passing information to, to realize that it could be a false status.

That's how many of the zignee and zwave to mqtt items work on my setup so that the device doesn't remove itself from HA, it just shows offline or that there's a problem. I've gone and put the retain=True in my setup to see if there are any adverse side effects of at least retaining it.

smwoodward commented 3 years ago

Ahh, something else that I just noticed that might be a good security thing to do, is not to publish the token to mqtt, just leave the token inside the appdaemon instance. Less places that it's shared out the better. Same with the usercode.

smwoodward commented 3 years ago

So far with adding the retain = True I haven't had any issues but everything seems to stay configured. Another example is customizing the entities (ie doors being labeled as doors and windows as windows in HA) did stick around after I took down the communications to the panel. Now the sensors appeared to disappear out of HA but the arm/disarm stuck around.

roopesh commented 3 years ago

How would appdaemon handle last will of mqtt items in this situation? That would likely be the best want to handle it is to retain and have a last will of "offline" or another status that would allow HA or what ever HA is also passing information to, to realize that it could be a false status.

I think this is the problem to solve i.e. availability first instead of retaining items giving a false status

smwoodward commented 3 years ago

I think this is the problem to solve i.e. availability first instead of retaining items giving a false status

What do you mean availability first?

roopesh commented 3 years ago

@smwoodward I'm thinking about how to use https://www.home-assistant.io/integrations/alarm_control_panel.mqtt/#availability and https://www.home-assistant.io/integrations/binary_sensor.mqtt/#availability

smwoodward commented 3 years ago

@roopesh ahh, that makes sense and that looks like it would be a better way to go.

roopesh commented 3 years ago

So I put in the availability stuff .. but when shutting down AppDaemon, the terminate() function runs after AD unloads MQTT. Which means the offline messages never publish. I'm a bit stuck there but will look for some options.

smwoodward commented 3 years ago

Hmm. Almost sounds like you might need to use some other mqtt function that isn't dependent to the AD mqtt.

I haven't seen the new code, but something that might be prudent would be once your plug-in loads to mount the items as offline until the connection to the panel is up and working properly and receives the ACK. Also where I've seen in the logs a couple of times where it stops listening to the socket that should be published as offline.

Maybe AD can change their terminate process to unload custom apps before unloading their plugins.

roopesh commented 3 years ago

Will not merge. Version 1.6.0 solves using alarm_control_panel and binary_sensor's availability. Please note this relies on AppDaemon's will_topic and birth_topic. If you haven't defined these, you're fine. If you have changed these from default values, see readme.