Zwave Lock: Support SLGA Migration#2944
Conversation
hcarter-775
requested review from
aleclorimer,
cbaumler,
cjswedes,
ctowns and
nand-nor
|
Duplicate profile check: Passed - no duplicate profiles detected. |
|
Invitation URL: |
Test Results 72 files 513 suites 0s ⏱️ Results for commit f24f62f. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against f24f62f |
|
For some reason my comment is failing to save when i try to add an in line comment, but I was wondering if in addition to the lockCodes.migrated attribute should we also set a persisted flag to indicated the migrated state? Just since otherwise it would fallback to the legacy handling if the state cache is cleared |
I agree with this: we should use the datastore. The state should always be available now that we are using the hubs state store to back the driver state_cache, but we have not always had this guarantee, and the pattern of expecting cached state to be present is not good IMO. Should we disable that for this driver, or make a change to how the hub handles state, it could have unintended consequences here. |
|
@cjswedes @nickolas-deboom If we're wondering about the state cache, I wonder how secure |
hcarter-775
force-pushed
the
re-structure/feature/chad-16364-zwave
branch
from
f5ab265 to
f24f62f
Compare
| version: 1 | ||
| - id: lockCodes | ||
| version: 1 | ||
| - id: lockCredentials |
There was a problem hiding this comment.
What if an existing device on one of these profiles that would not be migrated doesn't support these capabilities? I'm just wondering if new profiles should be created instead
| zwave_handlers = { | ||
| [cc.TIME] = { | ||
| [Time.GET] = time_get_handler -- used by DanaLock | ||
| [0x01] = time_get_handler -- used by DanaLock |
There was a problem hiding this comment.
Just curious but why did Time.GET change to a hardcoded value?
4 participants
Description of Change
Follow-up to #2937, where I've responded to some comments that myself and others have left in order to clarify certain functionality.
Note on structure: The
legacy-handlerssubdriver is nearly untouched from before- the only real difference is that the key-we logic was lifted out from there since it is so close in from to the "new", now default key-we overriden behavior.The other primary thing that was removed from the now-
legacy-handlerssubdriver is the oldaddedhandler, for the following 2 reasons. 1. Generally, devices should not be added to this driver any longer, and 2. We now use theaddedmain driver implementation to initially set the migrated keyword totrue.I find the fact that the entire migrated state depends on a keyword set only once in
addedto be a little precarious, but nothing stands out to me initially at this not working.Summary of Completed Tests