Wait for Max PIN length before generating credentials

This change fixes a bug where if the handler for `RequirePINforRemoteOperation`
runs before the `maxCodeLength` attribute of the `lockCodes` capability is set,
a default pin length of 4 would be used, which may be shorter than the minimum
pin length of the device specified by the `MinPINCodeLength` attribute.
Therefore, the read was removed from `init` and `added`, and was moved to the
handler for `MaxPINCodeLength` to guarantee that the max pin length is known
before attempting to generate the COTA credential.

Author: nickolas-deboom

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

@@ -143,6 +143,14 @@ end
 
 local function max_pin_code_len_handler(driver, device, ib, response)
   device:emit_event(capabilities.lockCodes.maxCodeLength(ib.data.value, {visibility = {displayed = false}}))
+  -- Device may require pin for remote operation if it supports COTA and PIN features.
+  local eps = device:get_endpoints(DoorLock.ID, {feature_bitmap = DoorLock.types.DoorLockFeature.CREDENTIALSOTA | DoorLock.types.DoorLockFeature.PIN_CREDENTIALS})
+  if #eps == 0 then
+    device.log.debug("Device will not require PIN for remote operation")
+    device:set_field(lock_utils.COTA_CRED, false, {persist = true})
+  else
+    device:send(DoorLock.attributes.RequirePINforRemoteOperation:read(device, eps[1]))
+  end
 end
 
 local function min_pin_code_len_handler(driver, device, ib, response)
@@ -598,22 +606,9 @@ local function do_configure(driver, device)
 end
 
 local function device_init(driver, device)
+  lock_utils.check_field_name_updates(device)
   device:set_component_to_endpoint_fn(component_to_endpoint)
   device:subscribe()
-
-  -- check if we have a missing COTA credential. Only run this if it has not been run before (i.e. in device added),
-  -- because there is a delay built into the COTA process and we do not want to start two COTA generations at the same time
-  -- in the event this was triggered on add.
-  if not device:get_field(lock_utils.COTA_READ_INITIALIZED) or not device:get_field(lock_utils.COTA_CRED) then
-    local eps = device:get_endpoints(DoorLock.ID, {feature_bitmap = DoorLock.types.DoorLockFeature.CREDENTIALSOTA | DoorLock.types.DoorLockFeature.PIN_CREDENTIALS})
-    if #eps == 0 then
-      device.log.debug("Device will not require PIN for remote operation")
-      device:set_field(lock_utils.COTA_CRED, false, {persist = true})
-    else
-      device:send(DoorLock.attributes.RequirePINforRemoteOperation:read(device, eps[1]))
-      device:set_field(lock_utils.COTA_READ_INITIALIZED, true, {persist = true})
-    end
-  end
 end
 
 local function device_added(driver, device)
@@ -645,16 +640,6 @@ local function device_added(driver, device)
       command = capabilities.lockCodes.commands.reloadAllCodes.NAME,
       args = {}
     })
-
-    --Device may require pin for remote operation if it supports COTA and PIN features.
-    eps = device:get_endpoints(DoorLock.ID, {feature_bitmap = DoorLock.types.DoorLockFeature.CREDENTIALSOTA | DoorLock.types.DoorLockFeature.PIN_CREDENTIALS})
-    if #eps == 0 then
-      device.log.debug("Device will not require PIN for remote operation")
-      device:set_field(lock_utils.COTA_CRED, false, {persist = true})
-    else
-      req:merge(DoorLock.attributes.RequirePINforRemoteOperation:read(device, eps[1]))
-      device:set_field(lock_utils.COTA_READ_INITIALIZED, true, {persist = true})
-    end
     device:send(req)
   end
 end

drivers/SmartThings/matter-lock/src/lock_utils.lua

@@ -25,7 +25,6 @@ local lock_utils = {
   COTA_CODE_NAME = "ST Remote Operation Code",
   COTA_CRED_INDEX = "cotaCredIndex",
   NONFUNCTIONAL = "nonFunctional",
-  COTA_READ_INITIALIZED = "cotaReadInitialized",
   BUSY_STATE = "busyState",
   COMMAND_NAME = "commandName",
   USER_NAME = "userName",
@@ -99,6 +98,21 @@ function lock_utils.code_deleted(device, code_slot)
   return lock_codes
 end
 
+local updated_fields = {
+  { current_field_name = "cotaReadInitialized", updated_field_name = nil }
+}
+
+function lock_utils.check_field_name_updates(device)
+  for _, field in ipairs(updated_fields) do
+    if device:get_field(field.current_field_name) then
+      if field.updated_field_name ~= nil then
+        device:set_field(field.updated_field_name, device:get_field(field.current_field_name), {persist = true})
+      end
+      device:set_field(field.current_field_name, nil)
+    end
+  end
+end
+
 --[[]]
 -- keys are the code slots that ST uses
 -- user_index and credential_index are used in the matter commands

drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua

@@ -70,8 +70,7 @@ local subscribed_attributes = {
   [capabilities.lockCredentials.ID] = {
     DoorLock.attributes.NumberOfPINUsersSupported,
     DoorLock.attributes.MaxPINCodeLength,
-    DoorLock.attributes.MinPINCodeLength,
-    DoorLock.attributes.RequirePINforRemoteOperation
+    DoorLock.attributes.MinPINCodeLength
   },
   [capabilities.lockSchedules.ID] = {
     DoorLock.attributes.NumberOfWeekDaySchedulesSupportedPerUser,
@@ -298,6 +297,14 @@ end
 -------------------------
 local function max_pin_code_len_handler(driver, device, ib, response)
   device:emit_event(capabilities.lockCredentials.maxPinCodeLen(ib.data.value, {visibility = {displayed = false}}))
+  -- Device may require pin for remote operation if it supports COTA and PIN features.
+  local eps = device:get_endpoints(DoorLock.ID, {feature_bitmap = DoorLock.types.DoorLockFeature.CREDENTIALSOTA | DoorLock.types.DoorLockFeature.PIN_CREDENTIALS})
+  if #eps == 0 then
+    device.log.debug("Device will not require PIN for remote operation")
+    device:set_field(lock_utils.COTA_CRED, false, {persist = true})
+  else
+    device:send(DoorLock.attributes.RequirePINforRemoteOperation:read(device, eps[1]))
+  end
 end
 
 --------------------------------------

drivers/SmartThings/matter-lock/src/test/test_aqara_matter_lock.lua

@@ -58,7 +58,6 @@ local function test_init()
   subscribe_request:merge(clusters.DoorLock.attributes.NumberOfPINUsersSupported:subscribe(mock_device))
   subscribe_request:merge(clusters.DoorLock.attributes.MaxPINCodeLength:subscribe(mock_device))
   subscribe_request:merge(clusters.DoorLock.attributes.MinPINCodeLength:subscribe(mock_device))
-  subscribe_request:merge(clusters.DoorLock.attributes.RequirePINforRemoteOperation:subscribe(mock_device))
   subscribe_request:merge(clusters.DoorLock.events.LockOperation:subscribe(mock_device))
   subscribe_request:merge(clusters.DoorLock.events.DoorLockAlarm:subscribe(mock_device))
   subscribe_request:merge(clusters.DoorLock.events.LockUserChange:subscribe(mock_device))

drivers/SmartThings/matter-lock/src/test/test_matter_lock_cota.lua

@@ -70,7 +70,6 @@ local function test_init()
   subscribe_request:merge(DoorLock.events.LockOperation:subscribe(mock_device))
   subscribe_request:merge(DoorLock.events.DoorLockAlarm:subscribe(mock_device))
   test.socket["matter"]:__expect_send({mock_device.id, subscribe_request})
-  test.socket.matter:__expect_send({mock_device.id, DoorLock.attributes.RequirePINforRemoteOperation:read(mock_device, 10)})
   test.mock_device.add_test_device(mock_device)
 end
 
@@ -108,8 +107,8 @@ local function expect_kick_off_cota_process(device)
   local req = DoorLock.attributes.MaxPINCodeLength:read(device, 10)
   req:merge(DoorLock.attributes.MinPINCodeLength:read(device, 10))
   req:merge(DoorLock.attributes.NumberOfPINUsersSupported:read(device, 10))
-  req:merge(DoorLock.attributes.RequirePINforRemoteOperation:read(device, 10))
   test.socket.matter:__expect_send({device.id, req})
+
   expect_reload_all_codes_messages(device)
   test.wait_for_events()
 
@@ -119,6 +118,19 @@ local function expect_kick_off_cota_process(device)
     DoorLock.attributes.NumberOfPINUsersSupported:build_test_report_data(device, 10, 16),
   })
 
+  test.wait_for_events()
+  test.socket.matter:__queue_receive({
+    device.id,
+    DoorLock.attributes.MaxPINCodeLength:build_test_report_data(device, 10, 8),
+  })
+  test.socket.capability:__expect_send(
+    device:generate_test_message(
+      "main",
+      capabilities.lockCodes.maxCodeLength(8, {visibility = {displayed = false}})
+    )
+  )
+  test.socket.matter:__expect_send({device.id, DoorLock.attributes.RequirePINforRemoteOperation:read(device, 10)})
+
   -- The creation of advance timers, advancing time, and waiting for events
   -- is done to ensure a correct order of operations and allow for all the
   -- `call_with_delay(0, ...)` calls to execute at the correct time.

drivers/SmartThings/matter-lock/src/test/test_new_matter_lock.lua

@@ -61,7 +61,6 @@ local function test_init()
   subscribe_request:merge(DoorLock.attributes.NumberOfPINUsersSupported:subscribe(mock_device))
   subscribe_request:merge(DoorLock.attributes.MaxPINCodeLength:subscribe(mock_device))
   subscribe_request:merge(DoorLock.attributes.MinPINCodeLength:subscribe(mock_device))
-  subscribe_request:merge(DoorLock.attributes.RequirePINforRemoteOperation:subscribe(mock_device))
   subscribe_request:merge(DoorLock.attributes.NumberOfWeekDaySchedulesSupportedPerUser:subscribe(mock_device))
   subscribe_request:merge(DoorLock.attributes.NumberOfYearDaySchedulesSupportedPerUser:subscribe(mock_device))
   subscribe_request:merge(DoorLock.events.LockOperation:subscribe(mock_device))
@@ -232,6 +231,7 @@ test.register_coroutine_test(
     test.socket.capability:__expect_send(
       mock_device:generate_test_message("main", capabilities.lockCredentials.maxPinCodeLen(8, {visibility = {displayed = false}}))
     )
+    test.socket.matter:__expect_send({mock_device.id, DoorLock.attributes.RequirePINforRemoteOperation:read(mock_device, 1)})
   end
 )
 

drivers/SmartThings/matter-lock/src/test/test_new_matter_lock_battery.lua

@@ -198,7 +198,6 @@ local function test_init_user_pin()
   subscribe_request:merge(DoorLock.attributes.NumberOfPINUsersSupported:subscribe(mock_device_user_pin))
   subscribe_request:merge(DoorLock.attributes.MaxPINCodeLength:subscribe(mock_device_user_pin))
   subscribe_request:merge(DoorLock.attributes.MinPINCodeLength:subscribe(mock_device_user_pin))
-  subscribe_request:merge(DoorLock.attributes.RequirePINforRemoteOperation:subscribe(mock_device_user_pin))
   subscribe_request:merge(DoorLock.events.LockOperation:subscribe(mock_device_user_pin))
   subscribe_request:merge(DoorLock.events.DoorLockAlarm:subscribe(mock_device_user_pin))
   subscribe_request:merge(DoorLock.events.LockUserChange:subscribe(mock_device_user_pin))
@@ -213,7 +212,6 @@ local function test_init_user_pin_schedule_unlatch()
   subscribe_request:merge(DoorLock.attributes.NumberOfPINUsersSupported:subscribe(mock_device_user_pin_schedule_unlatch))
   subscribe_request:merge(DoorLock.attributes.MaxPINCodeLength:subscribe(mock_device_user_pin_schedule_unlatch))
   subscribe_request:merge(DoorLock.attributes.MinPINCodeLength:subscribe(mock_device_user_pin_schedule_unlatch))
-  subscribe_request:merge(DoorLock.attributes.RequirePINforRemoteOperation:subscribe(mock_device_user_pin_schedule_unlatch))
   subscribe_request:merge(DoorLock.attributes.NumberOfWeekDaySchedulesSupportedPerUser:subscribe(mock_device_user_pin_schedule_unlatch))
   subscribe_request:merge(DoorLock.attributes.NumberOfYearDaySchedulesSupportedPerUser:subscribe(mock_device_user_pin_schedule_unlatch))
   subscribe_request:merge(DoorLock.events.LockOperation:subscribe(mock_device_user_pin_schedule_unlatch))