Matter Lock: Move AttributeList Read to Subscribe#2828
Matter Lock: Move AttributeList Read to Subscribe#2828hcarter-775 wants to merge 3 commits intomainfrom
Conversation
hcarter-775
requested review from
HunsupJung,
ctowns,
greens and
nickolas-deboom
|
Invitation URL: |
Test Results 72 files 489 suites 0s ⏱️ Results for commit 5c1241d. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 5c1241d |
| end | ||
| device:set_field(MODULAR_PROFILE_UPDATED, nil) | ||
| if device:get_field(profiling_data.BATTERY_SUPPORT) == nil then | ||
| device:add_subscribed_attribute(clusters.PowerSource.attributes.AttributeList) |
There was a problem hiding this comment.
Don't we need to check whether the PowerSource cluster is supported or not?
There was a problem hiding this comment.
The lines here occur in info_changed. The device_init event will always run before this when a device is created or a driver starts up, and in device_init we have this logic:
if #device:get_endpoints(clusters.PowerSource.ID, {feature_bitmap = clusters.PowerSource.types.PowerSourceFeature.BATTERY}) == 0 then
device:set_field(profiling_data.BATTERY_SUPPORT, battery_support.NO_BATTERY, {persist = true})
end
doing that check. Therefore, the only way BATTERY_SUPPORT can be nil her is if the PowerSource cluster with the FeatureMap we care about is supported on the device.
There was a problem hiding this comment.
The lines here occur in info_changed. The device_init event will always run before this when a device is created or a driver starts up, and in device_init we have this logic:
If so, why we need to call same logic below? the logic is already in the device_init
if device:get_field(profiling_data.BATTERY_SUPPORT) == nil then
device:add_subscribed_attribute(clusters.PowerSource.attributes.AttributeList)
end
There was a problem hiding this comment.
I think I added this as a kind of safety check, but you're right, it is not required in the current implementation. The AttributeList will always be added to the subscribed_attributes table used in subscribe in init if the nil condition is met by the time infoChanged would occur.
| end | ||
| if device:get_field(profiling_data.BATTERY_SUPPORT) == nil then |
There was a problem hiding this comment.
| end | |
| if device:get_field(profiling_data.BATTERY_SUPPORT) == nil then | |
| elseif device:get_field(profiling_data.BATTERY_SUPPORT) == nil then |
3 participants
Description of Change
For more consistent behavior, move the AttributeList read for Power Source to the subscritption block in the New Matter Lock subdriver. Follows the Matter Switch work done here.
Summary of Completed Tests
Unit tests are comprehensive. 5 onboardings completed successfully with VDA device (to ensure consistent behavior).