[CodeDriven] Convert Actions cluster to be code-driven#43471
[CodeDriven] Convert Actions cluster to be code-driven#43471mergify[bot] merged 11 commits intoproject-chip:masterfrom
Conversation
There was a problem hiding this comment.
Code Review
This pull request converts the Actions cluster to a code-driven model, introducing new ActionsCluster.h, ActionsCluster.cpp, and CodegenIntegration.cpp files, and refactoring existing actions-server.h and actions-server.cpp as a legacy facade. While this refactoring improves maintainability and testability, several security and stability issues were identified in the new ActionsCluster implementation. These include potential null pointer dereferences, a logic error in attribute reading, incompatible return types in InvokeCommand methods, and redundant checks in command handlers. Addressing these issues is crucial for the robustness and compliance of the cluster.
cfd1b02 to
775c7bb
Compare
|
PR #43471: Size comparison from ae4c1e9 to ac7ff51 Full report (11 builds for cc13x4_26x4, cc32xx, qpg, realtek, stm32)
|
ac7ff51 to
f42ba66
Compare
|
PR #43471: Size comparison from a92c842 to f42ba66 Full report (5 builds for cc32xx, realtek, stm32)
|
1bd336a to
16a0a48
Compare
|
PR #43471: Size comparison from a92c842 to 16a0a48 Full report (24 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32)
|
Codecov Report❌ Patch coverage is Additional details and impacted files@@ Coverage Diff @@
## master #43471 +/- ##
==========================================
+ Coverage 54.21% 54.26% +0.05%
==========================================
Files 1562 1563 +1
Lines 107371 107369 -2
Branches 13311 13316 +5
==========================================
+ Hits 58212 58266 +54
+ Misses 49159 49103 -56 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
|
PR #43471: Size comparison from a92c842 to 7066dd2 Full report (24 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, nxp, psoc6, qpg, realtek, stm32)
|
|
PR #43471: Size comparison from a92c842 to c066589 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
c066589 to
5d09b69
Compare
|
PR #43471: Size comparison from a94849f to 5d09b69 Full report (10 builds for cc13x4_26x4, cc32xx, nrfconnect, realtek, stm32)
|
|
PR #43471: Size comparison from a94849f to 93ddef9 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
93ddef9 to
aaa2300
Compare
|
PR #43471: Size comparison from a94849f to 2c7a59d Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
|
PR #43471: Size comparison from 1edbe71 to 2396b9c Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
Get rid of unnecessary checks Move private functions to be helper functions Cleanup code
- Rename OnStateChanged/OnActionFailed to GenerateEvent overloads taking
the event struct directly, reducing parameter count and matching the
underlying eventsGenerator API.
- Wrap ActionsCluster.cpp in namespace chip::app::Clusters to reduce
using-namespace boilerplate.
- Replace DispatchActionCommand template with inline decode/validate/
dispatch logic in InvokeCommand, as the template generalization added
complexity without benefit.
- Move BuildOptionalAttributes and BuildSetupURL (renamed ReadSetupURL)
to anonymous-namespace free functions in CodegenIntegration.cpp;
replace the fixed-size char[] buffer with std::string.
- Add singleton guard (sInstanceCount) to ActionsServer: the Actions
cluster is Node-scoped per the Matter spec and must live on a single
aggregator endpoint. Log an error on double-instantiation.
- Guard ActionListModified/EndpointListModified against calls for a
mismatched endpoint.
- Make Init/Shutdown idempotent via mRegistered flag to prevent spurious
error logs when the destructor calls Shutdown after an explicit call.
- Add README.md documenting the Node-scope constraint, per-endpoint
2396b9c to
9892390
Compare
|
PR #43471: Size comparison from af89497 to 9892390 Full report (34 builds for bl602, bl616, bl702, bl702l, cc13x4_26x4, cc32xx, efr32, esp32, nrfconnect, nxp, psoc6, qpg, realtek, stm32, telink)
|
…43471) * Move Actions cluster to be code driven * Linter error fix * Fix includes and dependencies * Refactor ActionsCluster Get rid of unnecessary checks Move private functions to be helper functions Cleanup code * Read SetURL from ember * Add docs, fix test build * Restyled by clang-format * Actions cluster: address code review feedback - Rename OnStateChanged/OnActionFailed to GenerateEvent overloads taking the event struct directly, reducing parameter count and matching the underlying eventsGenerator API. - Wrap ActionsCluster.cpp in namespace chip::app::Clusters to reduce using-namespace boilerplate. - Replace DispatchActionCommand template with inline decode/validate/ dispatch logic in InvokeCommand, as the template generalization added complexity without benefit. - Move BuildOptionalAttributes and BuildSetupURL (renamed ReadSetupURL) to anonymous-namespace free functions in CodegenIntegration.cpp; replace the fixed-size char[] buffer with std::string. - Add singleton guard (sInstanceCount) to ActionsServer: the Actions cluster is Node-scoped per the Matter spec and must live on a single aggregator endpoint. Log an error on double-instantiation. - Guard ActionListModified/EndpointListModified against calls for a mismatched endpoint. - Make Init/Shutdown idempotent via mRegistered flag to prevent spurious error logs when the destructor calls Shutdown after an explicit call. - Add README.md documenting the Node-scope constraint, per-endpoint * Revisit the comments of the cluster, separate the backward compatability tests * Enable and fix backwards-compatibility tests * Move the backward compatability tests to the same definintion --------- Co-authored-by: Restyled.io <commits@restyled.io>
Summary
Migrate
Actionscluster to be Code-DrivenRelated issues
43162
Testing
Tested using chip-all-clusters-app and matter-repl.
Added unit tests.
See: Pull Request Guidelines