search

vanackej / risco-mqtt-local

Provide Risco alarm system integration to Home assistant using local TCP communication (no cloud required) and MQTT
MIT License
24 stars 11 forks source link

Bypass Zone - Attribute to show bypassed state #39

Open ekkesa opened 2 years ago

ekkesa commented 2 years ago

Thank you for this - it is awesome to have control of the alarm without being dependent on cloud infrastructure.

I was using the Risco integration prior, and on the binary sensors they had an attribute which showed the status of the sensor's bypassed state.

bypassed: false

or

bypassed: true

If this is possible it will be awesome - I have automations which are dependent on knowing this state and will be awesome to be able to it again.

Thank you!

markxroberts commented 2 years ago

The state of the zone bypass switch reflects the zone bypass state - as well as being a trigger, it reads the status from the panel. Is there an advantage to make this a state attribute to the binary sensor too?

ekkesa commented 2 years ago

What I picked up when I switched from the cloud integration to yours was that some of my automations failed which used the bypass attribute - I did edit them to use the switch. It failed inconsistently, so I dug a little deeper. What I saw was that when the panel is slow to respond, the switch state changes in succession quickly - to on, then off, and on again when it gets confirmation from the panel. So any automation with that zone bypass switch as a trigger to off, fires incorrectly.

One could work around it, but the attribute might be cleaner from my perspective.

markxroberts commented 2 years ago

Understand. At the moment, attributes are only published at HA discovery, so this would need a bit of reworking. When we discussed setting up the bypass functions, we settled on a switch. An alternative would be a binary sensor and a separate button push to toggle. As the only command that can be sent to the panel is toggle, this would still work, and the binary sensor wouldn't be updated until the message came from the panel. In some ways this would be cleaner as the behaviour you describe couldn't happen. Which do you think is best?

ekkesa commented 2 years ago

I have literally been using your local implementation for about 24hours, so I'm still finding my way around it and exploring the everything what is available. It will be very biased if I give a opinion so early.

From an implementation perspective, I prefer to have a device with all it attributes in one place. So when looking at a particular binary_sensor all its info will be available in one glance, you don't have to remember to check the helper switch to see if it is disabled or not.

vanackej commented 2 years ago

Hi, Your switch should not go back and forth between states when changed.

From the MQTT Switch documentation :

When a state_topic is not available, the switch will work in optimistic mode. In this mode, the switch will immediately change state after every command. Otherwise, the switch will wait for state confirmation from the device (message from state_topic). The initial state is set to False / off in optimistic mode.

As the bypass switch discovery data includes a state_topic, it does not use optimistic mode. It should always wait for panel acknowledgement to change the bypass state. That's the behavior I can see on my HA setup with the addon currently.

Can you provide more logs of what's happening in the addon ? And the sequence of MQTT messages when you change the switch state ?

markxroberts commented 2 years ago

If you want to try my latest development version of this, it's available here: ghcr.io/markxroberts/risco-mqtt-local:v0.5.1 I've added zone attributes of zone bypass status and battery level. The bypass attribute works. I haven't tested the battery one.

vanackej commented 2 years ago

@markxroberts I've seen many devs in progress on one of your various dev branches : https://github.com/vanackej/risco-mqtt-local/compare/main...markxroberts:risco-mqtt-local:dev-outputs Let me know when you think some of them are ready to merge. I've got more free time these days to have a look and merge in the main HA addon

I know about the currently opened PR here : https://github.com/vanackej/risco-mqtt-local/pull/37 , but is it up to date with your latest commits ?

markxroberts commented 2 years ago

PR #37 only addresses the request for multiple panels.

The addition of outputs was a subsequent and separate piece of work. It works for private outputs as binary sensors and user outputs as switches. It's not as elegant as yours because of issue #38, so the status update works differently. It's been working for me for several weeks.

It's too hard maintaining multiple branches, so I've switched to the dev-outputs branch alone. Do you want to review all my changes together along with the modifications requested to #37? If so, I'll create a new PR later.

ekkesa commented 2 years ago

ghcr.io/markxroberts/risco-mqtt-local:v0.5.1

Awesome! I will try it out - Will just try to figure out how to get it up an running in a RPi4 docker.

ekkesa commented 2 years ago

Showing in the attributes but it is not updating when the zone is disabled. The status stays false.

Would it perhaps also be possible to to rename the attribute to "bypassed" to mirror the cloud integration (and prevent me from having to edit a bunch of automations :-) )

markxroberts commented 2 years ago

I’ve spotted the update problem and suggested a fix.

ekkesa commented 2 years ago

Awesome! Let me know, when and I'll update and test.

Thank you!