Update matter window covering edge driver - #3052
Conversation
571f3d8 to
efdf5b3
Compare
|
Edge driver development for R2 new feature (https://smartthings.atlassian.net/browse/CHAD-18661) |
c659987 to
f5b95e2
Compare
hcarter-775
left a comment
hcarter-775
left a comment
There was a problem hiding this comment.
Seems like the new fields may be unnecessary, and may be adding some unnecessary complexity
|
Duplicate profile check: Warning - duplicate profiles detected. |
|
Channel deleted. |
Test Results 73 files ±0 536 suites ±0 0s ⏱️ ±0s For more details on these errors, see this check. Results for commit 0b239eb. ± Comparison against base commit 60a0ce7. ♻️ This comment has been updated with latest results. |
|
Minimum allowed coverage is Generated by 🐒 cobertura-action against 0b239eb |
|
Hi, I just had the workflows run and there are some warnings showing up. Can you fix these warnings as well? Thanks! |
f5b95e2 to
ccbd180
Compare
accidental approval, unit tests and comments must still be addressed
ccbd180 to
734a936
Compare
|
The statelessWindowShadeLevelStep capability is newly introduced in version 62 (dev), so the driver tests on versions 59 to 61 have failed. |
734a936 to
d62b349
Compare
d62b349 to
f1bfc46
Compare
|
TC has been updated. Please recalculate the coverage |
f1bfc46 to
e53ee91
Compare
| test.register_coroutine_test( | ||
| "windowShade setShadeLevel with reverse polarity inverts level", function() | ||
| update_profile() | ||
| test.wait_for_events() | ||
| -- Set REVERSE_POLARITY field directly | ||
| local fields = require "sub_drivers.closure.closure_utils.fields" | ||
| mock_device:set_field(fields.REVERSE_POLARITY, true, {persist = true}) | ||
| test.socket.capability:__queue_receive({ | ||
| mock_device.id, | ||
| {capability = "windowShadeLevel", component = "windowShade1", command = "setShadeLevel", args = {75}}, | ||
| }) | ||
| -- With reverse=true, 75 should be inverted to 25 | ||
| test.socket.matter:__expect_send({ | ||
| mock_device.id, | ||
| clusters.ClosureDimension.server.commands.SetTarget(mock_device, 11, 25 * 100) | ||
| }) | ||
| end | ||
| ) |
There was a problem hiding this comment.
Let's add a test for this without reverse polarity.
There was a problem hiding this comment.
I see that this has not been taken care of. I think a test or two should be added in a follow-up PR for closures @hongye-samsung. Thanks
There was a problem hiding this comment.
The test for setShadeLevel without reverse polarity already exists (line 485-498 in test_matter_closure.lua). If you feel that test is not sufficient, we can add more in a follow-up PR.
There was a problem hiding this comment.
Ah, I see. Apologies, I think there was a miscommunication. I think we need some unit tests for stepShadeLevel for closures.
There was a problem hiding this comment.
we will add it in a follow-up PR.
Signed-off-by: Hong Ye/MDE Lab /SRC-Nanjing/Staff Engineer/Samsung Electronics <hong.ye@samsung.com>
e53ee91 to
0b239eb
Compare
|
We've fixed the issue and re-pushed the code. Please approve the check workflow. |
|
There are 2 failing tests here, un-related to this PR. I cannot rebase this PR over the outside fix, so I will merge this as-is, as the fix will be applied in the main branch. |
bf4fd79
into
SmartThingsCommunity:main
6 participants
Check all that apply
Type of Change
Checklist
Description of Change
Summary of Completed Tests
Co-Authored-By: Cline SR (50%)
CGAI & AI Review