DevicePoseProvider: improve getLocation() (#1454)

* Only take updated location when it is of better quality

* Rename to AndroidLocation for clarification

* be pedantic about licenses

* fix race condition in DevicePoseProvider.kt

* add possibility to explicitly skip cache upon requesting location updates for WeatherRepository.kt

* LocationsRepository.kt: use getLocation() in combineTransform() to properly wait for & update on new locations

* LocationItem.kt: fix getAzimuthDegrees() usage for userHeading

app/ui/src/main/java/de/mm20/launcher2/ui/launcher/search/SearchVM.kt

@@ -288,7 +288,7 @@ class SearchVM : ViewModel(), KoinComponent {
                         locationResults.mergeWith(
                             results.locations?.filterNot { hiddenKeys.contains(it.key) }
                                 ?.let { locations ->
-                                    devicePoseProvider.lastLocation?.let {
+                                    devicePoseProvider.lastCachedLocation?.let {
                                         locations.asSequence()
                                             .sortedWith { a, b ->
                                                 a.distanceTo(it).compareTo(b.distanceTo(it))

app/ui/src/main/java/de/mm20/launcher2/ui/launcher/search/location/LocationItem.kt

@@ -167,7 +167,7 @@ fun LocationItem(
 
     val userLocation by remember {
         viewModel.devicePoseProvider.getLocation()
-    }.collectAsStateWithLifecycle(viewModel.devicePoseProvider.lastLocation)
+    }.collectAsStateWithLifecycle(null)
 
     val targetHeading by remember(userLocation, location) {
         if (userLocation != null) {
@@ -180,9 +180,7 @@ fun LocationItem(
     }.collectAsStateWithLifecycle(null)
 
     val userHeading by remember {
-        if (userLocation != null) {
+        viewModel.devicePoseProvider.getAzimuthDegrees()
-            viewModel.devicePoseProvider.getAzimuthDegrees()
-        } else emptyFlow()
     }.collectAsStateWithLifecycle(null)
 
     val icon by viewModel.icon.collectAsStateWithLifecycle()

core/devicepose/src/main/java/de/mm20/launcher2/devicepose/DevicePoseProvider.kt

@@ -2,7 +2,6 @@ package de.mm20.launcher2.devicepose
 
 import android.Manifest
 import android.content.Context
-import android.hardware.GeomagneticField
 import android.hardware.Sensor
 import android.hardware.SensorEvent
 import android.hardware.SensorEventListener
@@ -14,33 +13,53 @@ import androidx.core.content.getSystemService
 import androidx.core.location.LocationListenerCompat
 import de.mm20.launcher2.ktx.PI
 import de.mm20.launcher2.ktx.checkPermission
+import de.mm20.launcher2.ktx.declination
 import kotlinx.coroutines.channels.awaitClose
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.callbackFlow
 import kotlinx.coroutines.flow.channelFlow
+import de.mm20.launcher2.ktx.foldOrNull
+import de.mm20.launcher2.ktx.isBetterThan
 import kotlinx.coroutines.flow.combine
+import java.util.concurrent.locks.ReentrantReadWriteLock
+import kotlin.concurrent.read
+import kotlin.concurrent.write
 
 class DevicePoseProvider internal constructor(
     private val context: Context
 ) {
-    var lastLocation: Location? = null
+    private val lastLocationLock = ReentrantReadWriteLock()
-        private set
+    var lastCachedLocation: Location? = null
+        get() { return lastLocationLock.read { field } }
+        private set(value) {
+            if (value == null) return
+            lastLocationLock.write {
+                if (value.isBetterThan(field)) {
+                    field = value
+                }
+            }
+        }
 
-    private var declination: Float? = null
+    /**
-    private fun updateDeclination(location: Location) {
+     * @param skipCache: when using `getLocation().firstOrNull()`, prefer `skipCache = false`,
-        declination = GeomagneticField(
+     *                   since otherwise, you may only receive an out of date location
-            location.latitude.toFloat(),
+     */
-            location.longitude.toFloat(),
+    fun getLocation(minTimeMs: Long = 1000, minDistanceM: Float = 1f, skipCache: Boolean = false) = channelFlow {
-            location.altitude.toFloat(),
+        // have a local copy to work with
-            location.time
+        var localLastLocation = lastCachedLocation
-        ).declination
+
-    }
+        fun updateLocation(update: Location) {
+            if (!update.isBetterThan(localLastLocation)) return
+            localLastLocation = update
+            trySend(update)
+        }
 
-    fun getLocation(minTimeMs: Long = 1000, minDistanceM: Float = 1f) = channelFlow {
         val locationCallback = LocationListenerCompat {
-            lastLocation = it
+            updateLocation(it)
-            updateDeclination(it)
+        }
-            trySend(it)
+
+        if (!skipCache && localLastLocation != null) {
+            trySend(localLastLocation)
         }
 
         context.getSystemService<LocationManager>()
@@ -50,20 +69,16 @@ class DevicePoseProvider internal constructor(
                 val hasCoarseAccess =
                     context.checkPermission(Manifest.permission.ACCESS_COARSE_LOCATION)
 
-                val location =
+                val previousLocation =
-                    (if (hasFineAccess) this@runCatching.getLastKnownLocation(LocationManager.GPS_PROVIDER) else null)
+                    hasFineAccess.foldOrNull { getLastKnownLocation(LocationManager.GPS_PROVIDER) } ?:
-                        ?: if (hasCoarseAccess) this@runCatching.getLastKnownLocation(
+                    hasCoarseAccess.foldOrNull { getLastKnownLocation(LocationManager.NETWORK_PROVIDER) }
-                            LocationManager.NETWORK_PROVIDER
-                        ) else null
 
-                if (location != null) {
+                if (previousLocation != null) {
-                    lastLocation = location
+                    updateLocation(previousLocation)
-                    updateDeclination(location)
-                    trySend(location)
                 }
 
                 if (hasFineAccess) {
-                    this@runCatching.requestLocationUpdates(
+                    requestLocationUpdates(
                         LocationManager.GPS_PROVIDER,
                         minTimeMs,
                         minDistanceM,
@@ -71,7 +86,7 @@ class DevicePoseProvider internal constructor(
                     )
                 }
                 if (hasCoarseAccess) {
-                    this@runCatching.requestLocationUpdates(
+                    requestLocationUpdates(
                         LocationManager.NETWORK_PROVIDER,
                         minTimeMs,
                         minDistanceM,
@@ -79,11 +94,12 @@ class DevicePoseProvider internal constructor(
                     )
                 }
             }?.onFailure {
-                Log.e("SearchableItemVM", "Failed to register location listener", it)
+                Log.e("DevicePoseProvider", "Failed to register location listener", it)
             }
 
         awaitClose {
             context.getSystemService<LocationManager>()?.removeUpdates(locationCallback)
+            lastCachedLocation = localLastLocation
         }
     }
 
@@ -101,7 +117,7 @@ class DevicePoseProvider internal constructor(
                     SensorManager.getRotationMatrixFromVector(rotationMatrix, event.values)
                     SensorManager.getOrientation(rotationMatrix, orientationAngles)
 
-                    trySend(orientationAngles[0] * 180f / Float.PI + (declination ?: 0f))
+                    trySend(orientationAngles[0] * 180f / Float.PI + (lastCachedLocation?.declination ?: 0f))
                 }
             }
 

core/ktx/src/main/java/de/mm20/launcher2/ktx/AndroidLocation.kt

@@ -0,0 +1,48 @@
+package de.mm20.launcher2.ktx
+
+import android.hardware.GeomagneticField
+import kotlin.time.Duration.Companion.minutes
+import kotlin.time.Duration.Companion.nanoseconds
+
+import android.location.Location as AndroidLocation
+
+/* https://github.com/streetcomplete/StreetComplete/blob/master/app/src/main/java/de/westnordost/streetcomplete/util/location/LocationUtils.kt
+ * GPL-3.0-or-later
+ */
+fun AndroidLocation?.isBetterThan(previous: AndroidLocation?): Boolean {
+    if (this == null) return false
+    if (longitude.isNaN() || latitude.isNaN()) return false
+    if (previous == null) return true
+
+    val locationTimeDiff = elapsedRealtimeNanos.nanoseconds - previous.elapsedRealtimeNanos.nanoseconds
+    val isMuchNewer = locationTimeDiff > 2.minutes
+    val isMuchOlder = locationTimeDiff < (-2).minutes
+    val isNewer = locationTimeDiff.isPositive()
+
+    val accuracyDelta = accuracy - previous.accuracy
+    val isLessAccurate = accuracyDelta > 0f
+    val isMoreAccurate = accuracyDelta < 0f
+    val isMuchLessAccurate = accuracyDelta > 200f
+
+    val isFromSameProvider = provider == previous.provider
+
+    return when {
+        // the user has likely moved
+        isMuchNewer -> true
+        // If the new location is more than two minutes older, it must be worse
+        isMuchOlder -> false
+        isMoreAccurate -> true
+        isNewer && !isLessAccurate -> true
+        isNewer && !isMuchLessAccurate && isFromSameProvider -> true
+        else -> false
+    }
+}
+
+val AndroidLocation.declination: Float
+    get() = GeomagneticField(
+        latitude.toFloat(),
+        longitude.toFloat(),
+        altitude.toFloat(),
+        time
+    ).declination
+

core/ktx/src/main/java/de/mm20/launcher2/ktx/Boolean.kt

@@ -1,5 +1,14 @@
 package de.mm20.launcher2.ktx
 
-inline fun Boolean.toInt(): Int {
+fun Boolean.toInt(): Int {
     return if (this) 1 else 0
-}
+}
+
+fun <T> Boolean.fold(
+    whenTrue: () -> T,
+    otherwise: () -> T
+): T = if (this) whenTrue() else otherwise()
+
+fun <T> Boolean.foldOrNull(
+    whenTrue: () -> T
+): T? = fold(whenTrue) { null }

data/locations/src/main/java/de/mm20/launcher2/locations/LocationsRepository.kt

@@ -11,21 +11,15 @@ import de.mm20.launcher2.search.Location
 import de.mm20.launcher2.search.SearchableRepository
 import kotlinx.collections.immutable.ImmutableList
 import kotlinx.collections.immutable.persistentListOf
-import kotlinx.collections.immutable.toImmutableList
 import kotlinx.collections.immutable.toPersistentList
-import kotlinx.coroutines.coroutineScope
+import kotlinx.coroutines.FlowPreview
 import kotlinx.coroutines.flow.Flow
 import kotlinx.coroutines.flow.MutableStateFlow
-import kotlinx.coroutines.flow.collectLatest
 import kotlinx.coroutines.flow.combineTransform
-import kotlinx.coroutines.flow.coroutineContext
 import kotlinx.coroutines.flow.emitAll
-import kotlinx.coroutines.flow.firstOrNull
 import kotlinx.coroutines.flow.flowOf
 import kotlinx.coroutines.flow.update
-import kotlinx.coroutines.job
 import kotlinx.coroutines.launch
-import kotlinx.coroutines.newCoroutineContext
 import kotlinx.coroutines.supervisorScope
 
 internal class LocationsRepository(
@@ -35,6 +29,7 @@ internal class LocationsRepository(
     private val permissionsManager: PermissionsManager,
 ) : SearchableRepository<Location> {
 
+    @OptIn(FlowPreview::class)
     override fun search(
         query: String,
         allowNetwork: Boolean
@@ -45,16 +40,16 @@ internal class LocationsRepository(
 
         val hasPermission = permissionsManager.hasPermission(PermissionGroup.Location)
 
-        return combineTransform(settings.data, hasPermission) { settingsData, permission ->
+        return combineTransform(
+            poseProvider.getLocation(minDistanceM = 50.0f),
+            settings.data,
+            hasPermission
+        ) { userLocation, settingsData, permission ->
             emit(persistentListOf())
             if (!permission || settingsData.providers.isEmpty()) {
                 return@combineTransform
             }
 
-            val userLocation = poseProvider.getLocation().firstOrNull()
-                ?: poseProvider.lastLocation
-                ?: return@combineTransform
-
             val providers = settingsData.providers.map {
                 when (it) {
                     "openstreetmaps" -> OsmLocationProvider(context, settings)

data/weather/src/main/java/de/mm20/launcher2/weather/WeatherRepository.kt

@@ -25,9 +25,7 @@ import org.koin.core.component.KoinComponent
 import org.koin.core.component.inject
 import java.time.Duration
 import java.util.*
-import kotlin.time.Duration.Companion.milliseconds
 import kotlin.time.Duration.Companion.minutes
-import kotlin.time.toJavaDuration
 
 interface WeatherRepository {
     fun getActiveProvider(): Flow<WeatherProviderInfo?>
@@ -263,8 +261,9 @@ class WeatherUpdateWorker(
     }
 
     @OptIn(FlowPreview::class)
-    private suspend fun getLastKnownLocation(): LatLon? =
+    private suspend fun getLastKnownLocation(): LatLon? = locationProvider.getLocation(skipCache = true)
-        locationProvider.getLocation().timeout(10.minutes).firstOrNull().or {
+        .timeout(10.minutes)
-            locationProvider.lastLocation
+        .firstOrNull()
-        }?.let { LatLon(it.latitude, it.longitude) }
+        .or { locationProvider.lastCachedLocation }
+        ?.let { LatLon(it.latitude, it.longitude) }
 }