Conversation
dhalbert
left a comment
There was a problem hiding this comment.
Thank you for working on this! I have some minor comments and some higher-level comments.
| // Async completion flag — ports may override by defining these in | ||
| // mpconfigport.h before this header is included. | ||
| // |
There was a problem hiding this comment.
In MicroPython, there is an asyncio.ThreadSafeFlag and also asyncio.event. Tjey are not in CPython asyncio, so I removed it from CircuitPython's asyncio. However, it would be worth looking at them to see if their semantics are similar to what you are doing here. They could be added as CircuitPython-specific classes if they would also be useful to someone writing CircuitPython async code.
There was a problem hiding this comment.
This is what I came up with for a standard awaitable object from the native core of CP. There is similar discussion about how to do this in CPython here: https://discuss.python.org/t/adding-a-c-api-for-coroutines-awaitables/22786/16
The third-party library for doing it is here too: https://pyawaitable.zintensity.dev/
tannewt
left a comment
There was a problem hiding this comment.
Thank you for working on this! I'm really excited to bring more async to CP. I think it'll really make it easy and performant to do the multiple things.
I've got a restructuring suggestion to remove the data packing and unpacking. In short, make another function specific macro to handle it for you. My example only showed the one argument case, yours is the kwarg case.
| common_hal_abusio_spi_write_cancel, | ||
| data); | ||
| } | ||
| static MP_DEFINE_CONST_FUN_OBJ_KW(abusio_spi_write_obj, 1, abusio_spi_write); |
There was a problem hiding this comment.
My intention was to introduce corresponding ASYNC macros to match the SYNC ones from MP. This will remove the need to pack and unpack all of the args because you can use a slightly different awaitable for each one. (Or one that has all of the args at the end of a variable length struct.)
| static MP_DEFINE_CONST_FUN_OBJ_KW(abusio_spi_write_obj, 1, abusio_spi_write); | |
| static CIRCUITPY_DEFINE_ASYNC_FUN_OBJ_KW(abusio_spi_write_obj, 1, abusio_spi_write); |
The macro will then assume there are corresponding abusio_spi_write (which calls start), common_hal_abusio_spi_write_cancel and common_hal_abusio_spi_write_end. There is a subtle behavior thing here because CPython validates arguments once the object is awaited, not before. That's why the awaitable is returned before validating args.
|
Thanks for the feedback! |
- Moved dual-task async example into docstring - Removed unneeded examples and setup - Revised naming - Added awaitable spi.lock() method - Moved unpack routings to shared-modules/abusio - Enabled abusio for all rp2xxx builds
|
Thanks for continuing work on this. I'm back from vacation so please let me know how I can help get you figure out the constructor stuff. |
|
Welcome back! I have been tied up at work and have not made tangible progress. I did read up on the ThreadSafeFlag, which is written in Python but based on IoBase. I think instead it would make more sense to treat the whole SPI object as a stream. I'm not sure how that would work with the chip select, but this seems like a pretty big paradigm shift that may be best left for later. I not yet looked at your suggestion to allow named arguments in the macros. Any help here would be appreciated. |
My intent would be to create ASYNC versions of these macros: Lines 377 to 386 in 47a9a55 That way you use almost the same pattern between the two. My example only covered the one argument case but the others should be the same formula with slightly different function signatures. |
common_hal_busio_spi_write_start() starts a write and returns; common_hal_busio_spi_write_end() waits for it without running background tasks, since the caller still holds the bus. The done flag uses the circuitpy_async_flag_t macros from adafruit#11040, so a port can later set it from an interrupt. Ports that have the write set CIRCUITPY_BUSIO_SPI_ASYNC. There is no Python API. raspberrypi: the blocking transfer and the async write share one DMA path. A blocking transfer claims its two channels per call as before; an async write keeps them until deinit. Writes shorter than 32 bytes or from flash/PSRAM stay synchronous. atmel-samd: samd51/same5x only, through shared_dma_transfer_start(). espressif: up to two queued ESP-IDF transactions (8 KB). Writes of four bytes or less, longer writes and word sizes other than 8 bits stay synchronous.
CIRCUITPY_DEFINE_ASYNC_FUN_OBJ_KW(obj, n_args_min, fun) defines a method that returns an awaitable. As with a CPython coroutine, its arguments are stored and only parsed when it is awaited. While an operation runs, the task waits on asyncio's I/O queue: the awaitable is a stream that polls readable once the operation's done flag is set, possibly from an interrupt. CIRCUITPY_DEFINE_ASYNC_POLL_FUN_OBJ_KW is for operations with no interrupt: the task tries again on each pass of the event loop. Based on the awaitable in adafruit#11040.
Implements asynchronous SPI on rp2040 and rp2350.
Created with the help of an LLM - please let me know if specific cleanup is needed.
This has been tested on a rp pico 2 W. The current tests are pretty basic:
async_spi.pyverifies that SPI can send long transfers without blockingspi_mode_test.pychecks that the phase and polarity options work as expectedWe have discussion of async busio over in #10856. What I am looking for here is discussion around