Thermostat ScheduleTypes Attribute: Add supporting delegate method #41160
+97
−16
Conversation
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
|
|
|
PR #41160: Size comparison from d1d4399 to 4aa48f8 Full report (6 builds for cc32xx, nrfconnect, realtek, stm32)
|
|
PR #41160: Size comparison from d1d4399 to 524aa54 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report❌ Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #41160 +/- ##
==========================================
- Coverage 50.94% 50.94% -0.01%
==========================================
Files 1378 1378
Lines 100698 100751 +53
Branches 13058 13070 +12
==========================================
+ Hits 51302 51326 +24
- Misses 49396 49425 +29 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #41160: Size comparison from d1d4399 to a34bf33 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41160: Size comparison from d1d4399 to 303f504 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41160: Size comparison from d1d4399 to 265e1b5 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
andy31415
reviewed
Sep 28, 2025
| callback attribute acceptedCommandList; | ||
| callback attribute attributeList; | ||
| ram attribute featureMap default = 0x123; | ||
| ram attribute featureMap default = 419; |
There was a problem hiding this comment.
maybe make this hex ... decimal for a bit set is even harder to figure out
There was a problem hiding this comment.
This file was generated by the zap scripts. Do I need to do anything special when using the zap tool?
There was a problem hiding this comment.
If I compare it with another example (air-purifier), FanControl featureMap is also written in decimal rather than hexadecimal.
connectedhomeip/examples/air-purifier-app/air-purifier-common/air-purifier-app.matter
Lines 3103 to 3124 in d1d4399
There was a problem hiding this comment.
In the end, should we use hexadecimal or decimal notation?
There was a problem hiding this comment.
This should be autogenerated by the zap_regen_all script and not hand edited (so whatever the tool does should be allowed).
There was a problem hiding this comment.
I ran scripts/tools/zap_regen_all.py
It produces the same decimal value in the thermostat.matter file
There was a problem hiding this comment.
When I use the ZAP tool to update the features, it puts a decimal value into the attributes value.
There was a problem hiding this comment.
I think you can edit the 'Default' value and @andy31415's comment was make this hex rather than decimal. Since Features are a bitmap in the spec it's easier to work out which features we're supporting.
There was a problem hiding this comment.
Using the ZAP tool, I set the Default value to 0x1A3
ram attribute featureMap default = 0x1A3;
I will have a look at updating the ZAP tool to put a hex value into this field for anyone else that runs into this problem.
|
PR #41160: Size comparison from d1d4399 to a444c47 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41160: Size comparison from d1d4399 to e8fede9 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41160: Size comparison from 1ceedce to c244b71 Full report (36 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nxp, psoc6, qpg, realtek, stm32, telink)
|
jamesharrow
reviewed
Oct 3, 2025
examples/thermostat/thermostat-common/src/thermostat-delegate-impl.cpp
Outdated
Show resolved
Hide resolved
|
PR #41160: Size comparison from 16876bc to 0df7671 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #41160: Size comparison from 1b2d06a to af7588f Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
jamesharrow
approved these changes
Oct 6, 2025
|
PR #41160: Size comparison from 7b16617 to 7a9c3a5 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
Is there anything else I need to do in order to move this PR along? |
In order for it to merge it needs 2 'approvers' (who are in a list of pull approvers) - and the CI needs to build cleanly, and all comments need to be resolved. It's best if the person who raised the comment clicks "Resolve comment" but sometimes they forget to come back and re-review. I've asked @andy31415 and @ReneJosefsen to take another look (click the circular arrows icon next to their name if you have that option). |
|
PR #41160: Size comparison from 5a37876 to c229301 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
lboue
reviewed
Oct 11, 2025
| callback attribute acceptedCommandList; | ||
| callback attribute attributeList; | ||
| ram attribute featureMap default = 0x123; | ||
| ram attribute featureMap default = 419; |
There was a problem hiding this comment.
make this hex rather than decimal:
Suggested change
| ram attribute featureMap default = 419; | |
| ram attribute featureMap default = 0x1a3; |
lboue
reviewed
Oct 11, 2025
| "singleton": 0, | ||
| "bounded": 0, | ||
| "defaultValue": "0x123", | ||
| "defaultValue": "419", |
There was a problem hiding this comment.
make this hex rather than decimal:
Suggested change
| "defaultValue": "419", | |
| "defaultValue": "0x1a3", |
|
PR #41160: Size comparison from 5a37876 to 1934547 Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
7 participants
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Summary
In this PR, I have added a new
GetScheduleTypeAtIndexfunction to theThermostatDelegate.I updated the Read function to invoke this callback for the ScheduleTypes attribute, replaced the default empty list.
This is my first PR to connectedhomeip, so all feedback is welcome! My goal is to implement the full Matter Schedule in a series of PRs.
Related issues
N/A
Testing
Added Python test: TC_TSTAT_4_4.py yamlNote
I'm would like to add python TC_TSTAT tests for this feature, but I think that's outside the scope of a non-CSA member.
Manual Testing
I used
matter-replto test the new attribute against the thermostat Linux exampleThis returned the new ScheduleTypes attribute, populated with the two values set in the example's delegate
Readability checklist
The checklist below will help the reviewer finish PR review in time and keep the
code readable:
descriptive
“When in Rome…”
rule (coding style)
See: Pull Request Guidelines