Add Android Assist VAD timing settings#6895
Conversation
|
CI actions were stuck, which is why I drafted/undrafted and closed/reopened this PR. Now available so we can get started with review :) |
| suspend fun setAssistVadSilenceSeconds(seconds: Double?) | ||
|
|
||
| suspend fun setAssistVadTimeoutSeconds(seconds: Double?) |
There was a problem hiding this comment.
Let's not exposes Double here and use Duration instead
| suspend fun setAssistVadSilenceSeconds(seconds: Double?) | |
| suspend fun setAssistVadTimeoutSeconds(seconds: Double?) | |
| suspend fun setAssistVadSilence(duration: Duration?) | |
| suspend fun setAssistVadTimeout(duration: Duration?) |
You'll need to adjust the rest of the codebase. Whenever we can we should avoid primitive types to represent time, in this specific case only the persistent layer and serialization layer should know what to do with the Duration object. it allow for instance to store milliseconds and send something like hours if we want, without having to change the whole chain of call.
| fun Double.toAssistVadSecondsString(): String = BigDecimal.valueOf(this).stripTrailingZeros().toPlainString() | ||
|
|
||
| fun Double?.toAssistVadSecondsInput(): String = this?.toAssistVadSecondsString().orEmpty() |
There was a problem hiding this comment.
Why use string here? While migrating to Duration I invite you to migrate to Long if you store the milliseconds or properly document this choice of design.
| fun String.normalizedAssistVadSecondsInputOrNull(): String? { | ||
| val normalized = replace(',', '.') | ||
| if (normalized.isEmpty()) return normalized | ||
| if (normalized.count { it == '.' } > 1) return null | ||
| return normalized.takeIf { it.all { char -> char.isDigit() || char == '.' } } | ||
| } | ||
|
|
||
| fun String?.toAssistVadSecondsOrNull(): Double? = | ||
| this?.normalizedAssistVadSecondsInputOrNull()?.toDoubleOrNull()?.takeIf { it.isFinite() && it > 0.0 } |
There was a problem hiding this comment.
This feel wrong and probably at the wrong place.
| _uiState.update { it.copy(isTestingWakeWord = testing, wakeWordDetected = false) } | ||
| } | ||
|
|
||
| fun onVadSilenceSecondsChanged(value: String) { |
There was a problem hiding this comment.
You should enforce the logic onto the fields and expose to the user when the value are not valid (if they can set an invalid value).
|
|
||
| Spacer(modifier = Modifier.height(HADimens.SPACE2)) | ||
|
|
||
| SectionHeader( |
There was a problem hiding this comment.
You need to regenerate the screenshot tests
|
Update the description of the PR to match the official template and add screenshots or even video of the new usage. |
Adds Assist VAD silence and timeout settings to Android voice input.