vouch-opensource / mcumgr-client

client for mcumgr commands
Apache License 2.0
31 stars 13 forks source link

Fix image upload hanging when MTU is increased #21

Closed bcluzel closed 8 months ago

bcluzel commented 9 months ago

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 commented 8 months ago

@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 https://github.com/vouch-opensource/mcumgr-client/issues/8 as well.

mfikes commented 8 months ago

@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 commented 8 months ago

Yes, our production bootloader has the patch and the larger buffer sizes. So looks like https://github.com/vouch-opensource/mcumgr-client/issues/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 commented 8 months ago

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 commented 8 months ago

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