adafruit / circuitpython

CircuitPython - a Python implementation for teaching coding with microcontrollers
https://circuitpython.org
Other
4.11k stars 1.22k forks source link

Add `fifo.InputFIFO` and `fifo.OutputFIFO` #8029

Open tannewt opened 1 year ago

tannewt commented 1 year ago

It is common to have write() and readinto() functions that take in WriteableBuffer and ReadableBuffer. The are some cases where you want to send or receive as you go. We've thought about this mostly in terms of async versions of these APIs. However, I wonder if it'd be ok to accept new FIFO types along side the fixed buffers. The corresponding methods would return immediately when given a FIFO. Then, data would be read or written via the FIFO. By taking in a new type, the old API is preserved and can still be used for fixed length transactions. (Under the hood, we could create a FIFO with the given buffer and then block based on it.)

The reason I'm thinking about this again is that I'm trying to sniff I2C using PIO. I2C traffic is bursty so the PIO's 8 entry FIFO isn't deep enough for me to keep up. If I had an InputFIFO, then I could allocate a larger incoming buffer. busio.UART already does this internally but a generic and common way to do it would be nice. The FIFOs would either use DMA or ringbuf underneath. This is similar to #7452.

Potential API:

class InputFIFO:
  def __init__(self, typecode="B", size=64):
    # Allocates the underlying ring buffer.

  @property
  def overflowed(self):
    # True when data was dropped.

  # Methods like a stream or like a list/array.

class OutputFIFO:
  def __init__(self, typecode="B", size=64):
    # Allocates the underlying ring buffer.

  @property
  def underflowed(self):
    # True when data was dropped.

  # Methods like a stream or like a list/array.

Ideally, I'd be able to do:

fifo_in = fifo.InputFIFO("b", 64)
sm.readinto(fifo_in)
for value in fifo_in:
  print(value)
dhalbert commented 1 year ago

This is an interesting generalization. The FIFO's could optionally be passed an event object that you could use to wake up an asyncio task when there was new data (InputFIFO) or the buffer was empty (OutputFIFO). I had mentioned something previously as a way of async-ifying the existing API's without new direct async def methods.

tannewt commented 1 year ago

@jepler What do you think of this? It could merge nicely with audio and synth stuff.

jepler commented 1 year ago

From what I understand about audio-frequency signal processing it mostly proceeds in lock-step, so fixed sized buffers are more commonly used. Furthermore, there may be a topology in which the output of one block is consumed as inputs by two or more blocks (or, in unusual circumstances, two copies of an output might be consumed by a single block), which is another way in which a FIFO model doesn't seem matched, since only one consumer gets to consume any particular item from a FIFO.

So, based on what I've learned, I'm not immediately seeing how this abstraction is appropriate for future audio work.

tannewt commented 1 year ago

the output of one block is consumed as inputs by two or more blocks

I wonder if a third class could be an intermediary between FIFOs then.

gneverov commented 1 year ago

I implemented something like this for my work on async PIO and audio. The code for the FIFO is here. It's use in PIO is here, and in audio here.

I however create the FIFO internally in the resource (PIO, audio, etc.) and do not expose it to users. This allows using the same old read/write methods, although exposing the FIFO object to Python can help reduce memory copies. If I become unblocked on the async work, you could start seeing this code in a PR.