Zwave lock: add explicit mins and migration validation

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

@@ -54,21 +54,30 @@ 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 = {}
   local lock_codes = get_lock_codes(device)
   local ordered_codes = {}
 
-  for code in pairs(lock_codes) do
-    table.insert(ordered_codes, code)
+  -- 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 pairs(lock_codes) do
+    local credential_index = tonumber(code_slot)
+    if math.type(credential_index) == "integer" and credential_index >= min_credential then
+      table.insert(ordered_codes, {slot = code_slot, index = credential_index})
+    else
+      device.log.warn(string.format("Dropping lock code slot %q during migration: not a valid credential index", tostring(code_slot)))
+    end
   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"})
+  table.sort(ordered_codes, function(a, b) return a.index < b.index end)
+  for _, code in ipairs(ordered_codes) do
+    local user_index = #lock_users + 1
+    table.insert(lock_users, {userIndex = user_index, userType = "guest", userName = lock_codes[code.slot]})
+    table.insert(lock_credentials, {userIndex = user_index, credentialIndex = code.index, credentialType = "pin"})
   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,23 @@ 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 def = assert(DEFS[table_name], string.format("table_helpers: unknown table %q", table_name))
   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 = assert(DEFS[table_name], string.format("table_helpers: unknown table %q", table_name))
+  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,39 @@ 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(
+  "Migrate compacts userIndex over gaps and sorts slots numerically",
+  function()
+    init_code_slot(0, "Invalid", mock_device)
+    init_code_slot(2, "Zach", mock_device)
+    init_code_slot(5, "Steven", mock_device)
+    init_code_slot(10, "Alex", mock_device)
+    test.socket.capability:__queue_receive({ mock_device.id, { capability = capabilities.lockCodes.ID, command = "migrate", args = {} } })
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockCredentials.credentials({{credentialIndex=2, credentialType="pin", userIndex=1}, {credentialIndex=5, credentialType="pin", userIndex=2}, {credentialIndex=10, credentialType="pin", userIndex=3}}, { visibility = { displayed = false } })))
+    test.socket.capability:__expect_send( mock_device:generate_test_message("main", capabilities.lockUsers.users({{userIndex=1, userName="Zach", userType="guest"}, {userIndex=2, userName="Steven", userType="guest"}, {userIndex=3, userName="Alex", 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()

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

@@ -368,6 +368,88 @@ test.register_coroutine_test(
   }
 )
 
+test.register_coroutine_test(
+  "deleteCredential: removes a credential whose userIndex maps to no user",
+  function()
+    -- seed via the persistent store, which the driver falls back to when the capability state cache is empty
+    mock_device:set_field("persistedCredentials",
+      { { userIndex = 7, credentialIndex = 1, credentialType = "pin" } }, { persist = true })
+
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      { capability = capabilities.lockCredentials.ID, command = "deleteCredential",
+        args = { 1, "pin" } },
+    })
+    test.socket.zwave:__expect_send(
+      UserCode:Set({ user_identifier = 1, user_id_status = UserCode.user_id_status.AVAILABLE }):build_test_tx(mock_device.id)
+    )
+    test.wait_for_events()
+
+    test.socket.zwave:__queue_receive({
+      mock_device.id,
+      UserCode:Report({ user_identifier = 1, user_id_status = UserCode.user_id_status.AVAILABLE }),
+    })
+    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,
+  {
+    min_api_version = 15
+  }
+)
+
+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 via the persistent store, which the driver falls back to when the capability state cache is empty
+    mock_device:set_field("persistedUsers",
+      { { userIndex = 3, userName = "Alice", userType = "guest" } }, { persist = true })
+    mock_device:set_field("persistedCredentials",
+      { { userIndex = 3, credentialIndex = 3, credentialType = "pin" } }, { persist = true })
+
+    test.socket.capability:__queue_receive({
+      mock_device.id,
+      { capability = capabilities.lockCredentials.ID, command = "deleteCredential",
+        args = { 3, "pin" } },
+    })
+    test.socket.zwave:__expect_send(
+      UserCode:Set({ user_identifier = 3, user_id_status = UserCode.user_id_status.AVAILABLE }):build_test_tx(mock_device.id)
+    )
+    test.wait_for_events()
+
+    -- the lock reports the slot as available, so the stale credential entry should be removed (the user is kept)
+    test.socket.zwave:__queue_receive({
+      mock_device.id,
+      UserCode:Report({ user_identifier = 3, user_id_status = UserCode.user_id_status.AVAILABLE }),
+    })
+    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 = 3, userIndex = 3 },
+          { state_change = true, visibility = { displayed = false } }
+        ))
+    )
+    test.wait_for_events()
+  end,
+  {
+    min_api_version = 15
+  }
+)
+
 test.register_coroutine_test(
   "deleteCredential: returns failure immediately when the credentialIndex does not exist in the table",
   function()