Fix image upload hanging when MTU is increased

[bcluzel]

Description

When working with Zephyr OS i came across a similar issue as discussed in #8.

I did some investigation and found that the client was waiting indefinitly for a response from the mcu, on the chunk start markers.

I'm assuming the mcu did not receive a valid frame and dropped it without sending an answer.

Changes

Lowered the read timeout and added a retry to send the unanswerd data chuncks again.

I was able to upload an image through uart at 921600b | MTU 1024 | line len 8192.

[Frank-Buss]

Thanks, looks good! The offset is sent as well, so Zephyr probably drops the packet as well, if there is a race condition, and it would probably also help with USB communication problems when something gets stuck. Just a few minor comments. Will have time to test it tomorrow, then I can merge it.

[Frank-Buss]

I tested it and it work.

[Frank-Buss]

@mfikes Maybe take a look at this PR as well. I tested it on Linux with the display, and it works. Maybe it fixes your problem you described here #8 as well.

[mfikes]

@Frank-Buss I tried it with the approach in #8 and it still hangs for me. Here is a truncated verbose output where I replaced lots of it with ... to eliminate a lot of the data

/Users/mfikes/Projects/mcumgr-client/target/release/mcumgr-client -v -m 4096 -l 8192 upload flash-slot3.bin
mcumgr-client 0.0.4, Copyright © 2024 Vouch.io LLC

21:38:20 [INFO] One bootloader device found, setting device to: /dev/cu.usbmodem14301
21:38:20 [INFO] upload file: flash-slot3.bin
21:38:20 [INFO] flashing to slot 3
21:38:20 [INFO] 14876672 bytes to transfer
21:38:20 [DEBUG] (1) mcumgr_client::image: try_length: 4096
21:38:20 [DEBUG] (1) mcumgr_client::image: req: ImageUploadReq { data: [61, 184, 243, 150, 0, 0, 0, … 16, 255], image_num: 3, len: Some(14876672), off: 0, data_sha: Some([160, 31, 121, … 8, 88, 189]), upgrade: None }
21:38:20 [DEBUG] (1) mcumgr_client::transfer: request header: NmpHdr { op: Write, flags: 0, len: 4164, group: Image, seq: 45, id: 1 }
21:38:20 [DEBUG] (1) mcumgr_client::transfer: serialized: 0200…50858bd
21:38:20 [DEBUG] (1) mcumgr_client::transfer: encoded with packet length and checksum: 104e0…2794
21:38:20 [DEBUG] (1) mcumgr_client::transfer: encoded: EE4C…QhYvSeU
21:38:20 [DEBUG] (1) mcumgr_client::image: new try_length: 2987
21:38:20 [DEBUG] (1) mcumgr_client::image: req: ImageUploadReq { data: [61, 184, …, 8, 88, 189]), upgrade: None }
21:38:20 [DEBUG] (1) mcumgr_client::transfer: request header: NmpHdr { op: Write, flags: 0, len: 3055, group: Image, seq: 45, id: 1 }
21:38:20 [DEBUG] (1) mcumgr_client::transfer: serialized: 020..58bd
21:38:20 [DEBUG] (1) mcumgr_client::transfer: encoded with packet length and checksum: 0bf…008
21:38:20 [DEBUG] (1) mcumgr_client::transfer: encoded: C/kC…gCA==

This is with our production bootloader on the target, which presumably has the CDC ACM driver bugfix in place.

[Frank-Buss]

Yes, our production bootloader has the patch and the larger buffer sizes. So looks like #8 is another problem and we can't close it with this PR. But nevertheless this PR looks useful for lost packages and it helps the author of the PR, so maybe check if it still works on your side with the default sizes for line length and MTU, and then you can merge it.

[Frank-Buss]

PS: the bootloader needs like 20 s to erase the flash before writing the first block. Also it should fail after 60 s, and not hang indefinitely.

[mfikes]

Yes, this PR works fine for me with mcumgr-client upload flash-slot3.bin.