From b145c5313924087d2b14e3344c1e8b410fd4f22e Mon Sep 17 00:00:00 2001 From: Kyle Madsen Date: Thu, 25 Aug 2022 12:22:50 -0700 Subject: [PATCH 1/2] Create a delegate for MapboxNavigation --- CHANGELOG.md | 1 + libnavigation-core/api/current.txt | 4 + .../core/lifecycle/RequireMapboxNavigation.kt | 139 +++++++ .../lifecycle/RequireMapboxNavigationTest.kt | 347 ++++++++++++++++++ .../view/AlternativeRouteActivity.kt | 91 ++--- .../qa_test_app/view/FeedbackActivity.kt | 55 +-- .../view/InactiveRouteStylingActivity.kt | 87 ++--- .../qa_test_app/view/MainActivity.kt | 4 + 8 files changed, 616 insertions(+), 112 deletions(-) create mode 100644 libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt create mode 100644 libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt diff --git a/CHANGELOG.md b/CHANGELOG.md index e70d14905d6..e5c7358cb19 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,6 +6,7 @@ Mapbox welcomes participation and contributions from everyone. #### Features - Improved the route refresh feature to also refresh _closures_ (`RouteLeg#closures`). [#6274](https://github.com/mapbox/mapbox-navigation-android/pull/6274) +- Added `requireMapboxNavigation` to offer a simple way to use `MapboxNavigationApp`. [#6233](https://github.com/mapbox/mapbox-navigation-android/pull/6233) #### Bug fixes and improvements ## Mapbox Navigation SDK 2.8.0-beta.2 - 01 September, 2022 diff --git a/libnavigation-core/api/current.txt b/libnavigation-core/api/current.txt index 6fe3d9d8ff5..b36d2562eee 100644 --- a/libnavigation-core/api/current.txt +++ b/libnavigation-core/api/current.txt @@ -337,6 +337,10 @@ package com.mapbox.navigation.core.lifecycle { method public com.mapbox.navigation.base.options.NavigationOptions createNavigationOptions(); } + public final class RequireMapboxNavigation { + method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static kotlin.properties.ReadOnlyProperty requireMapboxNavigation(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onCreatedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onStartedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onResumedObserver = null, kotlin.jvm.functions.Function0? onInitialize = null); + } + } package com.mapbox.navigation.core.navigator { diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt new file mode 100644 index 00000000000..1bc2656815e --- /dev/null +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt @@ -0,0 +1,139 @@ +@file:JvmName("RequireMapboxNavigation") + +package com.mapbox.navigation.core.lifecycle + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.core.MapboxNavigation +import kotlin.properties.ReadOnlyProperty +import kotlin.reflect.KProperty + +/** + * Extension function to make it simple to create the [RequireMapboxNavigationDelegate]. Below are + * a couple examples of how you may use the delegate. + * + * Default can be used when [MapboxNavigationApp] is setup elsewhere. + * ``` + * val mapboxNavigation by requireMapboxNavigation() + * ``` + * + * Initialize the [MapboxNavigationApp] when you are ready to use it + * ``` + * val mapboxNavigation by requireMapboxNavigation { + * MapboxNavigationApp.setup(..) + * } + * ``` + * + * Register subscriptions and setup MapboxNavigationApp + * ``` + * private val mapboxNavigation by requireMapboxNavigation( + * onResumedObserver = object : MapboxNavigationObserver { + * override fun onAttached(mapboxNavigation: MapboxNavigation) { + * mapboxNavigation.registerLocationObserver(locationObserver) + * mapboxNavigation.registerRoutesObserver(routesObserver) + * } + * override fun onDetached(mapboxNavigation: MapboxNavigation) { + * mapboxNavigation.unregisterLocationObserver(locationObserver) + * mapboxNavigation.unregisterRoutesObserver(routesObserver) + * } + * } + * ) { + * MapboxNavigationApp.setup( + * NavigationOptions.Builder(this) + * .accessToken(accessToken) + * .build() + * ) + * } + * ``` + * + * @see [RequireMapboxNavigationDelegate] for more details. + */ +@ExperimentalPreviewMapboxNavigationAPI +fun requireMapboxNavigation( + onCreatedObserver: MapboxNavigationObserver? = null, + onStartedObserver: MapboxNavigationObserver? = null, + onResumedObserver: MapboxNavigationObserver? = null, + onInitialize: (() -> Unit)? = null, +): ReadOnlyProperty = RequireMapboxNavigationDelegate( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + onInitialize = onInitialize +) + +/** + * Attaches a [LifecycleOwner] to [MapboxNavigationApp] and provides access to [MapboxNavigation]. + * + * You can choose to call [MapboxNavigationApp.setup] in the [onInitialize]. You can also setup in + * the onCreate calls, or any call that happens before this delegate is accessed. The delegate will + * crash if accessed when the app is not setup or an attached lifecycle has not been created. + * + * You can use the observers parameter to setup any subscriptions. This is important because the + * [MapboxNavigation] instance can be re-created with [MapboxNavigationApp.disable], or if all + * [MapboxNavigationApp.attach] lifecycles are destroyed. + * + * @param onCreatedObserver registered to the [Lifecycle.State.CREATED] lifecycle + * @param onStartedObserver registered to the [Lifecycle.State.STARTED] lifecycle + * @param onResumedObserver registered to the [Lifecycle.State.RESUMED] lifecycle + * @param onInitialize called when the property is read for the first time + */ +internal class RequireMapboxNavigationDelegate( + private val onCreatedObserver: MapboxNavigationObserver? = null, + private val onStartedObserver: MapboxNavigationObserver? = null, + private val onResumedObserver: MapboxNavigationObserver? = null, + private val onInitialize: (() -> Unit)? = null +) : ReadOnlyProperty { + + private lateinit var lifecycleOwner: LifecycleOwner + + private val lifecycleObserver = object : DefaultLifecycleObserver { + override fun onCreate(owner: LifecycleOwner) { + onCreatedObserver?.let { MapboxNavigationApp.registerObserver(it) } + } + + override fun onDestroy(owner: LifecycleOwner) { + onCreatedObserver?.let { MapboxNavigationApp.unregisterObserver(it) } + } + + override fun onStart(owner: LifecycleOwner) { + onStartedObserver?.let { MapboxNavigationApp.registerObserver(it) } + } + + override fun onStop(owner: LifecycleOwner) { + onStartedObserver?.let { MapboxNavigationApp.unregisterObserver(it) } + } + + override fun onResume(owner: LifecycleOwner) { + onResumedObserver?.let { MapboxNavigationApp.registerObserver(it) } + } + + override fun onPause(owner: LifecycleOwner) { + onResumedObserver?.let { MapboxNavigationApp.unregisterObserver(it) } + } + } + + /** + * Returns an instance of [MapboxNavigation], the first retrieval will attach the [thisRef] + * to [MapboxNavigationApp.attach]. If [MapboxNavigationApp.isSetup] is false after all + * observers and initializers, this property getter will crash. + * + * @param thisRef - the [LifecycleOwner] that needs access to [MapboxNavigation]. + * @param property - ignored + */ + override fun getValue(thisRef: LifecycleOwner, property: KProperty<*>): MapboxNavigation { + if (!this::lifecycleOwner.isInitialized) { + onInitialize?.invoke() + this.lifecycleOwner = thisRef + MapboxNavigationApp.attach(lifecycleOwner) + lifecycleOwner.lifecycle.addObserver(lifecycleObserver) + } + val mapboxNavigation = MapboxNavigationApp.current() + checkNotNull(mapboxNavigation) { + "MapboxNavigation cannot be null. Ensure that MapboxNavigationApp is setup and an" + + " attached lifecycle is at least CREATED." + } + return mapboxNavigation + } +} diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt new file mode 100644 index 00000000000..2d8ad731d18 --- /dev/null +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt @@ -0,0 +1,347 @@ +package com.mapbox.navigation.core.lifecycle + +import androidx.lifecycle.DefaultLifecycleObserver +import androidx.lifecycle.Lifecycle +import androidx.lifecycle.LifecycleOwner +import androidx.lifecycle.LifecycleRegistry +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI +import com.mapbox.navigation.base.options.NavigationOptions +import com.mapbox.navigation.core.MapboxNavigation +import io.mockk.every +import io.mockk.mockk +import io.mockk.mockkObject +import io.mockk.unmockkAll +import io.mockk.verify +import io.mockk.verifyOrder +import org.junit.After +import org.junit.Assert.assertTrue +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner + +@RunWith(RobolectricTestRunner::class) +class RequireMapboxNavigationTest { + + private class SystemUnderTest( + onCreatedObserver: MapboxNavigationObserver? = null, + onStartedObserver: MapboxNavigationObserver? = null, + onResumedObserver: MapboxNavigationObserver? = null, + onInitialize: (() -> Unit)? = null + ) : LifecycleOwner { + private val lifecycleRegistry = LifecycleRegistry(this) + .also { it.currentState = Lifecycle.State.INITIALIZED } + + override fun getLifecycle(): Lifecycle = lifecycleRegistry + + fun moveToState(state: Lifecycle.State) { + lifecycleRegistry.currentState = state + } + + @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) + val mapboxNavigation by requireMapboxNavigation( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + onInitialize = onInitialize + ) + } + + @Before + fun setup() { + mockkObject(MapboxNavigationApp) + } + + @After + fun teardown() { + unmockkAll() + } + + @Test(expected = IllegalStateException::class) + fun `crashes before setup`() { + SystemUnderTest().mapboxNavigation + } + + @Test + fun `mapboxNavigation is available when current is not null`() { + every { MapboxNavigationApp.current() } returns mockk() + + SystemUnderTest().mapboxNavigation + } + + @Test + fun `mapboxNavigation is available after MapboxNavigationApp#setup and lifecycle is CREATED`() { + mockMapboxNavigationAppBehavior() + val sut = SystemUnderTest() + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + + // This should not crash + sut.mapboxNavigation + } + + @Test + fun `multiple delegate references are the same instance`() { + mockMapboxNavigationAppBehavior() + val sut = SystemUnderTest() + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + val firstRef = sut.mapboxNavigation + val secondRef = sut.mapboxNavigation + + assertTrue(firstRef === secondRef) + } + + @Test(expected = IllegalStateException::class) + fun `delegate reference will crash if accessed after lifecycle is DESTROYED`() { + mockMapboxNavigationAppBehavior() + val sut = SystemUnderTest() + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + sut.mapboxNavigation + sut.moveToState(Lifecycle.State.DESTROYED) + + // This should crash + sut.mapboxNavigation + } + + @Test + fun `delegate references changes after MapboxNavigationApp is reset`() { + mockMapboxNavigationAppBehavior() + val sut = SystemUnderTest() + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + val firstRef = sut.mapboxNavigation + MapboxNavigationApp.disable() + MapboxNavigationApp.setup(mockk()) + val secondRef = sut.mapboxNavigation + + assertTrue(firstRef !== secondRef) + } + + @Test(expected = IllegalStateException::class) + fun `delegate reference will crash if accessed after MapboxNavigationApp#disable`() { + mockMapboxNavigationAppBehavior() + val sut = SystemUnderTest() + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + sut.mapboxNavigation + MapboxNavigationApp.disable() + + // This should crash + sut.mapboxNavigation + } + + @Test + fun `onInitialize can be used to setup the app`() { + mockMapboxNavigationAppBehavior() + + val sut = SystemUnderTest { + MapboxNavigationApp.setup(mockk()) + } + sut.moveToState(Lifecycle.State.CREATED) + + // This should not crash + sut.mapboxNavigation + } + + @Test + fun `observer callback order of resetting MapboxNavigationApp`() { + mockMapboxNavigationAppBehavior() + val onCreatedObserver = mockk(relaxed = true) + val onStartedObserver = mockk(relaxed = true) + val onResumedObserver = mockk(relaxed = true) + val sut = SystemUnderTest( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + ) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.RESUMED) + val firstRef = sut.mapboxNavigation + MapboxNavigationApp.disable() + MapboxNavigationApp.setup(mockk()) + val secondRef = sut.mapboxNavigation + + verifyOrder { + onCreatedObserver.onAttached(firstRef) + onStartedObserver.onAttached(firstRef) + onResumedObserver.onAttached(firstRef) + onCreatedObserver.onDetached(firstRef) + onStartedObserver.onDetached(firstRef) + onResumedObserver.onDetached(firstRef) + onCreatedObserver.onAttached(secondRef) + onStartedObserver.onAttached(secondRef) + onResumedObserver.onAttached(secondRef) + } + } + + @Test + fun `CREATED lifecycle state callback order`() { + mockMapboxNavigationAppBehavior() + val onCreatedObserver = mockk(relaxed = true) + val onStartedObserver = mockk(relaxed = true) + val onResumedObserver = mockk(relaxed = true) + val sut = SystemUnderTest( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + ) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + val firstRef = sut.mapboxNavigation + sut.moveToState(Lifecycle.State.DESTROYED) + + verifyOrder { + onCreatedObserver.onAttached(firstRef) + onCreatedObserver.onDetached(firstRef) + } + verify(exactly = 0) { + onStartedObserver.onDetached(any()) + onStartedObserver.onDetached(any()) + onResumedObserver.onDetached(any()) + onResumedObserver.onDetached(any()) + } + } + + @Test + fun `STARTED lifecycle state callback order`() { + mockMapboxNavigationAppBehavior() + val onCreatedObserver = mockk(relaxed = true) + val onStartedObserver = mockk(relaxed = true) + val onResumedObserver = mockk(relaxed = true) + val sut = SystemUnderTest( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + ) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.STARTED) + val firstRef = sut.mapboxNavigation + sut.moveToState(Lifecycle.State.CREATED) + + verifyOrder { + onCreatedObserver.onAttached(firstRef) + onStartedObserver.onAttached(firstRef) + onStartedObserver.onDetached(firstRef) + } + verify(exactly = 0) { + onCreatedObserver.onDetached(any()) + onResumedObserver.onAttached(any()) + onResumedObserver.onDetached(any()) + } + } + + @Test + fun `RESUMED lifecycle state callback order`() { + mockMapboxNavigationAppBehavior() + val onCreatedObserver = mockk(relaxed = true) + val onStartedObserver = mockk(relaxed = true) + val onResumedObserver = mockk(relaxed = true) + val sut = SystemUnderTest( + onCreatedObserver = onCreatedObserver, + onStartedObserver = onStartedObserver, + onResumedObserver = onResumedObserver, + ) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.RESUMED) + val firstRef = sut.mapboxNavigation + sut.moveToState(Lifecycle.State.STARTED) + + verifyOrder { + onCreatedObserver.onAttached(firstRef) + onStartedObserver.onAttached(firstRef) + onResumedObserver.onAttached(firstRef) + onResumedObserver.onDetached(firstRef) + } + verify(exactly = 0) { + onCreatedObserver.onDetached(any()) + onStartedObserver.onDetached(any()) + } + } + + /** + * This mocks the behavior of [MapboxNavigationApp] in order to showcase what is expected from + * the [RequireMapboxNavigationDelegate]. + */ + private fun mockMapboxNavigationAppBehavior() { + var mockMapboxNavigation: MapboxNavigation? = null + var isSetup = false + val registeredMapboxNavigationObservers = mutableListOf() + val attachedLifecycleOwner = mutableListOf() + + every { MapboxNavigationApp.current() } answers { + mockMapboxNavigation + } + every { MapboxNavigationApp.disable() } answers { + registeredMapboxNavigationObservers.forEach { it.onDetached(mockMapboxNavigation!!) } + mockMapboxNavigation = null + isSetup = false + MapboxNavigationApp + } + every { MapboxNavigationApp.attach(any()) } answers { + attachedLifecycleOwner.add(firstArg()) + firstArg().lifecycle.addObserver(object : DefaultLifecycleObserver { + override fun onCreate(owner: LifecycleOwner) { + if (isSetup) { + mockMapboxNavigation = mockk() + registeredMapboxNavigationObservers.forEach { observer -> + observer.onAttached(mockMapboxNavigation!!) + } + } + } + + override fun onDestroy(owner: LifecycleOwner) { + super.onDestroy(owner) + attachedLifecycleOwner.remove(firstArg()) + val isCreated = attachedLifecycleOwner.any { + it.lifecycle.currentState.isAtLeast(Lifecycle.State.CREATED) + } + if (!isCreated) { + registeredMapboxNavigationObservers.forEach { observer -> + observer.onDetached(mockMapboxNavigation!!) + } + mockMapboxNavigation = null + } + } + }) + MapboxNavigationApp + } + every { MapboxNavigationApp.setup(any()) } answers { + isSetup = true + val isCreated = attachedLifecycleOwner.any { + it.lifecycle.currentState.isAtLeast(Lifecycle.State.CREATED) + } + if (isCreated) { + mockMapboxNavigation = mockk() + registeredMapboxNavigationObservers.forEach { observer -> + observer.onAttached(mockMapboxNavigation!!) + } + } + MapboxNavigationApp + } + every { + MapboxNavigationApp.registerObserver(any()) + } answers { + registeredMapboxNavigationObservers.add(firstArg()) + firstArg().onAttached(mockMapboxNavigation!!) + MapboxNavigationApp + } + every { + MapboxNavigationApp.unregisterObserver(any()) + } answers { + registeredMapboxNavigationObservers.remove(firstArg()) + firstArg().onDetached(mockMapboxNavigation!!) + MapboxNavigationApp + } + } +} diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt index 2632c516328..312d96b50ac 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/AlternativeRouteActivity.kt @@ -26,6 +26,7 @@ import com.mapbox.maps.plugin.gestures.OnMapLongClickListener import com.mapbox.maps.plugin.gestures.gestures import com.mapbox.maps.plugin.locationcomponent.OnIndicatorPositionChangedListener import com.mapbox.maps.plugin.locationcomponent.location +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.extensions.applyDefaultNavigationOptions import com.mapbox.navigation.base.extensions.applyLanguageAndVoiceUnitOptions import com.mapbox.navigation.base.options.NavigationOptions @@ -36,8 +37,10 @@ import com.mapbox.navigation.base.route.RouterFailure import com.mapbox.navigation.base.route.RouterOrigin import com.mapbox.navigation.base.trip.model.RouteProgress import com.mapbox.navigation.core.MapboxNavigation -import com.mapbox.navigation.core.MapboxNavigationProvider import com.mapbox.navigation.core.directions.session.RoutesObserver +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp +import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver +import com.mapbox.navigation.core.lifecycle.requireMapboxNavigation import com.mapbox.navigation.core.replay.MapboxReplayer import com.mapbox.navigation.core.replay.ReplayLocationEngine import com.mapbox.navigation.core.replay.route.ReplayProgressObserver @@ -62,10 +65,11 @@ import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch import java.util.concurrent.TimeUnit +@SuppressLint("MissingPermission") class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { private companion object { - private const val TAG = "AlternativeRouteActvt" + private const val TAG = "AlternativeRouteActivity" } private val routeClickPadding = Utils.dpToPx(30f) @@ -80,20 +84,6 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { binding.mapView.camera } - private val mapboxNavigation: MapboxNavigation by lazy { - MapboxNavigationProvider.create( - NavigationOptions.Builder(this) - .accessToken(getMapboxAccessToken(this)) - .locationEngine(ReplayLocationEngine(mapboxReplayer)) - .routeAlternativesOptions( - RouteAlternativesOptions.Builder() - .intervalMillis(TimeUnit.SECONDS.toMillis(30)) - .build() - ) - .build() - ) - } - private val routeLineResources: RouteLineResources by lazy { RouteLineResources.Builder().build() } @@ -115,42 +105,49 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { MapboxRouteLineApi(routeLineOptions) } + @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) + private val mapboxNavigation: MapboxNavigation by requireMapboxNavigation( + onResumedObserver = object : MapboxNavigationObserver { + override fun onAttached(mapboxNavigation: MapboxNavigation) { + mapboxNavigation.registerLocationObserver(locationObserver) + mapboxNavigation.registerRouteProgressObserver(replayProgressObserver) + mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) + mapboxNavigation.registerRoutesObserver(routesObserver) + mapboxNavigation.registerRouteAlternativesObserver(alternativesObserver) + } + + override fun onDetached(mapboxNavigation: MapboxNavigation) { + mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) + mapboxNavigation.unregisterRouteProgressObserver(routeProgressObserver) + mapboxNavigation.unregisterLocationObserver(locationObserver) + mapboxNavigation.unregisterRoutesObserver(routesObserver) + mapboxNavigation.unregisterRouteAlternativesObserver(alternativesObserver) + } + }, + onInitialize = this::initNavigation + ) + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(binding.root) - initNavigation() initStyle() initListeners() } - override fun onStart() { - super.onStart() - mapboxNavigation.registerLocationObserver(locationObserver) - mapboxNavigation.registerRouteProgressObserver(replayProgressObserver) - mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) - mapboxNavigation.registerRoutesObserver(routesObserver) - mapboxNavigation.registerRouteAlternativesObserver(alternativesObserver) - } - - override fun onStop() { - super.onStop() - mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) - mapboxNavigation.unregisterRouteProgressObserver(routeProgressObserver) - mapboxNavigation.unregisterLocationObserver(locationObserver) - mapboxNavigation.unregisterRoutesObserver(routesObserver) - mapboxNavigation.unregisterRouteAlternativesObserver(alternativesObserver) - } - - override fun onDestroy() { - super.onDestroy() - routeLineApi.cancel() - routeLineView.cancel() - mapboxReplayer.finish() - mapboxNavigation.onDestroy() - } - private fun initNavigation() { + MapboxNavigationApp.setup( + NavigationOptions.Builder(this) + .accessToken(getMapboxAccessToken(this)) + .locationEngine(ReplayLocationEngine(mapboxReplayer)) + .routeAlternativesOptions( + RouteAlternativesOptions.Builder() + .intervalMillis(TimeUnit.SECONDS.toMillis(30)) + .build() + ) + .build() + ) + binding.mapView.location.apply { setLocationProvider(navigationLocationProvider) addOnIndicatorPositionChangedListener(onIndicatorPositionChangedListener) @@ -161,6 +158,13 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { mapboxReplayer.play() } + override fun onDestroy() { + super.onDestroy() + routeLineApi.cancel() + routeLineView.cancel() + mapboxReplayer.finish() + } + private val locationObserver: LocationObserver = object : LocationObserver { override fun onNewRawLocation(rawLocation: Location) { Log.d(TAG, "raw location $rawLocation") @@ -307,7 +311,6 @@ class AlternativeRouteActivity : AppCompatActivity(), OnMapLongClickListener { } } - @SuppressLint("MissingPermission") private fun initListeners() { binding.startNavigation.setOnClickListener { mapboxNavigation.startTripSession() diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt index 6226f44a96f..e56fa15740e 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/FeedbackActivity.kt @@ -28,8 +28,10 @@ import com.mapbox.navigation.base.route.RouterCallback import com.mapbox.navigation.base.route.RouterFailure import com.mapbox.navigation.base.route.RouterOrigin import com.mapbox.navigation.core.MapboxNavigation -import com.mapbox.navigation.core.MapboxNavigationProvider import com.mapbox.navigation.core.directions.session.RoutesObserver +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp +import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver +import com.mapbox.navigation.core.lifecycle.requireMapboxNavigation import com.mapbox.navigation.core.telemetry.events.FeedbackEvent import com.mapbox.navigation.core.telemetry.events.FeedbackHelper import com.mapbox.navigation.core.telemetry.events.FeedbackMetadata @@ -53,7 +55,7 @@ import kotlinx.coroutines.CoroutineScope import kotlinx.coroutines.Dispatchers import kotlinx.coroutines.launch -@ExperimentalPreviewMapboxNavigationAPI +@OptIn(ExperimentalPreviewMapboxNavigationAPI::class) class FeedbackActivity : AppCompatActivity() { private val binding: FeedbackActivityBinding by lazy { @@ -66,14 +68,6 @@ class FeedbackActivity : AppCompatActivity() { binding.mapView.camera } - private val mapboxNavigation: MapboxNavigation by lazy { - MapboxNavigationProvider.create( - NavigationOptions.Builder(this) - .accessToken(Utils.getMapboxAccessToken(this)) - .build() - ) - } - private val locationObserver = object : LocationObserver { override fun onNewRawLocation(rawLocation: Location) = Unit @@ -135,6 +129,30 @@ class FeedbackActivity : AppCompatActivity() { enableButtonStoreFeedbackMetadata(feedbackMetadataWrapper != null && value != null) } + private val mapboxNavigation by requireMapboxNavigation( + onCreatedObserver = object : MapboxNavigationObserver { + override fun onAttached(mapboxNavigation: MapboxNavigation) { + binding.mapView.location.apply { + setLocationProvider(navigationLocationProvider) + enabled = true + } + mapboxNavigation.registerLocationObserver(locationObserver) + mapboxNavigation.registerRoutesObserver(routesObserver) + } + + override fun onDetached(mapboxNavigation: MapboxNavigation) { + mapboxNavigation.unregisterLocationObserver(locationObserver) + mapboxNavigation.unregisterRoutesObserver(routesObserver) + } + } + ) { + MapboxNavigationApp.setup( + NavigationOptions.Builder(this) + .accessToken(Utils.getMapboxAccessToken(this)) + .build(), + ) + } + private companion object { private const val KEY_SP_FEEDBACK_METADATA = "KEY_SP_FEEDBACK_METADATA" private const val KEY_SP_FEEDBACK_SCREENSHOT = "KEY_SP_FEEDBACK_SCREENSHOT" @@ -144,20 +162,10 @@ class FeedbackActivity : AppCompatActivity() { super.onCreate(savedInstanceState) setContentView(binding.root) - initNavigation() initStyle() initListeners() } - private fun initNavigation() { - binding.mapView.location.apply { - setLocationProvider(navigationLocationProvider) - enabled = true - } - mapboxNavigation.registerLocationObserver(locationObserver) - mapboxNavigation.registerRoutesObserver(routesObserver) - } - @SuppressLint("MissingPermission") private fun initStyle() { binding.mapView.getMapboxMap().loadStyleUri(NavigationStyles.NAVIGATION_DAY_STYLE) { @@ -257,13 +265,6 @@ class FeedbackActivity : AppCompatActivity() { sharedPrefs.registerOnSharedPreferenceChangeListener(spListener) } - override fun onDestroy() { - super.onDestroy() - mapboxNavigation.unregisterLocationObserver(locationObserver) - mapboxNavigation.unregisterRoutesObserver(routesObserver) - mapboxNavigation.onDestroy() - } - private fun updateCamera(location: Location, keyPoints: List) { navigationLocationProvider.changePosition(location, keyPoints, null, null) val mapAnimationOptionsBuilder = MapAnimationOptions.Builder() diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt index 663f3aaaa3c..7b4986ccb3c 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt @@ -15,9 +15,12 @@ import com.mapbox.maps.plugin.animation.CameraAnimationsPlugin import com.mapbox.maps.plugin.animation.MapAnimationOptions import com.mapbox.maps.plugin.animation.camera import com.mapbox.maps.plugin.locationcomponent.location +import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.base.options.NavigationOptions import com.mapbox.navigation.core.MapboxNavigation -import com.mapbox.navigation.core.MapboxNavigationProvider +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp +import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver +import com.mapbox.navigation.core.lifecycle.requireMapboxNavigation import com.mapbox.navigation.core.replay.MapboxReplayer import com.mapbox.navigation.core.replay.ReplayLocationEngine import com.mapbox.navigation.core.replay.route.ReplayProgressObserver @@ -56,15 +59,6 @@ class InactiveRouteStylingActivity : AppCompatActivity() { binding.mapView.camera } - private val mapboxNavigation: MapboxNavigation by lazy { - MapboxNavigationProvider.create( - NavigationOptions.Builder(this) - .accessToken(Utils.getMapboxAccessToken(this)) - .locationEngine(ReplayLocationEngine(mapboxReplayer)) - .build() - ) - } - private val routeLineColorResources by lazy { RouteLineColorResources.Builder() .inActiveRouteLegsColor(Color.YELLOW) @@ -95,42 +89,63 @@ class InactiveRouteStylingActivity : AppCompatActivity() { MapboxRouteLineApi(options) } + @OptIn(ExperimentalPreviewMapboxNavigationAPI::class) + private val mapboxNavigation by requireMapboxNavigation( + onCreatedObserver = object : MapboxNavigationObserver { + override fun onAttached(mapboxNavigation: MapboxNavigation) { + + mapboxNavigation.registerLocationObserver(locationObserver) + mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) + mapboxNavigation.registerRouteProgressObserver(replayProgressObserver) + } + + override fun onDetached(mapboxNavigation: MapboxNavigation) { + mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) + mapboxNavigation.unregisterRouteProgressObserver(routeProgressObserver) + mapboxNavigation.unregisterLocationObserver(locationObserver) + mapboxNavigation.stopTripSession() + } + } + ) { + MapboxNavigationApp.setup( + NavigationOptions.Builder(this) + .accessToken(Utils.getMapboxAccessToken(this)) + .locationEngine(ReplayLocationEngine(mapboxReplayer)) + .build() + ) + } + override fun onCreate(savedInstanceState: Bundle?) { super.onCreate(savedInstanceState) setContentView(binding.root) - initNavigation() initStyle() initListeners() } - override fun onStop() { - super.onStop() - mapboxNavigation.unregisterRouteProgressObserver(replayProgressObserver) - mapboxNavigation.unregisterRouteProgressObserver(routeProgressObserver) - mapboxNavigation.unregisterLocationObserver(locationObserver) - mapboxNavigation.stopTripSession() - } - - override fun onDestroy() { - super.onDestroy() - routeLineApi.cancel() - routeLineView.cancel() - mapboxReplayer.finish() - mapboxNavigation.onDestroy() - } - - private fun initNavigation() { + @SuppressLint("MissingPermission") + private fun initListeners() { binding.mapView.location.apply { setLocationProvider(navigationLocationProvider) enabled = true } - mapboxNavigation.setRoutes(listOf(getRoute())) - mapboxNavigation.registerLocationObserver(locationObserver) - mapboxNavigation.registerRouteProgressObserver(replayProgressObserver) mapboxReplayer.pushRealLocation(this, 0.0) mapboxReplayer.playbackSpeed(1.5) mapboxReplayer.play() + binding.startNavigation.setOnClickListener { + val route = getRoute() + mapboxNavigation.startTripSession() + mapboxNavigation.setRoutes(listOf(route)) + binding.startNavigation.visibility = View.GONE + startSimulation(route) + } + } + + override fun onDestroy() { + super.onDestroy() + routeLineApi.cancel() + routeLineView.cancel() + mapboxReplayer.finish() } private fun initStyle() { @@ -177,16 +192,6 @@ class InactiveRouteStylingActivity : AppCompatActivity() { ) } - @SuppressLint("MissingPermission") - private fun initListeners() { - binding.startNavigation.setOnClickListener { - mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) - mapboxNavigation.startTripSession() - binding.startNavigation.visibility = View.GONE - startSimulation(mapboxNavigation.getRoutes()[0]) - } - } - private val routeProgressObserver = RouteProgressObserver { routeProgress -> // This is the most important part of this example. The route progress will be used to diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/MainActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/MainActivity.kt index 744d239b6a6..89762d12eee 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/MainActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/MainActivity.kt @@ -9,6 +9,7 @@ import androidx.appcompat.app.AppCompatActivity import androidx.lifecycle.lifecycleScope import androidx.recyclerview.widget.DividerItemDecoration import androidx.recyclerview.widget.LinearLayoutManager +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp import com.mapbox.navigation.qa_test_app.R import com.mapbox.navigation.qa_test_app.databinding.ActivityMainBinding import com.mapbox.navigation.qa_test_app.domain.TestActivityDescription @@ -53,6 +54,9 @@ class MainActivity : AppCompatActivity() { (binding.activitiesList.adapter as GenericListAdapter).swap( TestActivitySuite.testActivities ) + + // Each example is responsible for setting up their NavigationOptions. + MapboxNavigationApp.disable() } override fun onRequestPermissionsResult( From 8dbc73acda55dcbe23258472c6b035d21b6ce32d Mon Sep 17 00:00:00 2001 From: Kyle Madsen Date: Tue, 6 Sep 2022 11:52:43 -0700 Subject: [PATCH 2/2] Make requireMapboxNavigation attach during initialization --- CHANGELOG.md | 1 - libnavigation-core/api/current.txt | 2 +- .../core/lifecycle/RequireMapboxNavigation.kt | 37 ++++++++++--------- .../lifecycle/RequireMapboxNavigationTest.kt | 12 ++++++ .../view/InactiveRouteStylingActivity.kt | 1 - 5 files changed, 32 insertions(+), 21 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index e5c7358cb19..2a2e3cddf99 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -5,7 +5,6 @@ Mapbox welcomes participation and contributions from everyone. ## Unreleased #### Features - Improved the route refresh feature to also refresh _closures_ (`RouteLeg#closures`). [#6274](https://github.com/mapbox/mapbox-navigation-android/pull/6274) - - Added `requireMapboxNavigation` to offer a simple way to use `MapboxNavigationApp`. [#6233](https://github.com/mapbox/mapbox-navigation-android/pull/6233) #### Bug fixes and improvements diff --git a/libnavigation-core/api/current.txt b/libnavigation-core/api/current.txt index b36d2562eee..ef6b71499d3 100644 --- a/libnavigation-core/api/current.txt +++ b/libnavigation-core/api/current.txt @@ -338,7 +338,7 @@ package com.mapbox.navigation.core.lifecycle { } public final class RequireMapboxNavigation { - method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static kotlin.properties.ReadOnlyProperty requireMapboxNavigation(com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onCreatedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onStartedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onResumedObserver = null, kotlin.jvm.functions.Function0? onInitialize = null); + method @com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI public static kotlin.properties.ReadOnlyProperty requireMapboxNavigation(androidx.lifecycle.LifecycleOwner, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onCreatedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onStartedObserver = null, com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver? onResumedObserver = null, kotlin.jvm.functions.Function0? onInitialize = null); } } diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt index 1bc2656815e..662385a272b 100644 --- a/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt +++ b/libnavigation-core/src/main/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigation.kt @@ -7,12 +7,13 @@ import androidx.lifecycle.Lifecycle import androidx.lifecycle.LifecycleOwner import com.mapbox.navigation.base.ExperimentalPreviewMapboxNavigationAPI import com.mapbox.navigation.core.MapboxNavigation +import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp.lifecycleOwner import kotlin.properties.ReadOnlyProperty import kotlin.reflect.KProperty /** - * Extension function to make it simple to create the [RequireMapboxNavigationDelegate]. Below are - * a couple examples of how you may use the delegate. + * Extension function to make it simple to create the [RequireMapboxNavigationDelegate]. + * Below are a couple examples of how you may use the delegate. * * Default can be used when [MapboxNavigationApp] is setup elsewhere. * ``` @@ -51,12 +52,13 @@ import kotlin.reflect.KProperty * @see [RequireMapboxNavigationDelegate] for more details. */ @ExperimentalPreviewMapboxNavigationAPI -fun requireMapboxNavigation( +fun LifecycleOwner.requireMapboxNavigation( onCreatedObserver: MapboxNavigationObserver? = null, onStartedObserver: MapboxNavigationObserver? = null, onResumedObserver: MapboxNavigationObserver? = null, onInitialize: (() -> Unit)? = null, -): ReadOnlyProperty = RequireMapboxNavigationDelegate( +): ReadOnlyProperty = RequireMapboxNavigationDelegate( + lifecycleOwner = this, onCreatedObserver = onCreatedObserver, onStartedObserver = onStartedObserver, onResumedObserver = onResumedObserver, @@ -74,22 +76,23 @@ fun requireMapboxNavigation( * [MapboxNavigation] instance can be re-created with [MapboxNavigationApp.disable], or if all * [MapboxNavigationApp.attach] lifecycles are destroyed. * + * @param lifecycleOwner: LifecycleOwner * @param onCreatedObserver registered to the [Lifecycle.State.CREATED] lifecycle * @param onStartedObserver registered to the [Lifecycle.State.STARTED] lifecycle * @param onResumedObserver registered to the [Lifecycle.State.RESUMED] lifecycle - * @param onInitialize called when the property is read for the first time + * @param onInitialize called when the [lifecycleOwner] is [Lifecycle.State.CREATED] */ internal class RequireMapboxNavigationDelegate( + lifecycleOwner: LifecycleOwner, private val onCreatedObserver: MapboxNavigationObserver? = null, private val onStartedObserver: MapboxNavigationObserver? = null, private val onResumedObserver: MapboxNavigationObserver? = null, private val onInitialize: (() -> Unit)? = null -) : ReadOnlyProperty { - - private lateinit var lifecycleOwner: LifecycleOwner +) : ReadOnlyProperty { private val lifecycleObserver = object : DefaultLifecycleObserver { override fun onCreate(owner: LifecycleOwner) { + onInitialize?.invoke() onCreatedObserver?.let { MapboxNavigationApp.registerObserver(it) } } @@ -114,21 +117,19 @@ internal class RequireMapboxNavigationDelegate( } } + init { + MapboxNavigationApp.attach(lifecycleOwner) + lifecycleOwner.lifecycle.addObserver(lifecycleObserver) + } + /** - * Returns an instance of [MapboxNavigation], the first retrieval will attach the [thisRef] - * to [MapboxNavigationApp.attach]. If [MapboxNavigationApp.isSetup] is false after all - * observers and initializers, this property getter will crash. + * Returns an instance of [MapboxNavigation]. If [MapboxNavigationApp.isSetup] is false after + * all observers and initializers, this property getter will crash. * * @param thisRef - the [LifecycleOwner] that needs access to [MapboxNavigation]. * @param property - ignored */ - override fun getValue(thisRef: LifecycleOwner, property: KProperty<*>): MapboxNavigation { - if (!this::lifecycleOwner.isInitialized) { - onInitialize?.invoke() - this.lifecycleOwner = thisRef - MapboxNavigationApp.attach(lifecycleOwner) - lifecycleOwner.lifecycle.addObserver(lifecycleObserver) - } + override fun getValue(thisRef: Any, property: KProperty<*>): MapboxNavigation { val mapboxNavigation = MapboxNavigationApp.current() checkNotNull(mapboxNavigation) { "MapboxNavigation cannot be null. Ensure that MapboxNavigationApp is setup and an" + diff --git a/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt index 2d8ad731d18..7a2e6e97d75 100644 --- a/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt +++ b/libnavigation-core/src/test/java/com/mapbox/navigation/core/lifecycle/RequireMapboxNavigationTest.kt @@ -81,6 +81,18 @@ class RequireMapboxNavigationTest { sut.mapboxNavigation } + @Test + fun `onInitialize is called before first reference to the delegate`() { + mockMapboxNavigationAppBehavior() + val mockOnInitialize: (() -> Unit) = mockk(relaxed = true) + val sut = SystemUnderTest(onInitialize = mockOnInitialize) + + MapboxNavigationApp.setup(mockk()) + sut.moveToState(Lifecycle.State.CREATED) + + verify { mockOnInitialize.invoke() } + } + @Test fun `multiple delegate references are the same instance`() { mockMapboxNavigationAppBehavior() diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt index 7b4986ccb3c..813d8ebe1dc 100644 --- a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt +++ b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingActivity.kt @@ -93,7 +93,6 @@ class InactiveRouteStylingActivity : AppCompatActivity() { private val mapboxNavigation by requireMapboxNavigation( onCreatedObserver = object : MapboxNavigationObserver { override fun onAttached(mapboxNavigation: MapboxNavigation) { - mapboxNavigation.registerLocationObserver(locationObserver) mapboxNavigation.registerRouteProgressObserver(routeProgressObserver) mapboxNavigation.registerRouteProgressObserver(replayProgressObserver)