Skip to content

Commit 81ffcd4

Browse files
author
Łukasz Paczos
committed
fixes an issue where NavigationRoute#upcomingRoadObjects was not refreshed
1 parent 6af814c commit 81ffcd4

12 files changed

Lines changed: 123 additions & 39 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -23,6 +23,7 @@ Mapbox welcomes participation and contributions from everyone.
2323
- Fixed `RoadNameLabel` position issues by making sure that it updates when expanding and collapsing the info panel. [#6361](https://github.com/mapbox/mapbox-navigation-android/pull/6361)
2424
- Fixed `InfoPanel` overlapping the `RoadNameLabel` in free drive state with the device being in landscape orientation. [#6361](https://github.com/mapbox/mapbox-navigation-android/pull/6361)
2525
- Introduced `ViewStyleCustomization#mapScalebarParams` that allows for configuring map scalebar. [#6355](https://github.com/mapbox/mapbox-navigation-android/pull/6355)
26+
- 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)
2627

2728
## Mapbox Navigation SDK 2.8.0-rc.3 - 23 September, 2022
2829
### Changelog

‎instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/RouteRefreshTest.kt‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -139,17 +139,22 @@ class RouteRefreshTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::class.ja
139139
val initialRoutes = routeUpdates[0]
140140
val refreshedRoutes = routeUpdates[1]
141141

142-
mapboxNavigation.routeProgressUpdates()
142+
val routeProgress = mapboxNavigation.routeProgressUpdates()
143143
.filter { routeProgress -> isRefreshedRouteDistance(routeProgress) }
144144
.first()
145-
mapboxNavigation.roadObjectsOnRoute()
145+
val roadObjectsFromObserver = mapboxNavigation.roadObjectsOnRoute()
146146
.filter { upcomingRoadObjects ->
147147
upcomingRoadObjects.size == 2 &&
148148
upcomingRoadObjects.map { it.roadObject.id }
149149
.containsAll(listOf("11589180127444257", "14158569638505033"))
150150
}
151151
.first()
152152

153+
assertEquals(roadObjectsFromObserver, refreshedRoutes.first().upcomingRoadObjects)
154+
assertEquals(
155+
routeProgress.navigationRoute.upcomingRoadObjects,
156+
refreshedRoutes.first().upcomingRoadObjects
157+
)
153158
assertEquals(
154159
"the test works only with 2 routes",
155160
2,

‎instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/coroutines/Adapters.kt‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -1,3 +1,5 @@
1+
@file:OptIn(ExperimentalCoroutinesApi::class)
2+
13
package com.mapbox.navigation.instrumentation_tests.utils.coroutines
24

35
import com.mapbox.api.directions.v5.models.BannerInstructions
@@ -20,6 +22,7 @@ import com.mapbox.navigation.core.trip.session.BannerInstructionsObserver
2022
import com.mapbox.navigation.core.trip.session.RoadObjectsOnRouteObserver
2123
import com.mapbox.navigation.core.trip.session.RouteProgressObserver
2224
import com.mapbox.navigation.core.trip.session.VoiceInstructionsObserver
25+
import kotlinx.coroutines.ExperimentalCoroutinesApi
2326
import kotlinx.coroutines.channels.awaitClose
2427
import kotlinx.coroutines.flow.Flow
2528
import kotlinx.coroutines.flow.callbackFlow

‎libnavigation-base/src/main/java/com/mapbox/navigation/base/internal/route/NavigationRouteEx.kt‎

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -119,6 +119,13 @@ fun NavigationRoute.updateDirectionsRouteOnly(
119119
return copy(directionsResponse = refreshedResponse)
120120
}
121121

122+
/**
123+
* Used to rebuild any [NavigationRoute] fields that are backed by a native peer, which might've been refreshed.
124+
*
125+
* At the moment, all fields are `val`s, so a simple re-instantiation is enough.
126+
*/
127+
fun NavigationRoute.refreshNativePeer(): NavigationRoute = copy()
128+
122129
/**
123130
* Internal API used for testing purposes. Needed to avoid calling native parser from unit tests.
124131
*/

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -877,7 +877,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
877877
processedRoute.route.routeId == passedRoute.id
878878
}
879879
}
880-
directionsSession.setRoutes(routes, setRoutesInfo)
880+
directionsSession.setRoutes(processedRoutes.routes, setRoutesInfo)
881881
routesSetResult = ExpectedFactory.createValue(
882882
RoutesSetSuccess(
883883
ignoredAlternatives.associate {
@@ -927,7 +927,7 @@ class MapboxNavigation @VisibleForTesting internal constructor(
927927
return tripSession.setRoutes(routes, setRoutesInfo).apply {
928928
if (this is NativeSetRouteValue) {
929929
routeAlternativesController.processAlternativesMetadata(
930-
routes,
930+
this.routes,
931931
nativeAlternatives
932932
)
933933
}

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/MapboxTripSession.kt‎

Lines changed: 20 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -8,6 +8,7 @@ import com.mapbox.api.directions.v5.models.VoiceInstructions
88
import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
99
import com.mapbox.navigation.base.internal.factory.RoadFactory
1010
import com.mapbox.navigation.base.internal.factory.TripNotificationStateFactory.buildTripNotificationState
11+
import com.mapbox.navigation.base.internal.route.refreshNativePeer
1112
import com.mapbox.navigation.base.route.NavigationRoute
1213
import com.mapbox.navigation.base.trip.model.RouteLegProgress
1314
import com.mapbox.navigation.base.trip.model.RouteProgress
@@ -90,16 +91,30 @@ internal class MapboxTripSession(
9091
}
9192
is SetAlternativeRoutesInfo -> {
9293
NativeSetRouteValue(
94+
routes = routes,
9395
nativeAlternatives = navigator.setAlternativeRoutes(routes.drop(1))
9496
)
9597
}
9698
is SetRefreshedRoutesInfo -> {
9799
if (routes.isNotEmpty()) {
98100
val primaryRoute = routes.first()
99-
navigator.refreshRoute(primaryRoute).onValue {
100-
this@MapboxTripSession.primaryRoute = routes.first()
101-
roadObjects = primaryRoute.upcomingRoadObjects
102-
}.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(it) }).also {
101+
navigator.refreshRoute(primaryRoute).fold(
102+
{ NativeSetRouteError(it) },
103+
{ value ->
104+
val refreshedPrimaryRoute = primaryRoute.refreshNativePeer()
105+
this@MapboxTripSession.primaryRoute = refreshedPrimaryRoute
106+
roadObjects = refreshedPrimaryRoute.upcomingRoadObjects
107+
val refreshedRoutes = routes
108+
.drop(1)
109+
.toMutableList().apply {
110+
add(0, refreshedPrimaryRoute)
111+
}
112+
NativeSetRouteValue(
113+
routes = refreshedRoutes,
114+
nativeAlternatives = value
115+
)
116+
}
117+
).also {
103118
logD(
104119
"routes update (route IDs: ${routes.map { it.id }}) - refresh finished",
105120
LOG_CATEGORY
@@ -144,7 +159,7 @@ internal class MapboxTripSession(
144159
routeProgress = null
145160
}.mapValue {
146161
it.alternatives
147-
}.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(it) }).also {
162+
}.fold({ NativeSetRouteError(it) }, { NativeSetRouteValue(routes, it) }).also {
148163
logD(
149164
"native routes update (route IDs: ${routes.map { it.id }}) - finished",
150165
LOG_CATEGORY

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/NativeSetRouteResult.kt‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
package com.mapbox.navigation.core.trip.session
22

3+
import com.mapbox.navigation.base.route.NavigationRoute
34
import com.mapbox.navigator.RouteAlternative
45

56
/**
@@ -13,11 +14,12 @@ internal sealed class NativeSetRouteResult
1314
* @param nativeAlternatives Set routes.
1415
*/
1516
internal class NativeSetRouteValue(
17+
val routes: List<NavigationRoute>,
1618
val nativeAlternatives: List<RouteAlternative>
1719
) : NativeSetRouteResult() {
1820

1921
override fun toString(): String {
20-
return "NativeSetRouteValue(nativeAlternatives=$nativeAlternatives)"
22+
return "NativeSetRouteValue(routes=$routes, nativeAlternatives=$nativeAlternatives)"
2123
}
2224

2325
override fun equals(other: Any?): Boolean {
@@ -26,13 +28,16 @@ internal class NativeSetRouteValue(
2628

2729
other as NativeSetRouteValue
2830

31+
if (routes != other.routes) return false
2932
if (nativeAlternatives != other.nativeAlternatives) return false
3033

3134
return true
3235
}
3336

3437
override fun hashCode(): Int {
35-
return nativeAlternatives.hashCode()
38+
var result = routes.hashCode()
39+
result = 31 * result + nativeAlternatives.hashCode()
40+
return result
3641
}
3742
}
3843

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationBaseTest.kt‎

Lines changed: 6 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -294,9 +294,12 @@ internal open class MapboxNavigationBaseTest {
294294
)
295295
} returns tripSession
296296
every { tripSession.getRouteProgress() } returns routeProgress
297-
coEvery { tripSession.setRoutes(any(), any()) } returns NativeSetRouteValue(
298-
nativeAlternatives = emptyList()
299-
)
297+
coEvery { tripSession.setRoutes(any(), any()) } answers {
298+
NativeSetRouteValue(
299+
routes = firstArg(),
300+
nativeAlternatives = emptyList()
301+
)
302+
}
300303
}
301304

302305
private fun mockDirectionSession() {

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesCallbackTest.kt‎

Lines changed: 9 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -53,6 +53,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
5353
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
5454
)
5555
} returns NativeSetRouteValue(
56+
routes,
5657
listOf(
5758
routeAlternativeWithId(alternativeId1),
5859
routeAlternativeWithId(alternativeId2),
@@ -78,7 +79,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
7879
routes,
7980
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
8081
)
81-
} returns NativeSetRouteValue(emptyList())
82+
} returns NativeSetRouteValue(routes, emptyList())
8283

8384
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
8485

@@ -121,7 +122,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
121122
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_CLEAN_UP, initialLegIndex),
122123

123124
)
124-
} returns NativeSetRouteValue(emptyList())
125+
} returns NativeSetRouteValue(routes, emptyList())
125126

126127
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
127128

@@ -142,7 +143,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
142143
routes,
143144
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
144145
)
145-
} returns NativeSetRouteValue(emptyList())
146+
} returns NativeSetRouteValue(routes, emptyList())
146147

147148
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
148149

@@ -170,6 +171,7 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
170171
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
171172
)
172173
} returns NativeSetRouteValue(
174+
routes,
173175
listOf(routeAlternativeWithId("bad id 1"), routeAlternativeWithId("bad id 2"))
174176
)
175177

@@ -198,7 +200,10 @@ internal class MapboxNavigationSetNavigationRoutesCallbackTest : MapboxNavigatio
198200
routes,
199201
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
200202
)
201-
} returns NativeSetRouteValue(listOf(routeAlternativeWithId(alternativeId2)))
203+
} returns NativeSetRouteValue(
204+
routes,
205+
listOf(routeAlternativeWithId(alternativeId2))
206+
)
202207

203208
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex, callback)
204209

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/MapboxNavigationSetNavigationRoutesHistoryRecordingTest.kt‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -35,7 +35,7 @@ internal class MapboxNavigationSetNavigationRoutesHistoryRecordingTest :
3535
routes,
3636
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
3737
)
38-
} returns NativeSetRouteValue(emptyList())
38+
} returns NativeSetRouteValue(routes, emptyList())
3939

4040
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex)
4141

@@ -69,7 +69,7 @@ internal class MapboxNavigationSetNavigationRoutesHistoryRecordingTest :
6969
routes,
7070
BasicSetRoutesInfo(RoutesExtra.ROUTES_UPDATE_REASON_NEW, initialLegIndex)
7171
)
72-
} returns NativeSetRouteValue(emptyList())
72+
} returns NativeSetRouteValue(routes, emptyList())
7373

7474
mapboxNavigation.setNavigationRoutes(routes, initialLegIndex)
7575

0 commit comments

Comments
 (0)