Skip to content

Commit d8eeb04

Browse files
kmadsenkmadsen
authored
Remove stopTripSession from ReplayRouteSession (#6817)
1 parent 61415ea commit d8eeb04

File tree

11 files changed

+197
-117
lines changed

11 files changed

+197
-117
lines changed

‎changelog/unreleased/bugfixes/6817.md‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1 @@
1+
- Improved `MapboxNavigation#startReplayTripSession` and `ReplayRouteSession` so that the previous trip session does not need to be stopped. :warning: `ReplayRouteSession#onDetached` removed the call to `stopTripSession`.

‎libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/route/ReplayRouteSession.kt‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -106,7 +106,6 @@ class ReplayRouteSession : MapboxNavigationObserver {
106106
override fun onAttached(mapboxNavigation: MapboxNavigation) {
107107
this.replayRouteMapper = ReplayRouteMapper(options.replayRouteOptions)
108108
this.mapboxNavigation = mapboxNavigation
109-
mapboxNavigation.stopTripSession()
110109
mapboxNavigation.startReplayTripSession()
111110
mapboxNavigation.registerRouteProgressObserver(routeProgressObserver)
112111
mapboxNavigation.registerRoutesObserver(routesObserver)
@@ -139,7 +138,6 @@ class ReplayRouteSession : MapboxNavigationObserver {
139138
mapboxNavigation.mapboxReplayer.unregisterObserver(replayEventsObserver)
140139
mapboxNavigation.mapboxReplayer.stop()
141140
mapboxNavigation.mapboxReplayer.clearEvents()
142-
mapboxNavigation.stopTripSession()
143141
this.mapboxNavigation = null
144142
this.currentRoute = null
145143
}

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

Lines changed: 29 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -267,6 +267,28 @@ internal class MapboxTripSession(
267267
}
268268
}
269269

270+
private val onRawLocationUpdate: (Location) -> Unit = { rawLocation ->
271+
val locationHash = rawLocation.hashCode()
272+
logD(
273+
"updateRawLocation; system elapsed time: ${System.nanoTime()}; " +
274+
"location ($locationHash) elapsed time: ${rawLocation.elapsedRealtimeNanos}",
275+
LOG_CATEGORY
276+
)
277+
this.rawLocation = rawLocation
278+
locationObservers.forEach { it.onNewRawLocation(rawLocation) }
279+
mainJobController.scope.launch(start = CoroutineStart.UNDISPATCHED) {
280+
logD(
281+
"updateRawLocation; notify navigator for ($locationHash) - start",
282+
LOG_CATEGORY
283+
)
284+
navigator.updateLocation(rawLocation.toFixLocation())
285+
logD(
286+
"updateRawLocation; notify navigator for ($locationHash) - end",
287+
LOG_CATEGORY
288+
)
289+
}
290+
}
291+
270292
init {
271293
navigator.setNativeNavigatorRecreationObserver {
272294
if (fallbackVersionsObservers.isNotEmpty()) {
@@ -297,39 +319,14 @@ internal class MapboxTripSession(
297319
* Start MapboxTripSession
298320
*/
299321
override fun start(withTripService: Boolean, withReplayEnabled: Boolean) {
300-
if (state == TripSessionState.STARTED) {
301-
return
302-
}
303-
navigator.addNavigatorObserver(navigatorObserver)
304-
if (withTripService) {
305-
tripService.startService()
306-
}
307-
tripSessionLocationEngine.startLocationUpdates(withReplayEnabled) {
308-
updateRawLocation(it)
309-
}
310-
state = TripSessionState.STARTED
311-
}
312-
313-
private fun updateRawLocation(rawLocation: Location) {
314-
val locationHash = rawLocation.hashCode()
315-
logD(
316-
"updateRawLocation; system elapsed time: ${System.nanoTime()}; " +
317-
"location ($locationHash) elapsed time: ${rawLocation.elapsedRealtimeNanos}",
318-
LOG_CATEGORY
319-
)
320-
this.rawLocation = rawLocation
321-
locationObservers.forEach { it.onNewRawLocation(rawLocation) }
322-
mainJobController.scope.launch(start = CoroutineStart.UNDISPATCHED) {
323-
logD(
324-
"updateRawLocation; notify navigator for ($locationHash) - start",
325-
LOG_CATEGORY
326-
)
327-
navigator.updateLocation(rawLocation.toFixLocation())
328-
logD(
329-
"updateRawLocation; notify navigator for ($locationHash) - end",
330-
LOG_CATEGORY
331-
)
322+
if (state != TripSessionState.STARTED) {
323+
navigator.addNavigatorObserver(navigatorObserver)
324+
if (withTripService) {
325+
tripService.startService()
326+
}
327+
state = TripSessionState.STARTED
332328
}
329+
tripSessionLocationEngine.startLocationUpdates(withReplayEnabled, onRawLocationUpdate)
333330
}
334331

335332
/**

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

Lines changed: 29 additions & 25 deletions
Original file line numberDiff line numberDiff line change
@@ -24,55 +24,60 @@ import java.util.concurrent.TimeUnit
2424
* trip session is not using replay, use the [NavigationOptions.locationEngine].
2525
*/
2626
internal class TripSessionLocationEngine constructor(
27-
val navigationOptions: NavigationOptions
27+
private val navigationOptions: NavigationOptions,
28+
private val replayLocationEngineProvider: (MapboxReplayer) -> LocationEngine = {
29+
ReplayLocationEngine(it)
30+
}
2831
) {
2932

3033
val mapboxReplayer: MapboxReplayer by lazy { MapboxReplayer() }
3134

32-
private val replayLocationEngine: ReplayLocationEngine by lazy {
33-
ReplayLocationEngine(mapboxReplayer)
35+
private val replayLocationEngine: LocationEngine by lazy {
36+
replayLocationEngineProvider.invoke(mapboxReplayer)
3437
}
35-
private var locationEngine: LocationEngine = navigationOptions.locationEngine
38+
private var activeLocationEngine: LocationEngine? = null
3639
private var onRawLocationUpdate: (Location) -> Unit = { }
3740

41+
private val locationEngineCallback = object : LocationEngineCallback<LocationEngineResult> {
42+
override fun onSuccess(result: LocationEngineResult?) {
43+
logD(LOG_CATEGORY) {
44+
"successful location engine callback $result"
45+
}
46+
result?.locations?.lastOrNull()?.let {
47+
logIfLocationIsNotFreshEnough(it)
48+
onRawLocationUpdate(it)
49+
}
50+
}
51+
52+
override fun onFailure(exception: Exception) {
53+
logD("location on failure exception=$exception", LOG_CATEGORY)
54+
}
55+
}
56+
3857
@SuppressLint("MissingPermission")
3958
fun startLocationUpdates(isReplayEnabled: Boolean, onRawLocationUpdate: (Location) -> Unit) {
4059
logD(LOG_CATEGORY) {
4160
"starting location updates for ${if (isReplayEnabled) "replay " else ""}location engine"
4261
}
62+
stopLocationUpdates()
4363
this.onRawLocationUpdate = onRawLocationUpdate
44-
val locationEngine = if (isReplayEnabled) {
64+
activeLocationEngine = if (isReplayEnabled) {
4565
replayLocationEngine
4666
} else {
4767
navigationOptions.locationEngine
4868
}
49-
locationEngine.requestLocationUpdates(
69+
activeLocationEngine?.requestLocationUpdates(
5070
navigationOptions.locationEngineRequest,
5171
locationEngineCallback,
5272
Looper.getMainLooper()
5373
)
54-
locationEngine.getLastLocation(locationEngineCallback)
74+
activeLocationEngine?.getLastLocation(locationEngineCallback)
5575
}
5676

5777
fun stopLocationUpdates() {
5878
onRawLocationUpdate = { }
59-
locationEngine.removeLocationUpdates(locationEngineCallback)
60-
}
61-
62-
private var locationEngineCallback = object : LocationEngineCallback<LocationEngineResult> {
63-
override fun onSuccess(result: LocationEngineResult?) {
64-
logD(LOG_CATEGORY) {
65-
"successful location engine callback $result"
66-
}
67-
result?.locations?.lastOrNull()?.let {
68-
logIfLocationIsNotFreshEnough(it)
69-
onRawLocationUpdate(it)
70-
}
71-
}
72-
73-
override fun onFailure(exception: Exception) {
74-
logD("location on failure exception=$exception", LOG_CATEGORY)
75-
}
79+
activeLocationEngine?.removeLocationUpdates(locationEngineCallback)
80+
activeLocationEngine = null
7681
}
7782

7883
private fun logIfLocationIsNotFreshEnough(location: Location) {
@@ -88,7 +93,6 @@ internal class TripSessionLocationEngine constructor(
8893
}
8994

9095
private companion object {
91-
9296
private const val DELAYED_LOCATION_WARNING_THRESHOLD_MS = 500 // 0.5s
9397
private const val LOG_CATEGORY = "TripSessionLocationEngine"
9498
}

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/route/ReplayRouteSessionTest.kt‎

Lines changed: 2 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -84,17 +84,6 @@ class ReplayRouteSessionTest {
8484
unmockkAll()
8585
}
8686

87-
@Test
88-
fun `onAttached - should stop trip session and start replay session`() {
89-
sut.onAttached(mapboxNavigation)
90-
91-
verifyOrder {
92-
mapboxNavigation.stopTripSession()
93-
mapboxNavigation.startReplayTripSession()
94-
replayer.play()
95-
}
96-
}
97-
9887
@Test
9988
fun `onAttached - should reset trip session and replayer when navigation routes are cleared`() {
10089
val routesObserver = slot<RoutesObserver>()
@@ -139,13 +128,13 @@ class ReplayRouteSessionTest {
139128
}
140129

141130
@Test
142-
fun `onDetached - should stop trip session and replayer`() {
131+
fun `onDetached - should stop and clear the replayer`() {
143132
sut.onDetached(mapboxNavigation)
144133

145134
verifyOrder {
135+
replayer.unregisterObserver(any())
146136
replayer.stop()
147137
replayer.clearEvents()
148-
mapboxNavigation.stopTripSession()
149138
}
150139
}
151140

@@ -251,7 +240,6 @@ class ReplayRouteSessionTest {
251240
)
252241

253242
verifyOrder {
254-
mapboxNavigation.stopTripSession()
255243
mapboxNavigation.startReplayTripSession()
256244
replayer.play()
257245
replayer.pushEvents(any())

‎libnavigation-core/src/test/java/com/mapbox/navigation/core/trip/session/MapboxTripSessionTest.kt‎

Lines changed: 28 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -238,16 +238,39 @@ class MapboxTripSessionTest {
238238
@Test
239239
fun startSessionWithTripServiceCallStartAgain() {
240240
tripSession.start(true)
241-
242241
tripSession.start(false)
243-
244242
every { tripService.hasServiceStarted() } returns true
245-
246243
assertTrue(tripSession.isRunningWithForegroundService())
247-
verify(exactly = 1) { tripService.startService() }
248-
verify(exactly = 1) { tripSessionLocationEngine.startLocationUpdates(any(), any()) }
244+
tripSession.stop()
245+
246+
verifyOrder {
247+
navigator.addNavigatorObserver(any())
248+
tripService.startService()
249+
tripSessionLocationEngine.startLocationUpdates(any(), any())
250+
tripSessionLocationEngine.startLocationUpdates(any(), any())
251+
navigator.removeNavigatorObserver(any())
252+
tripSession.stop()
253+
tripSessionLocationEngine.stopLocationUpdates()
254+
}
255+
}
249256

257+
@Test
258+
fun startSessionWithReplayCallStartAgain() {
259+
tripSession.start(true, withReplayEnabled = false)
260+
tripSession.start(true, withReplayEnabled = true)
261+
tripSession.start(true, withReplayEnabled = false)
250262
tripSession.stop()
263+
264+
verifyOrder {
265+
navigator.addNavigatorObserver(any())
266+
tripService.startService()
267+
tripSessionLocationEngine.startLocationUpdates(false, any())
268+
tripSessionLocationEngine.startLocationUpdates(true, any())
269+
tripSessionLocationEngine.startLocationUpdates(false, any())
270+
navigator.removeNavigatorObserver(any())
271+
tripSession.stop()
272+
tripSessionLocationEngine.stopLocationUpdates()
273+
}
251274
}
252275

253276
@Test

0 commit comments

Comments
 (0)