-
Notifications
You must be signed in to change notification settings - Fork 484
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Update smoke co alarm fault support #2045
base: main
Are you sure you want to change the base?
Conversation
This change adds support for the battery percentage for devices that support the BatPercentRemaining attribute from the PowerSource cluster in addition to the battery level from the SmokeCoAlarm cluster.
Use battery if supported by the device, otherwise use batteryLevel if the percentage is not supported.
Duplicate profile check: Passed - no duplicate profiles detected. |
Test Results 66 files 421 suites 0s ⏱️ For more details on these failures, see this check. Results for commit d334cd0. ♻️ This comment has been updated with latest results. |
Minimum allowed coverage is Generated by 🐒 cobertura-action against d334cd0 |
Invitation URL: |
@@ -81,7 +83,6 @@ local function test_init() | |||
end | |||
test.socket.matter:__expect_send({mock_device.id, subscribe_request}) | |||
test.mock_device.add_test_device(mock_device) | |||
mock_device:expect_metadata_update({ profile = "smoke-co-temp-humidity-comeas" }) | |||
end | |||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Could you add UT(s) that test the hardwareFault capability being set by BatteryAlert?
if #battery_feature_eps > 0 then | ||
device:send(clusters.PowerSource.attributes.AttributeList:read()) | ||
else | ||
match_profile(device, battery_support.NO_BATTERY) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should we create profiles that exclude the battery capabilities? The way it is now, there will be no change in behavior if the battery feature is not supported, but that would mean that the batteryLevel capability would still be included in the profile (and would show up as blank), which isn't a huge deal but we could just create some -nobattery profiles
Description of Change
Support the SmokeCOAlarm attribute BatteryAlert through the hardwareFault capability. Support the batteryLevel capability with the PowerSource attribute BatChargeLevel, and include battery capability support with the PowerSource attribute BatPercentRemaining.
Summary of Completed Tests