mpremote: Add smart encoding selection for fs_writefile.#11
Open
andrewleech wants to merge 53 commits intomasterfrom
Open
mpremote: Add smart encoding selection for fs_writefile.#11andrewleech wants to merge 53 commits intomasterfrom
andrewleech wants to merge 53 commits intomasterfrom
Conversation
|
Code size report: |
Owner
Author
|
/review |
![mpy-reviewer[bot]](https://avatars.githubusercontent.com/in/2934917?s=60&v=4){kind=small}
There was a problem hiding this comment.
There's a critical bug in _detect_encoding_capabilities: the single dict eval includes __import__('deflate') first, so on any device without the deflate module, ImportError is raised mid-expression and the whole eval fails. The except Exception path then sets both deflate and base64 to False, meaning devices without deflate permanently lose base64 encoding and fall back to repr — a regression. The deflate and base64 capabilities need to be probed independently. Also, test_compression_ratio is defined twice in compression_utils.py.
tools/mpremote/mpremote/transport.py
Outdated
| try: | ||
| caps = self.eval( | ||
| "{" | ||
| "'deflate':hasattr(__import__('deflate'),'DeflateIO')," |
There was a problem hiding this comment.
If deflate is not available on the device, __import__('deflate') raises ImportError, aborting the entire dict construction. The except Exception at the outer level then sets both deflate and base64 to False — even though binascii.a2b_base64 is almost certainly present. On any device without deflate this PR is a regression vs repr.
The capabilities must be probed independently. Either do two separate eval calls, or use a device-side try/except around the deflate import:
caps = self.eval(
"{'base64':hasattr(__import__('binascii'),'a2b_base64'),"
"'bytesio':hasattr(__import__('io'),'BytesIO')}"
)
try:
has_dfl = self.eval("hasattr(__import__('deflate'),'DeflateIO')")
except Exception:
has_dfl = False
caps['deflate'] = has_dfl and caps.get('base64') and caps.get('bytesio')| if not sample: | ||
| return 1.0 | ||
| compressed = compress_chunk(sample) | ||
| return len(compressed) / len(sample) |
There was a problem hiding this comment.
test_compression_ratio is already defined at line 69. Remove the duplicate.
| @@ -0,0 +1,86 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
Please remove the shebang; this is a library module, not a standalone script.
tools/mpremote/mpremote/transport.py
Outdated
| # Setup imports and file handle on device | ||
| if encoding == "deflate": | ||
| self.exec( | ||
| "from binascii import a2b_base64 as _a\n" |
There was a problem hiding this comment.
The ROMFS path in commands.py imports under the full names (a2b_base64, BytesIO, DeflateIO, RAW). Using single-letter aliases (_a, _B, _D, _R) here is inconsistent. Please use the same names in both paths.
Owner
Author
|
/review |
There was a problem hiding this comment.
The .strip() fix in the ROMFS base64 path is correct and the performance gains are well documented. However there is a correctness bug in _detect_encoding_capabilities that will regress base64 support on devices without deflate, and a few smaller issues below.
tools/mpremote/mpremote/transport.py
Outdated
| try: | ||
| caps = self.eval( | ||
| "{" | ||
| "'deflate':hasattr(__import__('deflate'),'DeflateIO')," |
There was a problem hiding this comment.
This will raise ImportError on devices that don't have the deflate module, causing the entire eval() to fail. The except Exception then sets base64 to False too, which is a regression — devices without deflate but with binascii.a2b_base64 will silently lose base64 support and fall back to repr.
The old code probed each capability with a separate try/exec block precisely to avoid this. Either do the same here, or guard the import in the eval string, e.g. with a helper that catches ImportError on the device side.
tools/mpremote/mpremote/transport.py
Outdated
| "deflate": caps.get("deflate") and caps.get("bytesio") and caps.get("base64"), | ||
| "base64": caps.get("base64"), | ||
| } | ||
| except Exception: |
There was a problem hiding this comment.
Catching bare Exception here silently swallows errors that are not ImportError / transport errors (e.g. OOM on the device, serialisation bugs). Should be except (Exception,): at minimum with a comment, or ideally just except TransportExecError.
tools/mpremote/mpremote/commands.py
Outdated
| chunk_size = max(chunk_size, rom_min_write) | ||
|
|
||
| # Detect capabilities of the device to use the fastest method of transfer. | ||
| caps = transport._detect_encoding_capabilities() |
There was a problem hiding this comment.
_detect_encoding_capabilities is a private method (underscore prefix). Calling it directly from commands.py breaks the encapsulation the underscore signals. Either make it public or expose the capabilities through a higher-level API.
| @@ -0,0 +1,86 @@ | |||
| #!/usr/bin/env python3 | |||
There was a problem hiding this comment.
please remove the shebang — this is a library module, not a standalone script.
| def compress_chunk(data, wbits=DEFAULT_WBITS): | ||
| """Compress a single chunk using raw deflate. | ||
|
|
||
| Each chunk is independently compressed/decompressable, which is required |
There was a problem hiding this comment.
"decompressable" → "decompressible"
| Returns: | ||
| Ratio of compressed/original size (0.0-1.0+). Lower = better compression. | ||
| """ | ||
| sample = data[:sample_size] if len(data) > sample_size else data |
There was a problem hiding this comment.
data[:sample_size] if len(data) > sample_size else data can just be data[:sample_size] — Python slicing past the end is safe.
andrewleech
force-pushed
the
feature/smart-encoding-fs-writefile
branch
2 times, most recently
from
a46e4ae to
7f0d6a6
Compare
This commit updates the documentation for the `re` library, officially documenting non-capturing grouping rules (ie. "(?:...)"). The documentation mistakenly marked that feature as not supported, but is is indeed supported in the current iteration of the regex library. This closes micropython#18900. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This reverts commit 046013a. Looks like since the latest round of GitHub Actions updates, the Cache LRU algorithm is working as designed again. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Both the overall IRQ line and the per-channel IRQ, for good measure. Otherwise, soft reset will remove the handler before the finaliser for the DMA object(s) run and trigger IRQs if the channel is still active. Closes micropython#18765 This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Small tweak to avoid changes in other targets' lockfiles from printing warnings when building esp32 port. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Not currently building, and too many versions to concurrently support. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Also rename the prefix from can to pyb_can, in anticipation of machine.CAN tests. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Closes micropython#18922 Signed-off-by: Angus Gratton <angus@redyak.com.au>
The function arguments mean totally different things for Classic vs FDCAN hardware, but the argument name wasn't particularly clear for either. This commit shouldn't really change the binary firmware at all. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Simplifies the pattern of an optional arg which can be a list of at least a certain length, otherwise one is lazily initialised. Modify pyb.CAN and ESP-NOW APIs to use the helper. Note this changes the return type of pyb.CAN.recv() from tuple to list. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
API is different to the original machine.CAN proposal, as numerous shortcomings were found during initial implementation. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
These are oddly missing from the STM32G4 HAL, but the reference manual describes being able to use them and the implementations seem to work as expected. Note that unlike STM32H7 it doesn't seem like we must use this approach, because HAL_FDCAN_AddMessageToTxFifoQ() does seem to not have the issues with priority inversion seen on the H7. However it's simpler to use the same API for both... Signed-off-by: Angus Gratton <angus@redyak.com.au> Signed-off-by: Angus Gratton <angus@redyak.com.au>
Implemented according to API docs in a parent comment. Adds new multi_extmod/machine_can_* tests which pass when testing between NUCLEO_G474RE, NUCLEO_H723ZG and PYBDV11. This work was mostly funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
The DAR register field is for auto-retransmit, FDCAN doesn't support automatic restart to clear Bus Off. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Some MCUs (eg N6) have more timers which are 32-bit, and it's best to use this macro to work that out. Signed-off-by: Damien George <damien@micropython.org>
This functionality already exists in the TIM code, and can be reused by the upcoming PWM implementation. Signed-off-by: Damien George <damien@micropython.org>
This commit implements the standard `machine.PWM` class on stm32, using the common bindings in `extmod/machine_pwm.c`. Features implemented are: - construct a PWM object from a pin, with automatic selection of TIM instance and channel; - get and set freq, duty_u16 and duty_ns; - optionally invert the output. The PWM objects are static objects (partly in ROM, partly in RAM) so creating a PWM instance on the same pin will return exactly the same object. That's consistent with other peripherals in the stm32 port, and consistent with other PWM implementations (eg rp2). When creating a PWM object on a pin, if that pin has multiple TIM instances then only the first will be selected. A future extension could allow selecting the TIM/channel (eg similar to how ADCBlock allows selecting an ADC). Signed-off-by: Damien George <damien@micropython.org>
When assigning a TIMx_CHy to a pin, the second available alternate function is chosen (or the first if there is only one). This gives better overall static allocation of TIM's to pins. On most MCUs (eg F4, F7, H5, H7) picking the second gives TIM5_CH[1-4] for PA0-PA3, and TIM5 is a 32-bit timer. That leaves TIM2 (also usually on PA0-PA3) for other pins that only have TIM2. For STM32G0, STM32L432 and STM32L452 the heuristic is to simply use the first available alternate function because that gives TIM2 (a 32-bit timer) on PA0-PA3. The above heuristic guarantees that PA0-PA3 always get a 32-bit timer on all supported MCUs. Signed-off-by: Damien George <damien@micropython.org>
To be slightly more accurate computing the expected low/high times for the PWM output. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
So that TIM2_CH1 can be used. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Intended to catch problems where new features don't build in old ESP-IDF. Includes major refactor to the GitHub Actions Workflow for esp32 port, including making a reusable workflow for both Code Size and ESP32 build jobs. This work was funded through GitHub Sponsors. Signed-off-by: Angus Gratton <angus@redyak.com.au>
Signed-off-by: Damien George <damien@micropython.org>
This commit updates the listed limitations of the native emitter in the documentation related to how to increase speed of python code. Context managers are supported, as in functions marked as native can use the `with` statement in regular code. Generators can be used in native functions both on the emitting (ie. `yield <whatever>`) and on the receiving end. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
This commit fixes an issue related to the NimBLE initialisation procedure in low memory environments on ESP32 boards. MicroPython uses at least two different NimBLE stacks across the supported ports, mynewt (imported as an external library), and the one provided by Espressif in their own SDKs. The problem is that these two ports differ in the signature for `nimble_port_init(void)`, with mynewt returning `void`, and Espressif's returning a status code on failure. On ESP32 boards, allocating almost all the available heap and then turning on the Bluetooth stack would trigger a failure in the NimBLE initialisation function that is not handled by the NimBLE integration code, as there's no expectation of a recoverable condition. Since the stack initialisation would progress, then uninitialised memory accesses crash the board. Since we cannot really modify neither mynewt nor Espressif SDKs, the next best thing is to provide two conditional initialisation paths depending on a configuration setting. This would make Espressif ports recover from a failed initialisation whilst retaining the existing behaviour on other ports. This fixes micropython#14293. Signed-off-by: Alessandro Gatti <a.gatti@frob.it>
Because socket objects have a finaliser they must be created carefully, in case an exception is raised during the population of their members, eg invalid input argument or out-of-memory when allocating additional arrays. Prior to the fix in this commit, the finaliser would crash due to `incoming.udp_raw.array` being an invalid pointer in the following cases: - if a SOCK_RAW was created with a proto argument that was not an integer - if a SOCK_DGRAM or SOCK_RAW was created where the allocation of `lwip_incoming_packet_t` failed - if an integer was passed in for the socket type but it was not one of SOCK_STREAM, SOCK_DGRAM or SOCK_RAW Furthermore, if the allocation of `lwip_incoming_packet_t` failed then it may have led to corruption within lwIP when freeing `socket->pcb.raw` because that PCB was not fully set up with its callbacks. This commit fixes all of these issues by ensuring: - `pcb.tcp` and `incoming.udp_raw.array` are initialised to NULL early on - the proto argument is parsed before allocating the PCB - the allocation of `lwip_incoming_packet_t` occurs befor allocating the PCB - `incoming.udp_raw.array` is checked for NULL in the finaliser code The corresponding test (which already checked most of these causes of failure) has been updated to include a previously-uncovered scenario. Signed-off-by: Damien George <damien@micropython.org>
User callbacks allow code to respond to incoming messages without blocking or polling. User callbacks are optional, and if no callback is registered the code has no effect. The mechanism is the same as for TCP: when a connection is accepted or a TCP packet is received, a user callback is executed. Fixes issue micropython#3594. Signed-off-by: Jack Whitham <jack.d.whitham@gmail.com>
Add DMA, NPU and PDM IRQ priorities to irq.h. Signed-off-by: iabdalkader <i.abdalkader@gmail.com>
2.25k Seems necessary so it doesn't crash `thread/thread_stacksize1.py`. But 2.5k gives a little extra headroom to make that test actually pass. Signed-off-by: Damien George <damien@micropython.org>
With the recent addition of `machine.PWM` and `machine.CAN`, the internal flash of PYBD_SF3 overflows by about 300 bytes. This commit moves the inline assembler compiler functions from internal to external QSPI flash. That frees up about 3k internal flash, and shouldn't affect performance. Signed-off-by: Damien George <damien@micropython.org>
qstr literals are of type qstr_short_t (aka uint16_t) for efficiency, but when the type is passed to `mp_printf` it must be cast explicitly to type `qstr`. These locations were found using an experimental gcc plugin for `mp_printf` error checking. Signed-off-by: Jeff Epler <jepler@unpythonic.net>
This adds support for the standard `weakref` module, to make weak references to Python objects and have callbacks for when an object is reclaimed by the GC. This feature was requested by PyScript, to allow control over the lifetime of external proxy objects (distinct from JS<->Python proxies). Addresses issue micropython#646 (that's nearly a 12 year old issue!). Functionality added here: - `weakref.ref(object [, callback])` create a simple weak reference with optional callback to be called when the object is reclaimed by the GC - `weakref.finalize(object, callback, /, *args, **kwargs)` create a finalize object that holds a weak reference to an object and allows more convenient callback usage and state change The new module is enabled at the "everything" level. The implementation aims to be as efficient as possible, by adding another bit-per-block to the garbage collector, the WTB (weak table). Similar to the finalizer bit (FTB), if a GC block has its corresponding WTB bit set then a weak reference to that block is held. The details of that weak reference are stored in a global map, `mp_weakref_map`, which maps weak reference to ref/finalize objects, allowing the callbacks to be efficiently found when the object is reclaimed. With this feature enabled the overhead is: - 1/128th of the available memory is used for the new WTB table (eg a 128k heap now needs an extra 1k for the WTB). - Code size is increased. - At garbage collection time, there is a small overhead to check if the collected objects had weak references. This check is the same as the existing FTB finaliser scan, so shouldn't add much overhead. If there are weak reference objects alive (ref/finalize objects) then additional time is taken to call the callbacks and do some accounting to clean up the used weak reference. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
Needs a native exp file because native code doesn't print line numbers in the traceback. Signed-off-by: Damien George <damien@micropython.org>
Following a69425b, this is a convenient way to run a subset of tests. Signed-off-by: Damien George <damien@micropython.org>
The webassembly port needs some additional weakref tests due to the fact that garbage collection only happens when Python execution finishes and JavaScript resumes. The `tests/ports/webassembly/heap_expand.py` expected output also needs to be updated because the amount of GC heap got smaller (weakref WTB takes some of the available RAM). Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Damien George <damien@micropython.org>
It takes longer now that weakref is enabled in the coverage build. Signed-off-by: Damien George <damien@micropython.org>
This brings in: - sdcard: Send stop bit after multi-block read/write - sdcard: Compute CRC7 for all SPI commands - sdcard: Add read/write speed test to sdtest - lsm6dsox: Add pedometer support - lsm6dsox: Add pedometer example code - unix-ffi/re: Handle PCRE2_UNSET in group and groups methods - unix-ffi/re: Add tests for empty string match in ffi regex - unix-ffi/machine: Retrieve a unique identifier if one is known - senml/docs: Correct capitalization of 'MicroPython' - unix-ffi/_libc: Extend FreeBSD libc versions range - string: Convert string module to package and import templatelib Signed-off-by: Damien George <damien@micropython.org>
If a port enables t-strings then it is required to have the `string.templatelib` package (at least to run the tests). That's automatically the case if `MICROPY_PY_TSTRINGS` is enabled. If a port freezes in the micropython-lib `string` extension package then the latest version of this package will include the built-in `string.templatelib` classes. So the feature check for t-strings no longer needs to check if they are available. Signed-off-by: Damien George <damien@micropython.org>
Includes a fix to STA teardown to deinit tcpip and clear itf_state. Signed-off-by: Damien George <damien@micropython.org>
Signed-off-by: Matt Trentini <matt.trentini@gmail.com>
Signed-off-by: Dryw Wade <dryw.wade@sparkfun.com>
Signed-off-by: Matt Trentini <matthew.trentini@planetinnovation.com.au> Signed-off-by: Matt Trentini <matt.trentini@gmail.com>
Automatically detect device capabilities (deflate, base64, bytes.fromhex) and select the best encoding for file transfers. Deflate+base64 is used when the device supports it and data compresses well, base64 alone as a fallback, and repr as the universal fallback. Each capability is probed independently so a missing deflate module does not suppress base64 detection. Signed-off-by: Andrew Leech <andrew.leech@planetinnovation.com.au>
andrewleech
force-pushed
the
feature/smart-encoding-fs-writefile
branch
from
7f0d6a6 to
47d6725
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
11 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary
mpremote fs cpfile transfers to device are slow becausefs_writefile()usesrepr()encoding, expanding each byte to\xNN(~4x wire overhead for binary data).This adds automatic encoding selection with a three-tier fallback:
binascii.a2b_base64but data doesn't compress wellThe ROMFS deploy path is updated to share the new compression utilities and capability detection, replacing its inline
zlib.compressobj(wbits=-9), hardcoded wbits value, and 14-line try/except capability detection. Also fixes a missing.strip()on the ROMFS base64-only encoding path.Testing
64 transfer+readback integrity tests on STM32WB55 over 115200 baud UART with SPI flash. All verified via SHA-256 readback.
Random binary (incompressible, ratio ~1.0 — auto selects base64):
Python source (ratio ~0.4 — auto selects deflate):
Log data (ratio ~0.5 — auto selects deflate):
All zeros (ratio ~0.005 — auto selects deflate):
Auto-selection picks the fastest encoding for each data type in all cases.
Not tested on other ports or boards.
Trade-offs and Alternatives
chunk_sizedefault changes from256toNone(auto-sized per encoding). Callers omittingchunk_sizeget 256 for repr (matching prior behaviour). Explicit values are respected.binascii.a2b_base64fall back torepr()with no behaviour change.hasattr()and cached for the session.