adafruit / Adafruit_VS1053_Library

This is a Arduino library for the Adafruit VS1053 Codec Breakout and Music Maker Shields
https://www.adafruit.com/products/1381
133 stars 112 forks source link

Fix pausePlaying function to not make playingMusic true if there is n… #58

Closed jamesguan closed 4 years ago

jamesguan commented 5 years ago

Resolves #16

Thank you for creating a pull request to contribute to Adafruit's GitHub code! Before you open the request please review the following guidelines and tips to help it be more easily integrated:

Fixed the void Adafruit_VS1053_FilePlayer::pausePlaying(boolean pause) function to accurately set playingMusic bool true only when there is a current track. This will prevent the scenario where stopped function will return false while the current track has indeed stopped.

Also, this prevents crashes that might occur when the library tries to play a track that has been closed.

To duplicate the issue, play a sound file and wait till it is stopped. Call the pausePlaying function and then call the stopped function, you should incorrectly get false for the status.

Thank you again for contributing! We will try to test and integrate the change as soon as we can, but be aware we have many GitHub repositories to manage and can't immediately respond to every request. There is no need to bump or check in on a pull request (it will clutter the discussion of the request).

Also don't be worried if the request is closed or not integrated--sometimes the priorities of Adafruit's GitHub code (education, ease of use) might not match the priorities of the pull request. Don't fret, the open source community thrives on forks and GitHub makes it easy to keep your changes in a forked repo.

After reviewing the guidelines above you can delete this text from the pull request.

jamesguan commented 5 years ago

Review Ready

jamesguan commented 5 years ago

Is there anything extra I need to do to get this PR merged?

jacgoudsmit commented 5 years ago

Is there anything extra I need to do to get this PR merged?

Just be patient I guess.

Adafruit has a LOT of repositories on Github so it may take some time before they get to this one. Also I understand they're mostly focusing on CircuitPython right now.

In the meantime, your PR is visible and can be pulled by others who find it useful.

ladyada commented 5 years ago

hiya if other people can test the PRs that is helpful, we have 1200 repos and many PRs :)

TheNitek commented 4 years ago

I also just stumbled upon this bug and can confirm this fix works. (ESP8266)

TheNitek commented 4 years ago

Any chance to get this merged soon? Anything I can do to help?

TheNitek commented 4 years ago

Awesome, thx!

jamesguan commented 4 years ago

Yay!