Skip to content
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

mgmt: mcumgr: Deprecate returning rc responses when status is OK. #51555

Merged
merged 3 commits into from
Dec 20, 2022

Conversation

nordicjm
Copy link
Collaborator

@nordicjm nordicjm commented Oct 24, 2022

This change deprecates the previous mcumgr behaviour of return result codes when the status is 0 (OK), instead it will skip the rc field for these responses. If there is only an rc field with status 0 to return, then mcumgr will now instead just return an empty map.

Fixes #42590

  • Remove rc responses when status is 0
  • Add deprecation warning/notice
  • Release notes update
  • Documentation update
  • Manual tests (mcumgr command line tool functions fine with new behaviour)

doc/releases/release-notes-3.3.rst Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/Kconfig Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/fs_mgmt/src/fs_mgmt.c Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c Outdated Show resolved Hide resolved
subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c Outdated Show resolved Hide resolved
de-nordic
de-nordic previously approved these changes Nov 23, 2022
Copy link
Member

@carlescufi carlescufi left a comment

Choose a reason for hiding this comment

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

No need to deprecate, this is unstable. Please remove it altogether so we can declare it stable soon.

@nordicjm
Copy link
Collaborator Author

nordicjm commented Dec 5, 2022

No need to deprecate, this is unstable. Please remove it altogether so we can declare it stable soon.

With this change it's more to give users the choice to revert as some software that uses that might be unable to cope with the change, and they might need time to adapt the software.

@de-nordic
Copy link
Collaborator

No need to deprecate, this is unstable. Please remove it altogether so we can declare it stable soon.

Yes, but @nordicjm mentions important issue of no-Zephyr based tools, that may require some time to adapt.
I agree with @nordicjm that we should use deprecation process as a way for external projects to adapt.

subsys/mgmt/mcumgr/Kconfig Outdated Show resolved Hide resolved
This change changes the previous mcumgr behaviour of return result
codes when the status is 0 (OK) to being legacy behaviour, instead
it will skip the rc field for these responses. If there is only an
rc field with status 0 to return, then mcumgr will now instead just
return an empty map.

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Adds a note on rc responses to mcumgr commands where the status is
good that these responses will henceforth be emitted.

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
Updates documentation on when the rc response is returned.

Signed-off-by: Jamie McCrae <jamie.mccrae@nordicsemi.no>
@de-nordic de-nordic requested a review from carlescufi December 14, 2022 10:00
@de-nordic
Copy link
Collaborator

@carlescufi Please take another look. Thanks.

@carlescufi carlescufi merged commit 235ab90 into zephyrproject-rtos:main Dec 20, 2022
@nordicjm nordicjm deleted the mcurc branch January 6, 2023 10:40
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: Documentation area: mcumgr Release Notes To be mentioned in the release notes
Projects
None yet
4 participants