From 5048fd73c566d6482529f5aec414c776671ef510 Mon Sep 17 00:00:00 2001 From: Dzina Dybouskaya Date: Sun, 18 Jun 2023 17:01:16 +0300 Subject: [PATCH] start replaying route on route updates if not playing now --- changelog/unreleased/bugfixes/dd.md | 1 + .../core/ReplayRouteSessionTest.kt | 123 ++++++++++++++ .../navigation/core/replay/MapboxReplayer.kt | 4 + .../replay/history/ReplayEventSimulator.kt | 9 +- .../core/replay/route/ReplayRouteSession.kt | 16 +- .../core/replay/history/MapboxReplayerTest.kt | 151 +++++++++++++++++- .../replay/route/ReplayRouteSessionTest.kt | 36 +++-- .../navigation/testing/MainCoroutineRule.kt | 9 +- 8 files changed, 323 insertions(+), 26 deletions(-) create mode 100644 changelog/unreleased/bugfixes/dd.md create mode 100644 instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/ReplayRouteSessionTest.kt diff --git a/changelog/unreleased/bugfixes/dd.md b/changelog/unreleased/bugfixes/dd.md new file mode 100644 index 00000000000..37e306ca31f --- /dev/null +++ b/changelog/unreleased/bugfixes/dd.md @@ -0,0 +1 @@ +- Fixed an issue where RouteReplaySession might not have started playing a route if it was created approximately at the same time when routes were set to `MapboxNavigation`. \ No newline at end of file diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/ReplayRouteSessionTest.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/ReplayRouteSessionTest.kt new file mode 100644 index 00000000000..e538984ffbd --- /dev/null +++ b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/core/ReplayRouteSessionTest.kt @@ -0,0 +1,123 @@ +package com.mapbox.navigation.instrumentation_tests.core + +import android.location.Location +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.base.route.RouteRefreshOptions +import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.MapboxNavigationProvider +import com.mapbox.navigation.core.directions.session.RoutesExtra +import com.mapbox.navigation.core.replay.route.ReplayRouteSession +import com.mapbox.navigation.core.replay.route.ReplayRouteSessionOptions +import com.mapbox.navigation.instrumentation_tests.utils.routes.RoutesProvider +import com.mapbox.navigation.instrumentation_tests.utils.routes.RoutesProvider.toNavigationRoutes +import com.mapbox.navigation.testing.ui.BaseCoreNoCleanUpTest +import com.mapbox.navigation.testing.ui.utils.MapboxNavigationRule +import com.mapbox.navigation.testing.ui.utils.coroutines.routeProgressUpdates +import com.mapbox.navigation.testing.ui.utils.coroutines.routesUpdates +import com.mapbox.navigation.testing.ui.utils.coroutines.sdkTest +import com.mapbox.navigation.testing.ui.utils.coroutines.setNavigationRoutesAndWaitForUpdate +import com.mapbox.navigation.testing.ui.utils.getMapboxAccessTokenFromResources +import com.mapbox.navigation.testing.ui.utils.runOnMainSync +import kotlinx.coroutines.flow.filter +import kotlinx.coroutines.flow.first +import kotlinx.coroutines.flow.take +import kotlinx.coroutines.flow.toList +import org.junit.After +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Rule +import org.junit.Test +import java.util.concurrent.TimeUnit + +@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) +class ReplayRouteSessionTest : BaseCoreNoCleanUpTest() { + + @get:Rule + val mapboxNavigationRule = MapboxNavigationRule() + private lateinit var mapboxNavigation: MapboxNavigation + private lateinit var replayRouteSession: ReplayRouteSession + + override fun setupMockLocation(): Location { + return mockLocationUpdatesRule.generateLocationUpdate { + this.latitude = 0.0 + this.longitude = 0.0 + } + } + + @Before + fun setUp() { + runOnMainSync { + replayRouteSession = ReplayRouteSession() + replayRouteSession.setOptions( + ReplayRouteSessionOptions.Builder().locationResetEnabled(false).build() + ) + } + } + + @After + fun tearDown() { + runOnMainSync { + replayRouteSession.onDetached(mapboxNavigation) + } + } + + @Test + fun routeIsPlayedIfNoLocationUpdatesHappenedBefore() = sdkTest { + mapboxNavigation = MapboxNavigationProvider.create( + NavigationOptions.Builder(context) + .accessToken(getMapboxAccessTokenFromResources(context)) + .build() + ) + val routes = RoutesProvider.dc_very_short(context) + + replayRouteSession.onAttached(mapboxNavigation) + mapboxNavigation.setNavigationRoutesAndWaitForUpdate(routes.toNavigationRoutes()) + + val routeProgressUpdates = mapboxNavigation.routeProgressUpdates().take(5).toList() + val lastUpdate = routeProgressUpdates.last() + val firstUpdate = routeProgressUpdates.first() + assertTrue( + lastUpdate.distanceTraveled > firstUpdate.distanceTraveled + ) + } + + @Test + fun routeIsPlayedFromCurrentPositionAfterRefresh() = sdkTest { + mapboxNavigation = MapboxNavigationProvider.create( + NavigationOptions.Builder(context) + .accessToken(getMapboxAccessTokenFromResources(context)) + .routeRefreshOptions(routeRefreshOptions(3000)) + .build() + ) + val routes = RoutesProvider.dc_short_with_alternative_reroute(context).toNavigationRoutes() + + replayRouteSession.onAttached(mapboxNavigation) + mapboxNavigation.setNavigationRoutesAndWaitForUpdate(routes) + mapboxNavigation.routeProgressUpdates() + .filter { it.currentRouteGeometryIndex >= 4 } + .first() + + mapboxNavigation.routeProgressUpdates().filter { + it.currentRouteGeometryIndex == 4 + }.first() + + mapboxNavigation.routesUpdates() + .filter { it.reason == RoutesExtra.ROUTES_UPDATE_REASON_REFRESH } + .first() + + val routeProgressUpdates = mapboxNavigation.routeProgressUpdates().take(2).toList() + assertTrue(routeProgressUpdates.all { it.currentRouteGeometryIndex >= 4 }) + } + + private fun routeRefreshOptions(intervalMillis: Long): RouteRefreshOptions { + val routeRefreshOptions = RouteRefreshOptions.Builder() + .intervalMillis(TimeUnit.SECONDS.toMillis(30)) + .build() + RouteRefreshOptions::class.java.getDeclaredField("intervalMillis").apply { + isAccessible = true + set(routeRefreshOptions, intervalMillis) + } + return routeRefreshOptions + } +} diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/MapboxReplayer.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/MapboxReplayer.kt index eba562be3ac..b2b41a2f480 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/MapboxReplayer.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/MapboxReplayer.kt @@ -239,4 +239,8 @@ class MapboxReplayer { unregisterObservers() clearEvents() } + + internal fun isPlaying(): Boolean { + return replayEventSimulator.isPlaying() + } } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/history/ReplayEventSimulator.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/history/ReplayEventSimulator.kt index d21f5690a91..1661dec9f35 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/history/ReplayEventSimulator.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/history/ReplayEventSimulator.kt @@ -27,6 +27,7 @@ internal class ReplayEventSimulator( private var simulatorTimeOffset: Double = 0.0 private var simulatorTimeScale: Double = 1.0 + private var currentJob: Job? = null private var pivotIndex = 0 private var clearingPlayedEvents = false @@ -40,6 +41,8 @@ internal class ReplayEventSimulator( simulateEvents(replayEventsCallback) } } + }.also { + currentJob = it } } @@ -92,6 +95,10 @@ internal class ReplayEventSimulator( resetSimulatorClock() } + fun isPlaying(): Boolean { + return currentJob?.isActive == true && !isDonePlayingEvents() + } + private fun resetSimulatorClock() { simulatorTimeOffset = timeSeconds() historyTimeOffset = if (isDonePlayingEvents()) { @@ -129,7 +136,7 @@ internal class ReplayEventSimulator( } private fun isDonePlayingEvents(): Boolean { - return pivotIndex >= replayEvents.events.size + return (pivotIndex >= replayEvents.events.size) } private fun timeSeconds(): Double { diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/route/ReplayRouteSession.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/route/ReplayRouteSession.kt index 2557384b3f6..92e9e9a9000 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/route/ReplayRouteSession.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/replay/route/ReplayRouteSession.kt @@ -9,7 +9,6 @@ import com.mapbox.android.core.permissions.PermissionsManager import com.mapbox.api.directions.v5.DirectionsCriteria import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.route.NavigationRoute -import com.mapbox.navigation.base.trip.model.RouteProgress import com.mapbox.navigation.core.MapboxNavigation import com.mapbox.navigation.core.directions.session.RoutesObserver import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp @@ -65,7 +64,7 @@ class ReplayRouteSession : MapboxNavigationObserver { private val routeProgressObserver = RouteProgressObserver { routeProgress -> if (currentRoute?.id != routeProgress.navigationRoute.id) { currentRoute = routeProgress.navigationRoute - onRouteChanged(routeProgress) + onRouteChanged(routeProgress.navigationRoute, routeProgress.currentRouteGeometryIndex) } } @@ -74,6 +73,14 @@ class ReplayRouteSession : MapboxNavigationObserver { mapboxNavigation?.resetReplayLocation() currentRoute = null polylineDecodeStream = null + } else if (mapboxNavigation?.mapboxReplayer?.isPlaying() != true) { + // In order to get route progress updates, we need location updates. + // If we don't have any location updates, we don't get route progress updates + // and we'll never start navigating the route. + // If we have location updates, we'll update the route from RouteProgressObserver, + // because it has more information, e. g. current route geometry index. + currentRoute = result.navigationRoutes.first() + onRouteChanged(result.navigationRoutes.first(), 0) } } @@ -143,8 +150,7 @@ class ReplayRouteSession : MapboxNavigationObserver { this.currentRoute = null } - private fun onRouteChanged(routeProgress: RouteProgress) { - val navigationRoute = routeProgress.navigationRoute + private fun onRouteChanged(navigationRoute: NavigationRoute, currentIndex: Int) { val mapboxReplayer = mapboxNavigation?.mapboxReplayer ?: return mapboxReplayer.clearEvents() mapboxReplayer.play() @@ -162,7 +168,7 @@ class ReplayRouteSession : MapboxNavigationObserver { // Skip up to the current geometry index. There is some imprecision here because the // distance traveled is not equal to a route index. - polylineDecodeStream?.skip(routeProgress.currentRouteGeometryIndex) + polylineDecodeStream?.skip(currentIndex) pushMorePoints() } diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/history/MapboxReplayerTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/history/MapboxReplayerTest.kt index 45bcc67526f..3bb0ac76187 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/history/MapboxReplayerTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/history/MapboxReplayerTest.kt @@ -13,13 +13,17 @@ import io.mockk.verify import kotlinx.coroutines.ExperimentalCoroutinesApi import org.junit.After import org.junit.Assert.assertEquals +import org.junit.Assert.assertFalse import org.junit.Assert.assertTrue import org.junit.Before import org.junit.Rule import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner import java.util.concurrent.TimeUnit @ExperimentalCoroutinesApi +@RunWith(RobolectricTestRunner::class) class MapboxReplayerTest { @get:Rule @@ -28,7 +32,7 @@ class MapboxReplayerTest { private val replayEventsObserver: ReplayEventsObserver = mockk(relaxed = true) private var deviceElapsedTimeNanos = TimeUnit.HOURS.toNanos(11) - private val mapboxReplayer = MapboxReplayer() + private lateinit var mapboxReplayer: MapboxReplayer @Test fun `should play start transit and location in order`() = coroutineRule.runBlockingTest { @@ -569,6 +573,150 @@ class MapboxReplayerTest { assertEquals(3000L, timeCapture[2]) } + @Test + fun isPlaying_notStarted() { + assertFalse(mapboxReplayer.isPlaying()) + } + + @Test + fun isPlaying_startedButNoEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_startedAndHasEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + mapboxReplayer.play() + advanceTimeMillis(1000) + + assertTrue(mapboxReplayer.isPlaying()) + + advanceTimeMillis(2000) + mapboxReplayer.finish() + } + + @Test + fun isPlaying_startedButEventsAreAllPlayed() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + + advanceTimeMillis(3000) + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_stoppedHasNoEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.stop() + + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_stoppedHasPlayedEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + advanceTimeMillis(3000) + mapboxReplayer.stop() + + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_stoppedHasClearedEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + mapboxReplayer.clearEvents() + + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_finishedHasClearedEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + mapboxReplayer.finish() + + assertFalse(mapboxReplayer.isPlaying()) + } + + @Test + fun isPlaying_stoppedHasEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + mapboxReplayer.stop() + + assertFalse(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + + @Test + fun isPlaying_relaunchedHasEvents() = coroutineRule.runBlockingTest { + mapboxReplayer.play() + mapboxReplayer.pushEvents( + listOf( + ReplayEventGetStatus(1.0), + ReplayEventGetStatus(2.0), + ReplayEventGetStatus(3.0) + ) + ) + mapboxReplayer.stop() + mapboxReplayer.play() + + assertTrue(mapboxReplayer.isPlaying()) + + mapboxReplayer.finish() + } + /** * Helpers for moving the simulation clock */ @@ -577,6 +725,7 @@ class MapboxReplayerTest { fun setup() { mockkStatic(SystemClock::class) every { SystemClock.elapsedRealtimeNanos() } returns deviceElapsedTimeNanos + mapboxReplayer = MapboxReplayer() } @After diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/route/ReplayRouteSessionTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/route/ReplayRouteSessionTest.kt index 61a1dd52a27..2ac4d5ca1db 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/route/ReplayRouteSessionTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/replay/route/ReplayRouteSessionTest.kt @@ -320,7 +320,7 @@ class ReplayRouteSessionTest { } @Test - fun `onAttached - should skip to the current routeProgress distanceTraveled`() { + fun `onAttached - should skip to short routeProgress currentRouteGeometryIndex`() { val progressObserver = slot() val routesObserver = slot() every { @@ -331,42 +331,46 @@ class ReplayRouteSessionTest { val primaryRoute = activeRoutes.navigationRoutes.first() sut.onAttached(mapboxNavigation) - routesObserver.captured.onRoutesChanged(activeRoutes) progressObserver.captured.onRouteProgressChanged( mockk { every { navigationRoute } returns primaryRoute - every { currentRouteGeometryIndex } returns 15 + every { currentRouteGeometryIndex } returns 12 } ) val pushedEvents = slot>() verify { replayer.pushEvents(capture(pushedEvents)) } - verifySkipToIndex(pushedEvents.captured, primaryRoute, 15) + verifySkipToIndex(pushedEvents.captured, primaryRoute, 12) } @Test - fun `onAttached - should skip to short routeProgress currentRouteGeometryIndex`() { - val progressObserver = slot() + fun `onAttached - should start from index 0 when routes changes if not playing events`() { val routesObserver = slot() - every { - mapboxNavigation.registerRouteProgressObserver(capture(progressObserver)) - } just runs every { mapboxNavigation.registerRoutesObserver(capture(routesObserver)) } just runs val activeRoutes = mockActiveRoutesUpdatedResult() val primaryRoute = activeRoutes.navigationRoutes.first() + every { replayer.isPlaying() } returns false sut.onAttached(mapboxNavigation) routesObserver.captured.onRoutesChanged(activeRoutes) - progressObserver.captured.onRouteProgressChanged( - mockk { - every { navigationRoute } returns primaryRoute - every { currentRouteGeometryIndex } returns 12 - } - ) val pushedEvents = slot>() verify { replayer.pushEvents(capture(pushedEvents)) } - verifySkipToIndex(pushedEvents.captured, primaryRoute, 12) + verifySkipToIndex(pushedEvents.captured, primaryRoute, 0) + } + + @Test + fun `onAttached - should not start playing route when route appears if playing events`() { + val routesObserver = slot() + every { mapboxNavigation.registerRoutesObserver(capture(routesObserver)) } just runs + val activeRoutes = mockActiveRoutesUpdatedResult() + every { replayer.isPlaying() } returns true + + sut.onAttached(mapboxNavigation) + + routesObserver.captured.onRoutesChanged(activeRoutes) + + verify(exactly = 0) { replayer.pushEvents(any()) } } @Test diff --git a/libtesting-utils/src/main/java/com/mapbox/navigation/testing/MainCoroutineRule.kt b/libtesting-utils/src/main/java/com/mapbox/navigation/testing/MainCoroutineRule.kt index c3a41d0e343..c75522d484c 100644 --- a/libtesting-utils/src/main/java/com/mapbox/navigation/testing/MainCoroutineRule.kt +++ b/libtesting-utils/src/main/java/com/mapbox/navigation/testing/MainCoroutineRule.kt @@ -30,10 +30,13 @@ class MainCoroutineRule : TestRule { override fun evaluate() { Dispatchers.setMain(testDispatcher) - base.evaluate() + try { + base.evaluate() + } finally { + Dispatchers.resetMain() // Restore original main dispatcher + createdScopes.forEach { it.cleanupTestCoroutines() } + } - Dispatchers.resetMain() // Restore original main dispatcher - createdScopes.forEach { it.cleanupTestCoroutines() } } }