search

adafruit / circuitpython

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

Why are `audioio` and `audiopwmio` separate? #4257

Open tyomitch opened 3 years ago

tyomitch commented 3 years ago

These two standard modules implement exactly the same API.

None of the boards in https://circuitpython.readthedocs.io/en/latest/shared-bindings/support_matrix.html has both of them. If ever there appears a board that needs both, AudioOut constructor can select the implementation appropriate for the passed pin.

Thus, renaming audiopwmio to audioio, and PWMAudioOut to AudioOut, will not cause any conflicts, and will allow for more portable CircuitPython code, avoiding hacks such as in the example given in https://learn.adafruit.com/adafruit-circuit-playground-express/circuitpython-audio-out

try:
    from audioio import AudioOut
except ImportError:
    try:
        from audiopwmio import PWMAudioOut as AudioOut
    except ImportError:
        pass  # not always supported by every board!
dhalbert commented 3 years ago

audioio might be more properly called audiodacio. This is an interesting idea, if we are willing to hide the implementation under the covers. Another example of such a thing is generic touchio vs chip-specific touchio: we don't provide both, and we hid the choice in compilation options.

@tannewt @ladyada what do you think about hiding whether DAC or PWM is used, and leaving it up to the build flags to pick?

This could be a 7.0.0 breaking change, or we could be back-compatible for a while, and then remove audiopwmio in 8.0.0.

tannewt commented 3 years ago

I'd rather not hide the implementation because it has noticeable quality differences and impacts external design (adding an external filter for PWM audio). This is similar to bitbangio vs busio. It's better to be explicit.

It's important to note I2SOut is identical except for the constructor. Managing what audio form you want to output should be expected.

My preference would be to split touchio instead of modelling after it. I think it's more important for the native API to be clear than to not require two imports.

tyomitch commented 3 years ago

May I comment that displayio transparently supports displays of various colour depth (and therefore, noticeable quality differences) and underlying protocol (SPI / I2C / whatever), not requiring the programmer to be aware of the differences.

Also, managing what audio form to output is trivial when there's only one audio form available on any board: "you can choose any colour so long as it is black", so to say.

tannewt commented 3 years ago

May I comment that displayio transparently supports displays of various colour depth (and therefore, noticeable quality differences) and underlying protocol (SPI / I2C / whatever), not requiring the programmer to be aware of the differences.

You are right that board.DISPLAYIO does handle this for you. It doesn't for external displays though. So, I'd say if there is "built-in" audio output like a headphone jack, then we should have a board.HEADPHONES that is the appropriate audio output object.

Also, managing what audio form to output is trivial when there's only one audio form available on any board: "you can choose any colour so long as it is black", so to say.

I think the above board.HEADPHONES mostly solves what you want. Note that for boards with AudioOut only, we may want to support PWMAudioOut in the future.

Arduino does this magic use a DAC or PWM for analogWrite and I think its misleading because it produces two different signals with no way of knowing from the code. I much prefer them separate for clarity.

dglaude commented 3 years ago

The hardware reference design for the Pico/RP2040 ( https://datasheets.raspberrypi.org/rp2040/hardware-design-with-rp2040.pdf ) has a board in chapter 3 with both analogue PWM and digital I2S.

So hiding the difference between the two technology might not help there.