Skip to content
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

Why are audioio and audiopwmio separate? #4257

Open
tyomitch opened this issue Feb 24, 2021 · 5 comments
Open

Why are audioio and audiopwmio separate? #4257

tyomitch opened this issue Feb 24, 2021 · 5 comments

Comments

@tyomitch
Copy link
Collaborator

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
Copy link
Collaborator

dhalbert commented Feb 24, 2021

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
Copy link
Member

tannewt commented Feb 24, 2021

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.

@tannewt tannewt added this to the Long term milestone Feb 24, 2021
@tyomitch
Copy link
Collaborator Author

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
Copy link
Member

tannewt commented Feb 25, 2021

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
Copy link

dglaude commented Feb 27, 2021

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.

tyomitch added a commit to tyomitch/circuitpython that referenced this issue Apr 3, 2021
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

No branches or pull requests

4 participants