Updates to support the JF Datastore spec change (#12644)#71509
Updates to support the JF Datastore spec change (#12644)#71509vijs wants to merge 1 commit intoproject-chip:masterfrom
Conversation
github-actions
bot
added
examples
darwin
controller
app
and removed
review - pending
labels
pullapprove
bot
added
review - pending
examples
darwin
controller
app
and removed
examples
darwin
controller
app
labels
![gemini-code-assist[bot]](https://avatars.githubusercontent.com/in/956858?s=60&v=4){kind=small}
There was a problem hiding this comment.
Code Review
This pull request updates the Joint Fabric Datastore and Administrator cluster definitions, primarily adjusting field IDs, changing field nullability to optionality, and introducing a new status code enum for ICAC CSR responses. The changes also include corresponding updates to the SDK's generated code and server-side handlers. A critical issue was identified in the UpdateAdmin command handler where .Value() is called on Optional fields without checking for their presence, which will lead to application crashes if those fields are missing from the request.
| auto nodeId = commandData.nodeID; | ||
| auto friendlyName = commandData.friendlyName.Value(); | ||
| auto icac = commandData.icac.Value(); |
There was a problem hiding this comment.
The fields friendlyName and icac in UpdateAdminRequest have been changed from nullable to optional in the cluster specification. In the Matter SDK, optional fields are represented as chip::Optional, and calling .Value() on an empty Optional will trigger an assertion failure and crash the application. Since these fields are optional, the handler should pass the Optional objects directly to the UpdateAdmin method (which should be updated to accept them) instead of unconditionally extracting their values.
auto nodeId = commandData.nodeID;
auto friendlyName = commandData.friendlyName;
auto icac = commandData.icac;
|
PR #71509: Size comparison from 6cfa747 to a1edcc0 Full report (34 builds for bl602, bl616, 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 #71509 +/- ##
==========================================
- Coverage 54.32% 54.32% -0.01%
==========================================
Files 1577 1577
Lines 108257 108258 +1
Branches 13401 13402 +1
==========================================
Hits 58806 58806
- Misses 49451 49452 +1 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
1 participant
Summary
Updates to support the JF Datastore spec change (#12644)
Related issues
#71507
Testing
Verified using the below instructions.