NAVAND-1001: use immediate dispatcher to guarantee that language will change before voice instructions

CHANGELOG.md

@@ -28,6 +28,7 @@ Mapbox welcomes participation and contributions from everyone.
 - Fixed standalone `MapboxManeuverView` appearance when the app also integrates Drop-In UI. [#6774](https://github.com/mapbox/mapbox-navigation-android/pull/6774)
 - Introduced `NavigationViewListener.onSpeedInfoClicked` that would be triggered when `MapboxSpeedInfoView` is clicked upon. [#6770](https://github.com/mapbox/mapbox-navigation-android/pull/6770)
 - Each newly instantiated MapboxRouteArrowView class will initialize the layers with the provided options on the first render call. Previously this would only be done if the layers hadn't already been initialized.  [#6466](https://github.com/mapbox/mapbox-navigation-android/pull/6466)
+- Fixed an issue where the first voice instruction might have been played twice. [#6766](https://github.com/mapbox/mapbox-navigation-android/pull/6766)
 
 ## Mapbox Navigation SDK 2.10.0-rc.1 - 16 December, 2022
 ### Changelog

libnavui-voice/src/main/java/com/mapbox/navigation/ui/voice/api/MapboxAudioGuidance.kt

@@ -14,7 +14,6 @@ import kotlinx.coroutines.CoroutineDispatcher
 import kotlinx.coroutines.CoroutineScope
 import kotlinx.coroutines.Dispatchers
 import kotlinx.coroutines.ExperimentalCoroutinesApi
-import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.Job
 import kotlinx.coroutines.SupervisorJob
 import kotlinx.coroutines.flow.Flow
@@ -25,7 +24,6 @@ import kotlinx.coroutines.flow.combine
 import kotlinx.coroutines.flow.distinctUntilChanged
 import kotlinx.coroutines.flow.filter
 import kotlinx.coroutines.flow.first
-import kotlinx.coroutines.flow.flatMapConcat
 import kotlinx.coroutines.flow.flatMapLatest
 import kotlinx.coroutines.flow.map
 import kotlinx.coroutines.flow.updateAndGet
@@ -122,7 +120,7 @@ internal constructor(
     /**
      * Top level flow that will switch based on the language and muted state.
      */
-    @OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class)
+    @OptIn(ExperimentalCoroutinesApi::class)
     private fun audioGuidanceFlow(
         mapboxNavigation: MapboxNavigation
     ): Flow<MapboxAudioGuidanceState> {
@@ -145,17 +143,15 @@ internal constructor(
                 } else {
                     voiceInstructions
                         .filter { it.voiceInstructions != lastPlayedInstructions }
-                        .flatMapConcat {
+                        .map {
                             lastPlayedInstructions = it.voiceInstructions
-                            audioGuidance.speak(it.voiceInstructions)
-                        }
-                        .map { speechAnnouncement ->
-                            internalStateFlow.updateAndGet {
+                            val announcement = audioGuidance.speak(it.voiceInstructions)
+                            internalStateFlow.updateAndGet { state ->
                                 MapboxAudioGuidanceState(
-                                    isPlayable = it.isPlayable,
-                                    isMuted = it.isMuted,
-                                    voiceInstructions = it.voiceInstructions,
-                                    speechAnnouncement = speechAnnouncement
+                                    isPlayable = state.isPlayable,
+                                    isMuted = state.isMuted,
+                                    voiceInstructions = state.voiceInstructions,
+                                    speechAnnouncement = announcement
                                 )
                             }
                         }
@@ -193,7 +189,10 @@ internal constructor(
          * Construct an instance without registering to [MapboxNavigationApp].
          */
         @JvmStatic
-        fun create() = MapboxAudioGuidance(MapboxAudioGuidanceServices(), Dispatchers.Main)
+        fun create() = MapboxAudioGuidance(
+            MapboxAudioGuidanceServices(),
+            Dispatchers.Main.immediate
+        )
 
         /**
          * Get the registered instance or create one and register it to [MapboxNavigationApp].

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<SpeechAnnouncement?> {
+    /**
+     * 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<SpeechAnnouncement> =
-        callbackFlow {
-            val speechCallback =
-                MapboxNavigationConsumer<Expected<SpeechError, SpeechValue>> { 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() }
+    }
 }

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,20 +34,19 @@ class TestMapboxAudioGuidanceServices(
     }
 
     private val mapboxAudioGuidanceVoice = mockk<MapboxAudioGuidanceVoice> {
-        every { speak(any()) } answers {
+        coEvery { speak(any()) } coAnswers {
             val voiceInstructions = firstArg<VoiceInstructions?>()
             val speechAnnouncement: SpeechAnnouncement? = voiceInstructions?.let {
                 mockk {
                     every { announcement } returns it.announcement()!!
                     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
         }
     }
 

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<MapboxSpeechApi>(relaxUnitFun = true)
-    private val voiceInstructionsPlayer = mockk<MapboxVoiceInstructionsPlayer>(relaxUnitFun = true)
-    private val carAppAudioGuidanceVoice = MapboxAudioGuidanceVoice(
+    private val voiceInstructionsPlayer = mockk<MapboxVoiceInstructionsPlayer>(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<VoiceInstructions> {
-            every { announcement() } returns "Turn right on Market"
-        }
-        carAppAudioGuidanceVoice.speak(voiceInstructions).collect { speechAnnouncement ->
+            val voiceInstructions = mockk<VoiceInstructions> {
+                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<VoiceInstructions> {
             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<SpeechAnnouncement>()
+                    delay(1000) // simulate 1 second announcement playback duration
+                    secondArg<MapboxNavigationConsumer<SpeechAnnouncement>>()
+                        .accept(speechAnnouncement)
+                }
+                Unit
+            }
+
+            val played = mutableListOf<SpeechAnnouncement?>()
+            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<VoiceInstructions>().announcement()
             val speechValue = mockk<SpeechValue> {
-                every { announcement } returns mockk {
-                    every { announcement } returns announcementArg!!
-                }
+                every { announcement } returns SpeechAnnouncement.Builder(announcementArg!!).build()
             }
             val consumer = secondArg<MapboxNavigationConsumer<Expected<SpeechError, SpeechValue>>>()
             consumer.accept(ExpectedFactory.createValue(speechValue))