Skip to content

Commit 9f4bae6

Browse files
dzinaddzinad
committed
NAVSDK-777: meet code review
1 parent da27db1 commit 9f4bae6

File tree

8 files changed

+175
-21
lines changed

8 files changed

+175
-21
lines changed

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

Lines changed: 7 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -74,7 +74,7 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
7474
)
7575
}
7676

77-
@Test
77+
@Test(timeout = 10_000)
7878
fun route_refresh_on_demand_executes_before_refresh_interval() = sdkTest {
7979
val routeRefreshOptions = RouteRefreshOptions.Builder()
8080
.intervalMillis(TimeUnit.MINUTES.toMillis(1))
@@ -113,7 +113,7 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
113113
.build()
114114
RouteRefreshOptions::class.java.getDeclaredField("intervalMillis").apply {
115115
isAccessible = true
116-
set(routeRefreshOptions, 10_000L)
116+
set(routeRefreshOptions, 5_000L)
117117
}
118118
refreshHandler.jsonResponseModifier = DynamicResponseModifier()
119119
createMapboxNavigation(routeRefreshOptions)
@@ -129,20 +129,20 @@ class RouteRefreshOnDemandTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::
129129
}
130130
}
131131
mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
132-
delay(5000)
132+
delay(2500)
133133

134134
mapboxNavigation.refreshRoutesImmediately()
135135
mapboxNavigation.routesUpdates()
136136
.filter { it.reason == RoutesExtra.ROUTES_UPDATE_REASON_REFRESH }
137137
.first()
138138
assertEquals(1, routeRefreshes.size)
139139

140-
// no route refresh 6 seconds after refresh on demand
141-
delay(6000)
140+
// no route refresh 4 seconds after refresh on demand
141+
delay(4000)
142142
assertEquals(1, routeRefreshes.size)
143143

144-
delay(4000)
145-
// has new refresh 10 seconds after refresh on demand
144+
delay(1000)
145+
// has new refresh 5 seconds after refresh on demand
146146
mapboxNavigation.routesUpdates()
147147
.filter { it.reason == RoutesExtra.ROUTES_UPDATE_REASON_REFRESH }
148148
.take(2)

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

Lines changed: 72 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -173,6 +173,41 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
173173
)
174174
}
175175

176+
@Test
177+
fun notStartedUntilTimeElapses() = sdkTest {
178+
setupMockRequestHandlers(
179+
coordinates,
180+
R.raw.route_response_single_route_refresh,
181+
createRefreshHandler(
182+
R.raw.route_response_route_refresh_annotations,
183+
"route_response_single_route_refresh"
184+
)
185+
)
186+
187+
createMapboxNavigation(createRouteRefreshOptionsWithInvalidInterval(5000))
188+
mapboxNavigation.registerRouteRefreshStateObserver(observer)
189+
mapboxNavigation.startTripSession()
190+
val requestedRoutes = requestRoutes()
191+
mapboxNavigation.setNavigationRoutes(requestedRoutes)
192+
193+
delay(4000)
194+
195+
assertEquals(
196+
emptyList<String>(),
197+
observer.getStatesSnapshot()
198+
)
199+
200+
waitForRefresh()
201+
202+
assertEquals(
203+
listOf(
204+
RouteRefreshExtra.REFRESH_STATE_STARTED,
205+
RouteRefreshExtra.REFRESH_STATE_FINISHED_SUCCESS
206+
),
207+
observer.getStatesSnapshot()
208+
)
209+
}
210+
176211
@Test
177212
fun successfulFromSecondAttemptRefreshTest() = sdkTest {
178213
setupMockRequestHandlers(
@@ -267,7 +302,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
267302
)
268303
}
269304

270-
@Test
305+
@Test(timeout = 5_000)
271306
fun successfulRouteRefreshOnDemandTest() = sdkTest {
272307
setupMockRequestHandlers(
273308
coordinates,
@@ -296,7 +331,7 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
296331
)
297332
}
298333

299-
@Test
334+
@Test(timeout = 5_000)
300335
fun failedRouteRefreshOnDemandTest() = sdkTest {
301336
setupMockRequestHandlers(
302337
coordinates,
@@ -405,6 +440,41 @@ class RouteRefreshStateTest : BaseTest<EmptyTestActivity>(EmptyTestActivity::cla
405440
)
406441
}
407442

443+
@Test
444+
fun routeRefreshOnDemandFailsThenPlannedTest() = sdkTest {
445+
val refreshHandler = createRefreshHandler(
446+
R.raw.route_response_route_refresh_annotations,
447+
"route_response_single_route_refresh"
448+
)
449+
refreshHandler.jsonResponseModifier = DynamicResponseModifier()
450+
setupMockRequestHandlers(
451+
coordinates,
452+
R.raw.route_response_single_route_refresh,
453+
NthAttemptHandler(refreshHandler, 1)
454+
)
455+
456+
createMapboxNavigation(createRouteRefreshOptionsWithInvalidInterval(5_000))
457+
mapboxNavigation.registerRouteRefreshStateObserver(observer)
458+
mapboxNavigation.startTripSession()
459+
val requestedRoutes = requestRoutes()
460+
mapboxNavigation.setNavigationRoutesAndWaitForUpdate(requestedRoutes)
461+
462+
mapboxNavigation.refreshRoutesImmediately()
463+
delay(5000)
464+
465+
waitForRefreshes(2) // immediate + planned
466+
467+
assertEquals(
468+
listOf(
469+
RouteRefreshExtra.REFRESH_STATE_STARTED,
470+
RouteRefreshExtra.REFRESH_STATE_FINISHED_FAILED,
471+
RouteRefreshExtra.REFRESH_STATE_STARTED,
472+
RouteRefreshExtra.REFRESH_STATE_FINISHED_SUCCESS,
473+
),
474+
observer.getStatesSnapshot()
475+
)
476+
}
477+
408478
@Test
409479
fun routeRefreshOnDemandBetweenPlannedAttemptsTest() = sdkTest {
410480
val refreshHandler = createRefreshHandler(

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

Lines changed: 5 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -952,6 +952,11 @@ class MapboxNavigation @VisibleForTesting internal constructor(
952952
/**
953953
* Immediately refresh current navigation routes.
954954
* Listen for refreshed routes using [RoutesObserver].
955+
*
956+
* The on-demand refresh request is not guaranteed to succeed and call the [RoutesObserver],
957+
* [refreshRoutesImmediately] invocations cannot be coupled with
958+
* [RoutesObserver.onRoutesChanged] callbacks for state management.
959+
* You can use [registerRouteRefreshStateObserver] to monitor refresh statuses independently.
955960
*/
956961
fun refreshRoutesImmediately() {
957962
routeRefreshController.requestImmediateRouteRefresh(getNavigationRoutes())

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

Lines changed: 11 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -32,6 +32,7 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
3232
RetryRouteRefreshStrategy(maxRetryCount = 2)
3333
)
3434

35+
private var paused = false
3536
private var routesToRefresh: List<NavigationRoute>? = null
3637

3738
fun startRoutesRefreshing(routes: List<NavigationRoute>) {
@@ -63,13 +64,19 @@ internal class PlannedRouteRefreshController @VisibleForTesting constructor(
6364
}
6465

6566
override fun pause() {
66-
cancellableHandler.cancelAll()
67+
if (!paused) {
68+
paused = true
69+
cancellableHandler.cancelAll()
70+
}
6771
}
6872

6973
override fun resume() {
70-
routesToRefresh?.let {
71-
if (retryStrategy.shouldRetry()) {
72-
scheduleUpdateRetry(it)
74+
if (paused) {
75+
paused = false
76+
routesToRefresh?.let {
77+
if (retryStrategy.shouldRetry()) {
78+
scheduleUpdateRetry(it)
79+
}
7380
}
7481
}
7582
}

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

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@ internal object RouteRefreshStateChanger {
1818
),
1919
RouteRefreshExtra.REFRESH_STATE_CANCELED to listOf(
2020
RouteRefreshExtra.REFRESH_STATE_STARTED,
21+
RouteRefreshExtra.REFRESH_STATE_FINISHED_FAILED,
2122
null,
2223
),
2324
RouteRefreshExtra.REFRESH_STATE_FINISHED_FAILED to listOf(

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

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -43,15 +43,15 @@ internal class RouteRefresher(
4343
val refreshedRoutes = refreshRoutesOrNull(routes, routeProgressData, routeRefreshTimeout)
4444
return if (refreshedRoutes.any { it != null }) {
4545
RouteRefresherResult(
46-
true,
46+
success = true,
4747
refreshedRoutes.mapIndexed { index, navigationRoute ->
4848
navigationRoute ?: routes[index]
4949
},
5050
routeProgressData
5151
)
5252
} else {
5353
RouteRefresherResult(
54-
false,
54+
success = false,
5555
routes.map { removeExpiringDataFromRoute(it, routeProgressData.legIndex) },
5656
routeProgressData
5757
)

0 commit comments

Comments
 (0)