-
Notifications
You must be signed in to change notification settings - Fork 2.1k
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
[FS Example] Update the FS Example apps to support fabric sync setup process part I #34906
[FS Example] Update the FS Example apps to support fabric sync setup process part I #34906
Conversation
Review changes with SemanticDiff. |
examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp
Outdated
Show resolved
Hide resolved
PR #34906: Size comparison from a190f44 to 2b23007 Full report (84 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp
Outdated
Show resolved
Hide resolved
examples/fabric-bridge-app/fabric-bridge-common/include/CommissionerControl.h
Show resolved
Hide resolved
examples/fabric-bridge-app/fabric-bridge-common/include/CommissionerControl.h
Outdated
Show resolved
Hide resolved
examples/fabric-admin/commands/fabric-sync/FabricSyncCommand.cpp
Outdated
Show resolved
Hide resolved
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.
I understand this is under time pressure, but we need a path to make this updatable to non-string API to have it ready for integration. Please make these changes for now:
- enter a github issue to switch to ReadClient based API instead of string API
- requests (even if string based) should be done in the DeviceManager, so that both request and handling are in the same class. This will also make code migration easier in the future
- replace snprintf with StringBuilder
PR #34906: Size comparison from a190f44 to 046d480 Full report (84 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, efr32, esp32, linux, nrfconnect, nxp, psoc6, qpg, stm32, telink, tizen)
|
…process part I (project-chip#34906) * [FS Example] Update the FS Example apps to support fabric sync setup process * Move all command handling to device manager using StringBuilder
Updated the Fabric-Admin and Fabric-Bridge example apps to support the Fabric-Sync setup process.
Please note that this PR includes only the changes to the example apps. The proto update needed to complete the remaining tasks will be addressed in a separate PR for review.
Fixes #33224