diff --git a/changelog/unreleased/bugfixes/7662.md b/changelog/unreleased/bugfixes/7662.md new file mode 100644 index 00000000000..53b31f3bebd --- /dev/null +++ b/changelog/unreleased/bugfixes/7662.md @@ -0,0 +1 @@ +- Fixed an issue where the first voice instruction might have been played thrice when switching between regular session and replay with route being set. \ No newline at end of file diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/WaypointsTest.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/WaypointsTest.kt index 51f75f0e77d..0ed0ade8a92 100644 --- a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/WaypointsTest.kt +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/WaypointsTest.kt @@ -20,6 +20,7 @@ import com.mapbox.navigation.core.internal.extensions.flowLocationMatcherResult import com.mapbox.navigation.instrumentation_tests.R import com.mapbox.navigation.instrumentation_tests.activity.EmptyTestActivity import com.mapbox.navigation.instrumentation_tests.utils.ApproximateCoordinates +import com.mapbox.navigation.instrumentation_tests.utils.assertions.waitUntilHasSize import com.mapbox.navigation.instrumentation_tests.utils.http.MockDirectionsRequestHandler import com.mapbox.navigation.instrumentation_tests.utils.location.MockLocationReplayerRule import com.mapbox.navigation.instrumentation_tests.utils.readRawFileText @@ -37,7 +38,6 @@ import kotlinx.coroutines.delay import kotlinx.coroutines.flow.filter import kotlinx.coroutines.flow.take import kotlinx.coroutines.flow.toList -import kotlinx.coroutines.withTimeout import org.junit.Assert.assertEquals import org.junit.Assert.assertNull import org.junit.Before @@ -278,15 +278,6 @@ class WaypointsTest : BaseTest(EmptyTestActivity::class.java) assertEquals(expected.longitude(), actual.longitude(), 0.00001) } - private suspend fun List<*>.waitUntilHasSize(size: Int) { - val list = this - withTimeout(10000) { - while (list.size < size) { - delay(50) - } - } - } - private fun stayOnPosition(point: Point, bearing: Float = 0f) { mockLocationReplayerRule.loopUpdate( mockLocationUpdatesRule.generateLocationUpdate { diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/ui/VoiceInstructionsTest.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/ui/VoiceInstructionsTest.kt new file mode 100644 index 00000000000..a27839748cd --- /dev/null +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/ui/VoiceInstructionsTest.kt @@ -0,0 +1,97 @@ +package com.mapbox.navigation.instrumentation_tests.ui + +import android.location.Location +import androidx.test.espresso.Espresso +import com.mapbox.api.directions.v5.models.VoiceInstructions +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.base.options.RoutingTilesOptions +import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.MapboxNavigationProvider +import com.mapbox.navigation.core.replay.route.ReplayRouteSession +import com.mapbox.navigation.core.trip.session.VoiceInstructionsObserver +import com.mapbox.navigation.instrumentation_tests.utils.assertions.waitUntilHasSize +import com.mapbox.navigation.instrumentation_tests.utils.history.MapboxHistoryTestRule +import com.mapbox.navigation.instrumentation_tests.utils.location.MockLocationReplayerRule +import com.mapbox.navigation.instrumentation_tests.utils.location.stayOnPosition +import com.mapbox.navigation.instrumentation_tests.utils.routes.RoutesProvider +import com.mapbox.navigation.instrumentation_tests.utils.routes.RoutesProvider.toNavigationRoutes +import com.mapbox.navigation.testing.ui.BaseCoreNoCleanUpTest +import com.mapbox.navigation.testing.ui.utils.MapboxNavigationRule +import com.mapbox.navigation.testing.ui.utils.coroutines.sdkTest +import com.mapbox.navigation.testing.ui.utils.coroutines.setNavigationRoutesAsync +import com.mapbox.navigation.testing.ui.utils.getMapboxAccessTokenFromResources +import org.junit.Assert.assertEquals +import org.junit.Assert.assertNotEquals +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import java.net.URI + +class VoiceInstructionsTest : BaseCoreNoCleanUpTest() { + + @get:Rule + val mapboxNavigationRule = MapboxNavigationRule() + + @get:Rule + val mockLocationReplayerRule = MockLocationReplayerRule(mockLocationUpdatesRule) + + @get:Rule + val mapboxHistoryTestRule = MapboxHistoryTestRule() + + @Before + fun setup() { + Espresso.onIdle() + } + + override fun setupMockLocation(): Location { + val mockRoute = RoutesProvider.dc_very_short(context) + return mockLocationUpdatesRule.generateLocationUpdate { + latitude = mockRoute.routeWaypoints.first().latitude() + longitude = mockRoute.routeWaypoints.first().longitude() + } + } + + @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) + @Test + fun voiceInstructionIsDuplicatedOnceWhenReplayIsStarted() = sdkTest { + val mockRoute = RoutesProvider.dc_very_short(context) + mockWebServerRule.requestHandlers.addAll(mockRoute.mockRequestHandlers) + val voiceInstructions = mutableListOf() + val voiceInstructionsObserver = VoiceInstructionsObserver { + voiceInstructions.add(it) + } + val mapboxNavigation = createMapboxNavigation() + mapboxNavigation.registerVoiceInstructionsObserver(voiceInstructionsObserver) + stayOnPosition( + mockRoute.routeWaypoints.first(), + 0f, + ) { + mapboxNavigation.startTripSession() + mapboxNavigation.setNavigationRoutesAsync(mockRoute.toNavigationRoutes()) + voiceInstructions.waitUntilHasSize(1) + } + val relayRouteSession = ReplayRouteSession() + relayRouteSession.onAttached(mapboxNavigation) + voiceInstructions.waitUntilHasSize(3, timeoutMillis = 15000) + + // the first instruction is duplicated once as a result of starting replay session + assertEquals(voiceInstructions[0], voiceInstructions[1]) + // the first instruction id not duplicated anymore + assertNotEquals(voiceInstructions[1], voiceInstructions[2]) + } + + private fun createMapboxNavigation(): MapboxNavigation { + val navigationOptions = NavigationOptions.Builder(context) + .accessToken(getMapboxAccessTokenFromResources(context)) + .routingTilesOptions( + RoutingTilesOptions.Builder() + .tilesBaseUri(URI(mockWebServerRule.baseUrl)) + .build() + ) + .build() + return MapboxNavigationProvider.create(navigationOptions).also { + mapboxHistoryTestRule.historyRecorder = it.historyRecorder + } + } +} diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/assertions/ListAssertions.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/assertions/ListAssertions.kt new file mode 100644 index 00000000000..e3a58f57749 --- /dev/null +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/assertions/ListAssertions.kt @@ -0,0 +1,13 @@ +package com.mapbox.navigation.instrumentation_tests.utils.assertions + +import kotlinx.coroutines.delay +import kotlinx.coroutines.withTimeout + +suspend fun List<*>.waitUntilHasSize(size: Int, timeoutMillis: Long = 10000) { + val list = this + withTimeout(timeoutMillis) { + while (list.size < size) { + delay(50) + } + } +} diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt index dbc66dd8b7f..87cc8df09d8 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt @@ -1995,11 +1995,23 @@ class MapboxNavigation @VisibleForTesting internal constructor( private fun startSession(withTripService: Boolean, withReplayEnabled: Boolean) { runIfNotDestroyed { + val previousState = tripSession.getState() tripSession.start( withTripService = withTripService, withReplayEnabled = withReplayEnabled ) - resetTripSessionRoutes() + // It's possible that we are in a state when routes are set, + // but session is not started. In this case, as soon as routes are set, + // NN starts generating status updates (which results in our route progress updates, + // voice and banner instructions events, etc.). + // If we first set the routes and then start the session, we might lose all the events + // between these two actions (because native status observer is only registered + // when the session is started). To avoid that, + // we set the routes again on session start. Then NN will generate all the + // relevant events again, because it will treat the routes as completely new. + if (previousState == TripSessionState.STOPPED) { + resetTripSessionRoutes() + } notificationChannelField?.let { monitorNotificationActionButton(it.get(null) as ReceiveChannel) } diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt index 35a67944a88..0849a0fe05e 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt @@ -38,6 +38,7 @@ import com.mapbox.navigation.core.trip.session.NavigationSession import com.mapbox.navigation.core.trip.session.NavigationSessionState import com.mapbox.navigation.core.trip.session.TripSession import com.mapbox.navigation.core.trip.session.TripSessionLocationEngine +import com.mapbox.navigation.core.trip.session.TripSessionState import com.mapbox.navigation.core.trip.session.createSetRouteResult import com.mapbox.navigation.metrics.internal.EventsServiceProvider import com.mapbox.navigation.metrics.internal.TelemetryServiceProvider @@ -84,7 +85,9 @@ internal open class MapboxNavigationBaseTest { val cache: CacheHandle = mockk(relaxUnitFun = true) val navigator: MapboxNativeNavigator = mockk(relaxUnitFun = true) val tripService: TripService = mockk(relaxUnitFun = true) - val tripSession: TripSession = mockk(relaxUnitFun = true) + val tripSession: TripSession = mockk(relaxUnitFun = true) { + every { getState() } returns TripSessionState.STOPPED + } val locationEngine: LocationEngine = mockk(relaxUnitFun = true) val distanceFormatterOptions: DistanceFormatterOptions = mockk(relaxed = true) val routingTilesOptions: RoutingTilesOptions = mockk(relaxed = true) diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt index a35f5bbf6f5..a995efaef7e 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt @@ -122,7 +122,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { } @Test - fun `trip session route is reset after trip session is restarted`() { + fun `trip session route is reset after trip session is started`() { createMapboxNavigation() val primary = mockk() val routes = listOf(primary, mockk()) @@ -135,6 +135,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { } } mapboxNavigation.stopTripSession() + every { tripSession.getState() } returns TripSessionState.STOPPED mapboxNavigation.startTripSession() coVerify(exactly = 1) { @@ -145,6 +146,30 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { } } + @Test + fun `trip session route is not reset after trip session is started twice`() { + createMapboxNavigation() + val primary = mockk() + val routes = listOf(primary, mockk()) + val currentLegIndex = 3 + every { directionsSession.routes } returns routes + every { directionsSession.initialLegIndex } returns 2 + every { tripSession.getRouteProgress() } returns mockk { + every { currentLegProgress } returns mockk { + every { legIndex } returns currentLegIndex + } + } + mapboxNavigation.stopTripSession() + mapboxNavigation.startTripSession() + clearAllMocks(answers = false) + every { tripSession.getState() } returns TripSessionState.STARTED + mapboxNavigation.startTripSession() + + coVerify(exactly = 0) { + tripSession.setRoutes(any(), any()) + } + } + @Test fun `getZLevel returns current z level`() { createMapboxNavigation() @@ -1794,6 +1819,7 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() { directionsSession.routes } returns (firstArg() as DirectionsSessionRoutes).acceptedRoutes } + every { tripSession.getState() } returns TripSessionState.STOPPED pauseDispatcher { mapboxNavigation.setNavigationRoutes(inputRoutes)