Skip to content

Commit 0636da9

Browse files
RingerJKRingerJK
committed
fix calculating remaining waypoints + add tests
1 parent 94b2a61 commit 0636da9

File tree

3 files changed

+138
-32
lines changed

3 files changed

+138
-32
lines changed

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

Lines changed: 2 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -10,7 +10,6 @@ import com.mapbox.navigation.base.ExperimentalMapboxNavigationAPI
1010
import com.mapbox.navigation.base.internal.factory.RoadFactory
1111
import com.mapbox.navigation.base.internal.factory.TripNotificationStateFactory.buildTripNotificationState
1212
import com.mapbox.navigation.base.internal.route.refreshNativePeer
13-
import com.mapbox.navigation.base.internal.utils.internalWaypoints
1413
import com.mapbox.navigation.base.route.NavigationRoute
1514
import com.mapbox.navigation.base.trip.model.RouteLegProgress
1615
import com.mapbox.navigation.base.trip.model.RouteProgress
@@ -31,7 +30,7 @@ import com.mapbox.navigation.core.trip.session.eh.EHorizonObserver
3130
import com.mapbox.navigation.core.trip.session.eh.EHorizonSubscriptionManager
3231
import com.mapbox.navigation.navigator.internal.MapboxNativeNavigator
3332
import com.mapbox.navigation.navigator.internal.MapboxNativeNavigatorImpl
34-
import com.mapbox.navigation.navigator.internal.TripStatus
33+
import com.mapbox.navigation.navigator.internal.utils.calculateRemainingWaypoints
3534
import com.mapbox.navigation.utils.internal.JobControl
3635
import com.mapbox.navigation.utils.internal.ThreadController
3736
import com.mapbox.navigation.utils.internal.ifNonNull
@@ -48,7 +47,6 @@ import kotlinx.coroutines.Job
4847
import kotlinx.coroutines.cancelChildren
4948
import kotlinx.coroutines.launch
5049
import java.util.concurrent.CopyOnWriteArraySet
51-
import kotlin.math.max
5250

5351
/**
5452
* Default implementation of [TripSession]
@@ -75,7 +73,6 @@ internal class MapboxTripSession(
7573

7674
private companion object {
7775
private const val LOG_CATEGORY = "MapboxTripSession"
78-
private const val INDEX_OF_INITIAL_LEG_TARGET = 1
7976
}
8077

8178
override suspend fun setRoutes(
@@ -356,7 +353,7 @@ internal class MapboxTripSession(
356353
nativeBannerInstruction?.index
357354
)
358355
}
359-
val remainingWaypoints = calculateRemainingWaypoints(tripStatus)
356+
val remainingWaypoints = tripStatus.calculateRemainingWaypoints()
360357
val routeProgress = getRouteProgressFrom(
361358
tripStatus.route,
362359
tripStatus.navigationStatus,
@@ -378,33 +375,6 @@ internal class MapboxTripSession(
378375
triggerVoiceInstructionEvent(routeProgress, status)
379376
isOffRoute = tripStatus.navigationStatus.routeState == RouteState.OFF_ROUTE
380377
}
381-
382-
private fun calculateRemainingWaypoints(tripStatus: TripStatus): Int {
383-
val routeCoordinates = tripStatus.route?.internalWaypoints()
384-
return if (routeCoordinates != null) {
385-
val waypointsCount = routeCoordinates.size
386-
val nextWaypointIndex = normalizeNextWaypointIndex(
387-
tripStatus.navigationStatus.nextWaypointIndex
388-
)
389-
return waypointsCount - nextWaypointIndex
390-
} else {
391-
0
392-
}
393-
}
394-
395-
/**
396-
* On the Android side, we always start navigation from the current position.
397-
* So we expect that the next waypoint index will not be less than 1.
398-
* But the native part considers the origin as a usual waypoint.
399-
* It can return the next waypoint index 0. Be careful, this case isn't easy to reproduce.
400-
*
401-
* For example, nextWaypointIndex=0 leads to an incorrect rerouting.
402-
* We don't want to get to an initial position even it hasn't been reached yet.
403-
*/
404-
private fun normalizeNextWaypointIndex(nextWaypointIndex: Int) = max(
405-
INDEX_OF_INITIAL_LEG_TARGET,
406-
nextWaypointIndex
407-
)
408378
}
409379

410380
/**

‎libnavigator/src/main/java/com/mapbox/navigation/navigator/internal/utils/TripStatusEx.kt‎

Lines changed: 34 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,34 @@
1+
package com.mapbox.navigation.navigator.internal.utils
2+
3+
import com.mapbox.navigation.base.internal.utils.internalWaypoints
4+
import com.mapbox.navigation.navigator.internal.TripStatus
5+
import kotlin.math.max
6+
7+
private const val INDEX_OF_INITIAL_LEG_TARGET = 1
8+
9+
fun TripStatus.calculateRemainingWaypoints(): Int {
10+
val routeWaypoints = this.route?.internalWaypoints()
11+
return if (routeWaypoints != null) {
12+
val waypointsCount = routeWaypoints.size
13+
val nextWaypointIndex = normalizeNextWaypointIndex(
14+
this.navigationStatus.nextWaypointIndex
15+
)
16+
return waypointsCount - nextWaypointIndex
17+
} else {
18+
0
19+
}
20+
}
21+
22+
/**
23+
* On the Android side, we always start navigation from the current position.
24+
* So we expect that the next waypoint index will not be less than 1.
25+
* But the native part considers the origin as a usual waypoint.
26+
* It can return the next waypoint index 0. Be careful, this case isn't easy to reproduce.
27+
*
28+
* For example, nextWaypointIndex=0 leads to an incorrect rerouting.
29+
* We don't want to get to an initial position even it hasn't been reached yet.
30+
*/
31+
private fun normalizeNextWaypointIndex(nextWaypointIndex: Int) = max(
32+
INDEX_OF_INITIAL_LEG_TARGET,
33+
nextWaypointIndex
34+
)

‎libnavigator/src/test/java/com/mapbox/navigation/navigator/internal/router/utils/TripStatusExTest.kt‎

Lines changed: 102 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,102 @@
1+
package com.mapbox.navigation.navigator.internal.router.utils
2+
3+
import com.mapbox.geojson.Point
4+
import com.mapbox.navigation.base.internal.route.Waypoint
5+
import com.mapbox.navigation.base.internal.utils.WaypointFactory
6+
import com.mapbox.navigation.base.internal.utils.internalWaypoints
7+
import com.mapbox.navigation.base.route.NavigationRoute
8+
import com.mapbox.navigation.navigator.internal.TripStatus
9+
import com.mapbox.navigation.navigator.internal.utils.calculateRemainingWaypoints
10+
import io.mockk.every
11+
import io.mockk.mockk
12+
import junit.framework.Assert.assertEquals
13+
import org.junit.Test
14+
import org.junit.runner.RunWith
15+
import org.junit.runners.Parameterized
16+
17+
class TripStatusExTest {
18+
19+
@RunWith(Parameterized::class)
20+
class CalculateRemainingWaypointsTest(
21+
val description: String,
22+
val routeWithWaypoints: NavigationRoute?,
23+
val nextWaypointIndex: Int,
24+
val expectedResult: Int,
25+
) {
26+
companion object {
27+
@JvmStatic
28+
@Parameterized.Parameters(name = "{0}")
29+
fun data() = listOf(
30+
arrayOf(
31+
"null route returns 0",
32+
null,
33+
Int.MIN_VALUE,
34+
0,
35+
),
36+
arrayOf(
37+
"next waypoint index normalized is normalized to default initial leg target",
38+
mockk<NavigationRoute> {
39+
every {
40+
internalWaypoints()
41+
} returns provideMockListOfWaypoints(
42+
Waypoint.REGULAR,
43+
Waypoint.REGULAR
44+
)
45+
},
46+
0,
47+
1,
48+
),
49+
arrayOf(
50+
"all waypoints are taken into account to get remaining waypoints",
51+
mockk<NavigationRoute> {
52+
every {
53+
internalWaypoints()
54+
} returns provideMockListOfWaypoints(
55+
Waypoint.REGULAR,
56+
Waypoint.SILENT,
57+
Waypoint.EV_CHARGING,
58+
Waypoint.SILENT,
59+
Waypoint.REGULAR,
60+
Waypoint.REGULAR,
61+
Waypoint.SILENT,
62+
Waypoint.EV_CHARGING,
63+
Waypoint.EV_CHARGING,
64+
Waypoint.REGULAR,
65+
Waypoint.REGULAR,
66+
)
67+
},
68+
8,
69+
3,
70+
),
71+
)
72+
73+
private fun provideMockListOfWaypoints(
74+
@Waypoint.Type vararg types: Int
75+
): List<Waypoint> =
76+
types.map { type ->
77+
WaypointFactory.provideWaypoint(
78+
Point.fromLngLat(0.0, 0.0),
79+
"",
80+
null,
81+
type
82+
)
83+
}
84+
}
85+
86+
@Test
87+
fun testCases() {
88+
val tripStatus = TripStatus(
89+
routeWithWaypoints,
90+
mockk {
91+
every {
92+
nextWaypointIndex
93+
} returns this@CalculateRemainingWaypointsTest.nextWaypointIndex
94+
}
95+
)
96+
97+
val result = tripStatus.calculateRemainingWaypoints()
98+
99+
assertEquals(description, expectedResult, result)
100+
}
101+
}
102+
}

0 commit comments

Comments
 (0)