nrf: Properly configure asyncio at the makefile level.#19219
Open
dpgeorge wants to merge 3 commits into
Open
Conversation
Instead of unconditionally (ie when the `select` module is enabled). This way, a minimal board can enable the `select` module (eg for asyncio) but still have `select.select()` disabled by default to save size. Signed-off-by: Damien George <damien@micropython.org>
Prior to this change some of the smaller boards (eg nRF51) would have the .py asyncio code frozen but be missing both the `select` module and the core C-level `_asyncio` module, making `asyncio` useless. This commit lets asyncio be configured at the makefile level, in order to freeze the correct code. Both the `select` and `_asyncio` modules will be enabled if asyncio is enabled. asyncio is now enabled by default on all boards except MICROBIT (which is too small due to the other modules is has). This costs about +2100 bytes on boards that did not already have it enabled, eg PCA10028. Signed-off-by: Damien George <damien@micropython.org>
To match other ports, and allow the `pyproject.toml` F821 ignore rule to cover it. Signed-off-by: Damien George <damien@micropython.org>
Codecov Report✅ All modified and coverable lines are covered by tests. Additional details and impacted files@@ Coverage Diff @@
## master #19219 +/- ##
=======================================
Coverage 98.47% 98.47%
=======================================
Files 176 176
Lines 22845 22845
=======================================
Hits 22497 22497
Misses 348 348 ☔ View full report in Codecov by Sentry. 🚀 New features to boost your workflow:
|
5075116 to
fc6638b
Compare
|
Code size report: |
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
1 participant
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
Prior to this change some of the smaller boards (eg MICROBIT, most nRF51 boards) would have the .py asyncio code frozen but be missing both the
selectmodule and the core C-level_asynciomodule, makingasynciounusable.This PR lets asyncio be configured at the makefile level, in order to freeze the correct code and include the correct modules. Both the
selectand_asynciomodules will be enabled if asyncio is enabled.asyncio is now enabled by default on all boards except MICROBIT (which is too small due to the other modules is has). This costs about +2100 bytes on boards that did not already have it enabled, eg PCA10028.
For MICROBIT, this change removes the unusable, partial frozen asyncio code, freeing up 7800 bytes on that board.
Related side change: MICROPY_PY_SELECT_SELECT is now enabled only at the extra feature level, matching the setting for MICROPY_PY_SELECT. That helps keep code size down for boards that want asyncio but don't want the full
selectmodule.Testing
Built all nrf boards. They all build, ie fit in flash.
Ran the test suite on PCA10028, which now includes fully working asyncio. Prior to this fix
extmod/asyncio_as_uasyncio.pywould fail. Now it passes, along with all other asyncio tests that don't use floats (because this board doesn't have float enabled, those tests are skipped).Trade-offs and Alternatives
This increases code size for all nRF51 boards (except MICROBIT) by +2100 bytes. But the code still fits and having a working asyncio is very useful.
Alternative would be to fully disable asyncio on nRF51, but IMO it's worth having.
Generative AI
No generative AI tools used when creating this PR.