haywirecoder / homebridge-envisalink-ademco

Homebridge plug-in for Envisalink Ademco module
MIT License
16 stars 6 forks source link

Improve how sensor status is updated (if really possible) #42

Closed barros001 closed 1 year ago

barros001 commented 1 year ago

This plugin clearly states the following:

Ademco panels provide limited zone information to their peripherals. The panel only provide real-time information when a zone is faulted (opened) but not when it is restored (closed). However, the virtual key panel is continuously updated with zones information. This module auto set the faulted zone (opened) to restored (close) based the value set by openZoneTimeout attributes

But after toying around with the system for a while I can't help but think there's a better way to handle this, and I'll explain why. I opened the Home app on my phone and then opened one of my doors. I kept looking at my phone for 30 seconds and sure enough, 30 seconds later the sensor for that door was set to Closed. BUT, a fraction of a second later, the sensor was once again set to Open for another 30 seconds. I have a Honeywell Vista 20p panel.

This experiment tells me that there must be a way to tell whether a sensor is open or not at any given time. If anything, within the 30 second time, before reseting the sensor back to Close, a check could be performed to check it is still open or not, and skip setting it as Closed if so. And to the extend that this is possible, I wonder if we really need this 30 second timeout at all. I apologize if I'm missing something obvious here (I honestly haven't dug too deep into the system) but just by visually inspecting the system in action, it tells me that there's a way.

I'm a software engineer by the way and can try and help implement this whenever I have some downtime (if at all possible), but wanted to bring this up in case someone that's already familiar with how this plugin works could chime in.

haywirecoder commented 1 year ago

Hmm... Two comments:

  1. I am not able to reproduce your error case -- if the zone is open, the zone never timeout until 30 sec after the zone is closed in all my tests (e.g. I am not getting close and re-open occurrence). As long as the virtual panel indicates the zone faulted the plug-in reset timer continuously it's not a fixed timer. Debug dump would help to see if something is going wrong.
  2. I don't love the timer either... however, the panel does not provide status and the envisalink interface itself uses a countdown timer to determine when a zone closes. Polling (or zone dump) would just provide the module's current timer status of the zone which also reset the time continuously (you can refer to the TPI document in the document section). I found the age of the zone dumping has its own set of drawbacks. I actually implemented the code but found that I would end up with another timer on top of to dump zones (the net result is the same, with a slower response time). Remember, the envisalink module is the interface, not the panel. Open another suggestion once you review the document.
barros001 commented 1 year ago

I spent some time to better understand the plugin and the Envisalink TPI and I get it now. I have a couple of suggestions but before I get to it, let me reiterate two key points to make sure I really got it right:

If the above is correct, this explains why I was seeing a zone close and then re-open. I had my timeout settings set to 5 seconds for a test and probably didn't set it back to 30 seconds. Because TPI will only ping roughly every 10 seconds, a 5-second timeout would trigger before the next ping comes.

Here are a few suggestions I have after digging into the code:

Thoughts on that? If you think any of these suggestions are good and worth implementing, I can certainly help with that.

haywirecoder commented 1 year ago

Comments below:

  1. Create a 30-second time for each zone, independently. This will ensure each zone would be checked at the 30-second mark. The process doesn’t work as you are suggesting. When the first zone fault is added to a zone watch list with its open event time and the timer is set to 30-secs (or whatever the user defines), when the next zone fault is detected, the timer is not reset rather it is just added to the watch zone list with its open event time. When the timer is triggered, it reviews all the open zones and sets a new timer with the zone with the least time remaining. E.g., If zone 1 faults at 00.00, the timer is set to 30-sec, when zone 2 faults at 00.08, no new timer is set or reset. When the time the 30-sec timer is reached zone 1 is clear due to reaching the 30-secs mark, zone 2 is not cleared since it is not at 30-sec, the new timer is created and set for 8-secs. The reason I don’t create individual timers is there are known issues when too many timers are set, with larger systems those issues with timers start coming up.

  2. Can't we assume all zones are closed when the panel reads “Ready”? Can’t make that assumption, not all zone cause the panel not to be “Ready”. A “vent” zone for example will be a zone fault but the panel will be “ready”. The zone timer is also tracking other zone conditions such as the low battery, which also allows the panel to be “Ready”. I could clear all open when the system is in the armed state, but just choose to let naturally age out.

  3. updateZone method only checks for OPEN zones and ignores closed zone. updateZone never receives a close event, TPI never sends a close event. You have to dump the zone to get the status. Don’t understand the vendor’s logic for not sending the event when the timer runs out.

  4. Timeout to anything lower than a certain value (maybe 15 seconds) Agreed. A floor is defined in the UI, but it is pretty low (5-sec), I can increase it to 15-sec. 15 sec does sounds like a good number and can add that next release coming up next week. Other options would be for the system to “learn” the frequency of the update for the virtual keypad and allow a person to use the “learned” value. E.g., Use a default value and adjust after 1 day of sampling the response time. Continuously determine the value and adjust…will have to think about that one more.

barros001 commented 1 year ago

The process doesn’t work as you are suggesting.

activeZoneTimeOut = setTimeout(zoneTimerClose, minZoneTime * 1000);

I missed the minZoneTime detail above. It all makes sense now.

Can’t make that assumption, not all zone cause the panel not to be “Ready”.

I figured that was a lot of details I was missing. I don't really know how to configure a vent zone so I'd have to learn to play around and try to think of something. That could potentially be a config option for simpler home installs where the user would be OK with closing all zones on READY.

Other options would be for the system to “learn” the frequency of the update for the virtual keypad and allow a person to use the “learned” value

One thought that occurred me the other day... the panel sends all faulted zones, in a sequence and in a loop. For example, zone 1, zone 3, zone 1, zone 3. I'd imagine it would be possible to determine once a full cycle happened and at that point close all sections not in the list. Using the same example: zone 1, zone 3, zone 1, zone 3, zone 1, zone 1 (at this point zone 3 is closed), zone 1, zone 1..... you’d still need the timeout though for when a single zone is faulted, which kinda defeats the purpose.

Anyways, I think the delay is good enough of a solution. I'd imagine most automations would rely on immediate OPEN and not so much on CLOSE. Over-optimizing this might not be worth it. I'll leave this open for now but feel free to close if you believe this discussion has run its course.

haywirecoder commented 1 year ago

Closing... updated minimum timeout value increase to 15 sec in UX in latest release.