From 6a147aa1d8f6108bed114c0ab777dab8fb3a96b6 Mon Sep 17 00:00:00 2001 From: "Brian S. Stephan" Date: Tue, 11 Jul 2023 23:28:34 -0500 Subject: [PATCH] attempt to address USB corruption by padding more --- gp2040ce_bintools/builder.py | 7 +++++-- gp2040ce_bintools/pico.py | 9 ++++++--- tests/test_builder.py | 5 +++-- tests/test_pico.py | 11 ++++++++--- 4 files changed, 22 insertions(+), 10 deletions(-) diff --git a/gp2040ce_bintools/builder.py b/gp2040ce_bintools/builder.py index 7c01f8d..cbe2769 100644 --- a/gp2040ce_bintools/builder.py +++ b/gp2040ce_bintools/builder.py @@ -152,11 +152,14 @@ def write_new_config_to_usb(config: Message, endpoint_out: object, endpoint_in: # we don't write the whole area, just the minimum from the end of the storage section # nevertheless, the USB device needs writes to start at 256 byte boundaries logger.debug("serialized: %s", serialized) - padding = 256 - (len(serialized) % 256) + # not sure why this minimal padding isn't working but it leads to corruption + # maybe claims that erase need to be on 4096 byte sectors? + # padding = 256 - (len(serialized) % 256) + padding = 4096 - (len(serialized) % 4096) logger.debug("length: %s with %s bytes of padding", len(serialized), padding) binary = bytearray(b'\x00' * padding) + serialized logger.debug("binary for writing: %s", binary) - write(endpoint_out, endpoint_in, STORAGE_MEMORY_ADDRESS + (STORAGE_SIZE - len(binary)), binary) + write(endpoint_out, endpoint_in, STORAGE_MEMORY_ADDRESS + (STORAGE_SIZE - len(binary)), bytes(binary)) ############ diff --git a/gp2040ce_bintools/pico.py b/gp2040ce_bintools/pico.py index f9a2caa..f81c9ca 100644 --- a/gp2040ce_bintools/pico.py +++ b/gp2040ce_bintools/pico.py @@ -198,8 +198,10 @@ def write(out_end: usb.core.Endpoint, in_end: usb.core.Endpoint, location: int, location: memory address of where to start reading from content: the data to write """ - if (location % 256) != 0: - raise PicoAlignmentError("writes must start at 256 byte boundaries, please pad or align as appropriate!") + # not sure why 256 alignment isn't working but it leads to corruption + # maybe claims that erase need to be on 4096 byte sectors? + if (location % 4096) != 0: + raise PicoAlignmentError("writes must start at 4096 byte boundaries, please pad or align as appropriate!") # set up the data command_size = 8 @@ -209,13 +211,14 @@ def write(out_end: usb.core.Endpoint, in_end: usb.core.Endpoint, location: int, erase(out_end, in_end, location, len(content)) exit_xip(out_end, in_end) pico_token = 1 - logger.debug("writing %s bytes to %s", len(content), location) + logger.debug("writing %s bytes to %s", len(content), hex(location)) payload = struct.pack(PICOBOOT_CMD_STRUCT + PICOBOOT_CMD_READ_SUFFIX_STRUCT, PICO_MAGIC, pico_token, PICO_COMMANDS['WRITE'], command_size, len(content), location, len(content)) logger.debug("WRITE: %s", payload) out_end.write(payload) logger.debug("actually writing bytes now...") + logger.debug("payload: %s", content) out_end.write(content) res = in_end.read(256) logger.debug("res: %s", res) diff --git a/tests/test_builder.py b/tests/test_builder.py index 05ad858..905db80 100644 --- a/tests/test_builder.py +++ b/tests/test_builder.py @@ -163,8 +163,9 @@ def test_write_new_config_to_usb(config_binary): write_new_config_to_usb(config, end_out, end_in) # check that it got padded - padded_serialized = bytearray(b'\x00' * 4) + serialized - assert mock_write.call_args.args[2] % 256 == 0 + assert len(serialized) == 2044 + padded_serialized = bytearray(b'\x00' * 2052) + serialized + assert mock_write.call_args.args[2] % 4096 == 0 assert mock_write.call_args.args[3] == padded_serialized diff --git a/tests/test_pico.py b/tests/test_pico.py index ff0e0e5..ec502a3 100644 --- a/tests/test_pico.py +++ b/tests/test_pico.py @@ -205,14 +205,19 @@ def test_misaligned_write(): with pytest.raises(pico.PicoAlignmentError): _ = pico.write(end_out, end_in, 0x101FE0FF, b'\x00\x01\x02\x03') - _ = pico.write(end_out, end_in, 0x101FE100, b'\x00\x01\x02\x03') + # 256 byte alignment is what is desired, but see comments around there for + # why only 4096 seems to work right... + with pytest.raises(pico.PicoAlignmentError): + _ = pico.write(end_out, end_in, 0x101FE100, b'\x00\x01\x02\x03') + + _ = pico.write(end_out, end_in, 0x101FF000, b'\x00\x01\x02\x03') expected_writes = [ mock.call(struct.pack('