haywirecoder / homebridge-envisalink-ademco

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

Disarm via Home app not working and timing out #52

Closed barros001 closed 5 months ago

barros001 commented 5 months ago

I recently noticed that when I disarm my alarm using the home app, it will say Disarming... and will just stay there forever, or until I go to a different room and come back. This only happens when disarming via home app. If I disarm via physical keypad, everything works fine. I spent some time debugging but I cannot find anything wrong.

Have you notice this before?

I commented out the following logic below and it works now, but this was just a hack to get my system back operation correctly, and not a real solution:

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/index.js#L429

It's odd that it works arming to any state, but only fails when disarming.

barros001 commented 5 months ago

Actually it not really only when disarming. It also happens when arming, but only when arming Home (which I never use). Arming NIGHT always works because partition.homekitLastTargetState is NIGHT but data.mode when the partition updates is STAY (Vista20p doesn't communicate the night stay), so that if above resolves to true and it sets the target and current state again.

So there's something going on around here:

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/accessories/partitionAccessory.js#L237-L245

Maybe the callback is not really doing what it's supposed to?

barros001 commented 5 months ago

Found something else:

[3/21/2024, 4:17:52 PM] [homebridge-envisalink-ademco] Characteristic 'On': SET handler returned write response value, though the characteristic doesn't support write response. See https://homebridge.io/w/JtMGR for more info.
[3/21/2024, 4:17:52 PM] [homebridge-envisalink-ademco] Error: 
    at On.Characteristic.characteristicWarning (/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:3011:105)
    at /homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2627:22
    at /homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/util/once.ts:15:18
    at EnvisalinkCustomAccessory.setChime (/homebridge/node_modules/homebridge-envisalink-ademco/accessories/customAccessory.js:127:12)
    at On.<anonymous> (/homebridge/node_modules/homebridge-envisalink-ademco/accessories/customAccessory.js:53:58)
    at On.emit (node:events:517:28)
    at /homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2596:16
    at new Promise (<anonymous>)
    at On.<anonymous> (/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Characteristic.ts:2594:14)
    at step (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:193:27)
    at Object.next (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:174:57)
    at /homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:167:75
    at new Promise (<anonymous>)
    at Object.__awaiter (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:163:16)
    at On.Characteristic.handleSetRequest (/homebridge/node_modules/homebridge/node_modules/hap-nodejs/dist/lib/Characteristic.js:871:24)
    at Bridge.<anonymous> (/homebridge/node_modules/homebridge/node_modules/hap-nodejs/src/lib/Accessory.ts:1894:29)
    at step (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:193:27)
    at Object.next (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:174:57)
    at /homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:167:75
    at new Promise (<anonymous>)
    at Object.__awaiter (/homebridge/node_modules/homebridge/node_modules/tslib/tslib.js:163:16)
    at Bridge.Accessory.handleCharacteristicWrite (/homebridge/node_modules/homebridge/node_modules/hap-nodejs/dist/lib/Accessory.js:1550:24)

This is right on line 244 of the snippet I posted above (return callback(null, l_homekitState);).

barros001 commented 5 months ago

https://github.com/haywirecoder/homebridge-envisalink-ademco/commit/c96b5635018bcb7d0e1583e96eba5aea54f36c90#diff-a42daac90d722086efc9b384788a3b8303909beea05c07621bf9779309e8fa38R215

This is actually the culprit. It sets this.homekitLastTargetState and then

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/index.js#L429

will not set the characteristics. Removing that line should solve the issue.

I'm gonna raise a PR but the PR will include another proposed change that I want to hear your thoughts on, so maybe it's better to fix this specific issue separately.

haywirecoder commented 5 months ago

Interesting..I will need to investigate more. I use the Arming Home every night using automation and disarming is instant. Let see if I can re-produce the error condition in my system.

haywirecoder commented 5 months ago

Reviewing your posted error, the Characteristic issue is coming from customAccessory.js which controls the Chime button and other controls. Without additional logs, I am assuming the panel is sending a state of ' EXIT_DELAY', which was missing, thus mapping to a button state would have failed, which corrected now. Still investigating the orginal issue

barros001 commented 5 months ago

Reviewing your posted error, the Characteristic issue is coming from customAccessory.js which controls the Chime button and other controls

I'm not sure I understand the above.

Here's what I believe is happening. When we ARM/DISARM the alarm, the system will send the command and wait until we get feedback from the panel via updatepartition event.

This is where the command is sent. At this point we do not update the Characteristic because we're waiting for feedback from the panel first. https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/accessories/partitionAccessory.js#L237-L244

Here's where we get the feedback from the panel, but notice how we check if partition.homekitLastTargetState != partition.ENVISA_TO_HOMEKIT_TARGET[data.mode].

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/index.js#L429-L436

This condition will most likely be false (I'd say always but somehow it works sometimes - in my case it's actually very rare but I did see it working a few times - probably some other race condition) because we're setting partition.homekitLastTargetState to the expected value here:

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/efeff17bbf98119cf5084afd84eef0aae63a2114/accessories/partitionAccessory.js#L243

The plugin WILL receive the feedback from the panel via updatepartition event, but it will not set the Characteristic because it thinks it's already set. Removing L243 above solves the problem (but I'm not entirely sure why it was added in the first place, so it could potentially break something else that I'm not seeing).

haywirecoder commented 5 months ago
  1. My comment was in reference to the earlier post. The issue is with customAccessory.js refer to line 7. The issue is related to one of Alarm state not being defined in the module. https://github.com/haywirecoder/homebridge-envisalink-ademco/issues/52#issuecomment-2013652529

  2. If a user is making changes via Homekit (e.g. Home) plug-in doesn't have to set the state since the Homekit is already selecting the state once the user has selected it. The setTargetState method doesn't (and shouldn't) update the Characteristic of the alarm object. Hence why it doesn't in the plug-in (e.g. it's already done).

  3. There is a command timeout which default of 10 sec. If the command doesn't return the plug-in will reset the alarm object back to previous stated.

  4. Per my comment above updatepartition is not what updating state of alarm object when it user selected from Homekit. Its primary job is to handle event from the panel and update alarm object. It also clear timer refer to above. The reason partition.homekitLastTargetState != partition.ENVISA_TO_HOMEKIT_TARGET[data.mode] code is present is to prevent the 'flip-flap' state in the Homekit (e.g. set alarm Characteristic to disarm, when the alarm Characteristic is already), this causes issue with some automation.

What would be useful is the debug log when Disarm issue is occuring.

haywirecoder commented 5 months ago

Disregarding the request for logs, was able to reproduce the issue on my system associated with disarming.

barros001 commented 5 months ago

Cool, glad you were able to reproduce. Let me know if you need anything, ill be spotty over the weekend but I’ll respond as I can.

haywirecoder commented 5 months ago

The repo has a new version of the plug-in which addresses "Disarming..." until switching to another app (that was the pain of an issue!) This version also addresses the night mode using keypad text... you suggested. You may see the state change from "Home" to "Night" quickly when you select "Night", that is because the panel partition event fires, but is quickly replaced by the keypad event.

barros001 commented 5 months ago

Cool, I'll give it a try as soon as I can (it's night here and I can't mess with my alarm or I'll wake everybody up!). A couple comments/questions after reading the diff:

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/333ac913de139be2f2fbe4271d3c4f669a5537f6/accessories/partitionAccessory.js#L257-L258 Looks like you're missing an await here?

You may see the state change from "Home" to "Night" quickly when you select "Night", that is because the panel partition event fires, but is quickly replaced by the keypad event

I had the exact same issue with the solution I tried out. But looking at the code I saw this:

https://github.com/haywirecoder/homebridge-envisalink-ademco/blob/333ac913de139be2f2fbe4271d3c4f669a5537f6/accessories/partitionAccessory.js#L268-L271

I'm don't fully understand what this does yet (if you don't mind, can you explain, in case I can't figure it out by myself), but as per the comment you left, did you figure out a way to prevent the NIGHT>HOME>NIGHT issue?

haywirecoder commented 5 months ago

Hi

  1. Nice catch with sleep... I have updated the code and will post it

  2. The plug receives two updates -- one from the partition and the keypad. The partition event is fired as soon as the partition status is changed, it has no concept of NIGHT this is what happened in the original plug-in, it just reports as ARM-STAY. If the last status is set to "NIGHT" the first event the plug-in will get is ARM-STAY which it will see as a "change" from the previously recorded state. This will cause the UI to change to Home (what you see today). By setting it ARM-STAY the partition event will not see this as a change and will not change UI (e.g. which is now set to Night). Afterward, the keypad events take over, that is reporting correctly based on an additional reading of the text field, so it will report as NIGHT. It will see the last state as ARM-STAY and change it to NIGHT, requesting an update to the UI but since it is already set night HomeKit will ignore it.

So yes, It is how to avoid the NIGHT>HOME>NIGHT issue. There is a small chance, you will still see quick NIGH-HOME-NIGHT in UI due to the time of events, but in my test, I do not see any flip-flops.

haywirecoder commented 5 months ago

Updated as part of version 2.0.13

joshjohanning commented 5 months ago

Thanks for the fixes @haywirecoder and the issue reports @barros001! I finally updated from 1.x to 2.0.12 about a week ago and noticed the same thing here. 2.0.13 update resolves it for me as well and is snappy quick to update 😎