tsightler / ring-mqtt

Ring devices to MQTT Bridge
MIT License
578 stars 104 forks source link

Disarm fire alarm event does not change alarm panel to disarmed status #780

Closed iptvcld closed 5 months ago

iptvcld commented 8 months ago

Describe the Bug

Disarm fire alarm event does not change alarm panel to disarmed status, instead it remains in triggered state until you restart HA.

Below screenshot shows it remained in triggered until i rebooted HA

Steps to Reproduce

Alarm is in disarmed state, make the smoke detector catch smoke, the alarm will trigger and sound, once you disarm the alarm via keypad, the alarm shuts off but in HA, the alarm panel still shows triggered until you reboot HA and that's when it changes to disarmed

Expected Behavior

alarm panel entity should follow the real time state and when it was in a triggered state, it should change to disarmed once the the alarm has been disarmed

Log Output

NA

Screenshots

image

Config File

NA

Install Type

HA

Version

Latest

Operating System

HA

Architecture

HA

Machine Details

VM

tsightler commented 8 months ago

Wouldn't simply arming and disarming the alarm clear it without restarting? Did you try this?

iptvcld commented 8 months ago

as per my steps to reproduce, this is exactly what I did:

once you disarm the alarm via keypad, the alarm shuts off but in HA, the alarm panel still shows triggered until you reboot HA and that's when it changes to disarmed.

If i were to arm and disarm it just clear it - im sure that will work but i have automations that are pending based on the fact the state did not return back to disarmed after a triggered state

tsightler commented 8 months ago

I didn't ask anything about restarting HA, I asked about performing a manual arm/disarm cycle from the panel (since you are already standing there to disarm the fire alarm) because that tells me if the code is still working of if it is completely out-of-sync. You said you restarted HA, but of course that will work, that's brute force, I'm asking for information that will help me understand the sequence of events in a more nuanced way based on how the code handles these events, I'm in no way implying that you should have to do that.

iptvcld commented 8 months ago

understood, thanks. I am not able to trigger a manual fire event at this moment but I just pulled more events in case if this helps. I looked at the alarm info entity and that state went into a fire-alarm state at the correct time but this at least returned back to all-clear. Saying that, it did not return back to all-clear as soon as i disarmed the alarm. As per Ring app logs, i disarmed it at 2:13pm and the info alarm entity cleared it self at 2:18:01pm. Short delay here, whereas the main panel entity did not clear it self.

image

tsightler commented 8 months ago

I think this is probably a subtle logic error in the code. HA alarm entity has a single property to represent the overall alarm state, but Ring alarm doesn't really behave this way, it has a property for the current arming mode (none/some/all) and then different properties for various things like entry/exit delay, co/fire/burglar alarm, etc. which all have to be computed into an HA alarm state.

To get information such as the name of the arming/disarming user/entity the code monitors the event bus for impulse data to detect a mode switch as that's the only place this information is available in real-time. This code currently assumes that, after a triggered alarm event, there will always be a mode switch from some armed mode to the "disarm" mode. In your case, this isn't true, the smoke alarm occurs when the alarm is already in "disarmed" mode, and thus there is no mode switch message when the alarm event is cleared.

Honestly, I hate pretty much everything about this code path, it too prone to logic errors in strange corner cases like this that are hard to test as it requires a full understanding of all possible impulse types, which is pretty much impossible to know. I need to think about how to solve that, maybe rewrite the entire logic. Don't know when that will happen though, as I don't have much time to mess with this project these days.

iptvcld commented 8 months ago

Thanks for the detailed response, what you're saying makes complete sense as to what occurred for me today. My alarm was already in a disarmed state and when the fire event occurred, the HA alarm component got stuck in the triggered mode as the previous state was something it is not used to see. Hence the reason I have not seen this issue occur on a regular alarm event when I open one of the windows when the alarm is in armed home.

I like to use the HA panel for my status to trigger on events while away mode and home mode because then I can have different trigger events based on what state the home was in.

I tried using the ring Info entity but it was harder for me to get the state it was in as it's always all-clear when in away mode or home mode. I guess I can use a combination of both sensors to get the state on a trigger as well.

As always, I appreciate your time you have put into this project. It is very impressive and is my main purpose for HA

tsightler commented 8 months ago

I'll try to think of a way to solve this issue in the nearer term, the biggest problem is it's difficult to test because I have monitoring, so I have to put it in test mode, test smoke, which I can only do easily when nobody is here, but hopefully I can come up with something.

iptvcld commented 8 months ago

appreciate your help!

tsightler commented 8 months ago

I've identified that there is an impulse type of "alarm-cleared" that I need to monitor for to catch these events and update the current state. I'll need to code up some simple logic to deal with this case.

iptvcld commented 8 months ago

Since you mentioned alarm-cleared I thought I saw something that said in my Ring app.

This was the trigger image

This was when I deactivated my alarm via the key pad. image

This was when the smoke sensor alarm tone turned off when I blew the smoke away. image

tsightler commented 8 months ago

Dev branch has an attempt to fix this, if you have time test it out via the branch feature (set branch config option to "dev" and restart the addon) it would be much appreciated.

iptvcld commented 8 months ago

Thank you, I will perform the update tomorrow morning and then perform a test once I get the nod from my wife as she also works from home.

Once updated, I assume I should perform the same scenario? Leave alarm in disarmed state and trigger the smoke then disarm via keypad and check HA alarm sensor to see if state is disarmed?

Thank you for your time..

iptvcld commented 8 months ago

i change the branch field from addon to dev and pressed save and rebooted the addon. Below is the log output

Updating ring-mqtt to the dev version... Cloning into 'ring-mqtt-dev'... Installing node module dependencies, please wait... The ring-mqtt-dev branch has been updated. cont-init: info: /etc/cont-init.d/ring-mqtt.sh exited 0 s6-rc: info: service legacy-cont-init successfully started s6-rc: info: service legacy-services: starting services-up: info: copying legacy longrun ring-mqtt (no readiness notification) s6-rc: info: service legacy-services successfully started

Running ring-mqtt... 2024-01-24T14:49:59.405Z ring-mqtt Detected runmode: addon 2024-01-24T14:49:59.406Z ring-mqtt Configuration file: /data/options.json 2024-01-24T14:49:59.865Z ring-mqtt Reading latest data from state file: /data/ring-state.json 2024-01-24T14:49:59.865Z ring-mqtt Succesfully started the token generator web UI 2024-01-24T14:49:59.877Z ring-mqtt Discovered MQTT Host: core-mosquitto

tsightler commented 8 months ago

I'm not sure why you are posting the startup messages, they just look normal, are you attempting to say there is something wrong?

iptvcld commented 8 months ago

i was just making sure that what i did to change to Dev was correct. I also just performed a test.

Result: Once i deactivated the fire alarm via the keypad, the HA alarm state returned to Disarmed image

This is the HA sensor after is deactivated the alarm image

This is the alarm_info sensor status image

tsightler commented 8 months ago

So it looks like it worked.

iptvcld commented 8 months ago

Yes, it looks like it all worked this time, thank you! When do you think this update will be passed to the regular branch? I will change my branch setting back to that then.

Thank you!

tsightler commented 8 months ago

I can't be 100% sure as I need to make a decision if it is worth pushing this as a bugfix/dependency update, or if I should just wait until I have a little more time to sneak in a few additional updates which are backlogged as there has not been an updated version for a few months due to lack of time to work on this project. Worst case probably a few weeks, best case maybe sometime this weekend.

iptvcld commented 8 months ago

Hello, would you have a timeframe for this fix to be implemented? Thanks.

tsightler commented 8 months ago

I don't have any timeframe for the next release version, it's really just whenever I have time to get it done. Based on my current availability, I wouldn't expect it this month.

iptvcld commented 6 months ago

Hey, was just checking in to see when you think you would be able to perform this update? Thanks

tsightler commented 6 months ago

Really no idea, there's not much work that needs to be done to finish the things I had planned for the next release, but I have almost zero time for this project right now.

iptvcld commented 6 months ago

understood; so much work has gone into this and I would hate to see it get out of date and not function as efficiently. But at the end of the day, we are enjoying the product in which you have spent countless amounts of hours on. Thanks

tsightler commented 6 months ago

Maybe someone would like to pick it up, but the issue is that ring-client-api is also quite stagnant. I try to assist with the maintenance of that project as well, but we both have the same issue, almost no time for personal projects. And the start of this year has simply been crazy busy with nearly zero downtime to work on personal projects.

iptvcld commented 6 months ago

Alright, I will have a look at my automations again for this and try to change things around and not use the fire alarm in HA or try to make something to reset the status or reboot HA.. This is going to be messy and I am not sure what else is out there for an alarm that we can integrate in HA. I based getting Ring on this project but things change and no control over that.

tsightler commented 6 months ago

To be clear, the project is not going away, and if something was completely broken I would try to find time to fix it as quickly as possible because I know this project is used by over 10,000 people, but the issue here is a fairly minor bug that happens only with fire alarm, which usually happens very, very rarely, and the fix is already available in the dev branch, which you can can personally keep using forever (I always run the dev branch in my setup so I try not to break it, at least for long). So, from my perspective, this is a low-priority issue not worthy of pushing as a separate build, that's all.

However, I would say basing the decision to purchase any product that required the effort of reverse engineering to get working is always quite risky as Ring could technically block us completely at any time and make the project immediately unusable, just as Chamberlin did with MyQ. So far Ring has shown no signs of doing this, thye've even unofficially let us know of some old APIs we were using that were going away so we could migrate the code before this point, but this somewhat friendly behavior could change at any time.

iptvcld commented 6 months ago

Thanks for the clarifications and the idea about the Dev branch. I may just update from the Dev branch and rework my sensors with the updated (tested) changes and then don't update Dev and wait for the Master branch to eventually receive the new code release before i make the change over again.

tsightler commented 5 months ago

Finally managed to get an update pushed with the fixes. Sorry about the delay.

iptvcld commented 5 months ago

thank you!