Skip to content

Conversation

@gneworld
Copy link

Summary

add restore to indicate "restore the factory settings" and the previous "factory" represents the confirmation of restoring to factory settings.

Impact

resetcause

Testing

CI test

Signed-off-by: wanggang26 <wanggang26@xiaomi.com>
Copy link
Contributor

@cederom cederom left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

  • I like the idea but this changes existing logic and may impact existing applications.
  • After introducing restore existing applications may not catch the factory event and skip the factory defaults reset.
  • factory is already used to trigger the "perform reset to factory defaults".
  • After successful factory defaults reset one more reset can be triggered with NULL and device should land in a state that should be handled by a firmware accordingly based on existing configuration.
  • restore name is a bit ambiguous and may be confusing, its not self explanatory, maybe another keyword can be used?
  • Maybe something like defaults would fit here better? factory as before would cause factory defaults reset, and then call reset with defaults mark to note everything is default or needs to be set default (this state should be also unambiguous)?
  • Documentation needs an update in this area. Will try to work more with documentation in a free moment.
  • Long story short if new resetflag is to create new and unique state that would be fine, but if it changes existing behavior this will break compatibility and we do not want that.
  • I would like to hear what other devs think about this idea :-)

@xiaoxiang781216
Copy link
Contributor

  • I like the idea but this changes existing logic and may impact existing applications.
  • After introducing restore existing applications may not catch the factory event and skip the factory defaults reset.
  • factory is already used to trigger the "perform reset to factory defaults".
  • After successful factory defaults reset one more reset can be triggered with NULL and device should land in a state that should be handled by a firmware accordingly based on existing configuration.
  • restore name is a bit ambiguous and may be confusing, its not self explanatory, maybe another keyword can be used?
  • Maybe something like defaults would fit here better? factory as before would cause factory defaults reset, and then call reset with defaults mark to note everything is default or needs to be set default (this state should be also unambiguous)?
  • Documentation needs an update in this area. Will try to work more with documentation in a free moment.
  • Long story short if new resetflag is to create new and unique state that would be fine, but if it changes existing behavior this will break compatibility and we do not want that.
  • I would like to hear what other devs think about this idea :-)

@cederom this change is to align with nuttx side definition:
https://github.com/apache/nuttx/blob/master/include/sys/boardctl.h#L461-L471

@xiaoxiang781216 xiaoxiang781216 merged commit d407670 into apache:master Oct 6, 2024
@gneworld gneworld deleted the app3 branch October 7, 2024 11:59
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

4 participants