Async mode fix #29
Async mode fix #29
Conversation
Codecov ReportAttention:
Additional details and impacted files@@ Coverage Diff @@
## dev #29 +/- ##
===========================================
+ Coverage 69.13% 99.16% +30.03%
===========================================
Files 3 4 +1
Lines 81 120 +39
Branches 7 12 +5
===========================================
+ Hits 56 119 +63
+ Misses 24 0 -24
Partials 1 1 ☔ View full report in Codecov by Sentry. |
Local tests on OSs
|
| :param kwargs: keyword arguments | ||
| :type kwargs: dict | ||
| """ | ||
| super(NavaThread, self).__init__(*args, **kwargs) |
There was a problem hiding this comment.
According to super, you are trying to limit the parental initialization only to the NavaThread class, right?
There was a problem hiding this comment.
Yeah, it will help NavaThread to act like the original Thread in other aspects.
There was a problem hiding this comment.
Oh, I see. It seems a high-level trick. 🥇
| sys_platform = sys.platform | ||
| if sys_platform == "win32": | ||
| import winsound | ||
| winsound.PlaySound(None, winsound.SND_PURGE) |
There was a problem hiding this comment.
Did you test the case in Windows where you have multiple sounds playing asynchronously and try to stop just one of them?
There was a problem hiding this comment.
Currently, the winsound module cannot play multiple sounds simultaneously. But play/stop of one sound in async mode works like charm!
CHANGELOG.md
Outdated
| @@ -5,6 +5,11 @@ The format is based on [Keep a Changelog](http://keepachangelog.com/en/1.0.0/) | |||
| and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.html). | |||
|
|
|||
| ## [Unreleased] | |||
| ### Added | |||
| - Async mode | |||
There was a problem hiding this comment.
I think "async_mode parameter" under Changed is better.
There was a problem hiding this comment.
fixed --> 435e701
| :return: sound id as int | ||
| """ | ||
| params._play_threads_counter += 1 | ||
| sound_id = params._play_threads_counter + 1000 |
There was a problem hiding this comment.
Why did you add 1000 offset. If it's necessary, what do you think of transferring it to params.
There was a problem hiding this comment.
For better readability, we may change the format of the sound_id to something like Nava-1, Nava-2, etc. in the future.
nava/functions.py
Outdated
| play_flags = winsound.SND_FILENAME | (async_mode & winsound.SND_ASYNC) | ||
|
|
||
| if async_mode: | ||
| sound_thread = NavaThread(target=__play_win_flags, args=(sound_path, play_flags), daemon=True) |
There was a problem hiding this comment.
It's too long. Can you make it multiline?
There was a problem hiding this comment.
fixed --> b0f52fc
Reference Issues/PRs
#5 #27 #26
What does this implement/fix? Explain your changes.
NavaThreadclass addedstopfunction addedstop_allfunction addedautopep8scripts updatedREADME.mdupdatedCHANGELOG.mdupdatedAny other comments?