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/lib: RFC: Allow leaving out "rc" in successful respones and use "rc" only for SMP processing errors. #42590

Closed
de-nordic opened this issue Feb 8, 2022 · 0 comments · Fixed by #51555
Assignees
Labels
area: mcumgr Enhancement Changes/Updates/Additions to existing features RFC Request For Comments: want input from the community

Comments

@de-nordic
Copy link
Collaborator

de-nordic commented Feb 8, 2022

Is your enhancement proposal related to a problem? Please describe.
There is inconsistency on how mcumgr responds to commands regarding usage of "rc" field.
When there are simple error responses, it is quite consistent and error responses usually consist only of map containing "rc" as a key and error code (int) as value.
Successful responses are not that consistent for example:

  • reset command returns empty dictionary on success;
  • echo does not return "rc" on success, only echo string;
  • img update command will include "rc" in every response, but image list only in case of error; image erase always return "rc";
  • stat commands always include "rc" = 0 in successful response, for all commands;

Additional problem is that some commands may use non-0 "rc" in successful responses; shell is an example when successful response, where SMP request has been properly decoded executed and response encoded, can have "rc" error because even though SMP processing went fine the executed command returned error that is returned as "rc" - which is not correct usage!

Describe the solution you'd like
Consistency: allow to leave out "rc" if it is 0 (success), and if "rc" is only response allow to send no payload at all, header should be enough. Only SMP processing errors should trigger addition of "rc" field to response.

Describe alternatives you've considered
Leaving current state.

Additional context
TBD

@de-nordic de-nordic added the Enhancement Changes/Updates/Additions to existing features label Feb 8, 2022
@de-nordic de-nordic added area: mcumgr RFC Request For Comments: want input from the community labels Feb 8, 2022
@de-nordic de-nordic changed the title mgmt/mcumgr/lib: Allow leaving out "rc" in successful respones mgmt/mcumgr/lib: RFC: Allow leaving out "rc" in successful respones Feb 8, 2022
@de-nordic de-nordic changed the title mgmt/mcumgr/lib: RFC: Allow leaving out "rc" in successful respones mgmt/mcumgr/lib: RFC: Allow leaving out "rc" in successful respones and use "rc" only for SMP processing errors. Apr 4, 2022
@nordicjm nordicjm self-assigned this Oct 20, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
area: mcumgr Enhancement Changes/Updates/Additions to existing features RFC Request For Comments: want input from the community
Projects
None yet
2 participants