don't "optimize" the UF2 until the combination is fixed

naively adding UF2 blocks together is at least wrong by spec; it
produces one file with wrong block counts --- say I'm writing 3770
blocks, the naive uf2(firmware) + uf2(config) solution yields a file
that says it's 3642 blocks for a while, then 128 blocks. picotool kind
of does a "wtf" at this but writes it anyway, but I am not confident
this is safe, so I'll just do the dumb thing again for now (meaning we
pretty much always write 8128 or 8192 blocks when concatenating configs)

Signed-off-by: Brian S. Stephan <bss@incorporeal.org>
This commit is contained in:
Brian S. Stephan 2024-04-12 15:27:58 -05:00
parent 8e6a203398
commit 2ce0c4d7df
Signed by: bss
GPG Key ID: 3DE06D3180895FCB
2 changed files with 22 additions and 11 deletions

View File

@ -102,13 +102,24 @@ def concatenate_firmware_and_storage_files(firmware_filename: str,
new_binary = combine_firmware_and_config(firmware_binary, board_config_binary, user_config_binary,
replace_extra=replace_extra)
else:
new_binary = convert_binary_to_uf2(firmware_binary)
if board_config_binary:
new_binary += convert_binary_to_uf2(pad_config_to_storage_size(board_config_binary),
start=BOARD_CONFIG_BINARY_LOCATION)
if user_config_binary:
new_binary += convert_binary_to_uf2(pad_config_to_storage_size(user_config_binary),
start=USER_CONFIG_BINARY_LOCATION)
# this was kind of fine, but combining multiple calls of convert_binary_to_uf2 produced
# incorrect total block counts in the file, which picotool handled with some squirrely
# double-output behavior that has me worried it'd cause a real issue, so doing the
# crude padding + write of empty blocks, for now...
#
# new_binary = convert_binary_to_uf2(firmware_binary)
# if board_config_binary:
# new_binary += convert_binary_to_uf2(pad_config_to_storage_size(board_config_binary),
# start=BOARD_CONFIG_BINARY_LOCATION)
# if user_config_binary:
# new_binary += convert_binary_to_uf2(pad_config_to_storage_size(user_config_binary),
# start=USER_CONFIG_BINARY_LOCATION)
#
# the correct way to do the above would be to pass a list of {offset,binary_data} to convert...,
# and have it calculate the total block size before starting to write, and then iterating over
# the three lists. doable, just not on the top of my mind right now
new_binary = convert_binary_to_uf2(combine_firmware_and_config(firmware_binary, board_config_binary,
user_config_binary, replace_extra=replace_extra))
if combined_filename:
with open(combined_filename, 'wb') as combined:

View File

@ -121,8 +121,8 @@ def test_concatenate_to_uf2(tmp_path, firmware_binary, config_binary):
combined_filename=tmp_file)
with open(tmp_file, 'rb') as file:
content = file.read()
# size of the file should be 2x firmware in 256 byte blocks + 2x padded board config + 2x padding user config
assert len(content) == 2 * (math.ceil(len(firmware_binary)/256) * 256) + 2 * STORAGE_SIZE + 2 * STORAGE_SIZE
# size of the file should be 2x the binary version, and the binary is 2 MB
assert len(content) == 2 * 2 * 1024 * 1024
def test_concatenate_to_uf2_board_only(tmp_path, firmware_binary, config_binary):
@ -134,8 +134,8 @@ def test_concatenate_to_uf2_board_only(tmp_path, firmware_binary, config_binary)
combined_filename=tmp_file)
with open(tmp_file, 'rb') as file:
content = file.read()
# size of the file should be 2x firmware in 256 byte blocks + 2x padded board config + 2x padding user config
assert len(content) == 2 * (math.ceil(len(firmware_binary)/256) * 256) + 2 * STORAGE_SIZE
# size of the file should be 2x the binary version (minus user config space), and the binary is 2 MB - 16KB
assert len(content) == 2 * (2 * 1024 * 1024 - 16384)
def test_padding_firmware(firmware_binary):