NAVAND-1636: do not set routes to NN when starting a session

changelog/unreleased/bugfixes/dd.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.

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>(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 {

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<VoiceInstructions>()
+        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)
+
+        // 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
+        }
+    }
+}

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) {
+    val list = this
+    withTimeout(10000) {
+        while (list.size < size) {
+            delay(50)
+        }
+    }
+}

libnavigation-core/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt

@@ -1153,16 +1153,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         }
     }
 
-    private fun resetTripSessionRoutes() {
-        threadController.getMainScopeAndRootJob().scope.launch(Dispatchers.Main.immediate) {
-            routeUpdateMutex.withLock {
-                val routes = directionsSession.routes
-                val legIndex = latestLegIndex ?: directionsSession.initialLegIndex
-                setRoutesToTripSession(routes, SetRoutes.NewRoutes(legIndex))
-            }
-        }
-    }
-
     /**
      * Call to set routes to trip session.
      * This call should be synchronized and only one should be executing at a time.
@@ -1999,7 +1989,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
                 withTripService = withTripService,
                 withReplayEnabled = withReplayEnabled
             )
-            resetTripSessionRoutes()
             notificationChannelField?.let {
                 monitorNotificationActionButton(it.get(null) as ReceiveChannel<NotificationAction>)
             }

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 not reset after trip session is restarted`() {
         createMapboxNavigation()
         val primary = mockk<NavigationRoute>()
         val routes = listOf(primary, mockk())
@@ -137,11 +137,8 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
         mapboxNavigation.stopTripSession()
         mapboxNavigation.startTripSession()
 
-        coVerify(exactly = 1) {
-            tripSession.setRoutes(
-                routes,
-                SetRoutes.NewRoutes(currentLegIndex)
-            )
+        coVerify(exactly = 0) {
+            tripSession.setRoutes(any(), any())
         }
     }
 
@@ -1808,7 +1805,6 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
                 directionsSession.setNavigationRoutesFinished(
                     DirectionsSessionRoutes(acceptedRoutes, ignoredRoutes, setRoutesInfo)
                 )
-                tripSession.setRoutes(acceptedRoutes, setRoutesInfo)
             }
 
             val routesSlot = mutableListOf<DirectionsSessionRoutes>()