-
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
We should call correct API for factory reset in CLI command #31505
We should call correct API for factory reset in CLI command #31505
Conversation
PR #31505: Size comparison from 71a1ce3 to 740bb6f Full report (8 builds for cc32xx, mbed, nrfconnect, qpg)
|
740bb6f
to
c0cae19
Compare
PR #31505: Size comparison from 71a1ce3 to c0cae19 Increases (8 builds for cyw30739, esp32, linux)
Full report (52 builds for bl602, bl702, bl702l, cc13x4_26x4, cc32xx, cyw30739, esp32, k32w, linux, mbed, nrfconnect, psoc6, qpg)
|
@@ -42,7 +43,7 @@ int DeviceHelpHandler(int argc, char ** argv) | |||
static CHIP_ERROR FactoryResetHandler(int argc, char ** argv) | |||
{ | |||
streamer_printf(streamer_get(), "Performing factory reset ... \r\n"); | |||
DeviceLayer::ConfigurationMgr().InitiateFactoryReset(); | |||
chip::Server::GetInstance().ScheduleFactoryReset(); |
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.
This was done this way to avoid dependency between src/lib
and app
as app
was supposed to be the top-level component. I guess the best way to workaround it would be moving the shell commands to the app
directory but not sure how much work that would be...
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.
Ah, okay. Closing the PR
We should call
ScheduleFactoryReset()
and not theInitiateFactoryReset()
API for factory reset in CLI command