From cd2efaca8f6bb7e8c22d2c205461df9f21ff50a8 Mon Sep 17 00:00:00 2001 From: temcguir Date: Wed, 30 Sep 2026 22:55:07 +0000 Subject: [PATCH 1/4] Implement LocationManagerLocationProvider with multi-provider registration and guardrails Introduces a platform LocationProvider implementation backed by LocationManagerCompat in :core:location:location-manager. Key features: - Concurrently registers available platform providers (GPS_PROVIDER, NETWORK_PROVIDER, and FUSED_PROVIDER on API 31+) to ensure fast location acquisition indoors and outdoors. - Enforces runtime permission checks and device master location state (isLocationAvailable()), returning null when location services are turned off even if permissions are granted. - Restricts coarse-only permission queries to avoid attempting fine-only platform queries (GPS_PROVIDER and PASSIVE_PROVIDER). - Implements time-aware isBetterLocation() heuristics: - Fixes >2 minutes newer supersede older fixes regardless of accuracy, preventing coordinate anchoring while traveling. - Within the 2-minute window, fixes must be more accurate, newer and at least as accurate, or newer from the same provider and no more than 20 meters less accurate. - Evicts stale fixes older than 30 minutes from the cache and incoming updates. - Applies a 60-second hardware timeout to conserve battery when a fix <= 50 meters is not achieved, and automatically stops updates early once a fix <= 50 meters is accepted into the cache. - Periodically re-executes a warmup cycle every 5 minutes during long camera preview sessions to refresh coordinates. - Filters out NaN, infinite, and (0, 0) coordinates. - Adds 28 Robolectric unit tests covering multi-provider registration, anchor-breaking, permission revocation with cached fixes, out-of-order rejection, and periodic refresh. --- .../location-manager/build.gradle.kts | 57 ++ .../location-manager/consumer-rules.pro | 0 .../src/main/AndroidManifest.xml | 18 + .../LocationManagerLocationProvider.kt | 350 ++++++++++ .../LocationManagerLocationProviderTest.kt | 630 ++++++++++++++++++ settings.gradle.kts | 1 + 6 files changed, 1056 insertions(+) create mode 100644 core/location/location-manager/build.gradle.kts create mode 100644 core/location/location-manager/consumer-rules.pro create mode 100644 core/location/location-manager/src/main/AndroidManifest.xml create mode 100644 core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt create mode 100644 core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt diff --git a/core/location/location-manager/build.gradle.kts b/core/location/location-manager/build.gradle.kts new file mode 100644 index 000000000..42ffe7b45 --- /dev/null +++ b/core/location/location-manager/build.gradle.kts @@ -0,0 +1,57 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +plugins { + alias(libs.plugins.android.library) +} + +android { + namespace = "com.google.jetpackcamera.core.location.locationmanager" + compileSdk { + version = release(libs.versions.compileSdk.get().toInt()) { + minorApiLevel = libs.versions.compileSdkMinor.get().toInt() + } + } + + defaultConfig { + minSdk = libs.versions.minSdk.get().toInt() + testOptions.targetSdk = libs.versions.targetSdk.get().toInt() + lint.targetSdk = libs.versions.targetSdk.get().toInt() + + testInstrumentationRunner = "androidx.test.runner.AndroidJUnitRunner" + consumerProguardFiles("consumer-rules.pro") + } + + compileOptions { + sourceCompatibility = JavaVersion.VERSION_17 + targetCompatibility = JavaVersion.VERSION_17 + } + kotlin { + jvmToolchain(17) + } +} + +dependencies { + implementation(project(":core:location")) + implementation(libs.androidx.core.ktx) + implementation(libs.kotlinx.coroutines.core) + + // Testing + testImplementation(libs.junit) + testImplementation(libs.truth) + testImplementation(libs.androidx.test.core) + testImplementation(libs.robolectric) +} diff --git a/core/location/location-manager/consumer-rules.pro b/core/location/location-manager/consumer-rules.pro new file mode 100644 index 000000000..e69de29bb diff --git a/core/location/location-manager/src/main/AndroidManifest.xml b/core/location/location-manager/src/main/AndroidManifest.xml new file mode 100644 index 000000000..50c73ec4c --- /dev/null +++ b/core/location/location-manager/src/main/AndroidManifest.xml @@ -0,0 +1,18 @@ + + + + diff --git a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt new file mode 100644 index 000000000..efdd63d72 --- /dev/null +++ b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt @@ -0,0 +1,350 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.core.location.locationmanager + +import android.Manifest +import android.annotation.SuppressLint +import android.content.Context +import android.content.pm.PackageManager +import android.location.Location +import android.location.LocationManager +import android.os.Build +import android.os.Looper +import android.os.SystemClock +import android.util.Log +import androidx.core.content.ContextCompat +import androidx.core.location.LocationListenerCompat +import androidx.core.location.LocationManagerCompat +import androidx.core.location.LocationRequestCompat +import com.google.jetpackcamera.core.location.LocationProvider +import java.util.concurrent.TimeUnit +import java.util.concurrent.atomic.AtomicBoolean +import java.util.concurrent.atomic.AtomicReference +import kotlinx.coroutines.CompletableDeferred +import kotlinx.coroutines.coroutineScope +import kotlinx.coroutines.delay +import kotlinx.coroutines.isActive +import kotlinx.coroutines.withTimeoutOrNull + +private const val TAG = "LocationManagerLocationProvider" +private const val WARMUP_TIMEOUT_MS = 60_000L +private const val ACCURACY_THRESHOLD_METERS = 50f +private const val SIGNIFICANT_ACCURACY_DELTA_METERS = 20f +private val REFRESH_INTERVAL_MS = TimeUnit.MINUTES.toMillis(5) +private val STALE_LOCATION_THRESHOLD_NANOS = TimeUnit.MINUTES.toNanos(30) +private val SIGNIFICANT_TIME_DELTA_NANOS = TimeUnit.MINUTES.toNanos(2) + +/** + * Implementation of [LocationProvider] backed by the Android platform [LocationManager] via + * [LocationManagerCompat]. + * + * Concurrently registers available hardware providers ([LocationManager.GPS_PROVIDER], + * [LocationManager.NETWORK_PROVIDER], and [LocationManager.FUSED_PROVIDER] on API 31+) + * to provide fast acquisition indoors and outdoors. Implements time-aware comparator heuristics + * to prevent coordinate anchoring, evicts stale fixes older than 30 minutes, applies a + * 60-second hardware timeout to conserve battery, and periodically refreshes the location + * fix every 5 minutes during extended preview sessions. + * + * @param context Application context used to retrieve location services and verify permissions. + */ +class LocationManagerLocationProvider(private val context: Context) : LocationProvider { + + private val locationManager = context.getSystemService(Context.LOCATION_SERVICE) + as LocationManager + + // Written from the main looper; read from any thread by capture via getCurrentLocation(). + private val cachedLocation = AtomicReference(null) + private val isUpdating = AtomicBoolean(false) + private val activeWarmupDeferred = AtomicReference?>(null) + + // Interval between periodic refresh cycles (5 minutes by default, configurable for testing) + internal var refreshIntervalMs: Long = REFRESH_INTERVAL_MS + + private val locationListener = object : LocationListenerCompat { + override fun onLocationChanged(location: Location) { + handleLocationUpdate(location) + } + } + + override fun getCurrentLocation(): Location? { + if (!isLocationAvailable()) { + return null + } + + val location = cachedLocation.get() + if (location != null && isValidLocation(location) && !isStale(location)) { + return location + } + + return getBestLastKnownLocation()?.also { lastKnown -> + cachedLocation.compareAndSet(location, lastKnown) + } + } + + /** + * Runs a warmup session every [refreshIntervalMs] until cancelled. Each cycle re-checks + * permissions and enabled providers, so changes made mid-session are applied on the next cycle. + */ + override suspend fun runLocationUpdates() = coroutineScope { + while (isActive) { + runWarmupSession() + delay(refreshIntervalMs) + } + } + + @SuppressLint("MissingPermission") + private suspend fun runWarmupSession() { + if (!hasAnyLocationPermission()) return + + // Pre-seed cache with last known location if available + getBestLastKnownLocation()?.let { lastKnown -> + cachedLocation.compareAndSet(null, lastKnown) + } + + val activeProviders = getActiveProviders() + if (activeProviders.isEmpty()) return + + val warmupCompleted = CompletableDeferred() + activeWarmupDeferred.set(warmupCompleted) + + val request = LocationRequestCompat.Builder(1000L) + .setMinUpdateIntervalMillis(1000L) + .setMinUpdateDistanceMeters(0f) + .setQuality(LocationRequestCompat.QUALITY_HIGH_ACCURACY) + .build() + + var registeredCount = 0 + for (provider in activeProviders) { + try { + LocationManagerCompat.requestLocationUpdates( + locationManager, + provider, + request, + locationListener, + Looper.getMainLooper() + ) + registeredCount++ + Log.d(TAG, "Registered location updates for provider: $provider") + } catch (e: SecurityException) { + Log.e(TAG, "SecurityException requesting updates for $provider", e) + } catch (e: IllegalArgumentException) { + Log.e(TAG, "IllegalArgumentException requesting updates for $provider", e) + } + } + + if (registeredCount == 0) { + activeWarmupDeferred.set(null) + return + } + + isUpdating.set(true) + Log.d(TAG, "Started location updates across $registeredCount providers") + + try { + withTimeoutOrNull(WARMUP_TIMEOUT_MS) { + warmupCompleted.await() + } + } finally { + activeWarmupDeferred.set(null) + stopHardwareUpdates() + } + } + + @SuppressLint("MissingPermission") + private fun stopHardwareUpdates() { + if (isUpdating.getAndSet(false)) { + LocationManagerCompat.removeUpdates(locationManager, locationListener) + Log.d(TAG, "Stopped location updates across all providers.") + } + } + + @SuppressLint("MissingPermission") + private fun getBestLastKnownLocation(): Location? { + if (!isLocationAvailable()) return null + + var bestLocation: Location? = null + val hasFine = hasFinePermission() + val providers = buildList { + // GPS and PASSIVE providers require ACCESS_FINE_LOCATION. + if (hasFine) add(LocationManager.GPS_PROVIDER) + add(LocationManager.NETWORK_PROVIDER) + if (hasFine) add(LocationManager.PASSIVE_PROVIDER) + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && + LocationManagerCompat.hasProvider( + locationManager, + LocationManager.FUSED_PROVIDER + ) && + locationManager.isProviderEnabled(LocationManager.FUSED_PROVIDER) + ) { + add(LocationManager.FUSED_PROVIDER) + } + } + for (provider in providers) { + try { + val loc = locationManager.getLastKnownLocation(provider) + if (loc != null && isValidLocation(loc) && !isStale(loc)) { + if (bestLocation == null || isBetterLocation(loc, bestLocation)) { + bestLocation = loc + } + } + } catch (e: SecurityException) { + Log.w(TAG, "SecurityException reading last known location from $provider", e) + } catch (e: IllegalArgumentException) { + Log.w(TAG, "IllegalArgumentException reading last known location from $provider", e) + } + } + return bestLocation + } + + private fun handleLocationUpdate(location: Location) { + if (!isValidLocation(location) || isStale(location)) return + + var accepted = false + while (true) { + val oldLoc = cachedLocation.get() + if (!isBetterLocation(location, oldLoc)) break + if (cachedLocation.compareAndSet(oldLoc, location)) { + accepted = true + Log.d( + TAG, + "Updated cached location: provider=${location.provider}, " + + "acc=${location.accuracy}m" + ) + break + } + } + + // A rejected fix leaves the cache unchanged, so it must not end the warmup session. + if (accepted && location.accuracy <= ACCURACY_THRESHOLD_METERS) { + activeWarmupDeferred.get()?.complete(Unit) + stopHardwareUpdates() + } + } + + /** + * Evaluates whether a candidate [Location] fix is superior to the currently cached fix. + * + * A fix more than 2 minutes newer always wins, which prevents coordinate anchoring when + * travelling. Within the 2-minute window, a fix is accepted if it is more accurate, if it is + * newer and at least as accurate, or if it is newer, from the same provider, and no more than + * [SIGNIFICANT_ACCURACY_DELTA_METERS] less accurate. + * + * @param newLoc Candidate fix received from a platform location provider. + * @param currentLoc The currently cached fix, or `null`. + * @return `true` if [newLoc] should replace [currentLoc], `false` otherwise. + */ + private fun isBetterLocation(newLoc: Location, currentLoc: Location?): Boolean { + if (currentLoc == null) return true + if (isStale(currentLoc)) return true + + val timeDeltaNanos = newLoc.elapsedRealtimeNanos - currentLoc.elapsedRealtimeNanos + val isSignificantlyNewer = timeDeltaNanos > SIGNIFICANT_TIME_DELTA_NANOS + val isSignificantlyOlder = timeDeltaNanos < -SIGNIFICANT_TIME_DELTA_NANOS + + if (isSignificantlyNewer) return true + if (isSignificantlyOlder) return false + + val isNewer = timeDeltaNanos > 0 + val accuracyDelta = newLoc.accuracy - currentLoc.accuracy + val isMoreAccurate = accuracyDelta < 0f + val isLessAccurate = accuracyDelta > 0f + val isSignificantlyLessAccurate = accuracyDelta > SIGNIFICANT_ACCURACY_DELTA_METERS + val isFromSameProvider = newLoc.provider != null && newLoc.provider == currentLoc.provider + + return when { + isMoreAccurate -> true + isNewer && !isLessAccurate -> true + isNewer && isFromSameProvider && !isSignificantlyLessAccurate -> true + else -> false + } + } + + private fun isValidLocation(location: Location): Boolean { + if (location.latitude.isNaN() || location.longitude.isNaN()) return false + if (location.latitude.isInfinite() || location.longitude.isInfinite()) return false + if (location.latitude == 0.0 && location.longitude == 0.0) return false + return true + } + + private fun isStale(location: Location): Boolean { + val ageNanos = SystemClock.elapsedRealtimeNanos() - location.elapsedRealtimeNanos + return ageNanos > STALE_LOCATION_THRESHOLD_NANOS + } + + private fun hasAnyLocationPermission(): Boolean { + return hasFinePermission() || hasCoarsePermission() + } + + /** + * Returns `true` if a location permission is granted and location services are enabled on + * the device. Cached fixes must not be returned when the user has turned off system location, + * even though runtime permissions remain granted. + */ + private fun isLocationAvailable(): Boolean { + return hasAnyLocationPermission() && LocationManagerCompat.isLocationEnabled( + locationManager + ) + } + + private fun hasFinePermission(): Boolean { + return ContextCompat.checkSelfPermission( + context, + Manifest.permission.ACCESS_FINE_LOCATION + ) == PackageManager.PERMISSION_GRANTED + } + + private fun hasCoarsePermission(): Boolean { + return ContextCompat.checkSelfPermission( + context, + Manifest.permission.ACCESS_COARSE_LOCATION + ) == PackageManager.PERMISSION_GRANTED + } + + @SuppressLint("InlinedApi") + private fun getActiveProviders(): List { + if (!LocationManagerCompat.isLocationEnabled(locationManager)) { + return emptyList() + } + + val providers = mutableListOf() + val hasFine = hasFinePermission() + val hasCoarse = hasCoarsePermission() + + if (hasFine && + LocationManagerCompat.hasProvider(locationManager, LocationManager.GPS_PROVIDER) && + locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER) + ) { + providers.add(LocationManager.GPS_PROVIDER) + } + + if ((hasFine || hasCoarse) && + LocationManagerCompat.hasProvider(locationManager, LocationManager.NETWORK_PROVIDER) && + locationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER) + ) { + providers.add(LocationManager.NETWORK_PROVIDER) + } + + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.S && + (hasFine || hasCoarse) && + LocationManagerCompat.hasProvider(locationManager, LocationManager.FUSED_PROVIDER) && + locationManager.isProviderEnabled(LocationManager.FUSED_PROVIDER) + ) { + providers.add(LocationManager.FUSED_PROVIDER) + } + + return providers + } +} diff --git a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt new file mode 100644 index 000000000..c24fba6c6 --- /dev/null +++ b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt @@ -0,0 +1,630 @@ +/* + * Copyright (C) 2026 The Android Open Source Project + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package com.google.jetpackcamera.core.location.locationmanager + +import android.Manifest +import android.app.Application +import android.content.Context +import android.location.Location +import android.location.LocationManager +import android.os.SystemClock +import androidx.test.core.app.ApplicationProvider +import com.google.common.truth.Truth.assertThat +import java.util.concurrent.TimeUnit +import kotlinx.coroutines.CoroutineScope +import kotlinx.coroutines.Dispatchers +import kotlinx.coroutines.Job +import kotlinx.coroutines.launch +import org.junit.After +import org.junit.Before +import org.junit.Test +import org.junit.runner.RunWith +import org.robolectric.RobolectricTestRunner +import org.robolectric.Shadows.shadowOf +import org.robolectric.shadows.ShadowLocationManager +import org.robolectric.shadows.ShadowLooper + +@RunWith(RobolectricTestRunner::class) +class LocationManagerLocationProviderTest { + + private lateinit var context: Context + private lateinit var shadowLocationManager: ShadowLocationManager + private lateinit var locationProvider: LocationManagerLocationProvider + private var updatesJob: Job? = null + + @Before + fun setUp() { + context = ApplicationProvider.getApplicationContext() + val locationManager = context.getSystemService(Context.LOCATION_SERVICE) as LocationManager + shadowLocationManager = shadowOf(locationManager) + + shadowLocationManager.setLocationEnabled(true) + shadowLocationManager.setProviderEnabled(LocationManager.GPS_PROVIDER, true) + shadowLocationManager.setProviderEnabled(LocationManager.NETWORK_PROVIDER, true) + + locationProvider = LocationManagerLocationProvider(context) + ShadowLooper.idleMainLooper() + } + + @After + fun tearDown() { + cancelLocationUpdates() + } + + /** + * Launches [LocationManagerLocationProvider.runLocationUpdates] on the main dispatcher, + * cancelling any session that is already running. + */ + private fun launchLocationUpdates() { + updatesJob?.cancel() + updatesJob = CoroutineScope(Dispatchers.Main).launch { + locationProvider.runLocationUpdates() + } + } + + private fun cancelLocationUpdates() { + updatesJob?.cancel() + updatesJob = null + } + + private fun grantLocationPermissions(fine: Boolean = true, coarse: Boolean = true) { + val app = shadowOf(context as Application) + if (fine) app.grantPermissions(Manifest.permission.ACCESS_FINE_LOCATION) + if (coarse) app.grantPermissions(Manifest.permission.ACCESS_COARSE_LOCATION) + } + + private fun createLocation( + provider: String = LocationManager.GPS_PROVIDER, + latitude: Double = 37.4220, + longitude: Double = -122.0841, + accuracy: Float = 10f, + elapsedRealtimeNanos: Long = SystemClock.elapsedRealtimeNanos() + ) = Location(provider).apply { + this.latitude = latitude + this.longitude = longitude + this.accuracy = accuracy + this.elapsedRealtimeNanos = elapsedRealtimeNanos + } + + private fun deliver(location: Location) { + shadowLocationManager.simulateLocation(location) + ShadowLooper.idleMainLooper() + } + + @Test + fun getCurrentLocation_noLocationCached_returnsNull() { + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Test + fun getCurrentLocation_locationCached_returnsValidLocation() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation()) + + assertThat(locationProvider.getCurrentLocation()?.latitude).isEqualTo(37.4220) + } + + @Test + fun getCurrentLocation_nullIsland_returnsNull() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(latitude = 0.0, longitude = 0.0)) + + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Test + fun getCurrentLocation_fixWithinThirtyMinutes_returnsLocation() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver( + createLocation( + elapsedRealtimeNanos = + SystemClock.elapsedRealtimeNanos() - TimeUnit.MINUTES.toNanos(29) + ) + ) + + assertThat(locationProvider.getCurrentLocation()?.latitude).isEqualTo(37.4220) + } + + @Test + fun getCurrentLocation_fixOlderThanThirtyMinutes_returnsNull() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver( + createLocation( + elapsedRealtimeNanos = SystemClock.elapsedRealtimeNanos() - + TimeUnit.MINUTES.toNanos(30) - TimeUnit.SECONDS.toNanos(1) + ) + ) + + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_bothProvidersEnabled_registersGpsAndNetwork() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.getLocationUpdateListeners(LocationManager.GPS_PROVIDER)) + .isNotEmpty() + assertThat( + shadowLocationManager.getLocationUpdateListeners(LocationManager.NETWORK_PROVIDER) + ).isNotEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_gpsDisabled_registersNetworkOnly() { + grantLocationPermissions() + shadowLocationManager.setProviderEnabled(LocationManager.GPS_PROVIDER, false) + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.getLocationUpdateListeners(LocationManager.GPS_PROVIDER)) + .isEmpty() + assertThat( + shadowLocationManager.getLocationUpdateListeners(LocationManager.NETWORK_PROVIDER) + ).isNotEmpty() + + deliver(createLocation(provider = LocationManager.NETWORK_PROVIDER, accuracy = 25f)) + + assertThat(locationProvider.getCurrentLocation()?.provider) + .isEqualTo(LocationManager.NETWORK_PROVIDER) + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_systemLocationDisabled_doesNotRegister() { + grantLocationPermissions() + shadowLocationManager.setLocationEnabled(false) + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + assertThat(locationProvider.getCurrentLocation()).isNull() + + // A later session registers once system location is re-enabled. + shadowLocationManager.setLocationEnabled(true) + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_permissionGrantedMidSession_registersOnNextCycle() { + locationProvider.refreshIntervalMs = 500L + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + + grantLocationPermissions() + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + } + + @Test + fun getCurrentLocation_withoutPermission_returnsNull() { + shadowLocationManager.setLastKnownLocation( + LocationManager.GPS_PROVIDER, + createLocation() + ) + + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Test + fun getCurrentLocation_systemLocationDisabledWithCachedFix_returnsNull() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation()) + assertThat(locationProvider.getCurrentLocation()).isNotNull() + + shadowLocationManager.setLocationEnabled(false) + + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Test + fun getCurrentLocation_coarseOnly_ignoresGpsAndPassiveLastKnownLocations() { + grantLocationPermissions(fine = false, coarse = true) + shadowLocationManager.setLastKnownLocation( + LocationManager.GPS_PROVIDER, + createLocation(provider = LocationManager.GPS_PROVIDER) + ) + shadowLocationManager.setLastKnownLocation( + LocationManager.PASSIVE_PROVIDER, + createLocation(provider = LocationManager.PASSIVE_PROVIDER) + ) + + assertThat(locationProvider.getCurrentLocation()).isNull() + + shadowLocationManager.setLastKnownLocation( + LocationManager.NETWORK_PROVIDER, + createLocation(provider = LocationManager.NETWORK_PROVIDER, accuracy = 30f) + ) + + assertThat(locationProvider.getCurrentLocation()?.provider) + .isEqualTo(LocationManager.NETWORK_PROVIDER) + } + + @Test + fun locationUpdate_significantlyNewerFix_replacesMoreAccurateFix() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + // A 5m fix stops the warmup early. + deliver(createLocation(accuracy = 5f, elapsedRealtimeNanos = baseTimeNanos)) + assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(5f) + + // Next refresh cycle re-registers listeners. + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + // 40m worse than the cached fix exceeds the 20m margin and the provider differs, so + // only the >2 minute rule can accept this fix. + deliver( + createLocation( + provider = LocationManager.NETWORK_PROVIDER, + latitude = 37.7749, + longitude = -122.4194, + accuracy = 45f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.MINUTES.toNanos(2) + + TimeUnit.SECONDS.toNanos(1) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.latitude).isEqualTo(37.7749) + assertThat(current?.accuracy).isEqualTo(45f) + assertThat(current?.provider).isEqualTo(LocationManager.NETWORK_PROVIDER) + } + + @Test + fun locationUpdate_significantlyOlderFix_isRejected() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 15f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + deliver( + createLocation( + latitude = 37.9999, + longitude = -122.9999, + accuracy = 2f, + elapsedRealtimeNanos = baseTimeNanos - TimeUnit.MINUTES.toNanos(2) - + TimeUnit.SECONDS.toNanos(1) + ) + ) + + assertThat(locationProvider.getCurrentLocation()?.latitude).isEqualTo(37.4220) + assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(15f) + } + + @Test + fun locationUpdate_withinTwoMinutes_prefersMoreAccurateFix() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 25f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + deliver( + createLocation( + latitude = 37.4225, + longitude = -122.0845, + accuracy = 5f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(5f) + assertThat(locationProvider.getCurrentLocation()?.latitude).isEqualTo(37.4225) + } + + @Test + fun locationUpdate_withinTwoMinutes_rejectsLessAccurateFixFromOtherProvider() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 5f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + // 35m worse than the cached fix exceeds the 20m margin, and the provider differs. + deliver( + createLocation( + provider = LocationManager.NETWORK_PROVIDER, + latitude = 37.4230, + longitude = -122.0850, + accuracy = 40f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.accuracy).isEqualTo(5f) + assertThat(current?.provider).isEqualTo(LocationManager.GPS_PROVIDER) + } + + @Test + fun locationUpdate_withinTwoMinutes_rejectsSlightlyLessAccurateFixFromOtherProvider() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 10f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + // A newer fix from a different provider must be at least as accurate to replace the cache. + deliver( + createLocation( + provider = LocationManager.NETWORK_PROVIDER, + latitude = 37.4230, + longitude = -122.0850, + accuracy = 25f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.accuracy).isEqualTo(10f) + assertThat(current?.provider).isEqualTo(LocationManager.GPS_PROVIDER) + } + + @Test + fun locationUpdate_withinTwoMinutes_acceptsSlightlyLessAccurateFixFromSameProvider() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 10f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + // 15m worse from the same provider is within the 20m margin. + deliver( + createLocation( + latitude = 37.4230, + longitude = -122.0850, + accuracy = 25f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.accuracy).isEqualTo(25f) + assertThat(current?.latitude).isEqualTo(37.4230) + } + + @Test + fun locationUpdate_withinTwoMinutes_rejectsMuchLessAccurateFixFromSameProvider() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 5f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + deliver( + createLocation( + latitude = 37.4230, + longitude = -122.0850, + accuracy = 500f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(5f) + } + + @Suppress("DEPRECATION") + @Test + fun locationUpdate_rejectedAccurateFix_doesNotStopWarmup() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 5f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + + // Within the 50m threshold, but rejected by the comparator. + deliver( + createLocation( + provider = LocationManager.NETWORK_PROVIDER, + accuracy = 30f, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(5f) + } + + @Suppress("DEPRECATION") + @Test + fun locationUpdate_staleFix_isIgnoredAndDoesNotStopWarmup() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver( + createLocation( + accuracy = 10f, + elapsedRealtimeNanos = SystemClock.elapsedRealtimeNanos() - + TimeUnit.MINUTES.toNanos(31) + ) + ) + + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Test + fun getCurrentLocation_permissionRevokedWithCachedFix_returnsNull() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation()) + assertThat(locationProvider.getCurrentLocation()).isNotNull() + + shadowOf(context as Application).denyPermissions( + Manifest.permission.ACCESS_FINE_LOCATION, + Manifest.permission.ACCESS_COARSE_LOCATION + ) + + assertThat(locationProvider.getCurrentLocation()).isNull() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_cancelled_removesListeners() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + + cancelLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_cancelledBeforeStart_doesNotRegister() { + grantLocationPermissions() + + launchLocationUpdates() + cancelLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_restartedBeforeStart_registersOnce() { + grantLocationPermissions() + + launchLocationUpdates() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + assertThat(shadowLocationManager.getLocationUpdateListeners(LocationManager.GPS_PROVIDER)) + .hasSize(1) + assertThat( + shadowLocationManager.getLocationUpdateListeners(LocationManager.NETWORK_PROVIDER) + ).hasSize(1) + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_accurateFix_stopsAndRestartsAfterRefreshInterval() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + + deliver(createLocation(accuracy = 10f)) + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_warmupTimeout_stopsAndRestartsAfterRefreshInterval() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + // An 80m fix does not meet the accuracy threshold, so the warmup continues. + deliver(createLocation(provider = LocationManager.NETWORK_PROVIDER, accuracy = 80f)) + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + + ShadowLooper.idleMainLooper(60L, TimeUnit.SECONDS) + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + } + + @Suppress("DEPRECATION") + @Test + fun runLocationUpdates_cancelled_doesNotRestart() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + cancelLocationUpdates() + ShadowLooper.idleMainLooper(10L, TimeUnit.MINUTES) + + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + } +} diff --git a/settings.gradle.kts b/settings.gradle.kts index 8e451a53a..53935f396 100644 --- a/settings.gradle.kts +++ b/settings.gradle.kts @@ -80,6 +80,7 @@ include(":ui:debug") include(":ui:debug:testing") include(":core:location") include(":core:location:location-di") +include(":core:location:location-manager") include(":core:location:testing") include(":core:settings:datastore-proto") include(":core:model-proto") From 6075ce31d538b4e5a1eb7b9412efc98001851d0f Mon Sep 17 00:00:00 2001 From: temcguir Date: Thu, 1 Oct 2026 00:50:31 +0000 Subject: [PATCH 2/4] Address review feedback on LocationManagerLocationProvider - Retrieve LocationManager via ContextCompat.getSystemService and treat a missing service as location unavailable instead of crashing on construction. Context.getSystemService is documented to return null when a service does not exist. - Extract location request interval and distance into constants. - Skip unsupported providers before querying last known locations. --- .../LocationManagerLocationProvider.kt | 21 +++++++++++++------ .../LocationManagerLocationProviderTest.kt | 17 +++++++++++++++ 2 files changed, 32 insertions(+), 6 deletions(-) diff --git a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt index efdd63d72..0a440ceff 100644 --- a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt +++ b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt @@ -43,6 +43,8 @@ private const val TAG = "LocationManagerLocationProvider" private const val WARMUP_TIMEOUT_MS = 60_000L private const val ACCURACY_THRESHOLD_METERS = 50f private const val SIGNIFICANT_ACCURACY_DELTA_METERS = 20f +private const val LOCATION_UPDATE_INTERVAL_MS = 1_000L +private const val LOCATION_UPDATE_MIN_DISTANCE_METERS = 0f private val REFRESH_INTERVAL_MS = TimeUnit.MINUTES.toMillis(5) private val STALE_LOCATION_THRESHOLD_NANOS = TimeUnit.MINUTES.toNanos(30) private val SIGNIFICANT_TIME_DELTA_NANOS = TimeUnit.MINUTES.toNanos(2) @@ -62,8 +64,10 @@ private val SIGNIFICANT_TIME_DELTA_NANOS = TimeUnit.MINUTES.toNanos(2) */ class LocationManagerLocationProvider(private val context: Context) : LocationProvider { - private val locationManager = context.getSystemService(Context.LOCATION_SERVICE) - as LocationManager + // Context.getSystemService is documented to return null when a service is unavailable. Treat + // a missing LocationManager as "location not available" rather than crashing at construction. + private val locationManager: LocationManager? = + ContextCompat.getSystemService(context, LocationManager::class.java) // Written from the main looper; read from any thread by capture via getCurrentLocation(). private val cachedLocation = AtomicReference(null) @@ -107,6 +111,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr @SuppressLint("MissingPermission") private suspend fun runWarmupSession() { + val locationManager = locationManager ?: return if (!hasAnyLocationPermission()) return // Pre-seed cache with last known location if available @@ -120,9 +125,9 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr val warmupCompleted = CompletableDeferred() activeWarmupDeferred.set(warmupCompleted) - val request = LocationRequestCompat.Builder(1000L) - .setMinUpdateIntervalMillis(1000L) - .setMinUpdateDistanceMeters(0f) + val request = LocationRequestCompat.Builder(LOCATION_UPDATE_INTERVAL_MS) + .setMinUpdateIntervalMillis(LOCATION_UPDATE_INTERVAL_MS) + .setMinUpdateDistanceMeters(LOCATION_UPDATE_MIN_DISTANCE_METERS) .setQuality(LocationRequestCompat.QUALITY_HIGH_ACCURACY) .build() @@ -165,6 +170,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr @SuppressLint("MissingPermission") private fun stopHardwareUpdates() { + val locationManager = locationManager ?: return if (isUpdating.getAndSet(false)) { LocationManagerCompat.removeUpdates(locationManager, locationListener) Log.d(TAG, "Stopped location updates across all providers.") @@ -173,6 +179,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr @SuppressLint("MissingPermission") private fun getBestLastKnownLocation(): Location? { + val locationManager = locationManager ?: return null if (!isLocationAvailable()) return null var bestLocation: Location? = null @@ -191,7 +198,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr ) { add(LocationManager.FUSED_PROVIDER) } - } + }.filter { LocationManagerCompat.hasProvider(locationManager, it) } for (provider in providers) { try { val loc = locationManager.getLastKnownLocation(provider) @@ -294,6 +301,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr * even though runtime permissions remain granted. */ private fun isLocationAvailable(): Boolean { + val locationManager = locationManager ?: return false return hasAnyLocationPermission() && LocationManagerCompat.isLocationEnabled( locationManager ) @@ -315,6 +323,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr @SuppressLint("InlinedApi") private fun getActiveProviders(): List { + val locationManager = locationManager ?: return emptyList() if (!LocationManagerCompat.isLocationEnabled(locationManager)) { return emptyList() } diff --git a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt index c24fba6c6..21b944d07 100644 --- a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt +++ b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt @@ -18,6 +18,7 @@ package com.google.jetpackcamera.core.location.locationmanager import android.Manifest import android.app.Application import android.content.Context +import android.content.ContextWrapper import android.location.Location import android.location.LocationManager import android.os.SystemClock @@ -627,4 +628,20 @@ class LocationManagerLocationProviderTest { assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() } + + @Test + fun locationServiceUnavailable_doesNotCrashAndReturnsNull() { + grantLocationPermissions() + val noLocationContext = object : ContextWrapper(context) { + override fun getSystemService(name: String): Any? = + if (name == LOCATION_SERVICE) null else super.getSystemService(name) + } + + val provider = LocationManagerLocationProvider(noLocationContext) + updatesJob = CoroutineScope(Dispatchers.Main).launch { provider.runLocationUpdates() } + ShadowLooper.idleMainLooper() + + assertThat(provider.getCurrentLocation()).isNull() + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + } } From d8eff3c04fcd39dd48e2a6cdf3ed9698c54f7103 Mon Sep 17 00:00:00 2001 From: temcguir Date: Fri, 2 Oct 2026 20:38:20 +0000 Subject: [PATCH 3/4] Replace warmup naming in LocationManagerLocationProvider Rename WARMUP_TIMEOUT_MS to ACQUISITION_TIMEOUT_MS, runWarmupSession() to runUpdateSession(), and the completion deferred to accurateFixDeferred / accurateFixReceived, so the internals use the same location updates terminology as runLocationUpdates(). Test names and comments updated to match. No behavior change. --- .../LocationManagerLocationProvider.kt | 26 +++++++++---------- .../LocationManagerLocationProviderTest.kt | 10 +++---- 2 files changed, 18 insertions(+), 18 deletions(-) diff --git a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt index 0a440ceff..4408a92b1 100644 --- a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt +++ b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt @@ -40,7 +40,7 @@ import kotlinx.coroutines.isActive import kotlinx.coroutines.withTimeoutOrNull private const val TAG = "LocationManagerLocationProvider" -private const val WARMUP_TIMEOUT_MS = 60_000L +private const val ACQUISITION_TIMEOUT_MS = 60_000L private const val ACCURACY_THRESHOLD_METERS = 50f private const val SIGNIFICANT_ACCURACY_DELTA_METERS = 20f private const val LOCATION_UPDATE_INTERVAL_MS = 1_000L @@ -72,7 +72,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr // Written from the main looper; read from any thread by capture via getCurrentLocation(). private val cachedLocation = AtomicReference(null) private val isUpdating = AtomicBoolean(false) - private val activeWarmupDeferred = AtomicReference?>(null) + private val accurateFixDeferred = AtomicReference?>(null) // Interval between periodic refresh cycles (5 minutes by default, configurable for testing) internal var refreshIntervalMs: Long = REFRESH_INTERVAL_MS @@ -99,18 +99,18 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr } /** - * Runs a warmup session every [refreshIntervalMs] until cancelled. Each cycle re-checks + * Runs a location update session every [refreshIntervalMs] until cancelled. Each cycle re-checks * permissions and enabled providers, so changes made mid-session are applied on the next cycle. */ override suspend fun runLocationUpdates() = coroutineScope { while (isActive) { - runWarmupSession() + runUpdateSession() delay(refreshIntervalMs) } } @SuppressLint("MissingPermission") - private suspend fun runWarmupSession() { + private suspend fun runUpdateSession() { val locationManager = locationManager ?: return if (!hasAnyLocationPermission()) return @@ -122,8 +122,8 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr val activeProviders = getActiveProviders() if (activeProviders.isEmpty()) return - val warmupCompleted = CompletableDeferred() - activeWarmupDeferred.set(warmupCompleted) + val accurateFixReceived = CompletableDeferred() + accurateFixDeferred.set(accurateFixReceived) val request = LocationRequestCompat.Builder(LOCATION_UPDATE_INTERVAL_MS) .setMinUpdateIntervalMillis(LOCATION_UPDATE_INTERVAL_MS) @@ -151,7 +151,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr } if (registeredCount == 0) { - activeWarmupDeferred.set(null) + accurateFixDeferred.set(null) return } @@ -159,11 +159,11 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr Log.d(TAG, "Started location updates across $registeredCount providers") try { - withTimeoutOrNull(WARMUP_TIMEOUT_MS) { - warmupCompleted.await() + withTimeoutOrNull(ACQUISITION_TIMEOUT_MS) { + accurateFixReceived.await() } } finally { - activeWarmupDeferred.set(null) + accurateFixDeferred.set(null) stopHardwareUpdates() } } @@ -234,9 +234,9 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr } } - // A rejected fix leaves the cache unchanged, so it must not end the warmup session. + // A rejected fix leaves the cache unchanged, so it must not end the update session. if (accepted && location.accuracy <= ACCURACY_THRESHOLD_METERS) { - activeWarmupDeferred.get()?.complete(Unit) + accurateFixDeferred.get()?.complete(Unit) stopHardwareUpdates() } } diff --git a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt index 21b944d07..75e82baae 100644 --- a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt +++ b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt @@ -290,7 +290,7 @@ class LocationManagerLocationProviderTest { launchLocationUpdates() ShadowLooper.idleMainLooper() - // A 5m fix stops the warmup early. + // A 5m fix stops the updates early. deliver(createLocation(accuracy = 5f, elapsedRealtimeNanos = baseTimeNanos)) assertThat(locationProvider.getCurrentLocation()?.accuracy).isEqualTo(5f) @@ -476,7 +476,7 @@ class LocationManagerLocationProviderTest { @Suppress("DEPRECATION") @Test - fun locationUpdate_rejectedAccurateFix_doesNotStopWarmup() { + fun locationUpdate_rejectedAccurateFix_doesNotStopUpdates() { grantLocationPermissions() locationProvider.refreshIntervalMs = 500L val baseTimeNanos = SystemClock.elapsedRealtimeNanos() @@ -503,7 +503,7 @@ class LocationManagerLocationProviderTest { @Suppress("DEPRECATION") @Test - fun locationUpdate_staleFix_isIgnoredAndDoesNotStopWarmup() { + fun locationUpdate_staleFix_isIgnoredAndDoesNotStopUpdates() { grantLocationPermissions() launchLocationUpdates() ShadowLooper.idleMainLooper() @@ -598,14 +598,14 @@ class LocationManagerLocationProviderTest { @Suppress("DEPRECATION") @Test - fun runLocationUpdates_warmupTimeout_stopsAndRestartsAfterRefreshInterval() { + fun runLocationUpdates_acquisitionTimeout_stopsAndRestartsAfterRefreshInterval() { grantLocationPermissions() locationProvider.refreshIntervalMs = 500L launchLocationUpdates() ShadowLooper.idleMainLooper() - // An 80m fix does not meet the accuracy threshold, so the warmup continues. + // An 80m fix does not meet the accuracy threshold, so updates continue. deliver(createLocation(provider = LocationManager.NETWORK_PROVIDER, accuracy = 80f)) assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() From 3e91542e4465b0ff6977340948f114107dc96278 Mon Sep 17 00:00:00 2001 From: temcguir Date: Fri, 2 Oct 2026 22:07:07 +0000 Subject: [PATCH 4/4] Treat location fixes without accuracy as least accurate --- .../LocationManagerLocationProvider.kt | 15 +++- .../LocationManagerLocationProviderTest.kt | 71 ++++++++++++++++++- 2 files changed, 81 insertions(+), 5 deletions(-) diff --git a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt index 4408a92b1..c8547d882 100644 --- a/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt +++ b/core/location/location-manager/src/main/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProvider.kt @@ -235,7 +235,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr } // A rejected fix leaves the cache unchanged, so it must not end the update session. - if (accepted && location.accuracy <= ACCURACY_THRESHOLD_METERS) { + if (accepted && location.accuracyOrMax <= ACCURACY_THRESHOLD_METERS) { accurateFixDeferred.get()?.complete(Unit) stopHardwareUpdates() } @@ -247,7 +247,8 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr * A fix more than 2 minutes newer always wins, which prevents coordinate anchoring when * travelling. Within the 2-minute window, a fix is accepted if it is more accurate, if it is * newer and at least as accurate, or if it is newer, from the same provider, and no more than - * [SIGNIFICANT_ACCURACY_DELTA_METERS] less accurate. + * [SIGNIFICANT_ACCURACY_DELTA_METERS] less accurate. A fix that does not report accuracy + * ranks below any fix that does. * * @param newLoc Candidate fix received from a platform location provider. * @param currentLoc The currently cached fix, or `null`. @@ -265,7 +266,7 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr if (isSignificantlyOlder) return false val isNewer = timeDeltaNanos > 0 - val accuracyDelta = newLoc.accuracy - currentLoc.accuracy + val accuracyDelta = newLoc.accuracyOrMax - currentLoc.accuracyOrMax val isMoreAccurate = accuracyDelta < 0f val isLessAccurate = accuracyDelta > 0f val isSignificantlyLessAccurate = accuracyDelta > SIGNIFICANT_ACCURACY_DELTA_METERS @@ -279,6 +280,14 @@ class LocationManagerLocationProvider(private val context: Context) : LocationPr } } + /** + * The horizontal accuracy of this fix in meters, or [Float.MAX_VALUE] if the fix does not + * report one. [Location.getAccuracy] returns 0 when no accuracy is set, which would otherwise + * rank the fix as the most accurate possible. + */ + private val Location.accuracyOrMax: Float + get() = if (hasAccuracy()) accuracy else Float.MAX_VALUE + private fun isValidLocation(location: Location): Boolean { if (location.latitude.isNaN() || location.longitude.isNaN()) return false if (location.latitude.isInfinite() || location.longitude.isInfinite()) return false diff --git a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt index 75e82baae..9f19e9101 100644 --- a/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt +++ b/core/location/location-manager/src/test/java/com/google/jetpackcamera/core/location/locationmanager/LocationManagerLocationProviderTest.kt @@ -87,16 +87,17 @@ class LocationManagerLocationProviderTest { if (coarse) app.grantPermissions(Manifest.permission.ACCESS_COARSE_LOCATION) } + /** Creates a fix. A null [accuracy] leaves the fix without accuracy, so hasAccuracy() is false. */ private fun createLocation( provider: String = LocationManager.GPS_PROVIDER, latitude: Double = 37.4220, longitude: Double = -122.0841, - accuracy: Float = 10f, + accuracy: Float? = 10f, elapsedRealtimeNanos: Long = SystemClock.elapsedRealtimeNanos() ) = Location(provider).apply { this.latitude = latitude this.longitude = longitude - this.accuracy = accuracy + accuracy?.let { this.accuracy = it } this.elapsedRealtimeNanos = elapsedRealtimeNanos } @@ -520,6 +521,72 @@ class LocationManagerLocationProviderTest { assertThat(locationProvider.getCurrentLocation()).isNull() } + @Suppress("DEPRECATION") + @Test + fun locationUpdate_fixWithoutAccuracy_isCachedAndDoesNotStopUpdates() { + grantLocationPermissions() + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = null)) + + assertThat(locationProvider.getCurrentLocation()?.hasAccuracy()).isFalse() + assertThat(shadowLocationManager.locationUpdateListeners).isNotEmpty() + } + + @Test + fun locationUpdate_withinTwoMinutes_fixWithoutAccuracyDoesNotReplaceFixWithAccuracy() { + grantLocationPermissions() + locationProvider.refreshIntervalMs = 500L + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = 30f, elapsedRealtimeNanos = baseTimeNanos)) + ShadowLooper.idleMainLooper(500L, TimeUnit.MILLISECONDS) + + // Newer and from the same provider, but its accuracy is unknown. + deliver( + createLocation( + latitude = 37.4230, + longitude = -122.0850, + accuracy = null, + elapsedRealtimeNanos = baseTimeNanos + TimeUnit.SECONDS.toNanos(10) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.accuracy).isEqualTo(30f) + assertThat(current?.latitude).isEqualTo(37.4220) + } + + @Suppress("DEPRECATION") + @Test + fun locationUpdate_withinTwoMinutes_fixWithAccuracyReplacesFixWithoutAccuracy() { + grantLocationPermissions() + val baseTimeNanos = SystemClock.elapsedRealtimeNanos() + + launchLocationUpdates() + ShadowLooper.idleMainLooper() + + deliver(createLocation(accuracy = null, elapsedRealtimeNanos = baseTimeNanos)) + // Older than the cached fix and from another provider, but its accuracy is known. + deliver( + createLocation( + provider = LocationManager.NETWORK_PROVIDER, + accuracy = 40f, + elapsedRealtimeNanos = baseTimeNanos - TimeUnit.SECONDS.toNanos(10) + ) + ) + + val current = locationProvider.getCurrentLocation() + assertThat(current?.accuracy).isEqualTo(40f) + assertThat(current?.provider).isEqualTo(LocationManager.NETWORK_PROVIDER) + // 40m is within the 50m threshold, so the session ends. + assertThat(shadowLocationManager.locationUpdateListeners).isEmpty() + } + @Test fun getCurrentLocation_permissionRevokedWithCachedFix_returnsNull() { grantLocationPermissions()