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

Pomodoro Audio Notifications #999

Open
wants to merge 2 commits into
base: main
Choose a base branch
from

Conversation

PrabhuUdurg
Copy link

Hi! Most Pomodoro timers have a sound notification, which is really helpful. I added this feature also to the Pomodoro module. I used simpleaudio library to play sounds. Also, I added two variables at the beginning of the file, for the work and rest duration, to make the user change it easier, and for better maintainability.
Additionally, I added a sentence about how to change these parameters in the header.
Thank you

@tobi-wan-kenobi
Copy link
Owner

Thanks a lot for the PR!

I have a couple of small suggestions/requests:

  1. Please keep the 2 parameter hardcoded, those are just the default values and can be changed using "-p pomodoro.". Users should not have to modify the source code (because updates will override their changes)

  2. Instead of committing the wav, please let users specify (via parameter) the audio file they want to play. I don't know where that wav came from and want to avoid copyright/usage dispute problems. Any content not directly generated by bumblebee has to come from the user.

Hope that helps! Thank you!

@PrabhuUdurg
Copy link
Author

Hallo! I can remove the path and file, but then the library throws an error. How I should handle this, thank you

@tobi-wan-kenobi
Copy link
Owner

If the wav does not exist, I would suggest to just log an error (using log.error) and continue.

@PrabhuUdurg
Copy link
Author

Hallo! I made changes, now user can upload their own audio files, The error problem was handled by try except.

@tobi-wan-kenobi
Copy link
Owner

Thanks a lot for the prompt responses @PrabhuUdurg !

I added a couple more notes that, in my opinion, improve the functionality a bit & might hopefully help you better understand the design ideas behind the bumblebee modules (often not very apparent, I admin).

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