adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
MIT License
3.97k stars 1.16k forks source link

Add asyncio-awaitable (non-polling) keypad.EventQueue.get() equivalent #8412

Closed davidmcnabnz closed 8 months ago

davidmcnabnz commented 10 months ago

This is a request for the keypad.EventQueue class to offer an asyncio-friendly awaitable alternative to the polling-based .get() method.

This would allow asyncio-based programs to follow a true event-driven programming pattern, and steer away from polling-based logic (which does not encourage good programming habits).

Such an alternative, possibly called keypad.EventQueue.aget(self, timeout=None), would:

dhalbert commented 10 months ago

See https://github.com/adafruit/circuitpython/pull/6712, which I think may do what you want, in particular https://github.com/adafruit/circuitpython/pull/6712#issuecomment-1210090179

davidmcnabnz commented 10 months ago

I tried this approach in my setup:

Adafruit CircuitPython 8.2.6 on 2023-09-12; Raspberry Pi Pico W with rp2040
Board ID:raspberry_pi_pico_w

Sadly, the asyncio task silently terminates when it attempts to await the AsyncEventQueue context (just the instance in this case).

I even tried catching BaseException, but there's no exception - the task simply just disappears.

    async def run(self):
        """                                                                                   
        Use the newer asyncio-based event sourcing                                            
        """
        self.log("run: monitoring buttons")
        self.is_running = True
        self.keysObj = keypad.Keys(self.pins, value_when_pressed=False, pull=True)

        async with AsyncEventQueue(self.keysObj.events) as event_source:
            self.log("run: got event queue context")
            try:
                while self.is_running:
                    self.log("run: waiting for event")
                    ev = await event_source
                    self.log(f"run: ev={ev}")
                    await self.dispatch(ev)
            except BaseException as e:
                self.log(f"run: exception: {e}")
        self.log("run: exiting")

The log output stops after the run: waiting for event line. With the .run() task holding the entire program, its sudden disappearance takes the whole program down.

On the other hand, if I use the old in-Python polling pattern, everything Simply Just Works:

    async def run(self):
    """                                                                                   
        Continually poll all the buttons and invoke callbacks on push and release             
        :return:                                                                              
        """
    self.log("run: monitoring buttons")
    self.is_running = True
    self.keysObj = keypad.Keys(self.pins, value_when_pressed=False, pull=True)
    while self.is_running:
            await self.poll()

    async def poll(self):
    if self.poll_count % 10000 == 0:
            self.log(f"poll: count={self.poll_count}")
    self.poll_count += 1

    evt = self.keysObj.events.get()
    if evt:
            await self.dispatch(evt)
    await asyncio.sleep(0.0001)

Is there anything I'm missing here?

dhalbert commented 10 months ago

Is your AsyncEventQueue what is in the first post in #6712 or is it what is in https://github.com/adafruit/circuitpython/pull/6712#issuecomment-1210090179 ?

davidmcnabnz commented 10 months ago

I'm using the first one from that PR:

class AsyncEventQueue:
    """                                                                                                                                                                                         
    Helper class which moves the polling burden from Python code to underlying C code                                                                                                           
    Reference: https://github.com/adafruit/circuitpython/pull/6712                                                                                                                              
    """
    def __init__(self, events):
        self._events = events

    async def __await__(self):
        yield asyncio.core._io_queue.queue_read(self._events)
        return self._events.get()

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_value, traceback):
        pass
dhalbert commented 10 months ago

Could you try the simpler code in the subsequent post?

davidmcnabnz commented 10 months ago

@dhalbert I just now tried the 'simpler code', and it does work.

    async def run(self):
        """
        Continually poll all the buttons and invoke callbacks on push and release
        :return:
        """
        self.log("run: monitoring buttons")
        self.is_running = True
        self.keysObj = keypad.Keys(self.pins, value_when_pressed=False, pull=True)

        #while self.is_running:
        #    await self.poll()

        event_queue = self.keysObj.events
        while self.is_running:
            while not event_queue:
                await asyncio.sleep(0.0001)

                if self.poll_count % 10000 == 0:
                    self.log(f"poll: count={self.poll_count}")
                self.poll_count += 1

            evt = event_queue.get()
            await self.dispatch(evt)

However, it seems to put me right back into the in-Python polling loop, with no asyncio support.

davidmcnabnz commented 10 months ago

As a side note, my project is a MIDI pedal controller board, being used in live music looping, so pedal pushes need to be detected and actioned as instantly as possible, to avoid loop mis-alignments.

It has helped a lot to switch from round-robin pin polling (in my earlier prototype) to now feeding off keypad events, but I'd still like to be able to eliminate all polling from Python code.

dhalbert commented 10 months ago

@jepler Do you have a comment here? Did I misunderstand what you did in #6712 ?

davidmcnabnz commented 10 months ago

Given that the asyncio task simply "disappears" when using the first helper class in #6712, I'm suspecting an edge case somewhere inside the C code for asyncio.

This suspicion is based on the fact that no exception is being raised.

jepler commented 10 months ago

I just tried the following complete code on Adafruit CircuitPython 8.2.6 on 2023-09-12; Adafruit PyGamer with samd51j19:

import asyncio
import board
import keypad
import digitalio
import gc

async def blink(pin, interval, count):  # Don't forget the async!
    with digitalio.DigitalInOut(pin) as led:
        led.switch_to_output(value=False)
        for _ in range(count):
            led.value = True
            await asyncio.sleep(interval)  # Don't forget the await!
            led.value = False
            await asyncio.sleep(interval)  # Don't forget the await!

class AsyncEventQueue:
    def __init__(self, events):
        self._events = events

    async def __await__(self):
        yield asyncio.core._io_queue.queue_read(self._events)
        return self._events.get()

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_value, traceback):
        pass

async def key_task():
    print("waiting for keypresses")
    with keypad.ShiftRegisterKeys(clock=board.BUTTON_CLOCK, data=board.BUTTON_OUT, latch=board.BUTTON_LATCH, key_count=4, value_when_pressed=True) as keys, AsyncEventQueue(keys.events) as ev:
        while True:
            print(await ev)

async def main():  # Don't forget the async!
    tasks = [
        asyncio.create_task(blink(board.LED, 0.25, 10)),
        asyncio.create_task(key_task())
    ]
    await asyncio.gather(*tasks)  # Don't forget the await!
    print("done")

asyncio.run(main())

This worked with an older version of asyncio, but fails starting with fb068e2f1bbea264dd4a69a3bb973561509d738f (first in asyncio 0.5.18):

fb068e2f1bbea264dd4a69a3bb973561509d738f is the first bad commit
commit fb068e2f1bbea264dd4a69a3bb973561509d738f
Author: Phil Underwood <beardydoc@gmail.com>
Date:   Thu Nov 3 16:25:36 2022 +0000

    Convert IOQueue.read|write to coroutines that await Never before returning.

    Also update streams.py to reflect this and provide example code for using UART, USB_CDC and BLE

 asyncio/core.py             |  8 +++--
 asyncio/stream.py           | 20 ++++-------
 examples/serial_examples.py | 81 +++++++++++++++++++++++++++++++++++++++++++++
 examples/usb_cdc_boot.py    |  9 ++++-
 examples/usb_cdc_example.py | 34 -------------------
 5 files changed, 102 insertions(+), 50 deletions(-)
 create mode 100644 examples/serial_examples.py
 delete mode 100644 examples/usb_cdc_example.py

Modifying AsyncEventQueue like so (__await__ now uses await instead of yield)

class AsyncEventQueue:
    def __init__(self, events):
        self._events = events

    async def __await__(self):                                                  
        await asyncio.core._io_queue.queue_read(self._events)                   
        return self._events.get()                                               

    def __enter__(self):
        return self

    def __exit__(self, exc_type, exc_value, traceback):
        pass

Since the AsyncEventQueue was using an internal implementation detail of asyncio (_io_queue) this sort of breakage is not unexpected.

I have updated the AsyncEventQueue implementation in #6712 to reflect the new structure required. However, it would be nice to figure out where in our libraries the AsyncEventQueue implementation should go, so that users don't have to worry about minutae like this; there's simply no way a non core develop could be expected to sort this out themselves.

anecdata commented 10 months ago

When this gets sorted, it would be nice to have this added to the asyncio learn guide (along with async sockets, if supported).

davidmcnabnz commented 10 months ago

Thanks so much @jepler for your deep-dive on this. TBH the use of yield within an __await__ method did seem interesting.

Changing yield to await inside the helper class got it working.

Now, given that keypad.Keys follows a streaming paradigm, it felt appropriate to rework the helper into an async generator pattern. So I've updated the above to:

class KeysGen:
    """
    Helper class which moves the polling burden from Python code to underlying C code
    Reference: https://github.com/adafruit/circuitpython/pull/6712
    """
    def __init__(self, *args, **kw):
        self._keys = keypad.Keys(*args, **kw)
        self._events = self._keys.events

    def __aiter__(self):
        return self

    async def __anext__(self):
        await asyncio.core._io_queue.queue_read(self._events)
        return self._events.get()

Usage of the above now looks like:

    async def run(self):
        self.is_running = True
        gen = KeysGen(self.pins, value_when_pressed=False, pull=True)
        async for evt in gen:
            await self.dispatch(evt)
            if not self.is_running:
                break

This now works beautifully, and not a poll loop in sight.

Note - my earlier attempt to implement KeysGen as a subclass of keypad.Keys failed with a very weird OSError. To overcome this, I immediately changed it from an is-a to a has-a pattern.

Thanks again everyone for your help with this.

(I'm hoping at some point to tool up for core work - I have done a few years in C and embedded development)

jepler commented 10 months ago

I'm glad this got you un-stuck. async iterators are already beyond my knowledge level but conceptually that makes good sense.

davidmcnabnz commented 10 months ago

@jepler I can't strongly enough recommend learning more asyncio stuff.

The implementation of asyncio in CircuitPython is fairly minimal at the moment. But it works amazingly well. Many of the more familiar CPython facilities like asyncio.Queue are not difficult to implement.

With CircuitPython's strategic decision to exclude interrupts, for anything but the most basic programs, asyncio is the only line of defence against sloppy inefficient poll cycles. It facilitates writing of efficient, elegant and maintainable real-time concurrent processing programs.

dhalbert commented 8 months ago

Closing because I think this is solved.