Engine #50
Engine #50
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #50 +/- ##
==========================================
- Coverage 95.24% 95.16% -0.08%
==========================================
Files 4 4
Lines 147 165 +18
Branches 19 24 +5
==========================================
+ Hits 140 157 +17
Misses 2 2
- Partials 5 6 +1 ☔ View full report in Codecov by Sentry. |
| >>> sys_platform = sys.platform | ||
| >>> if sys_platform == "win32": | ||
| ... sound_id = nava.play(os.path.join("others", "test.wav"), async_mode=True, engine=nava.Engine.WINSOUND) | ||
| ... elif sys_platform == "darwin": | ||
| ... sound_id = nava.play(os.path.join("others", "test.wav"), async_mode=True, engine=nava.Engine.AFPLAY) | ||
| ... else: | ||
| ... sound_id = nava.play(os.path.join("others", "test.wav"), async_mode=True, engine=nava.Engine.ALSA) |
There was a problem hiding this comment.
Why didn't you test __play_auto directly here?
There was a problem hiding this comment.
This section of tests is not about the __play_auto function. Here, I tried to test each engine manually.
README.md
Outdated
|
|
||
| ```python | ||
| from nava import play, Engine | ||
| sound_id = play("alarm.wav", async_mode=True, engine=Engine.AFPLAY) |
There was a problem hiding this comment.
I guess it's better to remove async_mode for this example, to prevent confusion.
| class Engine(Enum): | ||
| """ | ||
| Nava engine class. | ||
|
|
||
| >>> import nava | ||
| >>> engine = nava.Engine.ALSA | ||
| """ | ||
|
|
||
| AUTO = "auto" | ||
| WINSOUND = "winsound" | ||
| ALSA = "alsa" | ||
| AFPLAY = "afplay" | ||
|
|
There was a problem hiding this comment.
Now when user requests for an unavailable engine (an engine which is available in the Engine enum but not in the user's system) we raise the following error:
nava.errors.NavaBaseError: Sound can not play due to some issues.
It's better to handle this exception differently somewhere. We may want to handle this in another PR, but I am raising it here just for the record.
There was a problem hiding this comment.
We should talk about it later. At the moment, all of the engines only work on one OS. In the future, it's possible that we'll have an engine that works on more than one OS. So, I believe that it's not a bad idea to give this option to the user to choose any engine anywhere.
Reference Issues/PRs
What does this implement/fix? Explain your changes.
engineparameter added toplayfunctionREADME.mdmodified__play_winfunction renamed to__play_winsound__play_macfunction renamed to__play_afplay__play_linuxfunction renamed to__play_alsaAny other comments?
Local tests on OSs