-
Notifications
You must be signed in to change notification settings - Fork 2.3k
Generate ICDManagement cluster with Alchemy #41353
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
Conversation
|
PR #41353: Size comparison from e03e558 to 722d17b Full report (37 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #41353 +/- ##
=======================================
Coverage 50.96% 50.96%
=======================================
Files 1378 1378
Lines 100598 100598
Branches 13023 13023
=======================================
Hits 51268 51268
Misses 49330 49330 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
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.
Pull Request Overview
This PR generates the ICDManagement cluster using Alchemy, the automated code generation tool. The changes primarily involve formatting updates, improved descriptions, and the addition of some missing attributes like max values and IDs for command arguments.
Key changes:
- Updated command descriptions to be more descriptive and clear
- Added "id" attributes to command arguments in XML definition
- Removed redundant
mandatoryConformelements and simplified conformance rules - Changed StayActiveRequest command privilege from
managetooperatein metadata
Reviewed Changes
Copilot reviewed 29 out of 29 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
zzz_generated/app-common/clusters/IcdManagement/Metadata.h |
Changed StayActiveRequest command privilege from manage to operate |
src/darwin/Framework/CHIP/zap-generated/MTRBaseClusters.h |
Updated command descriptions to be more detailed and clear |
src/controller/data_model/controller-clusters.matter |
Updated command descriptions and removed access privilege for StayActiveRequest |
src/app/zap-templates/zcl/data-model/chip/icd-management-cluster.xml |
Major Alchemy regeneration with formatting, added IDs, max values, and improved descriptions |
| Multiple example app .matter files | Updated command descriptions to match the new Alchemy-generated versions |
.github/workflows/tests.yaml |
Added icd-management-cluster.xml to the list of files checked in tests |
|
@tersal As a nit, i don't believe the all-cluster-app has the icd management cluster as part of its configuration. |
Yes, I checked and most likely not in the all-clusters app, luckily it seems that the lit-icd-app was built and a couple of ICDM tests also ran during the CI checks so I assume that nothing breaks there. I usually use the all clusters app as a first "sanity check" (even if no actual code was changed) but probably I should look for an example more carefully when doing this. |
|
Approved: StayActive moved from |
Summary
Alchemy generation of ICDManagement cluster.
mandatoryConformremoved when redundant.Related issues
Part of #37391
Testing
Compiled all-clusters app and ran unit tests. CI checks should check backwards compatibility and other examples.