Add some flexibility to NavigationLocationProvider

kmadsen · kmadsen · commit ec1428e11c5a · 2022-09-29T18:02:24.000-07:00
diff --git a/CHANGELOG.md b/CHANGELOG.md
@@ -14,6 +14,9 @@ Mapbox welcomes participation and contributions from everyone.
 - Mitigated a rare issue that caused a crash in alternative routes fork offset calculation when using `MapboxRouteLineApi#setNavigationRoutes`. [#6404](https://github.com/mapbox/mapbox-navigation-android/pull/6404)
 - Improved `MapboxNavigation#startTripSession(withForegroundService = true)` docs to indicate that it should only be called from a foreground state. [#6405](https://github.com/mapbox/mapbox-navigation-android/pull/6405)
 - Fixed an issue where registered `RouteProgressObserver`s were not invoked, while alternative routes were updated. [#6397](https://github.com/mapbox/mapbox-navigation-android/pull/6397)
+- Make `NavigationLocationProvider` an open class so it can be extended. [#6420](https://github.com/mapbox/mapbox-navigation-android/pull/6420)
+- Make `NavigationLocationProvider` inherit from `MapboxNavigationObserver` so it can be used with `MapboxNavigationApp`. [#6420](https://github.com/mapbox/mapbox-navigation-android/pull/6420)
+- Convert `LocationObserver` into a java interface with default functions so the functions are optional and new functions can be added. [#6420](https://github.com/mapbox/mapbox-navigation-android/pull/6420)
 
 ## Mapbox Navigation SDK 2.8.0 - 29 September, 2022
 ### Changelog
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/IndependentRouteGenerationActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/IndependentRouteGenerationActivity.kt
@@ -129,7 +129,6 @@ class IndependentRouteGenerationActivity : AppCompatActivity() {
     private val locationObserver = object : LocationObserver {
         private var initialUpdateDone = false
 
-        override fun onNewRawLocation(rawLocation: Location) {}
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             val enhancedLocation = locationMatcherResult.enhancedLocation
             if (!initialUpdateDone) {
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxBuildingHighlightActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxBuildingHighlightActivity.kt
@@ -149,7 +149,6 @@ class MapboxBuildingHighlightActivity : AppCompatActivity(), OnMapLongClickListe
     }
 
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {}
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxCustomStyleActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxCustomStyleActivity.kt
@@ -134,7 +134,6 @@ class MapboxCustomStyleActivity : AppCompatActivity(), OnMapLongClickListener {
     private val replayProgressObserver = ReplayProgressObserver(mapboxReplayer)
 
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {}
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxJunctionActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxJunctionActivity.kt
@@ -129,7 +129,6 @@ class MapboxJunctionActivity : AppCompatActivity(), OnMapLongClickListener {
     private val replayProgressObserver = ReplayProgressObserver(mapboxReplayer)
 
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {}
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxManeuverActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxManeuverActivity.kt
@@ -132,9 +132,6 @@ class MapboxManeuverActivity : AppCompatActivity(), OnMapLongClickListener {
     }
 
     private val locationObserver = object : LocationObserver {
-
-        override fun onNewRawLocation(rawLocation: Location) {}
-
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxNavigationActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxNavigationActivity.kt
@@ -178,10 +178,6 @@ class MapboxNavigationActivity : AppCompatActivity() {
 
     /* ----- Location and route progress callbacks ----- */
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {
-            // not handled
-        }
-
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             // update location puck's position on the map
             navigationLocationProvider.changePosition(
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxRouteLineAndArrowActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/MapboxRouteLineAndArrowActivity.kt
@@ -321,7 +321,6 @@ class MapboxRouteLineAndArrowActivity : AppCompatActivity(), OnMapLongClickListe
     }
 
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {}
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
diff --git a/examples/src/main/java/com/mapbox/navigation/examples/core/camera/MapboxCameraAnimationsActivity.kt b/examples/src/main/java/com/mapbox/navigation/examples/core/camera/MapboxCameraAnimationsActivity.kt
@@ -146,9 +146,6 @@ class MapboxCameraAnimationsActivity :
     private val poiSource = geoJsonSource("circle_source") {}.data("")
 
     private val locationObserver = object : LocationObserver {
-
-        override fun onNewRawLocation(rawLocation: Location) {}
-
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             val transitionOptions: (ValueAnimator.() -> Unit) =
                 if (locationMatcherResult.isTeleport) {
diff --git a/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/idling/FirstLocationIdlingResource.kt b/instrumentation-tests/src/androidTest/java/com/mapbox/navigation/instrumentation_tests/utils/idling/FirstLocationIdlingResource.kt
@@ -54,10 +54,6 @@ class FirstLocationIdlingResource(
 
     /** Used to communicate with [MapboxNavigation.registerLocationObserver] **/
     private val locationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {
-            // Do nothing
-        }
-
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             if (firstLocation == null) {
                 firstLocation = locationMatcherResult.enhancedLocation
diff --git a/libnavigation-core/api/current.txt b/libnavigation-core/api/current.txt
@@ -932,8 +932,8 @@ package com.mapbox.navigation.core.trip.session {
   }
 
   @UiThread public interface LocationObserver {
-    method public void onNewLocationMatcherResult(com.mapbox.navigation.core.trip.session.LocationMatcherResult locationMatcherResult);
-    method public void onNewRawLocation(android.location.Location rawLocation);
+    method public default void onNewLocationMatcherResult(com.mapbox.navigation.core.trip.session.LocationMatcherResult);
+    method public default void onNewRawLocation(android.location.Location);
   }
 
   public abstract sealed class NavigationSessionState {
diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/LocationObserver.java b/libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/LocationObserver.java
@@ -0,0 +1,33 @@
+package com.mapbox.navigation.core.trip.session;
+
+import android.location.Location;
+
+import androidx.annotation.NonNull;
+import androidx.annotation.UiThread;
+
+/**
+ * An interface which enables listening to location updates
+ *
+ * @see LocationMatcherResult
+ */
+@UiThread
+public interface LocationObserver {
+
+  /**
+   * Invoked as soon as a new [Location] has been received.
+   *
+   * @param rawLocation un-snapped update
+   */
+  default void onNewRawLocation(@NonNull Location rawLocation) {
+    // Override to capture
+  }
+
+  /**
+   * Provides the best possible location update, snapped to the route or map-matched to the road if possible.
+   *
+   * @param locationMatcherResult details about the status of the enhanced location.
+   */
+  default void onNewLocationMatcherResult(@NonNull LocationMatcherResult locationMatcherResult) {
+    // Override to capture
+  }
+}
diff --git a/libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/LocationObserver.kt b/libnavigation-core/src/main/java/com/mapbox/navigation/core/trip/session/LocationObserver.kt
diff --git a/libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/location/NavigationLocationProvider.kt b/libnavui-maps/src/main/java/com/mapbox/navigation/ui/maps/location/NavigationLocationProvider.kt
@@ -3,13 +3,17 @@ package com.mapbox.navigation.ui.maps.location
 import android.animation.ValueAnimator
 import android.annotation.SuppressLint
 import android.location.Location
+import androidx.annotation.CallSuper
 import com.mapbox.geojson.Point
 import com.mapbox.maps.plugin.locationcomponent.LocationComponentConstants
 import com.mapbox.maps.plugin.locationcomponent.LocationComponentPlugin
 import com.mapbox.maps.plugin.locationcomponent.LocationConsumer
 import com.mapbox.maps.plugin.locationcomponent.LocationProvider
 import com.mapbox.maps.plugin.locationcomponent.OnIndicatorPositionChangedListener
 import com.mapbox.navigation.core.MapboxNavigation
+import com.mapbox.navigation.core.lifecycle.MapboxNavigationApp
+import com.mapbox.navigation.core.lifecycle.MapboxNavigationObserver
+import com.mapbox.navigation.core.trip.session.LocationMatcherResult
 import com.mapbox.navigation.core.trip.session.LocationObserver
 import com.mapbox.navigation.ui.maps.internal.location.PuckAnimationEvaluator
 import com.mapbox.navigation.utils.internal.ifNonNull
@@ -53,7 +57,10 @@ import java.util.concurrent.CopyOnWriteArraySet
  * }
  * ```
  */
-class NavigationLocationProvider : LocationProvider {
+open class NavigationLocationProvider :
+    LocationProvider,
+    LocationObserver,
+    MapboxNavigationObserver {
 
     private val locationConsumers = CopyOnWriteArraySet<LocationConsumer>()
 
@@ -73,6 +80,17 @@ class NavigationLocationProvider : LocationProvider {
     var lastKeyPoints: List<Location> = emptyList()
         private set
 
+    /**
+     * Automatically updated when this class is registered to [MapboxNavigationApp]
+     */
+    @CallSuper
+    override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
+        changePosition(
+            locationMatcherResult.enhancedLocation,
+            locationMatcherResult.keyPoints,
+        )
+    }
+
     /**
      * Register the location consumer to the Location Provider.
      *
@@ -81,7 +99,7 @@ class NavigationLocationProvider : LocationProvider {
      * @param locationConsumer
      */
     @SuppressLint("MissingPermission")
-    override fun registerLocationConsumer(locationConsumer: LocationConsumer) {
+    final override fun registerLocationConsumer(locationConsumer: LocationConsumer) {
         if (locationConsumers.add(locationConsumer)) {
             ifNonNull(lastLocation, lastKeyPoints) { location, keyPoints ->
                 locationConsumer.notifyLocationUpdates(
@@ -103,7 +121,7 @@ class NavigationLocationProvider : LocationProvider {
      *
      * @param locationConsumer
      */
-    override fun unRegisterLocationConsumer(locationConsumer: LocationConsumer) {
+    final override fun unRegisterLocationConsumer(locationConsumer: LocationConsumer) {
         locationConsumers.remove(locationConsumer)
     }
 
@@ -122,6 +140,7 @@ class NavigationLocationProvider : LocationProvider {
      * @see LocationObserver
      * @see MapboxNavigation.registerLocationObserver
      */
+    @CallSuper
     fun changePosition(
         location: Location,
         keyPoints: List<Location> = emptyList(),
@@ -140,6 +159,27 @@ class NavigationLocationProvider : LocationProvider {
         lastKeyPoints = keyPoints
     }
 
+    /**
+     * Use [getInstance] to share a single instance of the [NavigationLocationProvider].
+     *
+     * @see [MapboxNavigationApp]
+     */
+    @CallSuper
+    override fun onAttached(mapboxNavigation: MapboxNavigation) {
+        mapboxNavigation.registerLocationObserver(this)
+    }
+
+    /**
+     * Disabled automatically with the [MapboxNavigationApp]. You can manually disable with
+     * [MapboxNavigationApp.unregisterObserver].
+     *
+     * @see [MapboxNavigationApp]
+     */
+    @CallSuper
+    override fun onDetached(mapboxNavigation: MapboxNavigation) {
+        mapboxNavigation.unregisterLocationObserver(this)
+    }
+
     private fun LocationConsumer.notifyLocationUpdates(
         location: Location,
         keyPoints: List<Location>,
@@ -176,4 +216,14 @@ class NavigationLocationProvider : LocationProvider {
             clientOptions?.also { apply(it) }
         }
     }
+
+    companion object {
+        /**
+         * Get the registered instance or create one and register it to [MapboxNavigationApp].
+         */
+        @JvmStatic
+        fun getInstance(): NavigationLocationProvider = MapboxNavigationApp
+            .getObservers(NavigationLocationProvider::class).firstOrNull()
+            ?: NavigationLocationProvider().also { MapboxNavigationApp.registerObserver(it) }
+    }
 }
diff --git a/libnavui-maps/src/test/java/com/mapbox/navigation/ui/maps/location/NavigationLocationProviderTest.kt b/libnavui-maps/src/test/java/com/mapbox/navigation/ui/maps/location/NavigationLocationProviderTest.kt
@@ -4,21 +4,23 @@ import android.animation.ValueAnimator
 import android.location.Location
 import com.mapbox.geojson.Point
 import com.mapbox.maps.plugin.locationcomponent.LocationConsumer
+import com.mapbox.navigation.core.MapboxNavigation
+import com.mapbox.navigation.core.trip.session.LocationMatcherResult
+import com.mapbox.navigation.core.trip.session.LocationObserver
 import io.mockk.every
+import io.mockk.just
 import io.mockk.mockk
+import io.mockk.runs
+import io.mockk.slot
 import io.mockk.verify
 import org.junit.Assert.assertEquals
-import org.junit.Before
+import org.junit.Assert.assertFalse
+import org.junit.Assert.assertTrue
 import org.junit.Test
 
 class NavigationLocationProviderTest {
 
-    private lateinit var navigationLocationProvider: NavigationLocationProvider
-
-    @Before
-    fun setup() {
-        navigationLocationProvider = NavigationLocationProvider()
-    }
+    private val navigationLocationProvider = NavigationLocationProvider()
 
     @Test
     fun `location is cached`() {
@@ -184,4 +186,69 @@ class NavigationLocationProviderTest {
             consumer.onBearingUpdated(any())
         }
     }
+
+    @Test
+    fun `onAttached will register a LocationObserver and receive raw locations`() {
+        val locationObserverSlot = slot<LocationObserver>()
+        val mapboxNavigation: MapboxNavigation = mockk {
+            every { registerLocationObserver(capture(locationObserverSlot)) } just runs
+        }
+        val rawLocationSlot = mutableListOf<Location>()
+        val navigationLocationProvider = object : NavigationLocationProvider() {
+            override fun onNewRawLocation(rawLocation: Location) {
+                rawLocationSlot.add(rawLocation)
+            }
+        }
+
+        // Expect 3 locations
+        navigationLocationProvider.onAttached(mapboxNavigation)
+        locationObserverSlot.captured.onNewRawLocation(mockk())
+        locationObserverSlot.captured.onNewRawLocation(mockk())
+        locationObserverSlot.captured.onNewRawLocation(mockk())
+
+        assertEquals(3, rawLocationSlot.size)
+    }
+
+    @Test
+    fun `onAttached will register a LocationObserver and receive map matched locations`() {
+        val locationObserverSlot = slot<LocationObserver>()
+        val mapboxNavigation: MapboxNavigation = mockk {
+            every { registerLocationObserver(capture(locationObserverSlot)) } just runs
+        }
+        val locationMatcherResultSlot = mutableListOf<LocationMatcherResult>()
+        val navigationLocationProvider = object : NavigationLocationProvider() {
+            override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
+                super.onNewLocationMatcherResult(locationMatcherResult)
+                locationMatcherResultSlot.add(locationMatcherResult)
+            }
+        }
+
+        navigationLocationProvider.onAttached(mapboxNavigation)
+        locationObserverSlot.captured.onNewLocationMatcherResult(
+            mockk { every { isOffRoad } returns false }
+        )
+        locationObserverSlot.captured.onNewLocationMatcherResult(
+            mockk { every { isOffRoad } returns true }
+        )
+
+        assertEquals(2, locationMatcherResultSlot.size)
+        assertFalse(locationMatcherResultSlot[0].isOffRoad)
+        assertTrue(locationMatcherResultSlot[1].isOffRoad)
+    }
+
+    @Test
+    fun `onDetached will unregister a LocationObserver and receive raw locations`() {
+        val mapboxNavigation: MapboxNavigation = mockk(relaxed = true)
+        val rawLocationSlot = mutableListOf<Location>()
+        val navigationLocationProvider = object : NavigationLocationProvider() {
+            override fun onNewRawLocation(rawLocation: Location) {
+                rawLocationSlot.add(rawLocation)
+            }
+        }
+
+        // Expect 3 locations
+        navigationLocationProvider.onDetached(mapboxNavigation)
+
+        verify { mapboxNavigation.unregisterLocationObserver(navigationLocationProvider) }
+    }
 }
diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/lifecycle/viewmodel/DropInLocationViewModel.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/lifecycle/viewmodel/DropInLocationViewModel.kt
@@ -26,10 +26,6 @@ class DropInLocationViewModel : ViewModel() {
     val locationLiveData: LiveData<Location> = _locationLiveData
 
     private val locationObserver: LocationObserver = object : LocationObserver {
-        override fun onNewRawLocation(rawLocation: Location) {
-            // no op
-        }
-
         override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {
             navigationLocationProvider.changePosition(
                 locationMatcherResult.enhancedLocation,
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
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
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
diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingWithRestrictionsActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/InactiveRouteStylingWithRestrictionsActivity.kt
diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/base/BaseNavigationActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/base/BaseNavigationActivity.kt
diff --git a/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/customnavview/MapboxNavigationViewCustomizedActivity.kt b/qa-test-app/src/main/java/com/mapbox/navigation/qa_test_app/view/customnavview/MapboxNavigationViewCustomizedActivity.kt

Original file line number	Diff line number	Diff line change
`@@ -149,7 +149,6 @@ class MapboxBuildingHighlightActivity : AppCompatActivity(), OnMapLongClickListe`
`149`	`149`	`}`
`150`	`150`
`151`	`151`	`private val locationObserver = object : LocationObserver {`
`152`		`- override fun onNewRawLocation(rawLocation: Location) {}`
`153`	`152`	`override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {`
`154`	`153`	`navigationLocationProvider.changePosition(`
`155`	`154`	`locationMatcherResult.enhancedLocation,`
Original file line number	Diff line number	Diff line change
`@@ -132,9 +132,6 @@ class MapboxManeuverActivity : AppCompatActivity(), OnMapLongClickListener {`
`132`	`132`	`}`
`133`	`133`
`134`	`134`	`private val locationObserver = object : LocationObserver {`
`135`		`-`
`136`		`- override fun onNewRawLocation(rawLocation: Location) {}`
`137`		`-`
`138`	`135`	`override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {`
`139`	`136`	`navigationLocationProvider.changePosition(`
`140`	`137`	`locationMatcherResult.enhancedLocation,`
Original file line number	Diff line number	Diff line change
`@@ -321,7 +321,6 @@ class MapboxRouteLineAndArrowActivity : AppCompatActivity(), OnMapLongClickListe`
`321`	`321`	`}`
`322`	`322`
`323`	`323`	`private val locationObserver = object : LocationObserver {`
`324`		`- override fun onNewRawLocation(rawLocation: Location) {}`
`325`	`324`	`override fun onNewLocationMatcherResult(locationMatcherResult: LocationMatcherResult) {`
`326`	`325`	`navigationLocationProvider.changePosition(`
`327`	`326`	`locationMatcherResult.enhancedLocation,`