Validate zigbee/zwave lock migraton indices

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

@@ -381,6 +381,7 @@ end
 
 local migrate = function(driver, device, command)
   local post_migration_consts = require "lock_utils.constants"
+  local table_utils = require "lock_utils.tables"
   local LockUsers = capabilities.lockUsers
   local LockCredentials = capabilities.lockCredentials
 
@@ -408,17 +409,32 @@ local migrate = function(driver, device, command)
   local users, credentials = {}, {}
   local cached_lock_codes = lock_utils.get_lock_codes(device)
   local ordered_code_slots = {}
+  local occupied_user_indices = {}
+  local min_user = table_utils.get_minimum(device, "users")
+  local min_credential = table_utils.get_minimum(device, "credentials")
   for code_slot_str, _ in pairs(cached_lock_codes) do
-    table.insert(ordered_code_slots, tonumber(code_slot_str))
+    local code_slot = tonumber(code_slot_str)
+    -- Slots that are not valid credential indices are dropped rather than migrated.
+    if math.type(code_slot) == "integer" and code_slot >= min_credential then
+      table.insert(ordered_code_slots, code_slot)
+      if code_slot >= min_user then occupied_user_indices[code_slot] = true end
+    end
   end
   table.sort(ordered_code_slots)
+  local next_free_user_index = min_user
   for _, code_slot in ipairs(ordered_code_slots) do
     local code_name = cached_lock_codes[tostring(code_slot)]
-    -- cached_lock_codes is formatted as { ["1"] = "Guest 1", ["2"] = "Guest 2" }, etc
-    if code_slot then
-      table.insert(users, { userIndex = code_slot, userType = "guest", userName = code_name })
-      table.insert(credentials, { userIndex = code_slot, credentialIndex = code_slot, credentialType = post_migration_consts.CRED_TYPE_PIN, credentialName = code_name })
+    -- A valid credential slot (e.g. 0) may not be a valid userIndex. Its user takes the lowest unused userIndex
+    -- without displacing any slot whose number is already a valid userIndex.
+    local user_index = code_slot
+    if code_slot < min_user then
+      while occupied_user_indices[next_free_user_index] do next_free_user_index = next_free_user_index + 1 end
+      user_index = next_free_user_index
+      occupied_user_indices[user_index] = true
     end
+    -- cached_lock_codes is formatted as { ["1"] = "Guest 1", ["2"] = "Guest 2" }, etc
+    table.insert(users, { userIndex = user_index, userType = "guest", userName = code_name })
+    table.insert(credentials, { userIndex = user_index, credentialIndex = code_slot, credentialType = post_migration_consts.CRED_TYPE_PIN, credentialName = code_name })
   end
   -- manually ensure user/cred state is persisted, then emit events to populate the new capabilities with the migrated data.
   device:set_field("persistedUsers", users, { persist = true })

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

@@ -15,13 +15,15 @@ local table_utils = {}
 --   match_key         Key used to identify entries in flat tables
 --   required_keys     Keys that must be non-nil when adding an entry
 --   persistent_field  device:set_field key used to back up the table across restarts
+--   minimum           Lowest valid match_key value; next_index() begins its search here
 --
 
 local DEFS = {
   users = {
     capability       = capabilities.lockUsers,
     attribute        = capabilities.lockUsers.users,
     max_entries      = capabilities.lockUsers.totalUsersSupported,
+    minimum          = 1,
     match_key        = "userIndex",
     required_keys    = {"userIndex", "userType"},
     persistent_field = "persistedUsers",
@@ -30,6 +32,7 @@ local DEFS = {
     capability       = capabilities.lockCredentials,
     attribute        = capabilities.lockCredentials.credentials,
     max_entries      = capabilities.lockCredentials.pinUsersSupported,
+    minimum          = 0,
     match_key        = "credentialIndex",
     required_keys    = {"userIndex", "credentialIndex", "credentialType"},
     persistent_field = "persistedCredentials",
@@ -128,19 +131,25 @@ function table_utils.find_all_entries_by(device, table_name, key, value)
   return matches
 end
 
--- Return the lowest positive integer not yet used as the match_key in the named table.
--- Used to auto-assign the next available slot for a new entry.
+-- Return the lowest integer, starting at the table's minimum, not yet used as the
+-- match_key in the named table. Used to auto-assign the next available slot for a new entry.
 function table_utils.next_index(device, table_name)
   local def = resolve_table_def(device, table_name)
   if not def then return 1 end
   local t = table_utils.get_state(device, def) or {}
   local occupied = {}
   for _, entry in ipairs(t) do occupied[entry[def.match_key]] = true end
-  local idx = 1
+  local idx = def.minimum
   while occupied[idx] do idx = idx + 1 end
   return idx
 end
 
+function table_utils.get_minimum(device, table_name)
+  local def = resolve_table_def(device, table_name)
+  if not def then return end
+  return def.minimum
+end
+
 function table_utils.get_max_entries(device, table_name)
   local def = resolve_table_def(device, table_name)
   if not def then return end

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

@@ -29,10 +29,23 @@ local mock_device = test.mock_device.build_test_zigbee_device(
     }
 )
 
+local zero_slot_mock_device = test.mock_device.build_test_zigbee_device(
+    {
+      profile = t_utils.get_profile_definition("base-lock.yml"),
+      data = {
+        lockCodes = json.encode({
+          ["0"] = "Zero",
+          ["1"] = "Zach"
+        }),
+      }
+    }
+)
+
 zigbee_test_utils.prepare_zigbee_env_info()
 local function test_init()
   test.disable_startup_messages()
   test.mock_device.add_test_device(mock_device)
+  test.mock_device.add_test_device(zero_slot_mock_device)
 end
 
 test.set_test_init_function(test_init)
@@ -73,4 +86,32 @@ test.register_coroutine_test(
     end
 )
 
+test.register_coroutine_test(
+    "Migrate keeps slot 0 as a credential with a valid, non-conflicting userIndex",
+    function()
+      test.socket.device_lifecycle:__queue_receive({ zero_slot_mock_device.id, "init" })
+      test.socket.zigbee:__queue_receive({ zero_slot_mock_device.id, DoorLock.attributes.MinPINCodeLength:build_test_attr_report(zero_slot_mock_device, 5) })
+      test.socket.zigbee:__queue_receive({ zero_slot_mock_device.id, DoorLock.attributes.MaxPINCodeLength:build_test_attr_report(zero_slot_mock_device, 10) })
+      test.socket.zigbee:__queue_receive({ zero_slot_mock_device.id, DoorLock.attributes.NumberOfPINUsersSupported:build_test_attr_report(zero_slot_mock_device, 4) })
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCodes.minCodeLength(5,  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCodes.maxCodeLength(10,  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCodes.maxCodes(4,  { visibility = { displayed = false } })))
+      test.wait_for_events()
+      test.socket.capability:__queue_receive({ zero_slot_mock_device.id, { capability = capabilities.lockCodes.ID, command = "migrate", args = {} } })
+      -- slot 0 is a valid credentialIndex but not a valid userIndex (minimum 1), so its user gets the next free userIndex, 2, without displacing slot 1.
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCredentials.supportedCredentials({"pin"},  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCredentials.minPinCodeLen(5,  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCredentials.maxPinCodeLen(10,  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCredentials.pinUsersSupported(4,  { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockUsers.totalUsersSupported(4, { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockUsers.users({{userIndex=2, userName="Zero", userType="guest"}, {userIndex=1, userName="Zach", userType="guest"}}, { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCredentials.credentials({{credentialIndex=0, credentialType="pin", userIndex=2, credentialName="Zero"}, {credentialIndex=1, credentialType="pin", userIndex=1, credentialName="Zach"}}, { visibility = { displayed = false } })))
+      test.socket.capability:__expect_send( zero_slot_mock_device:generate_test_message("main", capabilities.lockCodes.migrated(true,  { visibility = { displayed = false } })))
+      test.socket.zigbee:__expect_send({ zero_slot_mock_device.id, PowerConfiguration.attributes.BatteryPercentageRemaining:read(zero_slot_mock_device) })
+      test.socket.zigbee:__expect_send({ zero_slot_mock_device.id, DoorLock.attributes.LockState:read(zero_slot_mock_device) })
+      test.socket.zigbee:__expect_send({ zero_slot_mock_device.id, Alarm.attributes.AlarmCount:read(zero_slot_mock_device) })
+      test.wait_for_events()
+    end
+)
+
 test.run_registered_tests()

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

@@ -386,6 +386,83 @@ test.register_coroutine_test(
   end
 )
 
+test.register_coroutine_test(
+  "deleteCredential: removes a credential whose userIndex maps to no user",
+  function()
+    seed_credentials({ { userIndex = 7, credentialIndex = 1, credentialType = "pin" } })
+
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      { capability = capabilities.lockCredentials.ID, command = "deleteCredential",
+        args = { 1, "pin" } },
+    })
+
+    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.ClearPINCode(mock_device, 1) })
+    test.wait_for_events()
+
+    test.socket.zigbee:__queue_receive({
+      mock_device.id,
+      DoorLock.client.commands.ClearPINCodeResponse.build_test_rx(mock_device, ResponseStatus.PASS),
+    })
+
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message("main",
+        capabilities.lockCredentials.credentials({}, { visibility = { displayed = false } }))
+    )
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message("main",
+        capabilities.lockCredentials.commandResult(
+          { commandName = "deleteCredential", statusCode = "success", credentialIndex = 1, userIndex = 7 },
+          { state_change = true, visibility = { displayed = false } }
+        ))
+    )
+    test.wait_for_events()
+  end
+)
+
+test.register_coroutine_test(
+  "deleteCredential: removes a credential whose credentialIndex maps to no credential",
+  function()
+    -- the credential exists in state, but the lock has nothing stored in that slot
+    seed_users({ { userIndex = 3, userName = "Alice", userType = "guest" } })
+    seed_credentials({ { userIndex = 3, credentialIndex = 3, credentialType = "pin" } })
+
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      { capability = capabilities.lockCredentials.ID, command = "deleteCredential",
+        args = { 3, "pin" } },
+    })
+
+    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.ClearPINCode(mock_device, 3) })
+    test.wait_for_events()
+
+    -- clearing an empty slot succeeds on the lock, so the stale state entry should be removed
+    test.socket.zigbee:__queue_receive({
+      mock_device.id,
+      DoorLock.client.commands.ClearPINCodeResponse.build_test_rx(mock_device, ResponseStatus.PASS),
+    })
+
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message("main",
+        capabilities.lockCredentials.credentials({}, { visibility = { displayed = false } }))
+    )
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message("main",
+        capabilities.lockUsers.users({}, { visibility = { displayed = false } }))
+    )
+    test.socket.capability:__expect_send(
+      mock_device:generate_test_message("main",
+        capabilities.lockCredentials.commandResult(
+          { commandName = "deleteCredential", statusCode = "success", credentialIndex = 3, userIndex = 3 },
+          { state_change = true, visibility = { displayed = false } }
+        ))
+    )
+    test.wait_for_events()
+  end
+)
+
 test.register_coroutine_test(
   "deleteCredential: emits failure when the lock returns FAIL",
   function()

drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua

@@ -54,6 +54,7 @@ end
 --- @param cmd table
 local function migrate(driver, device, cmd)
   local LockCodesDefaults = require "st.zwave.defaults.lockCodes"
+  local table_utils = require "lock_utils.tables"
   local get_lock_codes = LockCodesDefaults.get_lock_codes
   local lock_users = {}
   local lock_credentials = {}
@@ -65,10 +66,17 @@ local function migrate(driver, device, cmd)
   end
 
   table.sort(ordered_codes)
-  for index = 1, #ordered_codes do
-    local code_slot, code_name = ordered_codes[index], lock_codes[ ordered_codes[index] ]
-    table.insert(lock_users, {userIndex = index, userType = "guest", userName = code_name})
-    table.insert(lock_credentials, {userIndex = index, credentialIndex = tonumber(code_slot), credentialType = "pin"})
+  -- Slots that are not valid credential indices (e.g. 0, which means "all users" in Z-Wave, or non-numeric)
+  -- are dropped rather than migrated.
+  local min_credential = table_utils.get_minimum(device, "credentials")
+  for _, code_slot in ipairs(ordered_codes) do
+    local credential_index = tonumber(code_slot)
+    if math.type(credential_index) == "integer" and credential_index >= min_credential then
+      local code_name = lock_codes[code_slot]
+      local user_index = #lock_users + 1
+      table.insert(lock_users, {userIndex = user_index, userType = "guest", userName = code_name})
+      table.insert(lock_credentials, {userIndex = user_index, credentialIndex = credential_index, credentialType = "pin"})
+    end
   end
   device:emit_event(capabilities.lockCredentials.credentials(lock_credentials, { visibility = { displayed = false } }))
   device:emit_event(capabilities.lockUsers.users(lock_users, { visibility = { displayed = false } }))

drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua

@@ -15,13 +15,15 @@ local table_utils = {}
 --   match_key         Key used to identify entries in flat tables
 --   required_keys     Keys that must be non-nil when adding an entry
 --   persistent_field  device:set_field key used to back up the table across restarts
+--   minimum           Lowest valid match_key value; next_index() begins its search here
 --
 
 local DEFS = {
   users = {
     capability       = capabilities.lockUsers,
     attribute        = capabilities.lockUsers.users,
     max_entries      = capabilities.lockUsers.totalUsersSupported,
+    minimum          = 1,
     match_key        = "userIndex",
     required_keys    = {"userIndex", "userType"},
     persistent_field = "persistedUsers",
@@ -30,6 +32,9 @@ local DEFS = {
     capability       = capabilities.lockCredentials,
     attribute        = capabilities.lockCredentials.credentials,
     max_entries      = capabilities.lockCredentials.pinUsersSupported,
+    -- Z-Wave User Identifiers are a sequence starting at 1; 0 is reserved to mean "all users".
+    -- This differs from the lockCredentials capability, which allows a credentialIndex of 0.
+    minimum          = 1,
     match_key        = "credentialIndex",
     required_keys    = {"userIndex", "credentialIndex", "credentialType"},
     persistent_field = "persistedCredentials",
@@ -128,19 +133,25 @@ function table_utils.find_all_entries_by(device, table_name, key, value)
   return matches
 end
 
--- Return the lowest positive integer not yet used as the match_key in the named table.
--- Used to auto-assign the next available slot for a new entry.
+-- Return the lowest integer, starting at the table's minimum, not yet used as the
+-- match_key in the named table. Used to auto-assign the next available slot for a new entry.
 function table_utils.next_index(device, table_name)
   local def = resolve_table_def(device, table_name)
   if not def then return 1 end
   local t = table_utils.get_state(device, def) or {}
   local occupied = {}
   for _, entry in ipairs(t) do occupied[entry[def.match_key]] = true end
-  local idx = 1
+  local idx = def.minimum
   while occupied[idx] do idx = idx + 1 end
   return idx
 end
 
+function table_utils.get_minimum(device, table_name)
+  local def = resolve_table_def(device, table_name)
+  if not def then return end
+  return def.minimum
+end
+
 function table_utils.get_max_entries(device, table_name)
   local def = resolve_table_def(device, table_name)
   if not def then return end

drivers/SmartThings/zwave-lock/src/test/test_lock_code_slga_migration.lua

@@ -98,6 +98,22 @@ test.register_coroutine_test(
   end
 )
 
+test.register_coroutine_test(
+  "Migrate drops slots with invalid credential indices",
+  function()
+    init_code_slot(0, "Invalid", mock_device)
+    init_code_slot(1, "Zach", mock_device)
+    test.socket.capability:__queue_receive({ mock_device.id, { capability = capabilities.lockCodes.ID, command = "migrate", args = {} } })
+    -- slot 0 is below the minimum credentialIndex for Z-Wave (1), so it is dropped
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCredentials.credentials({{credentialIndex=1, credentialType="pin", userIndex=1}}, { visibility = { displayed = false } })))
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockUsers.users({{userIndex=1, userName="Zach", userType="guest"}}, { visibility = { displayed = false } })))
+    test.socket.zwave:__expect_send(UserCode:UsersNumberGet({}):build_test_tx(mock_device.id))
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCredentials.supportedCredentials({"pin"},  { visibility = { displayed = false } })))
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCodes.migrated(true,  { visibility = { displayed = false } })))
+    test.wait_for_events()
+  end
+)
+
 test.register_coroutine_test(
     "Schlage-Lock device called 'migrate' command, validate codeLength is being properly set",
     function()