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

Fix for issue #31 #32

Closed markxroberts closed 2 years ago

markxroberts commented 2 years ago

This code change fixes issue #31. I haven't found any issues with this yet. For the bypass switches the alternative fix is to set the 'optimistic' flag to 'true'.

vanackej commented 2 years ago

I did it this way originally, but wasn't happy with it because it reported the previous known state even if it was really old and sometimes inaccurate. For an alarm system, I don't think it is a desired behavior. Wouldn't it be better to subscribe to home assistant availability topic, and send the current sensors state again on HA availability publication ? What do you think about it ?

markxroberts commented 2 years ago

I see the problem. I haven’t experienced these issues myself. When might this occur in reality? I find it more frustrating when the system reports an unknown state for all sensors at restart. The solution you describe is the most elegant!

On 3 Apr 2022, at 22:08, Johann Vanackere @.***> wrote:

I did it this way originally, but wasn't happy with it because it reported the previous known state even if it was really old and sometimes inaccurate. For an alarm system, I don't think it is a desired behavior. Wouldn't it be better to subscribe to home assistant availability topic, and send the current sensors state again on HA availability publication ? What do you think about it ?

— Reply to this email directly, view it on GitHub https://github.com/vanackej/risco-mqtt-local/pull/32#issuecomment-1086949781, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF2J4VZOEJK532THZWPMLTDVDICGVANCNFSM5SM4WYTQ. You are receiving this because you authored the thread.

markxroberts commented 2 years ago

Update to fix unavailability status following Home Assistant restart

ob1w4nken0b1 commented 2 years ago

Wow! Can't wait to test this

vanackej commented 2 years ago

Looks good. I'm not at home this week, I'll be able to test and merge next week. Thanks for the contribution 👍

markxroberts commented 2 years ago

I’ve shared the Javascript file directly with @ob1w4nken0b1 If you know how/where to replace this in your setup, you will be able to test.

On 16 Apr 2022, at 15:04, ob1w4nken0b1 @.***> wrote:

Wow! Can't wait to test this

— Reply to this email directly, view it on GitHub https://github.com/vanackej/risco-mqtt-local/pull/32#issuecomment-1100671770, or unsubscribe https://github.com/notifications/unsubscribe-auth/AF2J4V5KPCKCLYZB2SBNFD3VFLCHTANCNFSM5SM4WYTQ. You are receiving this because you authored the thread.