diff --git a/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua b/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua index 3bcb8adfe4..65cd6cca4b 100644 --- a/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua +++ b/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 } })) diff --git a/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua b/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua index bfe3562344..c6da0ac401 100644 --- a/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua +++ b/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua @@ -15,6 +15,7 @@ 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 = { @@ -22,6 +23,7 @@ local DEFS = { 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 diff --git a/drivers/SmartThings/zwave-lock/src/test/test_lock_code_slga_migration.lua b/drivers/SmartThings/zwave-lock/src/test/test_lock_code_slga_migration.lua index 21729e507d..a486f943e8 100644 --- a/drivers/SmartThings/zwave-lock/src/test/test_lock_code_slga_migration.lua +++ b/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() diff --git a/drivers/SmartThings/zwave-lock/src/test/test_lock_credentials_commands.lua b/drivers/SmartThings/zwave-lock/src/test/test_lock_credentials_commands.lua index 864162b29d..5d48a4d576 100644 --- a/drivers/SmartThings/zwave-lock/src/test/test_lock_credentials_commands.lua +++ b/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()