8000 stmhal: I2S (Inter-IC-Sound) peripheral support for pyb module by blmorris · Pull Request #1361 · micropython/micropython · GitHub
[go: up one dir, main page]

Skip to content

stmhal: I2S (Inter-IC-Sound) peripheral support for pyb module #1361

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 33 commits into from

Conversation

blmorris
Copy link
Contributor
@blmorris blmorris commented Jul 2, 2015

This PR is almost certainly not yet suitable for merging; there is still much to do but I would appreciate any feedback regarding style, design, implementation, or even just how to make it more 'pythonic' before I push it much further.
While there are still some details to implement, the I2S class in this PR is reasonably complete.
pyb.I2S() accepts a list or tuple of pin names to configure an I2S port; this port can then be initialized as either a Master or a Slave.
It is possible to create a 16-bit (half-word) array and transmit it in Master mode using i2s.send(arr)
and see the transfer on a logic analyzer; i2s.recv() also works although without being fed by an external codec there will be no data to read. Slave modes also work (testing them on my own hardware) but require an external codec to provide clock signals.
Please see comments for detailed usage, but for a quick start the following code should work, the data loopback is implemented by jumping pin X8 (B15, transmit) to X7 (B14, receive). Note that sometimes the send_recv command needs to be run a few times before it works consistently; I am investigating this.

import micropython
micropython.alloc_emergency_exception_buf(100)

import pyb, array

pins = ['B13', 'B12', 'B15', 'B14']
i2s = pyb.I2S(pins)
i2s.init(i2s.MASTER)
arr = array.array('h')
for i in range(32):
    arr.append(i + 512)

b = bytearray(64)
i2s.send_recv(arr, b)

Ultimately my goal with this project is to be able to do something like this and have it run in the background, playing audio while other processing can proceed:

i2s = pyb.I2S(pins, pyb.I2S.MASTER)
audio_file = open('/sd/my_audio_file.wav', 'rb')
play_my_audio_file(audio_file, i2s)

There are a few things in the critical path to get this working; specifically the DMA callback functions need to be implemented (I'm trying to figure out how to do this, could use some help ;-) ) and getting the SD Card working with DMA might help too (I'll look into this.)

There are several TODO's noted in i2s.c, and I expect others to come up as well.

Finally, I am planning to design a py-skin daughterboard with an I2S codec to help other people use I2S on the pyboard - it shouldn't take me nearly as long to design as this code did ;) Ping me if you are interested in such a board.

@dhylands
Copy link
Contributor
dhylands commented Jul 2, 2015

The DMA callbacks will wind up being interrupt handlers. You can look at the timer class for an example of how to call into python from C.

https://github.com/micropython/micropython/blob/master/stmhal/timer.c#L1335-L1351

@blmorris
Copy link
Contributor Author
blmorris commented Jul 6, 2015

Implemented a few of the smaller changes suggested by @dhylands

Unless someone suggests otherwise, I am going to plan on #1362 or something very close to it being implemented. Based on that, I will also implement a new pin_find_af_type() function in pin_named_pins.c as suggested in #1361 (comment)
This will allow me to make the pin validation logic substantially simpler and more robust by checking directly whether a pin supports a required function rather than comparing it to a hardcoded set of pin identifiers.

@dhylands
Copy link
Contributor
dhylands commented Jul 6, 2015

Looking at pin_defs_stmhal.h (https://github.com/micropython/micropython/blob/master/stmhal/pin_defs_stmhal.h#L51-L80) I made the PIN_TYPE_Xxx for each peripheral type start at zero and increment by 1.

This means that a pin_find_af_type function would need to take the pin, the pin function, and the pin type as arguments.

If, instead, we made all of the PIN_TYPE_xxx be unique (get rid of all of the = 0's) then the pin_find_pin_type function would only need to take a pin, and a PIN_TYPE_xxx constant.

The field to hold this is 8 bits wide:
https://github.com/micropython/micropython/blob/master/stmhal/pin.h#L38

Anwyays, I just thought I'd throw that out as a discussion point.

@dpgeorge
Copy link
Member
dpgeorge commented Jul 6, 2015

If, instead, we made all of the PIN_TYPE_xxx be unique (get rid of all of the = 0's) then the pin_find_pin_type function would only need to take a pin, and a PIN_TYPE_xxx constant.

@dhylands sounds good. Does it mean we can then remove AF_FN_xx constants, and the ->fn field?

@dhylands
Copy link
Contributor
dhylands commented Jul 6, 2015

If, instead, we made all of the PIN_TYPE_xxx be unique (get rid of all of the = 0's) then the pin_find_pin_type function would only need to take a pin, and a PIN_TYPE_xxx constant.

@dhylands sounds good. Does it mean we can then remove AF_FN_xx constants, and the ->fn field?

The timer code still uses AF_FN_TIM (https://github.com/micropython/micropython/blob/master/stmhal/timer.c#L873), but pin_find_af could be changed to accept a range of types to search for instead.

I don't think that there would be any net savings in flash usage though (the single byte saved would convert into a pad byte).

Hmm. There are 143 AF lines (for the PYBV10 build), each of which has a pointer to a register. There are only 26 unique registers referenced.

grep '  AF' pins_PYBV10.c | cut -d, -f5 | cut -d' ' -f2 | sort | uniq | wc -l

So if we replaced the 4-byte register pointer with a 1-byte register index, we save (143-26)x4 = 468 bytes with a small performance penalty (indirect lookup of register). Minus the slight increase in code size for the places that the indirection occurs.

@dhylands
Copy link
Contributor
dhylands commented Jul 6, 2015

Actually, we should check to see if those registers are even used at all. I know we use the gpio register, but it was factored out into the pin.

A quick test suggests that the register isn't used by C code at all. We do expose it to python though.

@dpgeorge
Copy link
Member
dpgeorge commented Jul 6, 2015

stmhal is not so strict on code size, so this would be more for code simplification than anything else.

Probably best to start with just making PIN_TYPE_xxx unique as you suggest, then go from there.

@blmorris
Copy link
Contributor Author

A few incremental commits addressing some of the suggestions raised above; in particular the hard-coded pins. (It will take a few days before I can work on it more.)
I also now need to figure out why the compile fails for the STM32F4DISC board.
Mostly I wanted to post at this point to ask @dhylands what you think of the GPIO initialization in https://github.com/blmorris/micropython/blob/i2s/stmhal/i2s.c#L217-L229
Instead of using the usual method of setting
GPIO_InitStructure.Alternate = ALTERNATE_FUNCTION_MACRO, I extract the alternate function data using pin_find_af() and then set GPIO_InitStructure.Alternate = (uint8_t)af->idx
Not as self-documenting as using the macros, but I thought is was a clean solution.

@dhylands
Copy link
Contributor

@blmorris Using af->idx looks right to me. However, you should probably have a check that af is non-NULL. If pin_find_af returns NULL because the desired alternate function wasn't found then you don't want to dereference af (i.e. use af-idx).

The reason your STM32F4DISC build is failing is because #define HAL_I2S_MODULE_ENABLED is commented out in the stmhal/boards/STM32F4DISC/stm32f4xx_hal_conf.h file.

@blmorris
Copy link
Contributor Author

@dhylands - Thanks again Dave. Fixed the STM32F4DISC as you suggested.

I don't check that the af is non-NULL here in the initialization because I have code to validate all of the pins as having I2S af's in pyb_i2s_make_new() (https://github.com/blmorris/micropython/blob/i2s/stmhal/i2s.c#L515-L599)

The I2S object should only be created with a pre-validated list of I2S pins. I could add the check in the GPIO initialization section of i2s_init() as well, but it seemed redundant to me.

@dhylands
Copy link
Contributor

The I2S object should only be created with a pre-validated list of I2S pins. I could add the check in the GPIO initialization section of i2s_init() as well, but it seemed redundant to me.

@blmorris That's just the defensive programmer side of me showing up. Personally, I'd still probably put a check, or at the very least an assert, just to cover yourself for changes in the future.

@blmorris
Copy link
Contributor Author

Progress on i2s streaming from an audio data file - I have hacked together a stream_out function which takes a handle to an open wave file and streams it over I2S to an audio codec.
The good news is that it seems to work somewhat reliably - I can listen to it! :-) … but the bad news is that once an i2s stream has started I can't do anything else in micropython without triggering a hard crash.

The code is quite hacky at this point, and I may find the problem while cleaning it up to push here for review, but I am hoping that someone else might see what I am doing wrong from a quick description.

The current implementation is rather simple and may be too naive. The I2S object struct now contains a pair of 2 kByte arrays (actually 2 pairs, one each for tx and rx) which are the ping-pong double buffers. The stream_out function verifies that the file handle is a readable stream, and reads 2048 bytes into the first array. This array gets passed to the HAL_I2S_Transmit_DMA function (which immediately returns b/c it is non-blocking) and then the next 2048 bytes are read into the second half of the double buffer.

The rest of the action takes place in the HAL_I2S_TxCpltCallback function which gets called when a transmit buffer is empty. The callback function passes the second half of the double buffer (which is filled by now) to HAL_I2S_Transmit_DMA, and then reads the next 2048 bytes to refill the half of the buffer which was just emptied. A ping-pong pointer gets toggled each time the callback executes to tell which buffers to read and to fill on the next execution.

This all works, and the audio playback is fine and glitch-free so far. I can confirm that I am getting 93.75 callbacks per second, which is exactly what is expected (2048 bytes is 512 16-bit stereo samples, 48kHz audio / 512 = 93.75). The question is why it crashes as soon as any other command gets issued from the REPL. (Hitting enter on an empty line does not cause a crash; I am using UART(4) for the REPL rather than USB.)

Thanks for reading, and I will post the actual code as soon as I can.

@danicampora
Copy link
Member

Hey Bryan congratulations on getting it to work! Your implementation as you explain it sounds logical. The reason of the crash is probably easy to find once you clean up the code a bit.

On Aug 21, 2015, at 8:40 PM, Bryan Morrissey notifications@github.com wrote:

Progress on i2s streaming from an audio data file - I have hacked together a stream_out function which takes a handle to an open wave file and streams it over I2S to an audio codec.
The good news is that it seems to work somewhat reliably - I can listen to it! :-) … but the bad news is that once an i2s stream has started I can't do anything else in micropython without triggering a hard crash.

The code is quite hacky at this point, and I may find the problem while cleaning it up to push here for review, but I am hoping that someone else might see what I am doing wrong from a quick description.

The current implementation is rather simple and may be too naive. The I2S object struct now contains a pair of 2 kByte arrays (actually 2 pairs, one each for tx and rx) which are the ping-pong double buffers. The stream_out function verifies that the file handle is a readable stream, and reads 2048 bytes into the first array. This array gets passed to the HAL_I2S_Transmit_DMA function (which immediately returns b/c it is non-blocking) and then the next 2048 bytes are read into the second half of the double buffer.

The rest of the action takes place in the HAL_I2S_TxCpltCallback function which gets called when a transmit buffer is empty. The callback function passes the second half of the double buffer (which is filled by now) to HAL_I2S_Transmit_DMA, and then reads the next 2048 bytes to refill the half of the buffer which was just emptied. A ping-pong pointer gets toggled each time the callback executes to tell which buffers to read and to fill on the next execution.

This all works, and the audio playback is fine and glitch-free so far. I can confirm that I am getting 93.75 callbacks per second, which is exactly what is expected (2048 bytes is 512 16-bit stereo samples, 48kHz audio / 512 = 93.75). The question is why it crashes as soon as any other command gets issued from the REPL. (Hitting enter on an empty line does not cause a crash; I am using UART(4) for the REPL rather than USB.)

Thanks for reading, and I will post the actual code as soon as I can.


Reply to this email directly or view it on GitHub.

@blmorris
Copy link
Contributor Author

Thanks Daniel! I could imagine a use case for this on the WiPy eventually, but first I need to get it working on the audio equipment I have already built.
Problem not solved yet, but here is my first attempt. See the stream_out function, stream_in is still just a stub. I know it may be difficult to test without an actual audio codec to stream to.

@blmorris
Copy link
Contributor Author

@dhylands - I just went back to re-read the first comment you left regarding implementing DMA callbacks in #1422, I suspect that I need to put in the nlr push and gc lock stuff - I overlooked that on my first draft. I'll try again in a few hours.

@blmorris
Copy link
Contributor Author

The i2s stream_out function that 8000 I am working on seems to have a basic incompatibility with the rest of MicroPython and I would appreciate any help to try to figure it out and resolve it.

At this point the stream function works as I described it a few days ago and a few comments up. I should note that it does work, in that it will playback a clean and glitch-free audio stream; the problem is that the whole system crashes as soon as any command is entered at the REPL.

As far as I can tell my relatively simple code implements the same basic procedure as the audio streaming examples in STM's HAL Cube package: a transmit is initiated by calling HAL_I2S_Transmit_DMA; the end of the transfer triggers the HAL function I2S_DMATxCplt which performs some housekeeping tasks before calling the user-defined function HAL_I2S_TxCpltCallback
My code for HAL_I2S_TxCpltCallback calls HAL_I2S_Transmit_DMA on the next buffer and reads the file stream to refill the buffer that was just emptied before returning.

I have tried several approaches to get this code to play nice with the rest of MicroPython:

  • First I tried wrapping my callback with the nlr_push/pop and gc_lock/unlock functions (in different combinations as well); this doesn't seem to be correct because it causes the crash to happen immediately instead of when a command is entered at the REPL. Also, the other example that I could find where the HAL callbacks are used in uPy is USB, and nlr/gc aren't used there.
  • I also tried removing the file read from the callback. In this case, the HAL_I2S_Transmit_DMA just sends the same buffer over and over; the crash still comes when a command is entered at the REPL.

Essentially every permutation that I have tried which calls HAL_I2S_Transmit_DMA from the DMA callback causes MicroPython to lock up, even when it is playing audio correctly. I can put simple statements in the callback - printf() or led toggles to indicate that something is happening - but in these cases the callback only gets triggered once. Whenever I have DMA callbacks running continuously in the background, any uPy command crashes the system as soon as it is interrupted by a DMA callback.

I hope that there is something simple and fairly obvious that I am missing here.

I should also note that I have all USB functionality turned off for these tests; the REPL is running on UART(4)

@dpgeorge
Copy link
Member

You are allocating your dma state on the stack (in pyb_i2s_stream_out, DMA_HandleTypeDef tx_dma;). When that function returns tx_dma is will be overwritten by the next function call, and the DMA IRQ handler will see garbage data. You only see this when you run a command at the REPL because that's the time when it clobbers tx_dma.

Solution is to put tx_dma in the pyb_i2s_obj_t instance. That instance is going to take quite a bit of ram! It can be optimised because the ST HAL is a bit bloated, but for now it's important to get the functionality.

@blmorris
Copy link
Contributor Author

@dpgeorge - Thanks! I'm off to a meeting now, will try this as soon as I get back. This feels like the right answer, the explanation fits what I'm seeing.

@blmorris
Copy link
Contributor Author

BINGO!!! :-)
That was it. I can play audio, execute commands from the REPL, and maintain background tasks using timer-scheduled callbacks (in this case, reading the voltage off the volume pot and setting the gain in my audio codec via I2C) all at the same time.

Still much to do - gracefully stopping the I2S transfer and closing the file stream at the end of the track is the first on the list ;) - but now it feels like I can get the rest of it done.

@dpgeorge -

Solution is to put tx_dma in the pyb_i2s_obj_t instance. That instance is going to take quite a bit of ram! It can be optimised because the ST HAL is a bit bloated, but for now it's important to get the functionality.

Adding in a pair of DMA_HandleTypeDef structs (tx_dma and rx_dma) won't make the biggest difference at this point: currently I have four audio data buffers attached to the instance (double buffers each for Rx and Tx) totaling 8192 bytes. I added a few sizeof statements to my code to find out exactly how big everything is:

pyb_i2s_obj_t size = 8484 bytes
DMA_HandleTypeDef size = 84 bytes
I2S_HandleTypeDef size = 72 bytes

The audio buffers could probably be 1/2 or 1/4 that size - I will need to experiment to find out - but I don't see using less than 512 bytes per buffer since that is the SD card block size.
Overall I don't think this is really a problem - at any time there can only be at most one or two I2S objects, and if you are using them it is probably the single most important thing your system is doing. I kind of expect that most people won't even have I2S compiled in to their firmware - at least not until I start sending I2S codec pyskin daughterboards out.

@dpgeorge
Copy link
Member

This is great that you have it working, it'll be a very good model for getting background DMA streaming working in other peripherals like SPI, I2C and UART.

in this case, reading the voltage off the volume pot and setting the gain in my audio codec via I2C

Can you elaborate on this: do you have a pyb.Timer instance calling a Python function on an interrupt which then uses pyb.ADC to read the pot, and then pyb.I2C to set the volume? All done in an interrupt? If so that is very neat.

@blmorris
Copy link
Contributor Author

Oops, my mistake. What we have is actually more mundane than that, just reading the pot and relaying it out through the DAC:

#volume control:

tim = Timer(1)
tim.init(freq=20)
dac = DAC(1)
pot = ADC('B0')
tim.callback(lambda t: dac.write(int(pot.read()>>4)))

I do wonder if a similar trick could work using I2C.

@dpgeorge
Copy link
Member

Ok, I see. Well, good that it works so well! (you probably don't need the int).

@blmorris
Copy link
Contributor Author

@dpgeorge -

Can you elaborate on this: do you have a pyb.Timer instance calling a Python function on an interrupt which then uses pyb.ADC to read the pot, and then pyb.I2C to set the volume? All done in an interrupt? If so that is very neat.

So, as I said in my previous message, I'm not actually doing that. However, the following code does do that (read the ADC and send the data via I2C all within a callback), and on my board it appears to work perfectly well:

tim = Timer(1)
tim.init(freq=100)
pot = ADC('B0')
tim.callback(lambda t: i2c.mem_write(pot.read()>>4, 0x34, 0, addr_size=1
67E6
6))

It doesn't do anything useful yet - I would need to reprogram the audio codec / DSP chip to make it work - but the callbacks run and I can demonstrate with either a scope or a logic analyzer that one byte of variable data is being sent. Pretty cool after all! It would be even better if I could use a bytearray to embed the variable data byte into a longer data buffer without allocating any memory; no idea if this could work, but I'll try it when I get a moment.

you probably don't need the int

Yes, looking back at some of the uPy code that I started to write last year I can find a lot of awkward and redundant code. On the bright side, that means I'm getting better at this ;)

@dpgeorge
Copy link
Member

It would be even better if I could use a bytearray to embed the variable data byte into a longer data buffer without allocating any memory;

Yes, you can do this. I'll give you a hint:

def set_volume(t, buf=bytearray(2)):
    # the buf variable is like a local static variable in C: it's only available
    # in this function and is the same from one call to the next

You could make buf a global variable, but making it static-local like above is neater.

@blmorris
Copy link
Contributor Author

@dpgeorge - Okay, I guess you are suggesting something like this:

def set_volume(t, buf=bytearray(3)):
    buf[0] = 0xa5
    buf[1] = t >> 4
    buf[2] = 0x5a
    return buf

tim = Timer(1)
tim.init(freq=100)
pot = ADC('B0')
tim.callback(lambda t: i2c.mem_write(set_volume(pot.read()), 0x34, 0, addr_size=16))

Just an example for demonstration - the extra numbers in the bytearray don't mean anything here - but it does work as shown and is a good model for something I could use. Pretty slick!

Since the restriction against allocating memory in a callback does trip users up, it might help to add a page to the documentation with examples of some relatively sophisticated things that can be done in a callback.

Getting pretty off-topic here, but at least its helping me.

from the stack to the i2s instance object
when tx_stream reaches end of data.

Also initial commit for i2s.stream_in(); not working yet.
…work!

Some comment changes, also debug statement to figure out why receive to file
wasn't working - the problem is found, but requires a major refactor to fix.
…nd duplex to work

Biggest functional change is that I2S DMA transfers will restart automatically
if a callback occurs prematurely.
Both functions can be started and stopped independently of each other.
Comment cleanup in preparation for review and merging.
Also implements Python-defined callbacks for stream and buffer
transfers that finish without being exlpicitly stopped
Can now do the following with stream_in and stream_out:
stream_in(f) or stream_in(f, len=n_samples)
stream_out(f) or stream_out(f, len=n_samples)
stop(), stop('stream_in'), stop('stream_out')
stream_in and stream_out can operate concurrently, and can be
started and stopped independently of each other
@blmorris
Copy link
Contributor Author
blmorris commented Dec 2, 2015

@Dylands: I just rebased and pushed my i2s branch; it still throws the same error.

My python source files are at https://github.com/blmorris/uPy_AudioCodec/src; the relevant files are codec.py and boot.py, codec_skin.py is an older version that isn't needed.
I also provided a copy of the non-working firmware in the firmware directory along with a previous working firmware. I am regretting not pushing a more recent working firmware, but the one that is there will work with the codec daughterboard and the codec.py that I provided.

You will need to replace the line

f = open('/sd/5_15-48kHz.wav', 'rb')

in codec.py with the name of a 16-bit stereo .wav file that you put on the SD card. If you only have wave files encoded at 44.1kHz, then you can also change the line

i2s.init(i2s.MASTER, mclkout=1)

to

i2s.init(i2s.MASTER, mclkout=1, audiofreq=44100)

@blmorris
Copy link
Contributor Author
blmorris commented Dec 2, 2015

To answer your other questions: no, not using the DAC code here, but I am using I2C in a callback in separately from I2S.

I will try your change to nlr_jump_fail now.

EDIT: I get OSError: 5 now

MicroPython v1.5.1-87-gcd55569-dirty on 2015-12-02; PYBv1.0 with STM32F405RG
Type "help()" for more information.
>>> import codec
26
96
>>> FATAL: uncaught exception 20003740
OSError: 5

FATAL ERROR:

@dhylands
Copy link
Contributor
dhylands commented Dec 3, 2015

I was able to reproduce the problem. I modified nlr_raise in nlr.h to look like:

#ifndef DEBUG
#define nlr_raise(val) do { printf("nlr_raise: %s: %d\n", __FILE__, __LINE__); nlr_jump(MP_OBJ_TO_PTR(val)); } while (0)
#else

and I see this:

>>> import codec
26
96
>>> nlr_raise: i2s.c: 1167
FATAL: uncaught exception 20003760
OSError: 5

FATAL ERROR:

Now to figure out why.

@dhylands
Copy link
Contributor
dhylands commented Dec 3, 2015

And changing sdcard to not use DMA/IRQs I get:

>>> import codec
26
96
>>> Callback!

so my immediate guess is that it has to do with the relative IRQ priorities.

@blmorris
Copy link
Contributor Author
blmorris commented Dec 3, 2015

@dhylands: Thanks, that's great! I disabled DMA in sdcard.c by changing lines 215 and 248 from

if (query_irq() == IRQ_STATE_ENABLED) {

to

if (0) {

and that got my I2S code working again. With that I can poke around and see if I can tweak things to get my code compatible with DMA on the sdcard.
Did you get to listen to the system too?

@dhylands
Copy link
Contributor
dhylands commented Dec 4, 2015

The issue is that the i2s code that is reading the file is being invoked from a DMA IRQ callback. Currently all DMA streams run at the same interrupt priority, so while the DMA callback is being invoked, all other DMA interrupts are disabled. Since the sdcard I/O is also using DMA interrupt, this is why there is a problem. A hack to get this to work was to lower the interrupt priority on the I2S streams from the 5 to 6.
I added this to dma.h:

uint8_t dma_get_irqn(DMA_Stream_TypeDef *dma_stream);

Added this to dma.c:

uint8_t dma_get_irqn(DMA_Stream_TypeDef *dma_stream) {
    int dma_id = get_dma_id(dma_stream);
    return dma_irqn[dma_id];
}

and this to i2s.c (after the dma_init calls in the i2s_init function):

        HAL_NVIC_SetPriority(dma_get_irqn(i2s_obj->tx_dma_stream), 6, 0);
        HAL_NVIC_SetPriority(dma_get_irqn(i2s_obj->rx_dma_stream), 6, 0);

and then i2s works while using DMA for the sdcard.

Another solution is that we can add a query_irq_level function and instead of checking if interrupts are disabled we query which level of interrupts are disabled and use the non DMA path if the DMA level (or higher priority) is disabled.

@blmorris
Copy link
Contributor Author
blmorris commented Dec 7, 2015

@dhylands: Thanks, I can confirm that setting the priority to 6 fixes I2S for me; using priority 5 in the same code reverts to the old error.

Functionality is restored to the point when I made this comment - #1361 (comment) - in a way that is compatible with the updated SD card code using DMA. So far, so good.

At this point, it seems that the options moving forward are to pursue something like the Software ISR solution; the system process/thread approach mentioned by @dpgeorge here: f4c1737#commitcomment-14798648; or put it on a queue to be processed later as Dave suggests in the next comment.

For the WiPy, if this eventually gets ported there, stream management can be delegated to the underlying RTOS.

@blmorris
Copy link
Contributor Author

I'm going to close this PR for now. It has served my purpose of developing a proof of concept, but the branch has gone stale in the couple of months since I last worked on it, and will require some restructuring to bring it in line with recent changes to master. I'll start this restructuring in a new branch so that I can keep this one as a working reference.

One question for @dpgeorge @dhylands and @danicampora - One thing that I implemented early on in my I2S work was a mechanism for passing a list of pins to the constructor, checking that they were valid for I2S, and configuring the port based on the set of pins passed (i.e. whether the port was simplex or duplex, and which pin was RX or TX was determined by the number and order of pins in the list. This was to support an audio processor board which I developed which is hard-wired to use a different set of pins for I2S2 than are chosen for SPI2 on the pyboard; I wanted to have the code be compatible with either board.)

One of the features that seemed to have consensus agreement in the new HW API discussion at #1430 was that HW port constructors should default to a specific set of pins, but also have the option to pass a list to configure a custom set of pins.
Since then, actual development in master has taken a different route, so that for most HW modules the default set of pins is now defined by macro in stmhal/boards/BOARD_NAME/mpconfigboard.h rather than in the module code itself.

I am content to follow this convention for default pins; my board has its own mpconfigboard.h so this does keep things simple. I just want to ask if there is any interest in pursuing the option to pass a pin list, and if so, if I should try to develop the code that I wrote into a general pin validation function that could be used by other HW modules as well.

@blmorris blmorris closed this Jan 19, 2016
@dhylands
Copy link
Contributor

It looks like the machine API proposes passing in pins=(A, B, C, D)
https://github.com/micropython/micropython/wiki/Hardware-API#use-cases

If no pins were passed in then it would use the default pins for UART0 as defined in mpconfigboard.h

I definitely think that it makes sense to specify the defaults in mpconfigboard.h.

dpgeorge pushed a commit that referenced this pull request Jul 5, 2021
This commit adds I2S protocol support for the esp32 and stm32 ports, via
a new machine.I2S class.  It builds on the stm32 work of blmorris, #1361.

Features include:
- a consistent I2S API across the esp32 and stm32 ports
- I2S configurations supported:
  - master transmit and master receive
  - 16-bit and 32-bit sample sizes
  - mono and stereo formats
  - sampling frequency
  - 3 modes of operation:
    - blocking
    - non-blocking with callback
    - uasyncio
  - internal ring buffer size can be tuned
- documentation for Pyboards and esp32-based boards
- tested on the following development boards:
  - Pyboard D SF2W
  - Pyboard V1.1
  - ESP32 with SPIRAM
  - ESP32

Signed-off-by: Mike Teachman <mike.teachman@gmail.com>
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.

5 participants
0