Added implementations for the JF Administrator and Datastore Servers #38842
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
doru91
reviewed
May 9, 2025
doru91
reviewed
May 9, 2025
doru91
reviewed
May 9, 2025
|
Please check my other comments for specific asks. General comment is that this is good work and probably can be merged as a basis for future JF DS work. At this moment probably it implements 10% of the JF DS text. |
chrisdecenzo
approved these changes
May 9, 2025
src/app/clusters/joint-fabric-datastore-server/joint-fabric-datastore-server.cpp
Outdated
Show resolved
Hide resolved
src/app/clusters/joint-fabric-datastore-server/joint-fabric-datastore-server.cpp
Outdated
Show resolved
Hide resolved
|
Please update per comments and resolve comments for approval. |
doru91
reviewed
May 13, 2025
src/app/clusters/joint-fabric-administrator-server/joint-fabric-administrator-server.cpp
Outdated
Show resolved
Hide resolved
vijs
force-pushed
the
feature/38841
branch
2 times, most recently
from
bce7641 to
5e86f37
Compare
|
PR #38842: Size comparison from f4d171a to 5e86f37 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38842: Size comparison from ebe16b7 to 1d047f7 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38842: Size comparison from 3388b3e to c7a4b41 Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
|
PR #38842: Size comparison from d6b2cee to 9c1da9e Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
doru91
reviewed
May 14, 2025
doru91
reviewed
May 14, 2025
|
PR #38842: Size comparison from 162df09 to b7069ac Full report (73 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
doru91
approved these changes
May 15, 2025
doru91
reviewed
May 15, 2025
heinsaar
pushed a commit
to heinsaar/connectedhomeip
that referenced
this pull request
May 15, 2025
This was referenced May 20, 2025
|
This PR should have had unit tests. |
andy31415
reviewed
May 22, 2025
| char mFriendlyNameBuffer[kFriendlyNameMaxSize]; | ||
| }; | ||
|
|
||
| class JointFabricDatastore |
There was a problem hiding this comment.
need documentation on usage/purpose.
Iw as looking to understand the include changes that claimed "not intended for embedded devices" ... but I don't have the intent of this class either. So what is it and what does it do?
andy31415
reviewed
May 22, 2025
| CHIP_ERROR SetAdministratorFabricIndex(FabricIndex fabricIndex) | ||
| { | ||
| mAdministratorFabricIndex = fabricIndex; | ||
| return CHIP_NO_ERROR; |
There was a problem hiding this comment.
why do these methods return CHIP_ERROR when it is impossible for them to fail? that adds extra overhead for callers (code and complexity). Do we envision these will ever fail?
andy31415
reviewed
May 22, 2025
| /** | ||
| * Used to notify of changes in the node list and more TODO. | ||
| */ | ||
| class Listener |
There was a problem hiding this comment.
Based on the description this is a NodeChangeListener. Maybe adjust the name?
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.
Implemented JF Administrator and Datastore Server methods to address #38841
Testing
Verified using docs/guides/joint_fabric_guide.md instructions.