extend OS_MGMT_RET_RC

[tomaszduda23]

extend OS_MGMT_RET_RC base on https://docs.zephyrproject.org/latest/services/device_mgmt/mcumgr.html#c.mcumgr_err_t

[JPHutchins]

Thanks, looking into this now!

[JPHutchins]

@tomaszduda23 It looks like the OS management enums should be updated from here:

https://github.com/zephyrproject-rtos/zephyr/blob/5c455aae88d14a527bdd438adf9da4958d52c17b/include/zephyr/mgmt/mcumgr/grp/os_mgmt/os_mgmt.h#L29-L50

The enums here, https://docs.zephyrproject.org/latest/services/device_mgmt/mcumgr.html#c.mcumgr_err_t, seem to be general SMP server errors.

This merits further discussion! Are the enum mcumgr_err_ts ever sent to the SMP client or are they only used internal to the SMP server? If they are to be sent to the client, then the client must have a way of knowing whether the return code is in the enum mcumgr_err_t space or the specified return code space of the SMP group.

Thanks for taking a look!

JP

[tomaszduda23]

It seems that both are mixed even in the same function https://github.com/nrfconnect/sdk-zephyr/blob/e9bb83320a4cd303135203fae18136a71e87f26e/subsys/mgmt/mcumgr/grp/os_mgmt/src/os_mgmt.c#L480C1-L483C47. MGMT_ERR_ENOTSUP is definitely possible. https://github.com/NordicSemiconductor/Android-nRF-Connect-Device-Manager does some mapping depending on group. I'm not sure if the mapping can be perfect though.

Maybe it would be better to allow int values for unknown values. I added this to not catch exception here https://github.com/tomaszduda23/smpclient/blob/d25c8035ae2858fd41a106058297b619d58fbcb5/smpclient/__init__.py#L62

[JPHutchins]

IMO you've found a mistake, or possibly legacy support for past mistakes, in Zephyr's SMP server implementation!

If you have time, take a look at the SMP spec here, https://docs.zephyrproject.org/latest/services/device_mgmt/smp_protocol.html, and see if that sheds any light on this.

BTW, no one seems to use the flag bits that are available in the header. It would be possible to use the flag bits to specify the namespace of the return code. Of course the spec would need to be revised.

[JPHutchins]

@tomaszduda23 I think that I see!

I made the mistake of implementing SMP Version 2 instead of SMP Version 1.

This PR should be updated to use the OS_MGMT RCs from here: https://docs.zephyrproject.org/latest/services/device_mgmt/mcumgr.html#c.mcumgr_err_t

Separately, this SMP library needs to understand that the error response is a union of SMP Version 1 and Version 2 errors. The mcumgr_err_t enums would be defined as the type of an SMP Version 1 return, which is what Zephyr implements (mostly, I think... 🤣 )

LMK what you think!

[tomaszduda23]

I was just about to point out the same thing.

[JPHutchins]

Another note is that smpclient, at the application layer, wants to discard this nonsense: https://github.com/intercreate/smpclient/blob/4942251bf4256643fd8878641aa08519a6693d9b/smpclient/generics.py#L17-L28

Edit: probably this won't work anymore since the application NEEDS to know that they are different types.