Set lock status correctly for Schlage BE469 Z-Wave Locks#18714
Closed
ahayworth wants to merge 2 commits intohome-assistant:devfrom
Closed
Set lock status correctly for Schlage BE469 Z-Wave Locks#18714ahayworth wants to merge 2 commits intohome-assistant:devfrom
ahayworth wants to merge 2 commits intohome-assistant:devfrom
Conversation
Some Z-Wave locks do not send a DOOR_LOCK_OPERATION_REPORT after a user manually manipulates a lock (via key, deadbolt, keypad, etc). Instead, these locks only send a DOOR_LOCK_OPERATION_REPORT in response to Z-Wave commands such as DOOR_LOCK_OPERATION_GET or DOOR_LOCK_OPERATION_SET. Based on my reading of the Z-Wave specification, it could be considered correct. In fact, OpenZWave seems to know about this, and queues a DOOR_LOCK_OPERATION_GET immediately after a DOOR_LOCK_OPERATION_SET: https://github.com/OpenZWave/open-zwave/blob/master/config/schlage/BE469.xml#L141 We can see that in action here on my own network with this lock: 2018-11-25 19:44:52.153 Info, Node019, Value::Set - COMMAND_CLASS_DOOR_LOCK - Locked - 0 - 1 - False 2018-11-25 19:44:52.153 Info, Node019, Value_Lock::Set - Requesting lock to be Unlocked 2018-11-25 19:44:52.153 Detail, Node019, Queuing (Send) DoorLockCmd_Set (Node=19): 0x01, 0x0a, 0x00, 0x13, 0x13, 0x03, 0x62, 0x01, 0x00, 0x25, 0x7b, 0xcb 2018-11-25 19:44:52.153 Detail, Node019, Queuing (Send) DoorLockCmd_Get (Node=19): 0x01, 0x09, 0x00, 0x13, 0x13, 0x02, 0x62, 0x02, 0x25, 0x7c, 0xcd 2018-11-25 19:44:53.718 Detail, Node019, Decrypted Packet: 0x00, 0x62, 0x03, 0x00, 0x00, 0x02, 0xfe, 0xfe 2018-11-25 19:44:53.718 Info, Node019, Received DoorLock report: DoorLock is Unsecure Unfortunately, when we call `update_properties()` for a Z-Wave lock, we don't really know what property has changed, or what message the Z-Wave stack received, or even what it plans to do next. So we potentially construct a state for the device that isn't actually valid. In fact, the right thing to do here is _nothing_ - ideally the component would wait until the DOOR_LOCK_OPERATION_GET completed, and _then_ update state. In home-assistant#17386, we began setting the lock state for certain models of locks based solely on the Alarm status. This is a good workaround for many models of locks, but it also depends on a lock *ALWAYS* sending an Alarm after *any* operation. That is unfortunately not the case for at least the Schlage BE469. In fact, we get an Alarm notification for every operation _aside_ from RF locks/unlocks. Long story short, this PR introduces yet another work around state - this time one based on clearing alarm status. When a user calls lock() or unlock(), we explicitly set any alarm_type or alarm_level to None. For a normal lock, this should be fine - we can't always assert that an alarm state will be present anyways. Moreover, if a user is manipulating a lock via RF we could assert that they are okay with clearing any alarm statuses - they're working with the device. But for the BE469, we can look for an enabled workaround flag, and a cleared alarm, and then decide that the value from a received door lock update is in fact correct, and set flags appropriately. The PR sets more things manually than I'd really like - it seems that this model really does send _just_ the bare minimum, and we can't rely on other things like access_control being set when an RF event happens.
Contributor
Author
|
Whoops, a wild bug-not-caught-in-testing appeared. Will re-open later when it's fixed. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Some Z-Wave locks do not send a
DOOR_LOCK_OPERATION_REPORTafter a usermanually manipulates a lock (via key, deadbolt, keypad, etc). Instead,
these locks only send a
DOOR_LOCK_OPERATION_REPORTin response to Z-Wavecommands such as
DOOR_LOCK_OPERATION_GETorDOOR_LOCK_OPERATION_SET.Based on my reading of the Z-Wave specification, it could be considered
correct. In fact, OpenZWave seems to know about this, and queues a
DOOR_LOCK_OPERATION_GETimmediately after aDOOR_LOCK_OPERATION_SET:https://github.com/OpenZWave/open-zwave/blob/master/config/schlage/BE469.xml#L141
We can see that in action here on my own network with this lock:
Unfortunately, when we call
update_properties()for a Z-Wave lock,we don't really know what property has changed, or what message the Z-Wave
stack received, or even what it plans to do next. So we potentially construct
a state for the device that isn't actually valid. In fact, the right thing
to do here is nothing - ideally the component would wait until the
DOOR_LOCK_OPERATION_GETcompleted, and then update state.In #17386, we began setting the lock state for certain models of locks
based solely on the Alarm status. This is a good workaround for many models
of locks, but it also depends on a lock ALWAYS sending an Alarm after any
operation. That is unfortunately not the case for at least the Schlage BE469.
In fact, we get an Alarm notification for every operation aside from RF
locks/unlocks.
Long story short, this PR introduces yet another work around state - this time
one based on clearing alarm status. When a user calls
lock()orunlock(),we explicitly set any
alarm_typeoralarm_levelto None. For a normal lock,this should be fine - we can't always assert that an alarm state will be present
anyways. Moreover, if a user is manipulating a lock via RF we could assert
that they are okay with clearing any alarm statuses - they're working with the
device. But for the BE469, we can look for an enabled workaround flag, and a cleared alarm, and then decide that the value from a received door lock update
is in fact correct, and set flags appropriately.
The PR sets more things manually than I'd really like - it seems that this
model really does send just the bare minimum, and we can't rely on other
things like access_control being set when an RF event happens.
Checklist:
tox. Your PR cannot be merged unless tests pass