Closure latest specification update and server changes #39760
Merged
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
There was a problem hiding this comment.
Code Review
This pull request updates the Closure cluster implementation to align with a new specification version. The changes primarily involve making the secureState field mandatory within the OverallCurrentStateStruct and adjusting conformance checks for various attributes and events accordingly. Most of the modifications are in auto-generated code files, which consistently reflect this specification update. The manual changes in the server logic appear correct and in line with the new requirements. I've identified one minor typo in a comment.
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
|
PR #39760: Size comparison from bce7d27 to b5768fd Full report (71 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from bce7d27 to e6f5a7a Full report (5 builds for cc32xx, stm32, tizen)
|
bzbarsky-apple
requested changes
Jun 30, 2025
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Show resolved
Hide resolved
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
AliTalebVelux
approved these changes
Jul 1, 2025
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Show resolved
Hide resolved
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
5 tasks
|
PR #39760: Size comparison from 299f5ed to e919980 Full report (66 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, linux, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from 5b51867 to 4f1b1ce Full report (63 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, linux, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from ddb2cbf to 2ebc132 Full report (45 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, nrfconnect, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from ddb2cbf to 6623e84 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from dd23712 to bb756c9 Full report (42 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from dd23712 to 4617059 Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
AliTalebVelux
approved these changes
Jul 10, 2025
mkardous-silabs
approved these changes
Jul 10, 2025
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Show resolved
Hide resolved
bzbarsky-apple
approved these changes
Jul 10, 2025
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-dimension-server/closure-dimension-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/closure-control-server/closure-control-cluster-logic.cpp
Outdated
Show resolved
Hide resolved
|
PR #39760: Size comparison from 777d399 to f39888e Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #39760: Size comparison from 7490ecc to 82ec8bd Full report (9 builds for cc13x4_26x4, cc32xx, stm32, tizen)
|
|
PR #39760: Size comparison from 7490ecc to 7e5c824 Full report (14 builds for cc13x4_26x4, cc32xx, nrfconnect, qpg, stm32, tizen)
|
|
PR #39760: Size comparison from 7490ecc to 4771b6d Full report (72 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
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
This PR implements the latest Closure Specification verison (44c5ab68e35160c155bb36b20cb08b62450850a4) and Provisional elements are avoided in XML with #ifdef based on PR https://github.com/CHIP-Specifications/connectedhomeip-spec/pull/11847
This PR contains XML code generated by ALchemy, ZAP generated code and server implemtnation changes.
Code changes :
this intially needed as we need this infor from application. Now LatchControlModes attribute give the data and
manual latch needed can be derived from spec " If the closure supports the MotionLatching (LT) feature, it SHALL either fulfill the latch order and update OverallTargetState.Latch or, if the ["LatchControlModes" attribute specify that manual intervention is required to latch, respond with INVALID_IN_STATE and remain in its current state.
this was intially needed for state transition validation. But after discussion the state transition diagram in specification is just for reference and should not be used for validation. so removing it
"
Testing
Build the closure app succesfully.
Ran the unit test cases for closure servers.
Note: Number of files are large due to XML and ZAP generated changes. Actual code changes are small.
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