diff --git a/drivers/SmartThings/zigbee-lock/src/legacy-handlers/init.lua b/drivers/SmartThings/zigbee-lock/src/legacy-handlers/init.lua index b9d5ce78ad..533260426f 100644 --- a/drivers/SmartThings/zigbee-lock/src/legacy-handlers/init.lua +++ b/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 }) diff --git a/drivers/SmartThings/zigbee-lock/src/lock_utils/tables.lua b/drivers/SmartThings/zigbee-lock/src/lock_utils/tables.lua index bfe3562344..52389f422b 100644 --- a/drivers/SmartThings/zigbee-lock/src/lock_utils/tables.lua +++ b/drivers/SmartThings/zigbee-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,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 diff --git a/drivers/SmartThings/zigbee-lock/src/test/test_lock_code_slga_migration.lua b/drivers/SmartThings/zigbee-lock/src/test/test_lock_code_slga_migration.lua index e29788fe36..3309f494d7 100644 --- a/drivers/SmartThings/zigbee-lock/src/test/test_lock_code_slga_migration.lua +++ b/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() \ No newline at end of file diff --git a/drivers/SmartThings/zigbee-lock/src/test/test_lock_credentials_commands.lua b/drivers/SmartThings/zigbee-lock/src/test/test_lock_credentials_commands.lua index 33293e784d..118f45c31c 100644 --- a/drivers/SmartThings/zigbee-lock/src/test/test_lock_credentials_commands.lua +++ b/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() diff --git a/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua b/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua index 3bcb8adfe4..ba477349ef 100644 --- a/drivers/SmartThings/zwave-lock/src/legacy-handlers/init.lua +++ b/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 } })) diff --git a/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua b/drivers/SmartThings/zwave-lock/src/lock_utils/tables.lua index bfe3562344..0d2ed4de4a 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,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 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..f6e9afd1e5 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,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() 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 5daee2191a..edb703c791 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()