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

Add I2SInOut library #227

Merged
merged 2 commits into from
Dec 5, 2024
Merged

Add I2SInOut library #227

merged 2 commits into from
Dec 5, 2024

Conversation

relic-se
Copy link
Contributor

A general purpose driver for I2S devices. Allows read-only and bidirectional operation. Write-only operation is allowed as well, but not recommended in lieu of audiobusio.I2SOut. Requires CircuitPython 9.2.1.

Because this library doesn't operate for a single device, it may be better placed within helpers. I'm also open to renaming this library in case it may create confusion with future additions to the CircuitPython core.

@FoamyGuy
Copy link
Contributor

I asked about the naming of this library during the weekly meeting today. The consensus was that it would be good to change the name slightly, but not necessarily due to possible conflict with a future core module.

Can you rename the module to pio_i2sinout please? That way it will be apparent from the name that it's only supported on Raspberry Pi pico MCUs with PIO. This would follow essentially the same naming convention used by https://github.com/adafruit/Adafruit_CircuitPython_PIO_UART albeit with the "adafruit" pre-prended as well since it's an Adafruit library.

@FoamyGuy
Copy link
Contributor

We also discussed helpers vs. drivers a bit, I think helpers is most appropriate for this library, though this one does sit sort of in the middle in that it's code that interfaces with hardware, but since it supports more than a single device helpers is more appropriate. PIO_UART was added under helpers in the adafruit bundle, so it'll be good to be consistent with that one.

@relic-se
Copy link
Contributor Author

I love the suggestions, @FoamyGuy ! I'll make those adjustments soon to the library to get it more consistent with the practices of PIO_UART.

@relic-se
Copy link
Contributor Author

@FoamyGuy The library has been updated accordingly. The only additional change is that I removed "InOut" as to more closely match PIO_UART. Usage is now codec = pio_i2s.I2S(...).

@FoamyGuy
Copy link
Contributor

FoamyGuy commented Dec 5, 2024

@relic-se I had a look in the actions log, it looks like the bundle build did not succeed because it found commits newer than the latest release in the new repo.

When you have a chance can you make a new release in https://github.com/relic-se/CircuitPython_PIO_I2S, I think it should be able to pass after that.

@relic-se
Copy link
Contributor Author

relic-se commented Dec 5, 2024

@FoamyGuy Gotcha. I've got a new release published: https://github.com/relic-se/CircuitPython_PIO_I2S/releases/tag/1.1.0.

Copy link
Contributor

@FoamyGuy FoamyGuy left a comment

Choose a reason for hiding this comment

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

Thank you!

@FoamyGuy FoamyGuy merged commit 5ce12ef into adafruit:main Dec 5, 2024
1 check passed
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.

2 participants