Conversation
There was a problem hiding this comment.
Thank you! I added a commit to make it so that the sentence parsing plugin allows a language to not have a translation for a specific sentence
Important note: The non-English sentence files are computer translated. I have no idea if they're accurate or cover reasonable user expectations.
Yeah they were quite poor quality and could have interfered with other skills
| return 1 | ||
| } | ||
|
|
||
| return ctx.parserFormatter!! |
There was a problem hiding this comment.
This will crash if there is no number parser. You should disable the volume up/down sentences for this skill if the number parser for the current language is null. I would suggest adding a fun withoutSentenceIds(vararg sentenceIds: String) in StandardRecognizerData, and then calling that function when building the MediaSkill in MediaInfo if the number parser is null. The function should throw if some passed sentence id is not present.
| audioManager.adjustStreamVolume( | ||
| AudioManager.STREAM_MUSIC, | ||
| AudioManager.ADJUST_RAISE, | ||
| if (it == 0) AudioManager.FLAG_SHOW_UI else 0 |
There was a problem hiding this comment.
Why FLAG_SHOW_UI only the first time? Please add a comment
| audioManager.dispatchMediaKeyEvent(KeyEvent(KeyEvent.ACTION_DOWN, KeyEvent.KEYCODE_MEDIA_PLAY)) | ||
| audioManager.dispatchMediaKeyEvent(KeyEvent(KeyEvent.ACTION_UP, KeyEvent.KEYCODE_MEDIA_PLAY)) |
There was a problem hiding this comment.
Maybe deduplicate this (and the below) with some member function
| .extractNumber(input) | ||
| .mixedWithText | ||
| .asSequence() | ||
| .filterIsInstance<org.dicio.numbers.unit.Number>() | ||
| .filter { it.isInteger } | ||
| .map { it.integerValue().toInt() } | ||
| .firstOrNull() | ||
| ?.coerceIn(1, 10) // Limit to 1-10 steps for safety | ||
| ?: 1 |
There was a problem hiding this comment.
| .extractNumber(input) | |
| .mixedWithText | |
| .asSequence() | |
| .filterIsInstance<org.dicio.numbers.unit.Number>() | |
| .filter { it.isInteger } | |
| .map { it.integerValue().toInt() } | |
| .firstOrNull() | |
| ?.coerceIn(1, 10) // Limit to 1-10 steps for safety | |
| ?: 1 | |
| .extractNumber(input) | |
| .first | |
| ?.takeIf { it.isInteger } | |
| ?.integerValue() | |
| ?.toInt() | |
| ?.coerceIn(1, 10) // Limit to 1-10 steps for safety | |
| ?: 1 |
Stypox
force-pushed
the
extend-media-controls
branch
from
2e7652e to
48de470
Compare
Stypox
force-pushed
the
extend-media-controls
branch
from
48de470 to
e277e78
Compare
|
I'm wondering, should we add support for setting absolute volumes? Is that even supported by audioManager (I guess yes?). Ref: #218 (comment) |
2 participants
This PR implements volume controls to the media skill. It covers both increase the volume by one (not much, not really useful, if I'm being honest) and by user specified increments.
I think this is a good approach as it allows fine control by the user. However, there is an alternative implementation where we pick some number as the increment and just have a "turn it up/down" sentence that turns audio down, say, 5 times.
Important note: The non-English sentence files are computer translated. I have no idea if they're accurate or cover reasonable user expectations. It's just a best effort to keep the compiler happy and not have language regression for the media skill.