Skip to content

esp32: add support for I2S #4471

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Closed
wants to merge 1 commit into from
Closed

esp32: add support for I2S #4471

wants to merge 1 commit into from

Conversation

miketeachman
Copy link
Contributor

@dpgeorge
Copy link
Member

Thanks very much for this contribution. There's a lot to understand, in particular because of the (non)blocking nature of this peripheral. Ideally you'd want to be able to stream audio (incoming and outgoing) in the background and make sure there are no gaps in the streaming.

Do I understand right that with the current design/implementation, a i2s.write(buf) will (if the DMA is empty) copy buf to the DMA buffer and return straightaway, and then the copying from the DMA to the output of the I2S peripheral happens in the background?

@miketeachman
Copy link
Contributor Author

Thanks for checking out this PR.

i2s.write(buf) works exactly as you indicated - if the DMA memory has enough space, all of buf is copied and the method returns. Copying from DMA memory to the I2S peripheral is always in the background. Similarly, if using a microphone, copying audio samples from the I2S peripheral to DMA memory is done in the background. For the microphone case, the i2s.readinto(buf) method is used to pull data out of DMA memory. The I2S constructor sets up the DMA memory - after the constructor completes, DMA operation is completely autonomous to the MicroPython runtime.

The optional timeout argument is interesting. With timeout=0, both the write and readinto methods are prevented from blocking - I have found this quite useful for uasyncio-based designs. With no blocking, an I2S service coro can afford to yield to the uasyncio scheduler. The autonomous I2S DMA operation keeps going in the background while the waiting coros run. The I2S service coro has to be given enough runtime to prevent overrun or underrun of the DMA memory.

I'm currently working on a MicroPython-based project called Street Sense which uses an I2S microphone. Using usayncio and this machine.I2S class, it's been possible to realize a gapless stream of microphone audio samples to a WAV file on an SDCard, while at the same time reading other sensors, logging to a SDCard, and updating an OLED display. The DMA background buffering is the key enabler for this functionality, which buffers audio samples when the SD Card is busy allocating blocks, other coros run, or when MicroPython is running a GC.

@junhuanchen
Copy link

Very useful module! XD!

@dpgeorge
Copy link
Member

Thanks @miketeachman for the explanation, it's clear why DMA is necessary.

I was thinking more about the general approach to asynchronous peripherals. I didn't make any good conclusions but had the following set of loose notes/ideas:

  • The concepts of non-blocking and asynchronous are related but not the same. Non-blocking here means a result is either immediately available (or actionable), or there is nothing and it returns straightaway. Async means something happens in the background and there needs to be "synchronisation points" between the background processing and the foreground logic. Buffering + non-blocking (like done in I2S here) can be considered asynchronous.
  • The I2S read behaviour in this PR is the same as existing UART read behaviour: the UART has an RX buffer which is filled in the background and then a synchronous uart.read() call will retrieve all available data. There's also a UART timeout which will tell it how long to wait for any incoming data before timing out and returning (which defaults to 0, non-blocking).
  • UART write is blocking, unlike I2S write behaviour here which buffers the output.
  • Consider a UART with a very slow baudrate (and a fast MCU): you don't want to waste time waiting for each character to be sent out, you want to queue the data and then do something else while its written out. If it were just non-blocking but without a buffer then it'd write a single character and return, and the code would do something else until it could write another character, etc. This is essentially a single-character buffer and is not very efficient. So it really needs to be fully asynchronous with a TX buffer, ability to query the status and cancel the write.
  • The buffer for read/write could either be a dedicated internal buffer (like with UART read, and with the I2S code here, the DMA buffers) or it could keep a reference to a user supplied buffer (eg when the buffers are huge and only one could fit in memory).
  • Consider WLAN.scan(): this is blocking but would benefit from a non-blocking / async version. Need a way to start the scan, query for status, cancel, and get results as they come in.
  • Consider connecting to WiFi: this is already an async API with WLAN.connect() to initiate the async connection, WLAN.status() to query status, WLAN.disconnect() to initiate an async disconnect (a non-async interface doesn't really make sense here, since connection persists for a very long time).
  • Consider SPI.write(framebuf) with a huge buffer (and potentially slow clock): there's no sense to buffer it, not enough RAM. Non-blocking doesn't really mean anything here because all it can do (synchronously) is either nothing or write the whole thing. So really needs to be asynchronous, with start, stop, status and repeat options.

To make it easier to understand, I'd consider the I2S implementation here basically a UART with an RX and TX buffer. That makes it pretty clear how it behaves, and how UART could be extended (with a TX buffer) to have similar async behaviour.

But when large buffers are involved I don't think it makes sense to have internal buffers allocated by the peripheral. Instead it should use the data buffer that is passed in (either to the read or write method). And for full flexibility one should be able to queue multiple buffers, and have callbacks for certain events. A simple API might then look like this:

SPI.awrite(list_of_buf, loop=False) # set queue of buffers and start write, optionally loop
SPI.areadinto(list_of_buf, loop=False) # set queue of buffers and start read, optionally loop
SPI.astatus() # return the index in list of the current buffer being read/written
SPI.apause() # pause output
SPI.aresume() # resume after pause
SPI.irq(handler, SPI.ASTART) # callback when a new buffer is started read/written

Some examples:

# display with double buffer continuously updating
spi.awrite([buf0, buf1], loop=True)
# update buf0 while buf1 is written, and vice versa

# audio streaming
i2s.awrite([buf0, buf1, buf2, buf3], loop=True)
# fill buffers that aren't currently being written out

@miketeachman
Copy link
Contributor Author

miketeachman commented May 24, 2019

Thanks for sharing these discussion points. I was hoping for some critical thought and suggestions on improving this PR.

I like the idea of moving the I2S implementation proposed in this PR to an asynchronous design. It makes sense to get the I2S implementation handling large buffers, rather than forcing a uPy application to slice a buffer and incrementally feed it to the I2S implementation.

What if it was possible to implement I2s with StreamWriter and StreamReader?:

for example, in machine_i2s.c, implement an i2s stream protocol:

STATIC const mp_stream_p_t i2s_stream_p = {
    .read = machine_i2s_stream_read,
    .write = machine_i2s_stream_write,
    .ioctl = machine_i2s_stream_ioctl,
    .is_text = false,
};

With uasyncio implementation (setup details omitted):
await i2s.awrite([list of buffers], loop=True)

awrite works in the background, performing gapless filling of DMA buffers. Returns when the entire buffer(s) has been copied to DMA memory.

careful awrite implementation seems important - awrite will need to yield to the scheduler at various points when the buffer(s) are being streamed to the peripheral. Otherwise, waiting coros are potentially blocked for too long. Awrite could likely yield after X samples have been written to the peripheral, or after a DMA buffer is emptied. Does that make sense?

In CircuitPython there is an I2SOut class with a looping construct.
https://circuitpython.readthedocs.io/en/3.x/shared-bindings/audiobusio/I2SOut.html
The focus of I2SOut is streaming samples contained in a single file (either one-shot, or looping). This is a good implementation for most hobbyists, but will far short for an application like streaming audio from an internet radio source.
Do you think it is valuable to follow the CircuitPython interface definition when possible?

@blmorris did a ton of work on I2S for the pyboard (circa 2015). He likely has some valuable contributions to this discussion.

Misc thoughts:

  • I see UARTs as more transactional peripherals. Application writes to the UART, then waits for a response. Streaming audio appears different - writing is continuous. Same for reading. Perhaps I2C, SPI can also be modeled as transactional?
  • I tend to view dev work thru the agile MVP viewpoint. Get "just enough" to users, get feedback, adapt,
    and release the next iteration. With an agile approach, I would be hesitant to support multiple buffers in the first release of I2S. e.g. Start with a single buffer, then add multiple buffers if the uPy community really needs them. Same idea for other options that might be proposed. Start small, expand when needed.
  • Do you think a uasyncio-only implementation would be adequate for a first release? If users really want to use I2S they can learn a bit of async design methods - it's not too hard.

Overall, I'm quite open to rethink and rework everything I've done so far. I was recently successful to order a PyBoard D so I can work on that port as well. I'd like to see the I2S API be consistent for both ESP32 and PyBoard D (potentially ESP8266 as well). Ideally, no port-specific configuration, or only some minimal port-specific options in the constructor.

@dpgeorge
Copy link
Member

What if it was possible to implement I2s with StreamWriter and StreamReader?

It's not a good idea to tie the peripheral behaviour to a particular async IO event loop, because:

With uasyncio implementation (setup details omitted):
await i2s.awrite([list of buffers], loop=True)

awrite() here must return a generator, and that generator must yield IOWrite objects. This is very tight coupling to a particular async IO implementation. Furthermore, the argument/payload of IOWrite must be a pollable object that is writable only when the I2S peripheral can make further progress with a write without blocking.

I fully support making I2S work with uasyncio (or another async framework) but all it really needs is to make the I2S object itself pollable (for read/write) and to have non-blocking read/write methods (which would queue data down to the peripheral). Then a simple bit of Python code would adapt it to run with uasyncio.

The focus of [CircuitPython's] I2SOut is streaming samples contained in a single file (either one-shot, or looping). This is a good implementation for most hobbyists, but will far short for an application like streaming audio from an internet radio source.
Do you think it is valuable to follow the CircuitPython interface definition when possible?

Honestly, no. I think it's too restrictive to just write a single buffer, as you mention.

I see UARTs as more transactional peripherals. Application writes to the UART, then waits for a response. Streaming audio appears different - writing is continuous. Same for reading.

I tend to agree, I2S/audio is something that runs continuously in the background at a fixed sample rate, and must be fed (or drained) to keep it happy. So it's a question of how to do that feeding.

Do you think a uasyncio-only implementation would be adequate for a first release?

I think we should make it adaptable to uasyncio, but not require it (and this is not difficult to do).

I'd like to see the I2S API be consistent for both ESP32 and PyBoard D

I agree. Also PYBv1.x.


How memory is managed is really important to how the API will work. In the simple case we can let the peripheral allocate the memory (on esp32 it must because the memory must be DMA capable). It has a ring buffer of blocks of a fixed size which are continuously read/written. The I2S object is writable when there is at least one block free, and readable when at least one is full and available to read from. The user then reads/writes using their own user-space buffer. Eg:

# rxblock is RX block size, rxnum is number of RX blocks; same for tx
i2s.init(id, sck, sd, ws, rxblock, rxnum, txblock, txnum, ...)
i2s.deinit()

i2s.start() # must be explicitly started
i2s.pause()

i2s.write(block_buf) # copy block to internal tx ring buffer
i2s.readinto(block_buf) # copy internal rx ring buffer to our block

The i2s object is also pollable for read/write: it's readable/writable when there is at least one block available. I guess write and readinto will be non-blocking by default. Probably blocking behaviour (with timeout) is not needed, at least in the first instance.

The above is pretty much exactly the same as the PR here. I guess it'd work quite well. The only downside is that it's not as efficient as it could be with memory, because the peripheral allocates RAM internally, and also the user must have their own buffer memory (at least one block in size).

An alternative would be to allow the user to get access to the internal buffers to read/write them directly. Eg:

# init, deinit, start, pause: same as above, buffers allocated internally

block_buf = i2s.get_tx_block() # must be a block available (check by polling)
block_buf[:] = ... # write to it
i2s.put_tx_block(block_buf) # queue for writing

block_buf = i2s.get_rx_block() # must be a block available (check by polling)
print(block_buf) # do something with it
i2s.put_rx_block(block_buf) # finished, can be reused by driver

It might not be necessary to have put_tx_block() and put_rx_block(): instead the driver just assumes they will be ready when it comes around to them next time in the ring buffer, and it's up to the user application to make sure they are ready by that time.

The first approach is definitely simpler, and for small enough blocks (eg 1024 bytes) doesn't waste too much memory.

Also, irq's can be added which fire when tx/rx is ready so one can use I2S in an interrupt based system if needed.


Using the above, uasyncio support can be done as follows (schematically):

def stream(i2s):
    while True:
        buf = ... # prepare buffer
        yield IOWrite(i2s)
        i2s.write(buf)

And with IRQs:

def write_next(i2s):
    buf = ... # prepare buffer
    i2s.write(buf)
i2s.irq(write_next, i2s.WRITABLE)

@miketeachman
Copy link
Contributor Author

miketeachman commented Jun 15, 2019

@dpgeorge Your comments helped a lot to give me some technical direction. Thanks !

I decided to build a working prototype based around the ideas you discussed below. I limited the prototype to the ESP32 and only the reading of audio samples. I'm hoping you can comment on it to see if this step aligns with the direction you're thinking...

I fully support making I2S work with uasyncio (or another async framework) but all it really needs is to make the I2S object itself pollable (for read/write) and to have non-blocking read/write methods (which would queue data down to the peripheral). Then a simple bit of Python code would adapt it to run with uasyncio.

Here are the main changes to the PR (and supporting modules), with link references to the commits in github.

Here is how the prototype works in a uPy application:

uasyncio:

import uasyncio as asyncio  # fast_io implementation preferred
from machine import I2S
audio=I2S(...)
buf = bytearray(number)

async def read_samples():
    while True:
        numread = await sreader.readinto(buf)
        <do something with audio samples in buf>

sreader = asyncio.StreamReader(audio) 
loop = asyncio.get_event_loop(ioq_len=1)  # ioq_len is fast_io specific
loop.create_task(read_samples())  
loop.run_forever()

non-blocking read:

from machine import I2S
audio=I2S(...)
buf = bytearray(number)

while True:
        numread = audio.readinto(buf)
        <do something with audio samples in buf>

Discussion:

  • I tried to keep the I2S API "platform neutral" - I think it should work for both PyBoards and Espressif products.
  • For the ESP32 implementation, there is no out-of-the-box way to poll the Espressif I2S driver for DMA data in ioctl(). (Drat!)The ESP-IDF UART driver has this, but unfortunately Espressif did not put this into their I2S driver. That led me to develop the "read peek" extension in the ESP-IDF I2S. The peek implementation seems clean and robust and levers the existing private data structures in the I2S driver. But, is it feasible? How supportive is Espressif for PRs to make a timely extension of a driver?
  • I think there is a "plan B" approach to polling in ioctl() that does not require an ESP-IDF extension. But, it's not elegant. You would try to read one sample from the I2S peripheral. If successful, that would indicate data is available. That one sample would be saved in the I2S object, and later added to the application buffer when the subsequent read() method is called. not pretty, but it should work and not affect performance.
  • changing the I2S write capability is very similar to the read. e.g. add a "write peek" extension to the ESP-IDF. And, implement the stream protocol write() in machine_i2s.c.
  • performance with uasyncio/fast_io was quite good provided that an IO queue is used. I was able to run gapless @ 44000 samples/s on the read (but no other processing of samples ...)
  • the uasyncio fast_io library is quite valuable for audio streaming. This library allows IO devices to work within a higher priority queue. This is quite important to support gapless audio streaming at higher bitrates, especially when there are other competing co-routines.
  • is it feasible to add readinto() to uasyncio? It appears to integrate seamlessly with the stream_readinto() function in stream.c.
  • as per your suggestion, I eliminated the "timeout" argument in the read() method to make it always non-blocking.
  • observation: the "richness" of the exception handling messages in the I2S.readinto() method is degraded by the stream implementation which only supports a numeric error code. This is a relatively minor point.
  • the I2S driver in ESP-IDF supports callback event handlers when 1) a DMA read buffer is filled and 2) a DMA write buffer is emptied. This could likely be used to implement the interrupt implementation you discuss.

What do you think? is this headed in the right direction?

My next step might be to attempt an implementation of this read prototype on a PyBoard V1.1 to verify that the I2S methods are really portable.

@peterhinch
Copy link
Contributor

performance with uasyncio/fast_io was quite good provided that an IO queue is used.

If you don't specify an IO queue the fast_io fork behaves as per the official version with regard to I/O scheduling, so this is expected behaviour. (I'm glad to see the fork is doing its job for I2S).

is it feasible to add readinto() to uasyncio?

I could certainly look at adding it to fast_io if your driver meets the following requirements.

The StreamReader.read(n) method calls the device's read(n) method. By the same token a readinto(buf, nbytes) method would expect to call a similar method of the device. For asyncio friendliness this device method should be nonblocking (it will only be called if the ioctl indicates a read data ready status). It should return the number of bytes read; this would then be returned by StreamReader.readinto(buf, nbytes).

@peterhinch
Copy link
Contributor

peterhinch commented Jun 16, 2019

I have pushed a version of fast_io which supports StreamReader.readinto(buf, nbytes=-1). It returns the number of bytes read, and calls your readinto method which should have a similar call signature, modelled on that for the UART. Your method should perform validation on the args, which I pass on verbatim.

I think this is a worthwhile and simple extension but have no means of testing it with real hardware: please report your results. If successful I'll document this as a permanent feature.

[EDIT]
I should add that if your readinto method returns 0 an assertion failure will be raised. If this occurs it means that either:

  • There is a bug in my code.
  • Your ioctl is reporting data available when it is not.
  • Your readinto method is returning 0 after ioctl has reported read data ready.

@miketeachman
Copy link
Contributor Author

miketeachman commented Jun 16, 2019

Thanks for adding this to your library! I had time for a quick browse and I'm 99% sure it's going to work with the stream capability that I added to the I2s module prototype. For the prototype, I forked fast_io and added the readinfo method, modeled using the streamreader read method and matching the interface called out by the stream_readinto method in py/stream.c. It is close to your implementation. In the last post I made in this discussion thread I included a link to the readinto code changes I made in the fast_io fork. Could you check it out and see what you think?

tomorrow I'll have time to pull in the fast_io changes and try them with my hardware setup. I'll report back what happens.

@peterhinch
Copy link
Contributor

peterhinch commented Jun 17, 2019

@miketeachman Sorry, I hadn't spotted your implementation. It looks good. But on further thought neither of our solutions has sufficient generality.

My solution is a simplification too far, because someone might use the method with a wrapped socket object: usocket.socket does support readinto(buf, nbytes). For generality we need the optional nbytes arg.

Your device driver has the option of not supporting this. You could simply decree in your docs that StreamReader.readinto(buf) may only be used with the default which will fill the buffer. Your device driver's readinto method could continue to support only a single buf arg. So if someone passes an nbytes value a TypeError will be thrown.

I'll shortly post an update based on your solution with nbytes added.
[EDIT]
Now done. Comments/test results welcome!

@miketeachman
Copy link
Contributor Author

miketeachman commented Jun 18, 2019

@peterhinch I pulled in your updates and tested it with the ESP32 hardware+INMP441 Microphone. It worked 100% for both the usayncio and the non-uasyncio calls to the readinto method. I tested with and without the nbytes arg. All worked.

It turns out that there is no need to detect the nbytes arg in the I2s driver. The nbytes arg ends up getting supported by the I2S driver as a side effect of adding a stream protocol to the driver - that's because the stream_readinto method in py/stream.c supports nbytes and the I2S readinto method is mapped to the stream_readinto method.

Thanks a lot for working on this! It will be great to use the fast_io master and not my fast_io fork.

note: all that debugging code I added to Streamreader.readinto can be stripped out. It was only in there to help me figure out how uselect works with uasyncio and streams.

@peterhinch
Copy link
Contributor

Thank you for testing. I have pushed an update removing the debug statements and some code comments which were duplicated from .read. The version no. is bumped to 0.25. I have documented it (.FASTPOLL.md).

You may need to prompt @dpgeorge on your other queries as we have rather monopolised the discussion since you raised them :)

@miketeachman
Copy link
Contributor Author

miketeachman commented Sep 24, 2019

updated PR commit to fix Makefile conflict

@MrSurly
Copy link
Contributor

MrSurly commented Sep 24, 2019

Just as an aside, there's a small project I did that utilizes the I2S support. Works great. Thanks for your hard work.

@AutomationD
Copy link

Gentle bump. Can this be merged please?

amotl added a commit to daq-tools/pycom-micropython that referenced this pull request Nov 19, 2019
amotl added a commit to daq-tools/pycom-micropython that referenced this pull request Nov 19, 2019
This adds I2S support from Genuine MicroPython by Mike Teachman.
Thanks, @miketeachman!

See also micropython/micropython#4471
amotl added a commit to daq-tools/pycom-micropython that referenced this pull request Dec 3, 2019
This commit adds I2S support from Genuine MicroPython by Mike Teachman.

See also micropython/micropython#4471
@tve
Copy link
Contributor

tve commented Feb 7, 2020

This is interesting, but from reading just the first PR comments it's evident that the interface is not simple and there are no docs?

@MrSurly
Copy link
Contributor

MrSurly commented Feb 7, 2020

PR comments it's evident that the interface is not simple and there are no docs?

Why not fork it and commit docs?

@tve
Copy link
Contributor

tve commented Feb 7, 2020

PR comments it's evident that the interface is not simple and there are no docs?

Why not fork it and commit docs?

'cause I'm not a hired doc writer ;-) I strongly believe that it's the responsibility of the author of a PR to write docs, i.e., to write down his/her intentions and how the thing is to be used. I'm happy to word-smith and provide suggestions, specially for non-english speakers.
(I'm actually of the old school of thought that writing the docs first avoids a lot of poor API designs 'cause you realize early on what makes sense in other contexts and minds and what doesn't.)

@miketeachman
Copy link
Contributor Author

@tve just want to check that you've seen these docs:
https://github.com/miketeachman/micropython-esp32-i2s-examples

That was my attempt to describe the API and give how-to examples.

I totally agree with you about writing docs first. As a project manager that was my go-to technique to get a workable API from the dev team. If the doc writer had troubles to write the documentation it usually meant that the API needed work.

Do you think that I should update the official MicroPython docs and update this PR? I'm happy to make this effort.

I'm about to start the next steps on this PR, including some simplification of the API, and making a common API across Espressif and Pyboard dev boards. More soon ...

@tve
Copy link
Contributor

tve commented Feb 8, 2020

I see, no, I didn't see those docs :-) If I were DG I wouldn't commit the PR without the docs, but I'm not DG :-) If I were you I wouldn't do the work to move the docs without an agreement that the PR is going to be committed, but I'm not you either :-) :-).

amotl added a commit to daq-tools/pycom-micropython that referenced this pull request Feb 27, 2020
This commit adds I2S support from Genuine MicroPython by Mike Teachman.

See also micropython/micropython#4471
amotl added a commit to daq-tools/pycom-micropython that referenced this pull request Mar 6, 2020
This commit adds I2S support from Genuine MicroPython by Mike Teachman.

See also micropython/micropython#4471
@miketeachman
Copy link
Contributor Author

I did the git submodule update before this for esp-idf. This is not my skill set so I am struggling. sorry to be such a pain.

@mtasatcom No worries. You're not alone. It's not easy to get a working uPy build, and adding a PR increases the difficulty!

I have two pre-built binaries in a google drive folder that might be easier to start with. There are binaries for both psram (WROVER module) and non-psram (WROOM module) versions.
https://drive.google.com/open?id=1AlhGXK56J0r8fLR3sivNQ3nvgz57CibY

@eliclement
Copy link

I did the git submodule update before this for esp-idf. This is not my skill set so I am struggling. sorry to be such a pain.

@mtasatcom No worries. You're not alone. It's not easy to get a working uPy build, and adding a PR increases the difficulty!

I have two pre-built binaries in a google drive folder that might be easier to start with. There are binaries for both psram (WROVER module) and non-psram (WROOM module) versions.
https://drive.google.com/open?id=1AlhGXK56J0r8fLR3sivNQ3nvgz57CibY

Thanks for sharing the builds Mike! Is this an async compatible version? I was first interested in testing your version (branch: esp32-i2s-june-6-2019-stream-experiment) but could not merge/build it successfully with the 1.12 version...
Hopefully the non-blocking I2S version can be merged soon for STM/ESP32. Great work so far!

@miketeachman
Copy link
Contributor Author

Is this an async compatible version?

Unfortunately I haven't done any more work on the uasyncio compatible version of this PR. However, there are some workarounds to adapt this PR to work with uasyncio. I use uasyncio extensively in another project called Streetsense. Here's link to the location in code where a microphone reading loop yields back to the scheduler, allowing other coros to run.
https://github.com/miketeachman/micropython-street-sense/blob/master/streetsense.py#L960
It's a bit clunky buy it works. The readme of this project outlines the uasyncio related libraries that are used.
https://github.com/miketeachman/micropython-street-sense

@eliclement
Copy link

Is this an async compatible version?

Unfortunately I haven't done any more work on the uasyncio compatible version of this PR. However, there are some workarounds to adapt this PR to work with uasyncio. I use uasyncio extensively in another project called Streetsense. Here's link to the location in code where a microphone reading loop yields back to the scheduler, allowing other coros to run.
https://github.com/miketeachman/micropython-street-sense/blob/master/streetsense.py#L960
It's a bit clunky buy it works. The readme of this project outlines the uasyncio related libraries that are used.
https://github.com/miketeachman/micropython-street-sense

Very nice project, congrats! This async code really helps a lot thanks.
Do you still have intentions to implementing I2S using Stream?

@miketeachman
Copy link
Contributor Author

Very nice project
thanks!
Do you still have intentions to implementing I2S using Stream?
Definitely. I'm interested in feedback on how people want this to work for them. Is it possible to describe your ideal I2S interface specification that would work with asyncio?

@nevercast
Copy link
Contributor

I'm currently implement I2S/DAC support in a C Module for MicroPython so that I may play small prebaked audio files on a device. This is for audible readouts of system status where other means of information is inaccessable (i.e. a display). I have WAV audio baked into the C module as char buffers, each field being a single word like, "One", "Point", "Three", etc.

While looking for information on how to get the I2S driver working with internal DAC in mono mode on Pin 25 as I still wish to maintain GPIO use of Pin 26, I stumbled across https://github.com/miketeachman/micropython-esp32-i2s-examples/ and in turn, this PR.

I'm happy to see that although it was created a year ago it's not totally dead, for me, I only need the means to initailize the driver, and write a single buffer that should be immediately played; non-blocking almost essential. An IRQ on play completion is also essential for my current implementation where, once an IRQ is fired for buffer underrun, I queue the next "file".

My project does not use asyncio and it would be a large effort to implement it now, since the project is already many thousands of lines of Python deep and product launch is end of year. I can imagine moving I2S out of a C Module and into Python would be a welcome change by the rest of the team, many of them not familiar with C or the IDF or MicroPython C internals.

I would not want to pull this PR into our own steam, maintaining a fork with extra functionality is expensive and time consuming when also trying to keep up to date with upstream master.

Is it reasonable to expect to see this feature land in master in the near future, and perhaps I can assist in some way in my personal unpaid time to bring that closer to reality. What's missing? Is it just suitable documentation? Damien's last comment in May '19 seemed to end on a target API, has that API been achieved?

@miketeachman
Copy link
Contributor Author

Is it reasonable to expect to see this feature land in master in the near future, and perhaps I can assist in some way in my personal unpaid time to bring that closer to reality. What's missing? Is it just suitable documentation? Damien's last comment in May '19 seemed to end on a target API, has that API been achieved?

@nevercast
Hi Josh,
I'm quite sure that I2S support will eventually be supported in the mainline releases. But, this PR needs more work to be accepted. As you suggested the I2S API needs more attention. This week I'm going to open a new discussion on the MicroPython forum to get feedback on the API. The API might look like this:

  1. Buffer oriented read and write methods, with 3 variants:
  • blocking
  • non-blocking with callback (this would serve your needs)
  • non-blocking with uasyncio (e.g. uses stream protocol, supporting ioctl, read, write)
  1. Stream oriented read and write methods

The API needs to be identical (or nearly identical) for the ESP32 and Pyboards.

I welcome help on this work ! I'll update here when the forum discussion is posted

@nevercast
Copy link
Contributor

Hi agian @miketeachman,

Looking through your examples, and it seems neither the examples nor readme give mention of using the internal DACs of the ESP32. Am I missing the example for this or is this not an intended usecase/not supported?

I don't wish to use an external I2S slave, instead I wish to use the internal DAC with I2S peripheral for non-blocking DMA.

I'd be interested to hear what concludes of the forum discussion too, thanks!

@miketeachman
Copy link
Contributor Author

miketeachman commented Jun 11, 2020

sorry, I misread your first post.

I don't wish to use an external I2S slave, instead I wish to use the internal DAC with I2S peripheral for non-blocking DMA.

For my immediate plans, I will not be implementing I2S driving the internal DAC. That would be a good feature to consider after all the I2S features are completed that support external I2S devices. The Lobo ESP32 port supports I2S/DAC. https://github.com/loboris/MicroPython_ESP32_psRAM_LoBo/wiki/dac

@dalex-at-gallifrey
Copy link

hi,
can anyone unpack this for me a bit?
"make a custom MicroPython build and integrate a pull request into the build"

i'm not very familiar with github processes, and i've spent a few days trying to decipher what i'm supposed to do.
i have the tool chain installed on an ubuntu virtual machine, and can 'make' the micropython binaries, including adding of c-code to run the ttgo display. and successfully flash the device. that's my level. the ttgo display is the reason why i don't want to use the prebuilt binaries supplied by mike.

for the current task, i have cloned the i2s repository locally, at the same folder level as the 'micropython' repository (i don't know if this is where is should be, or inside the 'micropython' folder somewhere?). even reading through other's problems with PR on this page, i still can't figure out the next steps. do i need to upload my custom micropython repository to github, or can integrate the i2s code locally. no idea!
david.

@miketeachman
Copy link
Contributor Author

@dalex-at-gallifrey
I think the easiest way for you to integrate the PR is to make some simple edits to 3 files in your build and add 1 file. The repo you cloned will include the edits and file you need

There is just one commit for this PR.
The commit will show the edits you need to make.
b45217e

The three files that need edits are:
Makefile: add one line
modmachine.c: add one line
modmachine.h: add one line

Add the following file:
machine_i2s.c

Then make and hopefully it will build cleanly, with the I2S feature in the resulting firmware binary.

Good luck!

@dalex-at-gallifrey
Copy link

@miketeachman

I think the easiest way for you to integrate the PR is to make some simple edits to 3 files in your build and add 1 file.

hey thanks mike! in my exhaustive search of this maze, i was definately on the wrong side, heading back towards the entrance.
should be simple from here with your helpful reorientation.

@dalex-at-gallifrey
Copy link

@miketeachman

from machine import I2S

worked a treat. thanks again.

@mdaeron
Copy link

mdaeron commented Aug 10, 2020

I've been playing a bit with machine.I2S on a TinyPICO and it works great, thanks for implementing it.

Quick question: if for some reason my code exits without calling I2S.deinit(), after I soft-reboot the board (crtl-D in REPL) I get the following message:

ValueError: I2S port is already in use

Apart from a hard reboot (unplugging and replugging my USB cable), is there a more elegant way to deinit() I2S?

@aguaviva
Copy link

Thanks guys for pushing this forward! I can't wait to see this in a build :)

@aguaviva
Copy link

Hey, what is missing in this MR? We are almost there!! :)

case ESP_OK:
break;
case ESP_ERR_INVALID_ARG:
mp_raise_msg(&mp_type_OSError, MP_ERROR_TEXT("I2S driver install: Parameter error"));
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hey Mike,

FYI

I encountered compilation errors that I traced back to the double-spaces in your error text messages (in this line for example the double space between "install:" and "Parameter").

Specifically: I'm running my own branch from v1.13 and hand-edited the one-line changes to modmachine.h, modmachine.c and the Makefile, and downloaded machine_i2s.c into the code base.

Launching the compilation process gave me the following error:
...
GEN build-TINYPICO/genhdr/compressed.collected
QSTR not updated
Compressed data updated
GEN build-TINYPICO/genhdr/compressed.data.h
Traceback (most recent call last):
File "../../py/makecompresseddata.py", line 205, in
main(sys.argv[1], word_compression)
File "../../py/makecompresseddata.py", line 165, in main
compressed_data = fn(error_strings)
File "../../py/makecompresseddata.py", line 78, in word_compression
return "".join(w[:-1] + "\{:03o}".format(0b10000000 | ord(w[-1])) for w in index)
File "../../py/makecompresseddata.py", line 78, in
return "".join(w[:-1] + "\{:03o}".format(0b10000000 | ord(w[-1])) for w in index)
IndexError: string index out of range
...

I traced this down to the index array variable inside makecompresseddata.py whose last element was empty quotes '' instead of a keyword. This empty quotes value itself comes from the top128 array a few lines earlier where there are 12 instances of this empty quotes values counted in the collection. The 12 instances relate to the 12 instances of double-spacing in the error messages of machine_i2s.c. Replacing those 12 instances with single spaces resolved the compilation issue.

top128 prints out as:
[('invalid', 30), ("can't", 35), ('must', 30), ("'%q'", 27), ('not', 32), ('object', 18), ('argument', 14), ('arguments', 11), ('function', 12), ('format', 15), ('be', 30), ('error', 15), ('convert', 11), ('for', 20), ('to', 25), ('Wifi', 15), ("'%s'", 14), ('keyword', 9), ('required', 8), ('of', 20), ('I2S', 15), ('string', 9), ('is', 19), ('specifier', 6), ('supported', 6), ('type', 11), ('index', 9), ('length', 7), ('range', 8), ('Invalid', 6), ('with', 9), ('%d', 14), ('Undocumented', 4), ('in', 14), ('arg', 10), ('empty', 7), ('implemented', 4), ("isn't", 7), ('multiple', 5), ('a', 18), ('assignment', 4), ('identifier', 4), ('or', 12), ('positional', 4), ('already', 5), ("doesn't", 5), ('Parameter', 4), ('UUID', 7), ('attribute', 4), ('expecting', 4), ('generator', 4), ('out', 8), ('syntax', 5), ('too', 8), ('expected', 4), ('from', 6), ('SPI', 7), ('allowed', 4), ('an', 9), ('and', 7), ('complex', 4), ('field', 5), ('has', 7), ('integer', 4), ('missing', 4), ('needs', 5), ('no', 9), ('outside', 4), ('set', 7), ('tuple', 5), ('unknown', 4), ('unsupported', 3), ('value', 5), ('%s', 8), ('Failed', 4), ('buffer', 4), ('memory', 4), ('number', 4), ('int', 6), ('name', 5), ('operation', 3), ('pin', 6), ('specified', 3), ('Error', 4), ('Internal', 3), ('found', 4), ('install:', 3), ('nonlocal', 3), ('param', 4), ('sequence', 3), ('valid', 4), ('Not', 5), ('bad', 5), ('between', 3), ('but', 5), ('indices', 3), ('struct:', 3), ('%q', 6), ('args', 4), ('characteristic', 2), ('dict', 4), ('list', 4), ('long', 4), ('mode', 4), ('on', 6), ('only', 4), ('zero', 4), ('Cannot', 3), ('TCP/IP', 3), ('before', 3), ('driver', 3), ('import', 3), ('method', 3), ('specification', 2), ('values', 3), ('incompatible', 2), ('issubclass()', 2), ('after', 3), ('bytes', 3), ('class', 3), ('given', 3), ('key', 4), ('local', 3), ('non-keyword', 2), ('store', 3), ('use', 4), ('wrong', 3), ('', 12)]

and index prints out as:
['invalid', "can't", 'must', "'%q'", 'not', 'object', 'argument', 'arguments', 'function', 'format', 'be', 'error', 'convert', 'for', 'to', 'Wifi', "'%s'", 'keyword', 'required', 'of', 'I2S', 'string', 'is', 'specifier', 'supported', 'type', 'index', 'length', 'range', 'Invalid', 'with', '%d', 'Undocumented', 'in', 'arg', 'empty', 'implemented', "isn't", 'multiple', 'a', 'assignment', 'identifier', 'or', 'positional', 'already', "doesn't", 'Parameter', 'UUID', 'attribute', 'expecting', 'generator', 'out', 'syntax', 'too', 'expected', 'from', 'SPI', 'allowed', 'an', 'and', 'complex', 'field', 'has', 'integer', 'missing', 'needs', 'no', 'outside', 'set', 'tuple', 'unknown', 'unsupported', 'value', '%s', 'Failed', 'buffer', 'memory', 'number', 'int', 'name', 'operation', 'pin', 'specified', 'Error', 'Internal', 'found', 'install:', 'nonlocal', 'param', 'sequence', 'valid', 'Not', 'bad', 'between', 'but', 'indices', 'struct:', '%q', 'args', 'characteristic', 'dict', 'list', 'long', 'mode', 'on', 'only', 'zero', 'Cannot', 'TCP/IP', 'before', 'driver', 'import', 'method', 'specification', 'values', 'incompatible', 'issubclass()', 'after', 'bytes', 'class', 'given', 'key', 'local', 'non-keyword', 'store', 'use', 'wrong', '']

I'm no expert, and am open to the idea that I have some other local (self-induced!) issue causing this error - but I'm surprised this compilation error has not happened with other users since the 12 instances should appear in most compilation runs and therefore manifest the error. This is probably more a case of some latent fragility within makecompresseddata.py (a split() on space gone wrong?) that only manifests when enough instances of this empty string make it into the top 128 words; but it seems easy enough to remove the double-spacing in your error messages to prevent future headaches anyway?

Hope this helps! Thanks for your I2S code! - I look forward to giving it a spin ...

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for taking the time to report this problem. I changed all instances of double-spaces to single-spaces, then rebased to the latest commit. Hopefully this change fixes the build problem.

self->id = i2s_id;

// is I2S peripheral already in use?
if (self->used) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Hi again Mike.

Is it a deliberate design choice to raise this exception in this way? I'm encountering the same error as reported by #4471 (comment) where I can't re-create my I2S object after a soft-reset because the port is reports as already in use (and in my use case, users can repeatedly upload new programs (similarly to the REPL + Ctrl-D) that triggers a soft reset of uPython).

I haven't yet thoroughly searched for some kind of static class method I can call to deinit/reset the I2S peripheral before I re-create my uPython I2S instance (and a quick scan of your examples repo seems to require an existing I2S object instance to call deinit() from, which I don't have post-reset) - so instead for now I just commented out this exception and now I can happily re-create the I2S object after a soft reset without issues. Doing this might however be a Bad Thing (tm) for reasons that are currently not obvious to me!

Feel free to let me know if I'm doing something stupid, but if not might I suggest a note under your esp32-i2s-examples about how to handle creating an I2S instance if the underlying hardware peripheral is still active from a previous initialization (ie: what should I do with the exception getting thrown if the I2S port is reported as already active?), and perhaps (maybe a horrible hack) a flag in the constructor parameters to choose to ignore the error and finish the constructor initialization code regardless?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can see how this exception is causing problems to upload new programs. I eliminated all checks around mapping an I2S hardware peripheral to an I2S instance. The exception no longer happens when re-uploading a program. There is a downside to this change -- it's now possible to instantiate multiple I2S objects that use the same I2S hardware peripheral. But, that situation doesn't cause problems, and other MicroPython machine modules already allow this.

The PR has been updated. I included a workaround for a bug that appears in a specific version of the ESP-IDF (v4.0.2).

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would tie into soft-reset and reinit I2S, I believe this solves the "re-upload" issues. https://github.com/micropython/micropython/blob/master/ports/esp32/main.c#L115

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I would tie into soft-reset and reinit I2S

That is totally the best way to solve the problem. I didn't know there was a hook to do this. Thanks a ton for reviewing this change and making the suggestion.

Copy link
Contributor Author

@miketeachman miketeachman Mar 1, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The previous changes were backed out and a new module init routine was added, that is called from the point of soft reset. A better implementation of a bug workaround was added as well. Rebased and squashed commits into a single commit. Tested with two supported ESP-IDF versions, v4.0.2 and v4.1.1

@@ -347,6 +347,7 @@ SRC_C = \
modesp32.c \
espneopixel.c \
machine_hw_spi.c \
machine_i2s.c \
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Note for anyone willing to compile their own build from master branch and merging in this PR:
The Makefile has changed a lot on master, and this line should now make it to ports/esp32/main/CMakeLists.txt in the set(MICROPY_SOURCE_PORT ...) instruction:

set(MICROPY_SOURCE_PORT
    ...
    ${PROJECT_DIR}/machine_i2s.c
)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

thanks for flagging this. I updated the PR to work with the new cmake build

Copy link

@olance olance Feb 25, 2021

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the amazing work!
Being able to compile the firmware and have sound flowing through my I2S DAC/Amp to a speaker in record time was quite a joyful moment ^_^

Usage guide and examples available at:
https://github.com/miketeachman/micropython-esp32-i2s-examples

UPDATE:  May 13, 2020
fixed compile errors caused by breaking change in MicroPython master

breaking change introduced in commit def76f on Apr 4, 2020
"all: Use MP_ERROR_TEXT for all error messages."

UPDATE:  Jan 14, 2021
fixed a conflict in modmachine.c

Update:  Feb 9, 2021
Removed double spaces in Exception error msgs
- to eliminate user reported compilation errors

Update:  Feb 25, 2021
esp32:  added machine_i2s.c to cmake build
esp32/i2s: apply formatting to machine_i2s.c

Update:  Feb 28, 2021
Initialize I2S module on a soft reset
Add workaround for I2S bug in ESP-IDF v4.0.2
(espressif/esp-idf#6625).

Signed-off-by: Mike Teachman
@cgreening
Copy link
Contributor

I think this PR has been superseded by this one: #7183?

Should this one be closed as it's a bit confusing?

@miketeachman
Copy link
Contributor Author

I think this PR has been superseded by this one: #7183?

This PR is now obsolete. A new PR for I2S on MicroPython is available that offers a simpler API, additional modes of operation, and support for Pyboards. Please refer to PR: #7183

This PR can be closed.

@dpgeorge dpgeorge closed this May 11, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.