Zigbee Lock: make code sync more robust (#3330)

* zigbee-lock: make credential code sync more robust

Sync the lock's PIN codes into the credentials/users tables more often
and more reliably, and recover from failures instead of stopping early.

- Start the sync from slot 0, and try it more often: on init (after a
  15s delay so NumberOfPINUsersSupported is known) and on refresh. The
  duplicate init sync in doConfigure is dropped.
- Move the sync checks into sync_device_state and continue_code_sync
  into lock_utils' sync section.
- Skip a sync while the device is busy, expiring stale busy state so a
  stalled sync can be retried, rather than clobbering a command in
  flight. Refresh waits 15s for updates to arrive.
- Persist the latest synced code index so a failed sync resumes from
  where it stopped, and ignore responses for any other index.
- Use a bump-able CODES_SYNC_VERSION: a persisted index started under a
  different version is cleared, which permits a re-sync when needed.
- Handle Default Responses during the sync. Default
  responses do not count towards the consecutive-unoccupied count that
  ends the sync.

Co-Authored-By: Claude Sonnet 5.5 <noreply@anthropic.com>

Author: hcarter-775

drivers/SmartThings/zigbee-lock/src/init.lua

@@ -46,6 +46,10 @@ function LockLifecycle.init(driver, device)
     -- We should check its PIN users if it should be re-profiled.
     device:send(clusters.DoorLock.attributes.NumberOfPINUsersSupported:read(device))
   end
+
+  -- retry sync on each driver init until it has been run to completion, in case of some kind of interruption (network, power, etc.)
+  -- ensure our user/credential state is accurate to the current device state
+  device.thread:call_with_delay(15, function() lock_utils.sync_device_state(device) end)
 end
 
 function LockLifecycle.do_configure(driver, device)
@@ -57,11 +61,6 @@ function LockLifecycle.do_configure(driver, device)
 
   device:send(device_management.build_bind_request(device, clusters.Alarms.ID, driver.environment_info.hub_zigbee_eui))
   device:send(clusters.Alarms.attributes.AlarmCount:configure_reporting(device, 0, 21600, 0))
-
-  if device:supports_capability(capabilities.lockCredentials) then
-    -- ensure our user/credential state is accurate to the current device state
-    device.thread:call_with_delay(15, function() lock_utils.sync_device_state(device) end)
-  end
 end
 
 function LockLifecycle.info_changed(driver, device, event, args)
@@ -100,6 +99,11 @@ local zigbee_lock_driver = {
     infoChanged = LockLifecycle.info_changed,
   },
   zigbee_handlers = {
+    global = {
+      [clusters.DoorLock.ID] = {
+        [0x0B] = zigbee_handlers.default_response, -- ZCL Default Response
+      }
+    },
     cluster = {
       [clusters.Alarms.ID] = {
         [clusters.Alarms.client.commands.Alarm.ID] = zigbee_handlers.alarm

drivers/SmartThings/zigbee-lock/src/lock_handlers/capabilities.lua

@@ -256,6 +256,9 @@ function CapabilityHandlers.refresh(driver, device, cmd)
        (device:get_latest_state("main", capabilities.lockUsers.ID, capabilities.lockUsers.totalUsersSupported.NAME) == nil) then
       device:send(clusters.DoorLock.attributes.NumberOfPINUsersSupported:read(device))
     end
+
+    -- Allow callers to try to sync device state- fails silently if the device is busy or already synced.
+    device.thread:call_with_delay(15, function() lock_utils.sync_device_state(device) end)
   elseif not device:supports_capability(capabilities.lockCodes) then
     -- Generically fingerprinted devices may support PINs even though the profile does not yet
     -- reflect it.  Read NumberOfPINUsersSupported so the response handler can trigger a profile

drivers/SmartThings/zigbee-lock/src/lock_handlers/zigbee_responses.lua

@@ -4,6 +4,7 @@
 local socket = require "cosock.socket"
 
 local clusters     = require "st.zigbee.zcl.clusters"
+local Status       = require "st.zigbee.generated.types.ZclStatus"
 local capabilities = require "st.capabilities"
 
 local consts     = require "lock_utils.constants"
@@ -141,27 +142,32 @@ function ZigbeeHandlers.get_pin_code_response(driver, device, zb_rx)
 
   if command_in_progress == consts.SYNC.CODES_FROM_LOCK then
     -- Only add entries for slots that are actually occupied on the lock.
-    local user_status = zb_rx.body.zcl_body.user_status.value
-    if user_status == clusters.DoorLock.types.DrlkUserStatus.OCCUPIED_ENABLED then
-      lock_utils.add_reported_credential(device, user_id)
-      -- reset the consecutive unoccupied codes counter since we found an occupied code
-      device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, 0)
-    else
-      local consecutive_unoccupied_codes = (device:get_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES) or 0) + 1
-      device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, consecutive_unoccupied_codes)
+    local occupied = zb_rx.body.zcl_body.user_status.value == clusters.DoorLock.types.DrlkUserStatus.OCCUPIED_ENABLED
+    if user_id < device:get_field(consts.SYNC.CODE_INDEX) then
+      device.log.warn_with({ hub_logs = true }, "GetPINCodeResponse for index %s already processed, expected %s", user_id, device:get_field(consts.SYNC.CODE_INDEX))
+      return -- return early if we've already processed this response.
+    elseif user_id ~= device:get_field(consts.SYNC.CODE_INDEX) then
+      device.log.warn_with({ hub_logs = true }, "Unexpected GetPINCodeResponse for index %s, expected %s", user_id, device:get_field(consts.SYNC.CODE_INDEX))
+      occupied = false -- treat this as an unoccupied slot to continue the sync
     end
+    lock_utils.continue_code_sync(device, occupied, false)
+  end
+end
 
-    if user_id >= tables.get_max_entries(device, "credentials") or
-     (device:get_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES) or 0) > 5 then
-      -- stop if we hit the max number of codes supported by the lock, or if we have received 5 consecutive unoccupied codes.
-      device:set_field(consts.SYNC.CODE_INDEX, nil)
-      lock_utils.clear_busy_state(device)
-    else
-      local synced_code_index = device:get_field(consts.SYNC.CODE_INDEX) + 1
-      device:set_field(consts.SYNC.CODE_INDEX, synced_code_index)
-      lock_utils.set_busy_state(device, consts.SYNC.CODES_FROM_LOCK, { checkingCode = synced_code_index })
-      device:send(clusters.DoorLock.server.commands.GetPINCode(device, synced_code_index))
-    end
+-- Handles a ZCL Default Response on the DoorLock cluster. During a code sync, a non-success Default Response
+-- to GetPINCode (e.g. the lock rejects an unsupported index) means no GetPINCodeResponse will arrive,
+-- so the request is retried once (in case the error was transient) and then the slot is treated as
+-- unoccupied, continuing to the next step rather than stalling the sync.
+function ZigbeeHandlers.default_response(driver, device, zb_rx)
+  local zcl_body = zb_rx.body.zcl_body
+  if device:get_field(consts.DRIVER_STATE.COMMAND_IN_PROGRESS) ~= consts.SYNC.CODES_FROM_LOCK then return end
+  if zcl_body.cmd.value ~= clusters.DoorLock.server.commands.GetPINCode.ID then return end
+  -- a SUCCESS default response is followed by the real GetPINCodeResponse
+  if zcl_body.status.value == Status.SUCCESS then
+    return
+  else  -- else, continue the code sync
+    device.log.warn_with({ hub_logs = true }, "Default Response for GetPINCode at index %s with status %s", device:get_field(consts.SYNC.CODE_INDEX), zcl_body.status.value)
+    lock_utils.continue_code_sync(device, false, true)
   end
 end
 

drivers/SmartThings/zigbee-lock/src/lock_utils/constants.lua

@@ -12,8 +12,12 @@ lock_constants.DRIVER_STATE = {
 
 lock_constants.SYNC = {
   CODES_FROM_LOCK = "syncCodesFromLock",
-  CODE_INDEX = "syncCodeIndex",
+  CODE_INDEX = "syncCodeIndex", -- persisted until codesSynced is set
+  CODE_INDEX_VERSION = "syncCodeIndexVersion", -- persisted; the CODES_SYNC_VERSION under which CODE_INDEX was started
+  RETRIED_CODE_INDEX = "syncRetriedCodeIndex", -- the code index whose GetPINCode request was already retried once
   CONSECUTIVE_UNOCCUPIED_CODES = "consecutiveUnoccupiedCodes",
+  CODES_SYNCED = "codesSynced", -- persisted; holds the CODES_SYNC_VERSION of the last completed code sync
+  CODES_SYNC_VERSION = 1, -- bump to force every device to re-sync its codes (a mismatch with codesSynced triggers it; with syncCodeIndexVersion, restarts from index 0)
 }
 
 lock_constants.COMMAND_RESULT = {

drivers/SmartThings/zigbee-lock/src/lock_utils/utils.lua

@@ -39,19 +39,68 @@ end
 -- [[ SYNC STATE MANAGEMENT ]] --
 
 function lock_utils.sync_device_state(device)
+  local capabilities = require "st.capabilities"
   local clusters = require "st.zigbee.zcl.clusters"
+
+  -- skip if unsupported, already synced, or busy (including a sync already in progress). is_device_busy
+  -- expires stale busy state, so a sync that stalled on a lost response can be retried.
+  if not (device:supports_capability(capabilities.lockCredentials) and
+    device:get_field(consts.SYNC.CODES_SYNCED) ~= consts.SYNC.CODES_SYNC_VERSION and
+    not lock_utils.is_device_busy(device)) then
+      return
+  end
+
+  -- a persisted index from a sync begun under a different sync version is stale; start over.
+  if device:get_field(consts.SYNC.CODE_INDEX_VERSION) ~= consts.SYNC.CODES_SYNC_VERSION then
+    device:set_field(consts.SYNC.CODE_INDEX, nil, { persist = true })
+  end
+
   -- Per spec, this attribute should be a boolean set to True if it is ok for the door lock server to send PINs over the air.
   device:send(clusters.DoorLock.attributes.SendPINOverTheAir:write(device, true))
 
   -- if consts.SYNC.CODE_INDEX is nil, we haven't started syncing codes from the lock yet,
-  -- so start the process from 1. note: the Zigbee "Master Code" is associated with index 0.
+  -- so start the process from 0. Persist the index so we can resume if the driver is restarted mid-sync.
   if (device:get_field(consts.SYNC.CODE_INDEX) == nil) then
-    device:set_field(consts.SYNC.CODE_INDEX, 1)
+    device:set_field(consts.SYNC.CODE_INDEX, 0, { persist = true })
+    device:set_field(consts.SYNC.CODE_INDEX_VERSION, consts.SYNC.CODES_SYNC_VERSION, { persist = true })
   end
   lock_utils.set_busy_state(device, consts.SYNC.CODES_FROM_LOCK)
   device:send(clusters.DoorLock.server.commands.GetPINCode(device, device:get_field(consts.SYNC.CODE_INDEX)))
 end
 
+-- Advance the sync-codes-from-lock flow after a response for the current code index (consts.SYNC.CODE_INDEX).
+-- `occupied` is true only when the lock reported an occupied slot at that index.
+-- `default_response` is true when the step was resolved by a Default Response: that is not treated as a
+-- failure, but it is not an occupied slot either, so it does not count toward the consecutive unoccupied codes.
+function lock_utils.continue_code_sync(device, occupied, default_response)
+  local clusters = require "st.zigbee.zcl.clusters"
+  local code_index = device:get_field(consts.SYNC.CODE_INDEX)
+  if code_index == nil then return end
+
+  if occupied then
+    lock_utils.add_reported_credential(device, code_index)
+    -- reset the consecutive unoccupied codes counter since we found an occupied code
+    device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, 0)
+  elseif not default_response then
+    device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, (device:get_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES) or 0) + 1)
+  end
+
+  if code_index >= tables.get_max_entries(device, "credentials") or
+   (device:get_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES) or 0) > 5 then
+    -- stop if we hit the max number of codes supported by the lock, or if we have received 5 consecutive unoccupied codes.
+    device:set_field(consts.SYNC.CODE_INDEX, nil, { persist = true })
+    device:set_field(consts.SYNC.CONSECUTIVE_UNOCCUPIED_CODES, nil)
+    device:set_field(consts.SYNC.RETRIED_CODE_INDEX, nil)
+    device:set_field(consts.SYNC.CODES_SYNCED, consts.SYNC.CODES_SYNC_VERSION, { persist = true })
+    lock_utils.clear_busy_state(device)
+  else
+    local next_index = code_index + 1
+    device:set_field(consts.SYNC.CODE_INDEX, next_index, { persist = true })
+    lock_utils.set_busy_state(device, consts.SYNC.CODES_FROM_LOCK)
+    device:send(clusters.DoorLock.server.commands.GetPINCode(device, next_index))
+  end
+end
+
 
 -- [[ CREDENTIAL TABLE MANAGEMENT ]] --
 
@@ -117,7 +166,7 @@ end
 -- is left in place
 function lock_utils.add_reported_credential(device, credential_index)
   if tables.find_entry(device, "credentials", credential_index) then return end
-  lock_utils.add_credential_and_user(device, {
+  return lock_utils.add_credential_and_user(device, {
     userIndex       = tables.next_index(device, "users"),
     credentialIndex = credential_index,
   }, true)

drivers/SmartThings/zigbee-lock/src/test/test_generic_fingerprint_profile_update.lua

@@ -34,6 +34,7 @@ test.register_coroutine_test(
     mock_device:expect_metadata_update({profile = "base-lock"})
 
     test.wait_for_events()
+    test.timer.__create_and_queue_test_time_advance_timer(15, "oneshot")
     test.socket.device_lifecycle:__queue_receive(mock_device:generate_info_changed({ profile = t_utils.get_profile_definition("base-lock.yml") }))
     test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCodes.migrated(true,  { visibility = { displayed = false } })))
     test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCredentials.supportedCredentials({"pin"},  { visibility = { displayed = false } })))
@@ -44,6 +45,11 @@ test.register_coroutine_test(
     test.socket.zigbee:__expect_send({ mock_device.id, DoorLock.attributes.MaxPINCodeLength:read(mock_device) })
     test.socket.zigbee:__expect_send({ mock_device.id, DoorLock.attributes.MinPINCodeLength:read(mock_device) })
     test.socket.zigbee:__expect_send({ mock_device.id, DoorLock.attributes.NumberOfPINUsersSupported:read(mock_device) })
+    test.wait_for_events()
+    -- the refresh handler delays the sync by 15 seconds
+    test.mock_time.advance_time(15)
+    test.socket.zigbee:__expect_send({ mock_device.id, DoorLock.attributes.SendPINOverTheAir:write(mock_device, true) })
+    test.socket.zigbee:__expect_send({ mock_device.id, DoorLock.server.commands.GetPINCode(mock_device, 0) })
   end,
   {
      min_api_version = 14