From 15d044edadda4a3ce7061e7dfd7d0f9e51a7917a Mon Sep 17 00:00:00 2001 From: Tomasz Rybakiewicz Date: Wed, 14 Dec 2022 15:11:45 -0500 Subject: [PATCH] Updated MapboxAudioGuidanceVoice to use coroutines to synchronize calls to both MapboxSpeechApi and MapboxVoiceInstructionsPlayer. As a result, MapboxAudioGuidanceVoice will wait until the player finishes playing the previous announcement before attempting to play the next one. Using coroutines also allows for API and Player cancellation alongside the parent Job. --- .../ui/voice/api/MapboxAudioGuidance.kt | 4 +- .../internal/MapboxAudioGuidanceVoice.kt | 70 ++++++++++--------- .../voice/TestMapboxAudioGuidanceServices.kt | 13 ++-- .../impl/MapboxAudioGuidanceVoiceTest.kt | 63 ++++++++++++----- 4 files changed, 90 insertions(+), 60 deletions(-) diff --git a/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/api/MapboxAudioGuidance.kt b/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/api/MapboxAudioGuidance.kt index d001fc90355..029a87c7142 100644 --- a/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/api/MapboxAudioGuidance.kt +++ b/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/api/MapboxAudioGuidance.kt @@ -27,6 +27,7 @@ import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.first import kotlinx.coroutines.flow.flatMapConcat import kotlinx.coroutines.flow.flatMapLatest +import kotlinx.coroutines.flow.flowOf import kotlinx.coroutines.flow.map import kotlinx.coroutines.flow.updateAndGet import kotlinx.coroutines.launch @@ -147,7 +148,8 @@ internal constructor( .filter { it.voiceInstructions != lastPlayedInstructions } .flatMapConcat { lastPlayedInstructions = it.voiceInstructions - audioGuidance.speak(it.voiceInstructions) + val announcement = audioGuidance.speak(it.voiceInstructions) + flowOf(announcement) } .map { speechAnnouncement -> internalStateFlow.updateAndGet { diff --git a/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/internal/MapboxAudioGuidanceVoice.kt b/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/internal/MapboxAudioGuidanceVoice.kt index 9bfa672ddbc..b6e13c1443e 100644 --- a/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/internal/MapboxAudioGuidanceVoice.kt +++ b/libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/internal/MapboxAudioGuidanceVoice.kt @@ -1,20 +1,13 @@ package com.mapbox.navigation.ui.voice.internal import com.mapbox.api.directions.v5.models.VoiceInstructions -import com.mapbox.bindgen.Expected -import com.mapbox.navigation.ui.base.util.MapboxNavigationConsumer import com.mapbox.navigation.ui.voice.api.MapboxSpeechApi import com.mapbox.navigation.ui.voice.api.MapboxVoiceInstructionsPlayer import com.mapbox.navigation.ui.voice.model.SpeechAnnouncement -import com.mapbox.navigation.ui.voice.model.SpeechError -import com.mapbox.navigation.ui.voice.model.SpeechValue -import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.channels.awaitClose -import kotlinx.coroutines.channels.onFailure -import kotlinx.coroutines.channels.onSuccess -import kotlinx.coroutines.flow.Flow -import kotlinx.coroutines.flow.callbackFlow -import kotlinx.coroutines.flow.flowOf +import kotlinx.coroutines.NonCancellable +import kotlinx.coroutines.suspendCancellableCoroutine +import kotlinx.coroutines.withContext +import kotlin.coroutines.resume /** * Controls voice guidance for the car. @@ -26,35 +19,44 @@ class MapboxAudioGuidanceVoice( private val mapboxSpeechApi: MapboxSpeechApi, private val mapboxVoiceInstructionsPlayer: MapboxVoiceInstructionsPlayer ) { - fun speak(voiceInstructions: VoiceInstructions?): Flow { + /** + * Load and play [SpeechAnnouncement]. + * This method will suspend until announcement finishes playback. + */ + suspend fun speak(voiceInstructions: VoiceInstructions?): SpeechAnnouncement? { return if (voiceInstructions != null) { - speechFlow(voiceInstructions) + val announcement = mapboxSpeechApi.generate(voiceInstructions) + try { + mapboxVoiceInstructionsPlayer.play(announcement) + announcement + } finally { + withContext(NonCancellable) { + mapboxSpeechApi.clean(announcement) + } + } } else { mapboxSpeechApi.cancel() mapboxVoiceInstructionsPlayer.clear() - flowOf(null) + null } } - @OptIn(ExperimentalCoroutinesApi::class) - private fun speechFlow(voiceInstructions: VoiceInstructions): Flow = - callbackFlow { - val speechCallback = - MapboxNavigationConsumer> { value -> - val speechAnnouncement = value.value?.announcement ?: value.error!!.fallback - mapboxVoiceInstructionsPlayer.play(speechAnnouncement) { - mapboxSpeechApi.clean(it) - trySend(speechAnnouncement).onSuccess { - close() - }.onFailure { - close() - } - } - } - mapboxSpeechApi.generate(voiceInstructions, speechCallback) - awaitClose { - mapboxSpeechApi.cancel() - mapboxVoiceInstructionsPlayer.clear() - } + private suspend fun MapboxSpeechApi.generate( + instructions: VoiceInstructions + ): SpeechAnnouncement = suspendCancellableCoroutine { cont -> + generate(instructions) { value -> + val announcement = value.value?.announcement ?: value.error!!.fallback + cont.resume(announcement) } + cont.invokeOnCancellation { cancel() } + } + + private suspend fun MapboxVoiceInstructionsPlayer.play( + announcement: SpeechAnnouncement + ): SpeechAnnouncement = suspendCancellableCoroutine { cont -> + play(announcement) { + cont.resume(announcement) + } + cont.invokeOnCancellation { clear() } + } } diff --git a/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/TestMapboxAudioGuidanceServices.kt b/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/TestMapboxAudioGuidanceServices.kt index ede31eef73a..8f694ec2c70 100644 --- a/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/TestMapboxAudioGuidanceServices.kt +++ b/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/TestMapboxAudioGuidanceServices.kt @@ -8,13 +8,13 @@ import com.mapbox.navigation.ui.voice.internal.MapboxVoiceInstructionsState import com.mapbox.navigation.ui.voice.internal.impl.MapboxAudioGuidanceServices import com.mapbox.navigation.ui.voice.model.SpeechAnnouncement import io.mockk.Runs +import io.mockk.coEvery import io.mockk.every import io.mockk.just import io.mockk.mockk import kotlinx.coroutines.delay import kotlinx.coroutines.flow.MutableStateFlow import kotlinx.coroutines.flow.flowOf -import kotlinx.coroutines.flow.onEach class TestMapboxAudioGuidanceServices( private val deviceLanguage: String = "en" @@ -34,7 +34,7 @@ class TestMapboxAudioGuidanceServices( } private val mapboxAudioGuidanceVoice = mockk { - every { speak(any()) } answers { + coEvery { speak(any()) } coAnswers { val voiceInstructions = firstArg() val speechAnnouncement: SpeechAnnouncement? = voiceInstructions?.let { mockk { @@ -42,12 +42,11 @@ class TestMapboxAudioGuidanceServices( every { ssmlAnnouncement } returns it.ssmlAnnouncement() } } - flowOf(speechAnnouncement).onEach { - if (it != null) { - // Simulate a real speech announcement by delaying the TestCoroutineScope - delay(SPEECH_ANNOUNCEMENT_DELAY_MS) - } + if (speechAnnouncement != null) { + // Simulate a real speech announcement by delaying the TestCoroutineScope + delay(SPEECH_ANNOUNCEMENT_DELAY_MS) } + speechAnnouncement } } diff --git a/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/internal/impl/MapboxAudioGuidanceVoiceTest.kt b/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/internal/impl/MapboxAudioGuidanceVoiceTest.kt index 00b1da07f51..3c8c2d90e62 100644 --- a/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/internal/impl/MapboxAudioGuidanceVoiceTest.kt +++ b/libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/internal/impl/MapboxAudioGuidanceVoiceTest.kt @@ -15,7 +15,8 @@ import io.mockk.every import io.mockk.mockk import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi -import kotlinx.coroutines.flow.collect +import kotlinx.coroutines.delay +import kotlinx.coroutines.launch import org.junit.Assert.assertEquals import org.junit.Rule import org.junit.Test @@ -27,28 +28,28 @@ class MapboxAudioGuidanceVoiceTest { val coroutineRule = MainCoroutineRule() private val speechApi = mockk(relaxUnitFun = true) - private val voiceInstructionsPlayer = mockk(relaxUnitFun = true) - private val carAppAudioGuidanceVoice = MapboxAudioGuidanceVoice( + private val voiceInstructionsPlayer = mockk(relaxed = true) + private val sut = MapboxAudioGuidanceVoice( speechApi, voiceInstructionsPlayer ) @Test - fun `voice instruction should be played as SpeechAnnouncement`() = coroutineRule.runBlockingTest { - mockSuccessfulSpeechApi() - mockSuccessfulVoiceInstructionsPlayer() + fun `voice instruction should be played as SpeechAnnouncement`() = + coroutineRule.runBlockingTest { + mockSuccessfulSpeechApi() + mockSuccessfulVoiceInstructionsPlayer() - val voiceInstructions = mockk { - every { announcement() } returns "Turn right on Market" - } - carAppAudioGuidanceVoice.speak(voiceInstructions).collect { speechAnnouncement -> + val voiceInstructions = mockk { + every { announcement() } returns "Turn right on Market" + } + val speechAnnouncement = sut.speak(voiceInstructions) assertEquals("Turn right on Market", speechAnnouncement!!.announcement) } - } @Test fun `null should clean up the api and player`() = coroutineRule.runBlockingTest { - carAppAudioGuidanceVoice.speak(null).collect() + sut.speak(null) verify { speechApi.cancel() } verify { voiceInstructionsPlayer.clear() } @@ -70,18 +71,44 @@ class MapboxAudioGuidanceVoiceTest { val voiceInstructions = mockk { every { announcement() } returns "This message fails" } - carAppAudioGuidanceVoice.speak(voiceInstructions).collect { speechAnnouncement -> - assertEquals("Turn right on Market", speechAnnouncement!!.announcement) - } + val speechAnnouncement = sut.speak(voiceInstructions) + assertEquals("Turn right on Market", speechAnnouncement!!.announcement) } + @Test + fun `should wait until previous instruction finishes playback before playing next one`() = + coroutineRule.runBlockingTest { + mockSuccessfulSpeechApi() + every { voiceInstructionsPlayer.play(any(), any()) } answers { + launch { + val speechAnnouncement = firstArg() + delay(1000) // simulate 1 second announcement playback duration + secondArg>() + .accept(speechAnnouncement) + } + Unit + } + + val played = mutableListOf() + launch { + listOf( + VoiceInstructions.builder().announcement("A").build(), + VoiceInstructions.builder().announcement("B").build() + ).forEach { + val announcement = sut.speak(it) // suspend until playback finishes + played.add(announcement) + } + } + advanceTimeBy(1500) // advance time to 50% of announcement B playback time + + assertEquals(1, played.size) + } + private fun mockSuccessfulSpeechApi() { every { speechApi.generate(any(), any()) } answers { val announcementArg = firstArg().announcement() val speechValue = mockk { - every { announcement } returns mockk { - every { announcement } returns announcementArg!! - } + every { announcement } returns SpeechAnnouncement.Builder(announcementArg!!).build() } val consumer = secondArg>>() consumer.accept(ExpectedFactory.createValue(speechValue))