Process out-of-order vfs sectors for Universal Hex in block format #1092
Conversation
|
The CI was failing because the So I've merged PR #1097 here to save enough flash to get these to builds successfully again. I don't mind if we merge it all together in this PR, or merge PR #1097 first and then I rebase this PR afterwards, either way works for me. |
|
I merged #1097, so you can rebase. Could you have a look at https://github.com/mbrossard/DAPLink/tree/feature/vfs-tests? It's a way to unit test a part of the flashing logic. I think you should cherry-pick the |
| { | ||
| uint8_t result = 0; | ||
| // We can only check image compatibility on the first block of flash data | ||
| if (addr != 0) { |
There was a problem hiding this comment.
The first block of flash data is not always at address 0x0.
There was a problem hiding this comment.
Right, this didn't meant the first block received that contains flash data, but that the check should only be performed on the block that contains the first 512 bytes of flash data, which might arrive at any point.
I can update the comment to make this more clear.
There was a problem hiding this comment.
I think addr is not the offset in the image, but the address at which the block is intended to be flashed.
There was a problem hiding this comment.
Right, I'll need to double check this, but my assumption here is that as this is being called within flash_decoder_write(addr, data, size), that would be the address in the target flash where the data block will be written.
So, this function is meant to let pass through all other data blocks as being compatible, and only perform the check on the raw data going to flash address 0x0. At that point it checks the vector table to see if it's an M4 or M0+ image and if it fails the check it makes DAPLink stop flashing and throw an error.
This is really only useful for Intel Hex files (not for Universal or UF2), to detect an old V1 Intel Hex file being flashed into a V2. Although it could also be useful for UF2 files without the family ID, but realistically the micro:bit editors will not really produce these, so those might only be produced by other tools, and those users are likely to be more advanced.
There was a problem hiding this comment.
Sorry I missed that it was in source/board/microbitv2/microbitv2.c.
I was expecting to check against something like g_board_info.target_cfg.flash_regions[0].start which in your case is always 0.
When blocks are received out of order the data passed to board_detect_incompatible_image() via flash_decoder_validate_target_image() might not be the first block of flash data, as expected by the microbitv2 imcompatibility detection, so pass the address to the function and perform the check.
When Universalh Hex was created two formats where specified, the segment format, which is currently used due to faster parsing, and the block format. The block format was design so that each 512-bytes of hex characters contain enough info to be self contained, however it was never tested in a systen that sent blocks out-of-order. The original vfs_manager expects all file blocks to arrive sequentially in order. This update adds a check of "self contained blocks" and a sticky flag that process these blocks whatever order they arrive and ignores all other non-self-contained vfs sectors received.
microbit-carlos
force-pushed
the
feature/out-of-order
branch
from
106fa09 to
f083cac
Compare
|
Thanks @mathias-arm! |
Tested in a VM with macOS 13.1, which had the issue where files larger than 1 MB would be sent out-of-order:
13.0 also had an issue where the OS was trying to read back some metadata files it was writtin to the MICROBIT drive, and that caused issues when DAPLink sent back zeros. That metadata issue was fixed in 13.1, but the out-of-order was still present until 13.2, so 13.1 is the best version to test this.