It could be an option to rename the I2S.TX mode to I2S.TX_ACTIVE but that would be a breaking change, so I don't suggest to do that (but conceptually this naming helps to understand how the modes work).

There may still be the opportunity to implement this breaking constructor change. In both the library and quick reference docs, I2S is clearly called out as a Technical Preview, that may be changed.

The I2S class is currently available as a Technical Preview. During the preview period, feedback from users is encouraged. Based on this feedback, the I2S class API and implementation may be changed.

There may still be the opportunity to implement this breaking constructor change. In both the library and quick reference docs, I2S is clearly called out as a Technical Preview, that may be changed.

Yes, that's true!

Considering what a possible I2C peripheral class would look like, it would probably be machine.I2CPeripheral. But we would definitely keep machine.I2C, ie it would not become machine.I2CController.

With that in mind, it kind of makes sense to have I2S.TX and I2S.TX_PASSIVE because the "master" mode is the default mode and doesn't need to be annotated as such.

But on the other hand, I2S is different to I2C with controller/peripheral, because we are using the same class just in a different mode, and it already has TX/RX mode, unlike I2C.

The main argument for changing I2S.TX to I2S.TX_ACTIVE (and RX to RX_ACTIVE) would be that it forces the user to consider exactly how the device is working, which side of the I2S is driving the WS/CLK lines. And in fact when in RX mode it's not obvious that RX_ACTIVE would be the default (which it currently is). One might think that a receiver is passive by default. So maybe it is worth spelling this out explicitly with the modes, and changing TX/RX.

@miketeachman from my measurements, the rate argument to the I2S constructor seems to have units "frames per second", where a frame is the L and R channel. Eg in stereo, 32-bit mode with a rate of 8000, the SCK clocks at 512kHz, which is 8000 frames/second * 64 bits/frame = 512000 bits/second.

But the docs suggest that it is samples per second.

I think it makes sense to be frames per second, because for rate=44100 you want both L and R channels to sample at that rate.

A CD audio stream is typically described as having a 44.1KHz sample rate, in that each channel is sampled at that rate (just over 2*Nyquist). The bit rate on the disk is 44.1K*16*2. The I2S args match those numbers:

config = {
    'sck' : Pin('W29'),
    'ws' : Pin('W16'),
    'sd' : Pin('Y4'),
    'mode' : I2S.TX,
    'bits' : 16,  # Sample size in bits/channel
    'format' : I2S.STEREO,
    'rate' : 44100,  # Sample rate in Hz
    'ibuf' : BUFSIZE,  # Buffer size
    }

So the value of 44100 is the "audio sample rate" in terms of sampling theory and the frame rate in terms of the data stream. Perhaps the docs should clarify this.

So the value of 44100 is the "audio sample rate" in terms of sampling theory and the frame rate in terms of the data stream. Perhaps the docs should clarify this.

Yes this matches with what I measure.

Codecov Report

Merging #8451 (3c1ef0f) into master (9e3e67b) will not change coverage.
The diff coverage is n/a.

@@           Coverage Diff           @@
##           master    #8451   +/-   ##
=======================================
  Coverage   98.25%   98.25%           
=======================================
  Files         154      154           
  Lines       20275    20275           
=======================================
  Hits        19922    19922           
  Misses        353      353           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 9e3e67b...3c1ef0f. Read the comment docs.

But the docs suggest that it is samples per second.

This is definitely misleading. It would be better to change the docs to read "specifies audio sampling rate (Hz)"

This adds support for passive (aka "slave") mode on stm32 machine.I2S.

I'm curious on the motivation to add support for the passive mode. e.g is MicroPython support needed for I2S DAC devices that only function in a controller mode?

I chose the terminology "passive" instead of "peripheral" because I think "passive" is more descriptive here.

Sorry, I disagree. To me, it seems unnecessary and confusing to add two new terms ("passive" and "active") into the MicroPython lexicon. The existing terms "controller" and "peripheral" accurately describe the I2S operation modes and provide consistency across other machine module classes, such as I2C and SPI. In electrical engineering, the terms "active" and "passive" are used to categorize electrical components.

When looking at how the mode constants are constructed, there could be consideration to be consistent with the macros defined in the STM32 API: I2S_MODE_MASTER_RX , where the SD data direction appears at the end of the term.

Based on this discussion, I suggest the following for the mode argument:

mode = I2S.CONTROLLER_TX
mode = I2S.CONTROLLER_RX
mode = I2S.PERIPHERAL_TX
mode = I2S.PERIPHERAL_RX

or, a more compact version

mode = I2S.CON_TX
mode = I2S.CON_RX
mode = I2S.PER_TX
mode = I2S.PER_RX

I'm curious on the motivation to add support for the passive mode. e.g is MicroPython support needed for I2S DAC devices that only function in a controller mode?

One use case that triggered this PR is to have system-wide synchronisation of the I2S streams with other peripherals. So the I2S clock and WS signal are generated externally, they are fed into the I2S source and sink (both in passive mode) and then these signals also drive other (non-I2S) peripherals so they are in sync with the sampling of the I2S device. (And in this application the non-I2S peripherals cannot be clocked directly from an active I2S device.)

To me, it seems unnecessary and confusing to add two new terms ("passive" and "active") into the MicroPython lexicon.
...
In electrical engineering, the terms "active" and "passive" are used to categorize electrical components

Yes that's a fair point. And in the network and BLE API the term active is also used to enable/disable an interface. So maybe it's not a good idea to overload it even more.

My main reasoning with active/passive is that when two devices are communicating over I2S (eg two pyboards!) neither are really the controller or peripheral, rather they are peers. And then active/passive just refers to which one is driving the lines. But it would be more common to have an actual I2S peripheral (eg mic, speaker), in which case controller/peripheral would make sense.

Also, mode=TX_ACTIVE might be misunderstood as "actively transmitting", ie "on". And the opposite would be TX_INACTIVE.

So I'm happy to use controller/peripheral.

there could be consideration to be consistent with the macros defined in the STM32 API: I2S_MODE_MASTER_RX

I don't think we should look to the STM32 Cube HAL constants to make decisions (that didn't always work out well in the past). But I think you're right that we should put controller/peripheral first in the constant name, because that's conceptually the first thing you need to decide: "am I implementing a controller or peripheral?".

I think CON and PER are too short and confusing if you don't know what they are abbreviations for. So I suggest to go with:

mode = I2S.CONTROLLER_TX
mode = I2S.CONTROLLER_RX
mode = I2S.PERIPHERAL_TX
mode = I2S.PERIPHERAL_RX

Changes unrelated to passive mode are moved to #8457. This PR will be rebased on that once merged.

One use case that triggered this PR is to have system-wide synchronisation of the I2S streams with other peripherals. So the I2S clock and WS signal are generated externally, they are fed into the I2S source and sink (both in passive mode) and then these signals also drive other (non-I2S) peripherals so they are in sync with the sampling of the I2S device. (And in this application the non-I2S peripherals cannot be clocked directly from an active I2S device.)

That's a very interesting I2S system. I was trying to guess what the application does, but ultimately was stumped. Although, I was imagining a retro mirror ball and lights somewhere in the mix :)

I've rebased this on latest master, and changed the names to use controller/peripheral.

Still need to test non-blocking and asyncio mode.

For a regression test, I tested this PR against the examples in https://github.com/miketeachman/micropython-i2s-examples/tree/peripheral-pr, using a Pyboard D SF2. All examples worked.