fixes an issue where NavigationRoute#upcomingRoadObjects was not re…

…freshed
mapbox · Sep 23, 2022 · 2c7aed7 · 2c7aed7
1 parent 3a65560
commit 2c7aed7
Show file tree

Hide file tree

Showing 12 changed files with 124 additions and 39 deletions.
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -10,6 +10,7 @@ Mapbox welcomes participation and contributions from everyone.
 - Fixed the issue with incorrect geometry indices for `RouteLeg#incidents` and `RouteLeg#closures` after refresh. [#6364](https://github.com/mapbox/mapbox-navigation-android/pull/6364)
 - Improved alternatives id robustness by adding new alternatives to existing instead of replacing them during `MapboxNavigation#requestAlternativeRoutes`. [#6373](https://github.com/mapbox/mapbox-navigation-android/pull/6373)
 - Improved stop detector for auto profile. [#6373](https://github.com/mapbox/mapbox-navigation-android/pull/6373)
+- Fixed an issue where `NavigationRoute#upcomingRoadObjects` was not refreshed. This issue did not impact the deprecated `RoadObjectsOnRouteObserver`. [#6378](https://github.com/mapbox/mapbox-navigation-android/pull/6378)
 
 ## Mapbox Navigation SDK 2.8.0-rc.2 - 16 September, 2022
 ### Changelog

diff --git a/...src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRefreshTest.kt b/...src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRefreshTest.kt
@@ -139,17 +139,22 @@ class RouteRefreshTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.ja
             val initialRoutes = routeUpdates[0]
             val refreshedRoutes = routeUpdates[1]
 
-            mapboxNavigation.routeProgressUpdates()
+            val routeProgress = mapboxNavigation.routeProgressUpdates()
                 .filter { routeProgress -> isRefreshedRouteDistance(routeProgress) }
                 .first()
-            mapboxNavigation.roadObjectsOnRoute()
+            val roadObjectsFromObserver = mapboxNavigation.roadObjectsOnRoute()
                 .filter { upcomingRoadObjects ->
                     upcomingRoadObjects.size == 2 &&
                         upcomingRoadObjects.map { it.roadObject.id }
                             .containsAll(listOf("11589180127444257", "14158569638505033"))
                 }
                 .first()
 
+            assertEquals(roadObjectsFromObserver, refreshedRoutes.first().upcomingRoadObjects)
+            assertEquals(
+                routeProgress.navigationRoute.upcomingRoadObjects,
+                refreshedRoutes.first().upcomingRoadObjects
+            )
             assertEquals(
                 "the test works only with 2 routes",
                 2,

diff --git a/...androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/coroutines/Adapters.kt b/...androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/coroutines/Adapters.kt
@@ -1,3 +1,5 @@
+@file:OptIn(ExperimentalCoroutinesApi::class)
+
 package com.mapbox.navigation.instrumentation_tests.utils.coroutines
 
 import com.mapbox.api.directions.v5.models.BannerInstructions
@@ -20,6 +22,7 @@ import com.mapbox.navigation.core.trip.session.BannerInstructionsObserver
 import com.mapbox.navigation.core.trip.session.RoadObjectsOnRouteObserver
 import com.mapbox.navigation.core.trip.session.RouteProgressObserver
 import com.mapbox.navigation.core.trip.session.VoiceInstructionsObserver
+import kotlinx.coroutines.ExperimentalCoroutinesApi
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow

diff --git a/...igation-base/src/main/java/com/mapbox/navigation/base/internal/route/NavigationRouteEx.kt b/...igation-base/src/main/java/com/mapbox/navigation/base/internal/route/NavigationRouteEx.kt
@@ -119,6 +119,13 @@ fun NavigationRoute.updateDirectionsRouteOnly(
     return copy(directionsResponse = refreshedResponse)
 }
 
+/**
+ * Used to rebuild any [NavigationRoute] fields that are backed by a native peer, which might've been refreshed.
+ *
+ * At the moment, all fields are `val`s, so a simple re-instantiation is enough.
+ */
+fun NavigationRoute.refreshNativePeer(): NavigationRoute = copy()
+
 /**
  * Internal API used for testing purposes. Needed to avoid calling native parser from unit tests.
  */

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
@@ -880,7 +880,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
                                     processedRoute.route.routeId == passedRoute.id
                                 }
                             }
-                        directionsSession.setRoutes(routes, setRoutesInfo)
+                        directionsSession.setRoutes(processedRoutes.routes, setRoutesInfo)
                         routesSetResult = ExpectedFactory.createValue(
                             RoutesSetSuccess(
                                 ignoredAlternatives.associate {
@@ -930,7 +930,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
         return tripSession.setRoutes(routes, setRoutesInfo).apply {
             if (this is NativeSetRouteValue) {
                 routeAlternativesController.processAlternativesMetadata(
-                    routes,
+                    this.routes,
                     nativeAlternatives
                 )
             }

diff --git a/...avigation-core/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt b/...avigation-core/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt
@@ -8,6 +8,7 @@ import com.mapbox.api.directions.v5.models.VoiceInstructions
 import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
 import com.mapbox.navigation.base.internal.factory.RoadFactory
 import com.mapbox.navigation.base.internal.factory.TripNotificationStateFactory.buildTripNotificationState
+import com.mapbox.navigation.base.internal.route.refreshNativePeer
 import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigation.base.trip.model.RouteLegProgress
 import com.mapbox.navigation.base.trip.model.RouteProgress
@@ -90,16 +91,30 @@ internal class MapboxTripSession(
             }
             is SetAlternativeRoutesInfo -> {
                 NativeSetRouteValue(
+                    routes = routes,
                     nativeAlternatives = navigator.setAlternativeRoutes(routes.drop(1))
                 )
             }
             is SetRefreshedRoutesInfo -> {
                 if (routes.isNotEmpty()) {
                     val primaryRoute = routes.first()
-                    navigator.refreshRoute(primaryRoute).onValue {
-                        this@MapboxTripSession.primaryRoute = routes.first()
-                        roadObjects = primaryRoute.upcomingRoadObjects
-                    }.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(it) }).also {
+                    navigator.refreshRoute(primaryRoute).fold(
+                        { NativeSetRouteError(it) },
+                        { value ->
+                            val refreshedPrimaryRoute = primaryRoute.refreshNativePeer()
+                            this@MapboxTripSession.primaryRoute = refreshedPrimaryRoute
+                            roadObjects = refreshedPrimaryRoute.upcomingRoadObjects
+                            val refreshedRoutes = routes
+                                .drop(1)
+                                .toMutableList().apply {
+                                    add(0, refreshedPrimaryRoute)
+                                }
+                            NativeSetRouteValue(
+                                routes = refreshedRoutes,
+                                nativeAlternatives = value
+                            )
+                        }
+                    ).also {
                         logD(
                             "routes update (route IDs: ${routes.map { it.id }}) - refresh finished",
                             LOG_CATEGORY
@@ -144,7 +159,7 @@ internal class MapboxTripSession(
             routeProgress = null
         }.mapValue {
             it.alternatives
-        }.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(it) }).also {
+        }.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(routes, it) }).also {
             logD(
                 "native routes update (route IDs: ${routes.map { it.id }}) - finished",
                 LOG_CATEGORY

diff --git a/...gation-core/src/main/java/com/mapbox/navigation/core/trip/session/NativeSetRouteResult.kt b/...gation-core/src/main/java/com/mapbox/navigation/core/trip/session/NativeSetRouteResult.kt
@@ -1,5 +1,6 @@
 package com.mapbox.navigation.core.trip.session
 
+import com.mapbox.navigation.base.route.NavigationRoute
 import com.mapbox.navigator.RouteAlternative
 
 /**
@@ -13,11 +14,12 @@ internal sealed class NativeSetRouteResult
  * @param nativeAlternatives Set routes.
  */
 internal class NativeSetRouteValue(
+    val routes: List<NavigationRoute>,
     val nativeAlternatives: List<RouteAlternative>
 ) : NativeSetRouteResult() {
 
     override fun toString(): String {
-        return "NativeSetRouteValue(nativeAlternatives=$nativeAlternatives)"
+        return "NativeSetRouteValue(routes=$routes, nativeAlternatives=$nativeAlternatives)"
     }
 
     override fun equals(other: Any?): Boolean {
@@ -26,13 +28,16 @@ internal class NativeSetRouteValue(
 
         other as NativeSetRouteValue
 
+        if (routes != other.routes) return false
         if (nativeAlternatives != other.nativeAlternatives) return false
 
         return true
     }
 
     override fun hashCode(): Int {
-        return nativeAlternatives.hashCode()
+        var result = routes.hashCode()
+        result = 31 * result + nativeAlternatives.hashCode()
+        return result
     }
 }
 

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
@@ -290,9 +290,12 @@ internal open class MapboxNavigationBaseTest {
             )
         } returns tripSession
         every { tripSession.getRouteProgress() } returns routeProgress
-        coEvery { tripSession.setRoutes(any(), any()) } returns NativeSetRouteValue(
-            nativeAlternatives = emptyList()
-        )
+        coEvery { tripSession.setRoutes(any(), any()) } answers {
+            NativeSetRouteValue(
+                routes = firstArg(),
+                nativeAlternatives = emptyList()
+            )
+        }
     }
 
     private fun mockDirectionSession() {

diff --git a/...c/test/java/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesCallbackTest.kt b/...c/test/java/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesCallbackTest.kt
@@ -53,6 +53,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
                 )
             } returns NativeSetRouteValue(
+                routes,
                 listOf(
                     routeAlternativeWithId(alternativeId1),
                     routeAlternativeWithId(alternativeId2),
@@ -78,7 +79,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     routes,
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
                 )
-            } returns NativeSetRouteValue(emptyList())
+            } returns NativeSetRouteValue(routes, emptyList())
 
             mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
 
@@ -121,7 +122,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_CLEAN_UP, initialLegIndex),
 
                 )
-            } returns NativeSetRouteValue(emptyList())
+            } returns NativeSetRouteValue(routes, emptyList())
 
             mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
 
@@ -142,7 +143,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     routes,
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
                 )
-            } returns NativeSetRouteValue(emptyList())
+            } returns NativeSetRouteValue(routes, emptyList())
 
             mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
 
@@ -170,6 +171,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
                 )
             } returns NativeSetRouteValue(
+                routes,
                 listOf(routeAlternativeWithId("bad id 1"), routeAlternativeWithId("bad id 2"))
             )
 
@@ -198,7 +200,10 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
                     routes,
                     BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
                 )
-            } returns NativeSetRouteValue(listOf(routeAlternativeWithId(alternativeId2)))
+            } returns NativeSetRouteValue(
+                routes,
+                listOf(routeAlternativeWithId(alternativeId2))
+            )
 
             mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
 

diff --git a/...ava/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesHistoryRecordingTest.kt b/...ava/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesHistoryRecordingTest.kt
@@ -35,7 +35,7 @@ internal class MapboxNavigationSetNavigationRoutesHistoryRecordingTest :
                 routes,
                 BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
             )
-        } returns NativeSetRouteValue(emptyList())
+        } returns NativeSetRouteValue(routes, emptyList())
 
         mapboxNavigation.setNavigationRoutes(routes, initialLegIndex)
 
@@ -69,7 +69,7 @@ internal class MapboxNavigationSetNavigationRoutesHistoryRecordingTest :
                 routes,
                 BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
             )
-        } returns NativeSetRouteValue(emptyList())
+        } returns NativeSetRouteValue(routes, emptyList())
 
         mapboxNavigation.setNavigationRoutes(routes, initialLegIndex)