Skip to content

Change to allow WaveFile and MP3Decoder to accept a file path #6931

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

Merged
merged 4 commits into from
Sep 21, 2022
Merged

Change to allow WaveFile and MP3Decoder to accept a file path #6931

merged 4 commits into from
Sep 21, 2022

Conversation

snkYmkrct
Copy link

@snkYmkrct snkYmkrct commented Sep 20, 2022

Hello 🐈‍⬛ 🐈‍⬛

I made the changes required by issue #5713 , following the pattern used with OnDiskBitmap.
WaveFile and MP3Decoder can now accept either an open file pointer, or a string file path as parameter.

Changes tested on the Adafruit Pygamer and Pyportal Titano.

Please let me know if this looks good.

Copy link

@jepler jepler 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! This looks very simple and sensible as a change. I do have a note about the documentation -- I only added a single comment but it looks like it applies equally to wave and mp3 sections.

In addition to that, if it is easy can also see whether you can make assigning a string to the mp3.file property can be made to work? If it is not easy, we can leave it for a future PR.

@snkYmkrct
Copy link
Author

snkYmkrct commented Sep 20, 2022

@jepler
I made the requested documentation changes.

I am a beginner at working with something like CircuitPython core (first core contribution actually), so I can't speak to how easy it would be to add a string to the mp3.file, but I can definitely look into it.

@dhalbert dhalbert requested a review from jepler September 21, 2022 00:31
Copy link

@jepler jepler 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! It looks good now. Changing how the mp3decoder's file attribute works can be done later.

@jepler jepler merged commit 96179f6 into adafruit:main Sep 21, 2022
@snkYmkrct
Copy link
Author

Thank you! 😸

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