NAVAND-1842 Don't reset route request when off-route is flaky (#9824)

yuryybk · github-actions[bot] · commit 472b19076138 · 2025-07-31T15:27:35.000Z
* NAVAND-1842 Don't reset route request when off-route is flaky

GitOrigin-RevId: 817cc2e36acabd18833f3bd5daa575aa26bae892
diff --git a/changelog/unreleased/bugfixes/9824.md b/changelog/unreleased/bugfixes/9824.md
@@ -0,0 +1 @@
+- Don't reset the re-route request when on-route/off-route events are flaky.
diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/CoreRerouteTest.kt
@@ -237,13 +237,24 @@ class CoreRerouteTest(
         mockWebServerRule.requestHandlers.add(requestHandler)
         mockWebServerRule.requestHandlers.add(rerouteRequestHandler)
 
-        val rerouteStateTransitionAssertion = RerouteStateTransitionAssertion(
-            mapboxNavigation.getRerouteController()!!,
-        ) {
-            requiredState(RerouteState.Idle)
-            requiredState(RerouteState.FetchingRoute)
-            requiredState(RerouteState.Interrupted)
-            requiredState(RerouteState.Idle)
+        // TODO This behavior should be the same for both NN and Nav SDK reroute controllers
+        // Keep the single flow when NN will apply fix
+        val rerouteStateTransitionAssertion = if (!runOptions.nativeReroute) {
+            RerouteStateTransitionAssertion(
+                mapboxNavigation.getRerouteController()!!,
+            ) {
+                requiredState(RerouteState.Idle)
+                requiredState(RerouteState.FetchingRoute)
+            }
+        } else {
+            RerouteStateTransitionAssertion(
+                mapboxNavigation.getRerouteController()!!,
+            ) {
+                requiredState(RerouteState.Idle)
+                requiredState(RerouteState.FetchingRoute)
+                requiredState(RerouteState.Interrupted)
+                requiredState(RerouteState.Idle)
+            }
         }
 
         val originalRoutes = mapboxNavigation.requestRoutes(
diff --git a/navigation/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt b/navigation/src/main/java/com/mapbox/navigation/core/MapboxNavigation.kt
@@ -2190,8 +2190,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(
     private fun createInternalOffRouteObserver() = OffRouteObserver { offRoute ->
         if (offRoute) {
             rerouteOnDeviation()
-        } else {
-            rerouteController?.interrupt()
         }
     }
 
@@ -2257,10 +2255,12 @@ class MapboxNavigation @VisibleForTesting internal constructor(
 
     private fun rerouteOnDeviation() {
         rerouteController?.rerouteOnDeviation { result: RerouteResult ->
-            internalSetNavigationRoutes(
-                result.routes,
-                SetRoutes.Reroute(result.initialLegIndex),
-            )
+            if (tripSession.isOffRoute) {
+                internalSetNavigationRoutes(
+                    result.routes,
+                    SetRoutes.Reroute(result.initialLegIndex),
+                )
+            }
         }
     }
 
diff --git a/navigation/src/main/java/com/mapbox/navigation/core/reroute/MapboxRerouteController.kt b/navigation/src/main/java/com/mapbox/navigation/core/reroute/MapboxRerouteController.kt
@@ -56,6 +56,8 @@ internal class MapboxRerouteController @VisibleForTesting constructor(
 
     private var runningRerouteCausedByRouteReplan = false
 
+    private var lastSignature: GetRouteSignature? = null
+
     constructor(
         directionsSession: DirectionsSession,
         tripSession: TripSession,
@@ -86,6 +88,9 @@ internal class MapboxRerouteController @VisibleForTesting constructor(
             if (value is RerouteState.Idle) {
                 runningRerouteCausedByRouteReplan = false
             }
+            if (value !is RerouteState.FetchingRoute) {
+                lastSignature = null
+            }
             observers.forEach { it.onRerouteStateChanged(field) }
         }
 
@@ -122,7 +127,10 @@ internal class MapboxRerouteController @VisibleForTesting constructor(
     }
 
     override fun rerouteOnDeviation(callback: RoutesCallback) {
-        rerouteInternal(deviationSignature, callback)
+        // Do not reroute if we are already fetching a route for deviation
+        if (state != RerouteState.FetchingRoute || lastSignature != deviationSignature) {
+            rerouteInternal(deviationSignature, callback)
+        }
     }
 
     override fun rerouteOnParametersChange(callback: RoutesCallback) {
@@ -134,6 +142,7 @@ internal class MapboxRerouteController @VisibleForTesting constructor(
         signature: GetRouteSignature,
         callback: RoutesCallback,
     ) {
+        this.lastSignature = signature
         val ignoreDeviationToAlternatives = runningRerouteCausedByRouteReplan
         logD(LOG_CATEGORY) {
             "Starting reroute"
diff --git a/navigation/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt b/navigation/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt
@@ -235,8 +235,8 @@ internal class MapboxTripSession(
             stateObservers.forEach { it.onSessionStateChanged(value) }
         }
 
-    private var isOffRoute: Boolean = false
-        set(value) {
+    override var isOffRoute: Boolean = false
+        private set(value) {
             if (field == value) {
                 return
             }
diff --git a/navigation/src/main/java/com/mapbox/navigation/core/trip/session/TripSession.kt b/navigation/src/main/java/com/mapbox/navigation/core/trip/session/TripSession.kt
@@ -19,6 +19,7 @@ internal interface TripSession {
     fun getRawLocation(): Location?
     val zLevel: Int?
     val locationMatcherResult: LocationMatcherResult?
+    val isOffRoute: Boolean
     fun getRouteProgress(): RouteProgress?
     fun getState(): TripSessionState
 
diff --git a/navigation/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt b/navigation/src/test/java/com/mapbox/navigation/core/MapboxNavigationTest.kt
@@ -512,10 +512,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
             it.onOffRouteStateChanged(false)
         }
 
-        verify(exactly = 1) { defaultRerouteController.interrupt() }
-        verify(ordering = Ordering.ORDERED) {
+        verify(exactly = 0) { defaultRerouteController.interrupt() }
+        verify(exactly = 1) {
             tripSession.registerOffRouteObserver(any())
-            defaultRerouteController.interrupt()
         }
     }
 
@@ -552,9 +551,12 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
         val observers = mutableListOf<OffRouteObserver>()
         verify { tripSession.registerOffRouteObserver(capture(observers)) }
 
+        every { tripSession.isOffRoute } returns true
+
         observers.forEach {
             it.onOffRouteStateChanged(true)
         }
+
         coVerify(exactly = 1) {
             directionsSession.setNavigationRoutesFinished(
                 DirectionsSessionRoutes(
@@ -1469,6 +1471,8 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {
             } returns NativeSetRouteValue(newRoutes, emptyList())
             createMapboxNavigation()
 
+            every { tripSession.isOffRoute } returns true
+
             observers.forEach {
                 it.onOffRouteStateChanged(true)
             }
diff --git a/navigation/src/test/java/com/mapbox/navigation/core/reroute/MapboxRerouteControllerTest.kt b/navigation/src/test/java/com/mapbox/navigation/core/reroute/MapboxRerouteControllerTest.kt
@@ -861,19 +861,13 @@ class MapboxRerouteControllerTest {
             rerouteController.rerouteOnDeviation(internalRouteCallback)
             routeRequestCallback1.captured.onCanceled(routeOptions1, RouterOrigin.ONLINE)
         }
-        verify(exactly = 1) { directionsSession.cancelRouteRequest(1L) }
-
-        routeRequestCallback2.captured.onRoutesReady(
-            listOf(mockk(relaxed = true)),
-            RouterOrigin.ONLINE,
-        )
+        verify(exactly = 0) { directionsSession.cancelRouteRequest(1L) }
 
         verifyOrder {
+            primaryRerouteObserver.onRerouteStateChanged(RerouteState.Idle)
             primaryRerouteObserver.onRerouteStateChanged(RerouteState.FetchingRoute)
             primaryRerouteObserver.onRerouteStateChanged(RerouteState.Interrupted)
             primaryRerouteObserver.onRerouteStateChanged(RerouteState.Idle)
-            primaryRerouteObserver.onRerouteStateChanged(RerouteState.FetchingRoute)
-            primaryRerouteObserver.onRerouteStateChanged(ofType<RerouteState.RouteFetched>())
         }
     }
 
@@ -1154,6 +1148,11 @@ class MapboxRerouteControllerTest {
             clearMocks(routeOptionsUpdater, answers = false)
             rerouteController.rerouteOnDeviation(internalRouteCallback)
 
+            routeRequestCallback.captured.onRoutesReady(
+                listOf(mockk(relaxed = true)),
+                RouterOrigin.ONLINE,
+            )
+
             verify(exactly = 1) {
                 routeOptionsUpdater.update(
                     mockRoute.toBuilder().avoidManeuverRadius(expectedMetersRadius).build(),
@@ -1162,11 +1161,6 @@ class MapboxRerouteControllerTest {
                 )
             }
         }
-
-        routeRequestCallback.captured.onRoutesReady(
-            listOf(mockk(relaxed = true)),
-            RouterOrigin.ONLINE,
-        )
     }
 
     @Test
@@ -1204,6 +1198,11 @@ class MapboxRerouteControllerTest {
             clearMocks(routeOptionsUpdater, answers = false)
             rerouteController.rerouteOnDeviation(internalRouteCallback)
 
+            routeRequestCallback.captured.onRoutesReady(
+                listOf(mockk(relaxed = true)),
+                RouterOrigin.ONLINE,
+            )
+
             verify(exactly = 1) {
                 routeOptionsUpdater.update(
                     mockRo.toBuilder().avoidManeuverRadius(if (result) 200.0 else null).build(),
@@ -1212,11 +1211,6 @@ class MapboxRerouteControllerTest {
                 )
             }
         }
-
-        routeRequestCallback.captured.onRoutesReady(
-            listOf(mockk(relaxed = true)),
-            RouterOrigin.ONLINE,
-        )
     }
 
     @Test

Original file line number	Diff line number	Diff line change
`@@ -0,0 +1 @@`
	`1`	`+- Don't reset the re-route request when on-route/off-route events are flaky.`
Original file line number	Diff line number	Diff line change
`@@ -2190,8 +2190,6 @@ class MapboxNavigation @VisibleForTesting internal constructor(`
`2190`	`2190`	`private fun createInternalOffRouteObserver() = OffRouteObserver { offRoute ->`
`2191`	`2191`	`if (offRoute) {`
`2192`	`2192`	`rerouteOnDeviation()`
`2193`		`- } else {`
`2194`		`- rerouteController?.interrupt()`
`2195`	`2193`	`}`
`2196`	`2194`	`}`
`2197`	`2195`
`@@ -2257,10 +2255,12 @@ class MapboxNavigation @VisibleForTesting internal constructor(`
`2257`	`2255`
`2258`	`2256`	`private fun rerouteOnDeviation() {`
`2259`	`2257`	`rerouteController?.rerouteOnDeviation { result: RerouteResult ->`
`2260`		`- internalSetNavigationRoutes(`
`2261`		`- result.routes,`
`2262`		`- SetRoutes.Reroute(result.initialLegIndex),`
`2263`		`- )`
	`2258`	`+ if (tripSession.isOffRoute) {`
	`2259`	`+ internalSetNavigationRoutes(`
	`2260`	`+ result.routes,`
	`2261`	`+ SetRoutes.Reroute(result.initialLegIndex),`
	`2262`	`+ )`
	`2263`	`+ }`
`2264`	`2264`	`}`
`2265`	`2265`	`}`
`2266`	`2266`
Original file line number	Diff line number	Diff line change
`@@ -235,8 +235,8 @@ internal class MapboxTripSession(`
`235`	`235`	`stateObservers.forEach { it.onSessionStateChanged(value) }`
`236`	`236`	`}`
`237`	`237`
`238`		`- private var isOffRoute: Boolean = false`
`239`		`- set(value) {`
	`238`	`+ override var isOffRoute: Boolean = false`
	`239`	`+ private set(value) {`
`240`	`240`	`if (field == value) {`
`241`	`241`	`return`
`242`	`242`	`}`
Original file line number	Diff line number	Diff line change
`@@ -512,10 +512,9 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {`
`512`	`512`	`it.onOffRouteStateChanged(false)`
`513`	`513`	`}`
`514`	`514`
`515`		`- verify(exactly = 1) { defaultRerouteController.interrupt() }`
`516`		`- verify(ordering = Ordering.ORDERED) {`
	`515`	`+ verify(exactly = 0) { defaultRerouteController.interrupt() }`
	`516`	`+ verify(exactly = 1) {`
`517`	`517`	`tripSession.registerOffRouteObserver(any())`
`518`		`- defaultRerouteController.interrupt()`
`519`	`518`	`}`
`520`	`519`	`}`
`521`	`520`
`@@ -552,9 +551,12 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {`
`552`	`551`	`val observers = mutableListOf<OffRouteObserver>()`
`553`	`552`	`verify { tripSession.registerOffRouteObserver(capture(observers)) }`
`554`	`553`
	`554`	`+ every { tripSession.isOffRoute } returns true`
	`555`	`+`
`555`	`556`	`observers.forEach {`
`556`	`557`	`it.onOffRouteStateChanged(true)`
`557`	`558`	`}`
	`559`	`+`
`558`	`560`	`coVerify(exactly = 1) {`
`559`	`561`	`directionsSession.setNavigationRoutesFinished(`
`560`	`562`	`DirectionsSessionRoutes(`
`@@ -1469,6 +1471,8 @@ internal class MapboxNavigationTest : MapboxNavigationBaseTest() {`
`1469`	`1471`	`} returns NativeSetRouteValue(newRoutes, emptyList())`
`1470`	`1472`	`createMapboxNavigation()`
`1471`	`1473`
	`1474`	`+ every { tripSession.isOffRoute } returns true`
	`1475`	`+`
`1472`	`1476`	`observers.forEach {`
`1473`	`1477`	`it.onOffRouteStateChanged(true)`
`1474`	`1478`	`}`