Skip to content

Commit 8bb70a5

Browse files
dzinaddzinad
committed
NAVAND-1001: use immediate dispatcher to guarantee that language will change before voice instructions (#6766)
1 parent d301b97 commit 8bb70a5

File tree

5 files changed

+100
-72
lines changed

5 files changed

+100
-72
lines changed

‎CHANGELOG.md

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ Mapbox welcomes participation and contributions from everyone.
88
- Fixed an issue with `NavigationView` that caused info panel to shrink in landscape mode with a full screen theme. [#6811](https://github.com/mapbox/mapbox-navigation-android/pull/6811)
99
- Fixed standalone `MapboxManeuverView` appearance when the app also integrates Drop-In UI. [#6810](https://github.com/mapbox/mapbox-navigation-android/pull/6810)
1010
- 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)
11+
- Fixed an issue where the first voice instruction might have been played twice. [#6766](https://github.com/mapbox/mapbox-navigation-android/pull/6766)
1112

1213
## Mapbox Navigation SDK 2.10.0-rc.1 - 16 December, 2022
1314
### Changelog

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

Lines changed: 12 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,6 @@ import kotlinx.coroutines.CoroutineDispatcher
1414
import kotlinx.coroutines.CoroutineScope
1515
import kotlinx.coroutines.Dispatchers
1616
import kotlinx.coroutines.ExperimentalCoroutinesApi
17-
import kotlinx.coroutines.FlowPreview
1817
import kotlinx.coroutines.Job
1918
import kotlinx.coroutines.SupervisorJob
2019
import kotlinx.coroutines.flow.Flow
@@ -25,7 +24,6 @@ import kotlinx.coroutines.flow.combine
2524
import kotlinx.coroutines.flow.distinctUntilChanged
2625
import kotlinx.coroutines.flow.filter
2726
import kotlinx.coroutines.flow.first
28-
import kotlinx.coroutines.flow.flatMapConcat
2927
import kotlinx.coroutines.flow.flatMapLatest
3028
import kotlinx.coroutines.flow.map
3129
import kotlinx.coroutines.flow.updateAndGet
@@ -122,7 +120,7 @@ internal constructor(
122120
/**
123121
* Top level flow that will switch based on the language and muted state.
124122
*/
125-
@OptIn(ExperimentalCoroutinesApi::class, FlowPreview::class)
123+
@OptIn(ExperimentalCoroutinesApi::class)
126124
private fun audioGuidanceFlow(
127125
mapboxNavigation: MapboxNavigation
128126
): Flow<MapboxAudioGuidanceState> {
@@ -145,17 +143,15 @@ internal constructor(
145143
} else {
146144
voiceInstructions
147145
.filter { it.voiceInstructions != lastPlayedInstructions }
148-
.flatMapConcat {
146+
.map {
149147
lastPlayedInstructions = it.voiceInstructions
150-
audioGuidance.speak(it.voiceInstructions)
151-
}
152-
.map { speechAnnouncement ->
153-
internalStateFlow.updateAndGet {
148+
val announcement = audioGuidance.speak(it.voiceInstructions)
149+
internalStateFlow.updateAndGet { state ->
154150
MapboxAudioGuidanceState(
155-
isPlayable = it.isPlayable,
156-
isMuted = it.isMuted,
157-
voiceInstructions = it.voiceInstructions,
158-
speechAnnouncement = speechAnnouncement
151+
isPlayable = state.isPlayable,
152+
isMuted = state.isMuted,
153+
voiceInstructions = state.voiceInstructions,
154+
speechAnnouncement = announcement
159155
)
160156
}
161157
}
@@ -193,7 +189,10 @@ internal constructor(
193189
* Construct an instance without registering to [MapboxNavigationApp].
194190
*/
195191
@JvmStatic
196-
fun create() = MapboxAudioGuidance(MapboxAudioGuidanceServices(), Dispatchers.Main)
192+
fun create() = MapboxAudioGuidance(
193+
MapboxAudioGuidanceServices(),
194+
Dispatchers.Main.immediate
195+
)
197196

198197
/**
199198
* 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

Lines changed: 36 additions & 34 deletions
Original file line numberDiff line numberDiff line change
@@ -1,20 +1,13 @@
11
package com.mapbox.navigation.ui.voice.internal
22

33
import com.mapbox.api.directions.v5.models.VoiceInstructions
4-
import com.mapbox.bindgen.Expected
5-
import com.mapbox.navigation.ui.base.util.MapboxNavigationConsumer
64
import com.mapbox.navigation.ui.voice.api.MapboxSpeechApi
75
import com.mapbox.navigation.ui.voice.api.MapboxVoiceInstructionsPlayer
86
import com.mapbox.navigation.ui.voice.model.SpeechAnnouncement
9-
import com.mapbox.navigation.ui.voice.model.SpeechError
10-
import com.mapbox.navigation.ui.voice.model.SpeechValue
11-
import kotlinx.coroutines.ExperimentalCoroutinesApi
12-
import kotlinx.coroutines.channels.awaitClose
13-
import kotlinx.coroutines.channels.onFailure
14-
import kotlinx.coroutines.channels.onSuccess
15-
import kotlinx.coroutines.flow.Flow
16-
import kotlinx.coroutines.flow.callbackFlow
17-
import kotlinx.coroutines.flow.flowOf
7+
import kotlinx.coroutines.NonCancellable
8+
import kotlinx.coroutines.suspendCancellableCoroutine
9+
import kotlinx.coroutines.withContext
10+
import kotlin.coroutines.resume
1811

1912
/**
2013
* Controls voice guidance for the car.
@@ -26,35 +19,44 @@ class MapboxAudioGuidanceVoice(
2619
private val mapboxSpeechApi: MapboxSpeechApi,
2720
private val mapboxVoiceInstructionsPlayer: MapboxVoiceInstructionsPlayer
2821
) {
29-
fun speak(voiceInstructions: VoiceInstructions?): Flow<SpeechAnnouncement?> {
22+
/**
23+
* Load and play [SpeechAnnouncement].
24+
* This method will suspend until announcement finishes playback.
25+
*/
26+
suspend fun speak(voiceInstructions: VoiceInstructions?): SpeechAnnouncement? {
3027
return if (voiceInstructions != null) {
31-
speechFlow(voiceInstructions)
28+
val announcement = mapboxSpeechApi.generate(voiceInstructions)
29+
try {
30+
mapboxVoiceInstructionsPlayer.play(announcement)
31+
announcement
32+
} finally {
33+
withContext(NonCancellable) {
34+
mapboxSpeechApi.clean(announcement)
35+
}
36+
}
3237
} else {
3338
mapboxSpeechApi.cancel()
3439
mapboxVoiceInstructionsPlayer.clear()
35-
flowOf(null)
40+
null
3641
}
3742
}
3843

39-
@OptIn(ExperimentalCoroutinesApi::class)
40-
private fun speechFlow(voiceInstructions: VoiceInstructions): Flow<SpeechAnnouncement> =
41-
callbackFlow {
42-
val speechCallback =
43-
MapboxNavigationConsumer<Expected<SpeechError, SpeechValue>> { value ->
44-
val speechAnnouncement = value.value?.announcement ?: value.error!!.fallback
45-
mapboxVoiceInstructionsPlayer.play(speechAnnouncement) {
46-
mapboxSpeechApi.clean(it)
47-
trySend(speechAnnouncement).onSuccess {
48-
close()
49-
}.onFailure {
50-
close()
51-
}
52-
}
53-
}
54-
mapboxSpeechApi.generate(voiceInstructions, speechCallback)
55-
awaitClose {
56-
mapboxSpeechApi.cancel()
57-
mapboxVoiceInstructionsPlayer.clear()
58-
}
44+
private suspend fun MapboxSpeechApi.generate(
45+
instructions: VoiceInstructions
46+
): SpeechAnnouncement = suspendCancellableCoroutine { cont ->
47+
generate(instructions) { value ->
48+
val announcement = value.value?.announcement ?: value.error!!.fallback
49+
cont.resume(announcement)
5950
}
51+
cont.invokeOnCancellation { cancel() }
52+
}
53+
54+
private suspend fun MapboxVoiceInstructionsPlayer.play(
55+
announcement: SpeechAnnouncement
56+
): SpeechAnnouncement = suspendCancellableCoroutine { cont ->
57+
play(announcement) {
58+
cont.resume(announcement)
59+
}
60+
cont.invokeOnCancellation { clear() }
61+
}
6062
}

‎libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/TestMapboxAudioGuidanceServices.kt

Lines changed: 6 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -8,13 +8,13 @@ import com.mapbox.navigation.ui.voice.internal.MapboxVoiceInstructionsState
88
import com.mapbox.navigation.ui.voice.internal.impl.MapboxAudioGuidanceServices
99
import com.mapbox.navigation.ui.voice.model.SpeechAnnouncement
1010
import io.mockk.Runs
11+
import io.mockk.coEvery
1112
import io.mockk.every
1213
import io.mockk.just
1314
import io.mockk.mockk
1415
import kotlinx.coroutines.delay
1516
import kotlinx.coroutines.flow.MutableStateFlow
1617
import kotlinx.coroutines.flow.flowOf
17-
import kotlinx.coroutines.flow.onEach
1818

1919
class TestMapboxAudioGuidanceServices(
2020
private val deviceLanguage: String = "en"
@@ -34,20 +34,19 @@ class TestMapboxAudioGuidanceServices(
3434
}
3535

3636
private val mapboxAudioGuidanceVoice = mockk<MapboxAudioGuidanceVoice> {
37-
every { speak(any()) } answers {
37+
coEvery { speak(any()) } coAnswers {
3838
val voiceInstructions = firstArg<VoiceInstructions?>()
3939
val speechAnnouncement: SpeechAnnouncement? = voiceInstructions?.let {
4040
mockk {
4141
every { announcement } returns it.announcement()!!
4242
every { ssmlAnnouncement } returns it.ssmlAnnouncement()
4343
}
4444
}
45-
flowOf(speechAnnouncement).onEach {
46-
if (it != null) {
47-
// Simulate a real speech announcement by delaying the TestCoroutineScope
48-
delay(SPEECH_ANNOUNCEMENT_DELAY_MS)
49-
}
45+
if (speechAnnouncement != null) {
46+
// Simulate a real speech announcement by delaying the TestCoroutineScope
47+
delay(SPEECH_ANNOUNCEMENT_DELAY_MS)
5048
}
49+
speechAnnouncement
5150
}
5251
}
5352

‎libnavui-voice/src/test/java/com/mapbox/navigation/ui/voice/internal/impl/MapboxAudioGuidanceVoiceTest.kt

Lines changed: 45 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,8 @@ import io.mockk.every
1515
import io.mockk.mockk
1616
import io.mockk.verify
1717
import kotlinx.coroutines.ExperimentalCoroutinesApi
18-
import kotlinx.coroutines.flow.collect
18+
import kotlinx.coroutines.delay
19+
import kotlinx.coroutines.launch
1920
import org.junit.Assert.assertEquals
2021
import org.junit.Rule
2122
import org.junit.Test
@@ -27,28 +28,28 @@ class MapboxAudioGuidanceVoiceTest {
2728
val coroutineRule = MainCoroutineRule()
2829

2930
private val speechApi = mockk<MapboxSpeechApi>(relaxUnitFun = true)
30-
private val voiceInstructionsPlayer = mockk<MapboxVoiceInstructionsPlayer>(relaxUnitFun = true)
31-
private val carAppAudioGuidanceVoice = MapboxAudioGuidanceVoice(
31+
private val voiceInstructionsPlayer = mockk<MapboxVoiceInstructionsPlayer>(relaxed = true)
32+
private val sut = MapboxAudioGuidanceVoice(
3233
speechApi,
3334
voiceInstructionsPlayer
3435
)
3536

3637
@Test
37-
fun `voice instruction should be played as SpeechAnnouncement`() = coroutineRule.runBlockingTest {
38-
mockSuccessfulSpeechApi()
39-
mockSuccessfulVoiceInstructionsPlayer()
38+
fun `voice instruction should be played as SpeechAnnouncement`() =
39+
coroutineRule.runBlockingTest {
40+
mockSuccessfulSpeechApi()
41+
mockSuccessfulVoiceInstructionsPlayer()
4042

41-
val voiceInstructions = mockk<VoiceInstructions> {
42-
every { announcement() } returns "Turn right on Market"
43-
}
44-
carAppAudioGuidanceVoice.speak(voiceInstructions).collect { speechAnnouncement ->
43+
val voiceInstructions = mockk<VoiceInstructions> {
44+
every { announcement() } returns "Turn right on Market"
45+
}
46+
val speechAnnouncement = sut.speak(voiceInstructions)
4547
assertEquals("Turn right on Market", speechAnnouncement!!.announcement)
4648
}
47-
}
4849

4950
@Test
5051
fun `null should clean up the api and player`() = coroutineRule.runBlockingTest {
51-
carAppAudioGuidanceVoice.speak(null).collect()
52+
sut.speak(null)
5253

5354
verify { speechApi.cancel() }
5455
verify { voiceInstructionsPlayer.clear() }
@@ -70,18 +71,44 @@ class MapboxAudioGuidanceVoiceTest {
7071
val voiceInstructions = mockk<VoiceInstructions> {
7172
every { announcement() } returns "This message fails"
7273
}
73-
carAppAudioGuidanceVoice.speak(voiceInstructions).collect { speechAnnouncement ->
74-
assertEquals("Turn right on Market", speechAnnouncement!!.announcement)
75-
}
74+
val speechAnnouncement = sut.speak(voiceInstructions)
75+
assertEquals("Turn right on Market", speechAnnouncement!!.announcement)
7676
}
7777

78+
@Test
79+
fun `should wait until previous instruction finishes playback before playing next one`() =
80+
coroutineRule.runBlockingTest {
81+
mockSuccessfulSpeechApi()
82+
every { voiceInstructionsPlayer.play(any(), any()) } answers {
83+
launch {
84+
val speechAnnouncement = firstArg<SpeechAnnouncement>()
85+
delay(1000) // simulate 1 second announcement playback duration
86+
secondArg<MapboxNavigationConsumer<SpeechAnnouncement>>()
87+
.accept(speechAnnouncement)
88+
}
89+
Unit
90+
}
91+
92+
val played = mutableListOf<SpeechAnnouncement?>()
93+
launch {
94+
listOf(
95+
VoiceInstructions.builder().announcement("A").build(),
96+
VoiceInstructions.builder().announcement("B").build()
97+
).forEach {
98+
val announcement = sut.speak(it) // suspend until playback finishes
99+
played.add(announcement)
100+
}
101+
}
102+
advanceTimeBy(1500) // advance time to 50% of announcement B playback time
103+
104+
assertEquals(1, played.size)
105+
}
106+
78107
private fun mockSuccessfulSpeechApi() {
79108
every { speechApi.generate(any(), any()) } answers {
80109
val announcementArg = firstArg<VoiceInstructions>().announcement()
81110
val speechValue = mockk<SpeechValue> {
82-
every { announcement } returns mockk {
83-
every { announcement } returns announcementArg!!
84-
}
111+
every { announcement } returns SpeechAnnouncement.Builder(announcementArg!!).build()
85112
}
86113
val consumer = secondArg<MapboxNavigationConsumer<Expected<SpeechError, SpeechValue>>>()
87114
consumer.accept(ExpectedFactory.createValue(speechValue))

0 commit comments

Comments
 (0)