-
-
Notifications
You must be signed in to change notification settings - Fork 2
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
NavaThread
exception
#53
Conversation
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## dev #53 +/- ##
===========================================
+ Coverage 67.88% 96.48% +28.60%
===========================================
Files 4 4
Lines 165 170 +5
Branches 24 24
===========================================
+ Hits 112 164 +52
+ Misses 47 0 -47
Partials 6 6 ☔ View full report in Codecov by Sentry. |
self._play_process.wait() | ||
if not self._loop: | ||
break | ||
except Exception: # pragma: no cover |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@sadrasabouri Currently, it's not straightforward to test this exception, but we will address it in a future PR. Please disregard it for this PR.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
LGTM.
Just a point I wanted to mention here for future reference. Since we are using WINSOUND
API, but call alsa
directly from the system, they still have difference.
For example in MacOS, for async mode, when running Winsound, it gave the error and process is done, but when calling Alsa, it stuck and needed a Ctrl + C
to exit.
You can merge it, I just don't merge it so that you see my message here.
What's the meaning of stuck? It's really weird! This process run on another thread. Can you provide a reproducible example? |
Yes, checkout this on Ubuntu (for example) and see the differnece: play("path/to/file.wav", engine=Engine.WINSOUND) and play("path/to/file.wav", engine=Engine.AFPLAY) |
We decided to fix the raised issue in a separate PR. |
Reference Issues/PRs
What does this implement/fix? Explain your changes.
NavaThread
exception bug fixedAny other comments?
Local tests on OSs