Thanks!

[JPHutchins]

Hello @Frank-Buss and thank you for this implementation! I'd like to contribute to this work. Some ideas I have:

• unit tests
• integration tests
• CI/CD in github workflows
• release workflow that automates the build and distribution of binaries for Windows, Linux, and Mac

LMK where you see priorities.

Cheers, J.P.

[JPHutchins]

Another thought is breaking some of the modules out to lib. The idea that mcumgr-client could be a dependency to other packages rather than a monolithic app.

[Frank-Buss]

A release workflow would be nice, because then it is easier for end-customers to use it, without installing Rust and compiling themselves. I think also it wasn't tested on Windows so far, but should work.

Extracting the mcumgr functionality as a crate might be a good idea as well. Would need some planning, but I guess it would be nice if it could be used for the server functionality as well. Then it could be integrated in the firmware on microcontrollers, when it is Rust based (might need to be non-std then).

[JPHutchins]

Windows testing is definitely in progress here! Build and initial upload went smooth as could be. I'm dealing with Zephyr/MCUBoot not wanting to do the swap, but I think that's on them, not mcumgr-client.

[Frank-Buss]

We had some problems with this, too. I solved this by pre-confirming the images, then it didn't reverse the update. Something like this after the build:

python3 `west topdir`/bootloader/mcuboot/scripts/imgtool.py sign --confirm --header-size 0x400 --align 32 --version 0.0.0+0 --slot-size 0xe0000 build/zephyr/zephyr.bin firmware-image-slot1.bin

I think with OTA and nRF Connect, there are some extra special mgm commands for confirming.

[JPHutchins]

Thanks for the insight! I think it may be that the images are not being marked to "test" - where "test" means check the header, do swap, decrypt, verify signature, and boot (once). Trying to track this down in newtmgr:

https://github.com/apache/mynewt-newtmgr/blob/5c3d1e7ebce4d14075fdb7c05948ec4c78e742bf/newtmgr/cli/image.go#L112-L141

BTW, do you have a source for the mcumgr protocol (AKA "NMP" or "SMP"?) spec? I am aware of this https://github.com/zephyrproject-rtos/mcumgr/blob/master/protocol.md but it does not seem exhaustive.

[JPHutchins]

Perhaps this function, https://github.com/zephyrproject-rtos/zephyr/blob/1a0b1124c6294b9f3851b54fc7d0aeeba12a5f34/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt_state.c#L567, this is called by this function, https://github.com/zephyrproject-rtos/zephyr/blob/1a0b1124c6294b9f3851b54fc7d0aeeba12a5f34/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt_state.c#L664, which is assigned to this function pointer, https://github.dev/zephyrproject-rtos/zephyr/blob/1a0b1124c6294b9f3851b54fc7d0aeeba12a5f34/subsys/mgmt/mcumgr/grp/img_mgmt/src/img_mgmt.c#L814, is accessible from mcumgr-client via this API https://github.com/zephyrproject-rtos/zephyr/blob/1a0b1124c6294b9f3851b54fc7d0aeeba12a5f34/subsys/mgmt/mcumgr/smp/src/smp.c#L159

It takes a smp_hdr which I'll have to match up to the rust implementation

struct smp_hdr {
#ifdef CONFIG_LITTLE_ENDIAN
    uint8_t  nh_op:3;       /* MGMT_OP_[...] */
    uint8_t  nh_version:2;
    uint8_t  _res1:3;
#else
    uint8_t  _res1:3;
    uint8_t  nh_version:2;
    uint8_t  nh_op:3;       /* MGMT_OP_[...] */
#endif
    uint8_t  nh_flags;      /* Reserved for future flags */
    uint16_t nh_len;        /* Length of the payload */
    uint16_t nh_group;      /* MGMT_GROUP_ID_[...] */
    uint8_t  nh_seq;        /* Sequence number */
    uint8_t  nh_id;         /* Message ID within group */
};

Where the op is MGMT_OP_WRITE (1)

It's hard for me to understand exactly what img_mgmt_state_write() is doing. It sorta looks like you give it the "hash" of the image that you want to boot into on next reset and it sets it. Generally a user would want "confirm" to be false.

I should note that in my tests I am doing the mcumgr upgrade from within MCUBoot. So, the behavior I expect is that MCUBoot performs the test swap immediately after the transfer is complete.

[JPHutchins]

I see it is described here https://docs.zephyrproject.org/latest/services/device_mgmt/mcumgr.html, the section that goes:

To test a new upgrade image the test command is used:

mcumgr <connection-options> image test <hash>

This command should mark a test upgrade, which means that after the next reboot the bootloader will execute the upgrade and jump into the new image. If no other image operations are executed on the newly running image, it will revert back to the image that was previously running on the device on the subsequent reset. When a test is requested, flags will be updated with pending to inform that a new image will be run after a reset:

$ mcumgr -c acm0 image test e8cf0dcef3ec8addee07e8c4d5dc89e64ba3fae46a2c5267fc4efbea4ca0e9f4
Images:
 image=0 slot=0
  version: 1.0.0
  bootable: true
  flags: active confirmed
  hash: 86dca73a3439112b310b5e033d811ec2df728d2264265f2046fced5a9ed00cc7
 image=0 slot=1
  version: 1.1.0
  bootable: true
  flags: pending
  hash: e8cf0dcef3ec8addee07e8c4d5dc89e64ba3fae46a2c5267fc4efbea4ca0e9f4
Split status: N/A (0)

Requiring the user to cut and paste the hash is silly when they have IDs like "0" and "1" that are plainly obvious. The client already knows the hash. So if I pursue adding this to mcumg-client, I propose mcumgr-client [OPTIONS] test <image id, eg 0, or 1, usually>.

[Frank-Buss]

I don't have more documentation of the protocol, other than the Zephyr document. I often looked at the Go source code, and its binary dumps, while developing the Rust version.

Sounds good with the new "test" command.

[JPHutchins]

Good news, Zephyr is maintaining a better doc here: https://docs.zephyrproject.org/latest/services/device_mgmt/smp_protocol.html

[Frank-Buss]

Closing this issue, since the release workflow is implemented. But feel free to create individual new issues. Might be nice to have CI/CD, with the tests, like using the test_serial_port for a kind of full integration test, or adding Rust #[test] things and unit tests.