Feature/support keypair generation

[HunsupJung]

Type of Change

• WWST Certification Request
  • If this is your first time contributing code:
    • I have reviewed the README.md file
    • I have reviewed the CODE_OF_CONDUCT.md file
    • I have signed the CLA
  • I plan on entering a WWST Certification Request or have entered a request through the WWST Certification console at developer.smartthings.com
• Bug fix
• New feature
• Refactor

Checklist

• I have performed a self-review of my code
• I have commented my code in hard-to-understand areas
• I have verified my changes by testing with a device or have communicated a plan for testing
• I am adding new behavior, such as adding a sub-driver, and have added and run new unit tests to cover the new behavior

Description of Change

• https://smartthings.atlassian.net/browse/REQ-23925
• https://smartthings.atlassian.net/wiki/spaces/STHK/pages/4707090966/Support+Keypair+generation

Summary of Completed Tests

[github-actions]

Invitation URL:
https://bestow-regional.api.smartthings.com/invite/adMKXL5K4J29

[github-actions]

Test Results

   72 files    493 suites   0s ⏱️
2 699 tests 2 699 ✅ 0 💤 0 ❌
4 560 runs  4 560 ✅ 0 💤 0 ❌

Results for commit 8da3bdc.

♻️ This comment has been updated with latest results.

[HunsupJung]

@tpmanley
I entered the Git command incorrectly and the PR was closed, but I couldn't open it again to see if I didn't have the authority, so I made a new one. I'm sorry to bother you.

[github-actions]

File	Coverage
All files	80%	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/new-matter-lock/init.lua	77%	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/new-matter-lock/can_handle.lua	90%	✅
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/lazy_load_subdriver.lua	57%	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/lock_utils.lua	68%	❌
/home/runner/work/SmartThingsEdgeDrivers/SmartThingsEdgeDrivers/drivers/SmartThings/matter-lock/src/init.lua	92%	✅

Minimum allowed coverage is 90%

Generated by 🐒 cobertura-action against 8da3bdc

[HunsupJung]

@tpmanley

Thanks for the making the requested changes @HunsupJung . The driver code is looking pretty good to me now. In addition to what @ctowns suggested, can you also take a look at the unit tests failures reported by CI and try to add tests for the new code?

First of all, I corrected all the errors.

[HunsupJung]

@ctowns
if test code uses lock-modular, the following message appears as if lockAliro capability is not supported, making it impossible to write test code. What do you suggest I do?

INFO  || <MatterDevice: 00000000-1111-2222-3333-000000000001 [nil]> received InteractionResponse: <InteractionResponse || type: REPORT_DATA, response_blocks: [<InteractionResponseInfoBlock || status: SUCCESS, <InteractionInfoBlock || endpoint: 0x01, cluster: DoorLock, attribute: AliroReaderVerificationKey, data: OctetString1: "\x04\xA9\xCB\xE4\x18\xEB\x09\x66\x16\x43\xE2\xA4\xA8\x46\xB8\xED\xFE\x27\x86\x98\x30\x2E\x9F\xB4\x3E\x9B\xFF\xD3\xE3\x10\xCC\x2C\x2C\x7F\xF4\x02\xE0\x6E\x40\xEA\x3C\xE1\x29\x43\x52\x73\x36\x68\x3F\xC5\xB1\xCB\x0C\x6A\x7C\x3F\x0B\x5A\xFF\x78\x35\xDF\x21\xC6\x24">>]>
TRACE || Found MatterMessageDispatcher handler in matter-lock -> New Matter Lock Handler
WARN  || Attempted to generate event for 00000000-1111-2222-3333-000000000001.main but it does not support capability Lock Aliro

[hcarter-775]

@HunsupJung when you're setting up the test, you can make a mock device with the following profile
profile = t_utils.get_profile_definition("lock-modular.yml", {enabled_optional_capabilities = {{"main", {"lockAliro"}}}}),

Then run the test as normal. This will make the tested profile include the optional capability. You can add as many optional capabilities as you want in this way.

[HunsupJung]

@tpmanley
I completed update. Could you review it again?

[tpmanley]

Thanks for resolving my two comments. There are a couple of conflicts right now so what I'd recommend is squash and rebase your changes into a single commit on top of latest main branch and then we can give it hopefully the last final review.

[HunsupJung]

@tpmanley
I did squash and rebase the commits.

[hcarter-775]

@gharveymn it doesn't seem like we have any way to represent this naturally in the generated clusters. Do you have any context for why? And do you have any recomendations on how to integrate it in the driver "naturally"?

I'm thinking we may want to handle it as:

Suggested change

	local DoorLockFeatureMapAttr = {ID = 0xFFFC, cluster = DoorLock.ID}
	local DoorLock.attributes.FeatureMap = {
	ID = 0xFFFC,
	NAME = "FeatureMap",
	_cluster = require "st.matter.clusters.DoorLock",
	base_type = require "st.matter.clusters.DoorLock.types.Feature",
	}

I'm mainly thinking of 1. whether/how we'd subscribe to this, and 2. whether we can make unit tests for it.

[gharveymn]

Not really too much context. I wasn't involved in the PRs which did the filtering. We also filter two other attributes: GeneratedCommandList and ClusterRevision, but those make sense since we don't act as a server in Lua-land.

I don't really see a reason not to add cluster-specific attribute definitions in scripting-engine for each of these where the base type is types.Feature. You may run into an issue where the Feature type doesn't exist for certain clusters, but you can just work around that by making the base type U32 or something.

The way you proposed would be a start, but I think you would be missing certain functions which would be a blocker for (1) and (2). Perhaps you can make some adjustments to the ZAP generation for scripting-engine locally, and then export the attribute definition as an embedded cluster attribute for now.

[hcarter-775]

@HunsupJung to integrate this more cleanly, I think we should add the following file DoorLock/server/attributes/FeatureMap.lua for the embedded cluster definition:

local cluster_base = require "st.matter.cluster_base"
local data_types = require "st.matter.data_types"
local TLVParser = require "st.matter.TLV.TLVParser"

local FeatureMap = {
  ID = 0xFFFC,
  NAME = "FeatureMap",
  _cluster = require "st.matter.clusters.DoorLock",
  base_type = require "st.matter.clusters.DoorLock.types.Feature",
}

function FeatureMap:augment_type(data_type_obj)
  for i, v in ipairs(data_type_obj.elements) do
    data_type_obj.elements[i] = data_types.validate_or_build_type(v, FeatureMap.element_type)
  end
end

function FeatureMap:new_value(...)
  local o = self.base_type(table.unpack({...}))

  return o
end

function FeatureMap:read(device, endpoint_id)
  return cluster_base.read(
    device,
    endpoint_id,
    self._cluster.ID,
    self.ID,
    nil
  )
end

function FeatureMap:subscribe(device, endpoint_id)
  return cluster_base.subscribe(
    device,
    endpoint_id,
    self._cluster.ID,
    self.ID,
    nil
  )
end

function FeatureMap:set_parent_cluster(cluster)
  self._cluster = cluster
  return self
end

function FeatureMap:build_test_report_data(
  device,
  endpoint_id,
  value,
  status
)
  local data = data_types.validate_or_build_type(value, self.base_type)

  return cluster_base.build_test_report_data(
    device,
    endpoint_id,
    self._cluster.ID,
    self.ID,
    data,
    status
  )
end

function FeatureMap:deserialize(tlv_buf)
  local data = TLVParser.decode_tlv(tlv_buf)

  return data
end

setmetatable(FeatureMap, {__call = FeatureMap.new_value, __index = FeatureMap.base_type})
return FeatureMap

Then we can maybe, at the top of the file, update the version check to something like:

if version.api < 10 then
  clusters.DoorLock = require "DoorLock"
else
  clusters.DoorLock.attributes.FeatureMap = require "DoorLock.server.attributes.FeatureMap"
end

Besides making this "cleaner", I think we should create unit tests for this newly introduced logic, and we will need this to create proper unit tests.

[hcarter-775]

This will not work currently, since it will no longer check for profiling data (that function is done at the start of match_profile). We should either add a parameter to match_profile called modular_only or something, or just move that profiling data function into the sub match profile functions.

[HunsupJung]

It does not need to read PowerSource cluster again since it is only related to the DoorLock cluster. (I mean that match_profile can be used here.)

If it needs to check DPS feature, I will add it in DPS PR.

[hcarter-775]

I still think we need this profiling_data check as a safety precaution.

Since we will subscribe to this attribute and the power source attribute list attribute when a device is first onboarded, it is possible that this handler will run before the power source attribute list handler. We cannot guarantee that ordering. This may cause 2 profile updates in a short period, which causes significant latency, so we should avoid this.

[HunsupJung]

It makes sense. I will consider this part and modify.

[hcarter-775]

This line will only affect the in-memory version of the st_store. This means that every time the cloud syncs with the driver, this will be overwritten with the previous data that was added on device creation.

I do not think that there is any built-in way to update this info in the cloud today, so this is an error-prone solution. I suggest that we save a separate field called

local latestDoorLockFeatureMap = "__latest_door_lock_feature_map"

or something like that.

Then we can do this:

Suggested change

	ep_cluster.feature_map = ib.data.value
	set_field_for_endpoint(device, latestDoorLockFeatureMap, device_ep.endpoint_id, ib.data.value, { persist = true })

where we take these api from Matter Switch: set_field_for_endpoint and get_field_for_endpoint.

Then, in match_profile_modular we should check whether this table has been initialized, and if it has, replace the in-memory version like you do here. Something like:

        local clus_has_feature = function(feature_bitmap)
          return DoorLock.are_features_supported(feature_bitmap, get_field_for_endpoint(device, latestDoorLockFeatureMap, device_ep.endpoint_id) or ep_cluster.feature_map)
        end

in match_profile_modular.

[HunsupJung]

I modified it.

[hcarter-775]

To support the changes discussed here, I think we have to modify this.

Suggested change

	local clus_has_feature = function(feature_bitmap)
	return DoorLock.are_features_supported(feature_bitmap, lock_utils.get_field_for_endpoint(device, latestDoorLockFeatureMap, device_ep.endpoint_id) or ep_cluster.feature_map)
	end

[hcarter-775]

Since this is not explicitly related to the lock capability, I think we should just add the line in device_init:

device:add_subscribed_attribute(DoorLockFeatureMapAttr)