Add AndroidLocationProvider

app/build.gradle.kts

@@ -207,7 +207,6 @@ dependencies {
     implementation("com.github.QuadFlask:colorpicker:0.0.15")
     implementation("com.github.openid:AppAuth-Android:0.8.0")
 
-    genericImplementation("com.mapbox.mapboxsdk:mapbox-android-core:3.1.1")
     genericImplementation("org.osmdroid:osmdroid-android:6.1.10@aar")
 
     googleplayImplementation("com.google.firebase:firebase-crashlytics:${Versions.crashlytics}")

app/src/androidTest/java/org/tasks/injection/TestModule.kt

@@ -6,18 +6,20 @@ import com.todoroo.astrid.dao.Database
 import dagger.Module
 import dagger.Provides
 import dagger.hilt.InstallIn
-import dagger.hilt.android.components.ApplicationComponent
 import dagger.hilt.android.qualifiers.ApplicationContext
+import dagger.hilt.components.SingletonComponent
 import org.mockito.Mockito.mock
 import org.tasks.TestUtilities
 import org.tasks.jobs.WorkManager
+import org.tasks.location.LocationManager
+import org.tasks.location.MockLocationManager
 import org.tasks.preferences.PermissionChecker
 import org.tasks.preferences.PermissivePermissionChecker
 import org.tasks.preferences.Preferences
 import javax.inject.Singleton
 
 @Module
-@InstallIn(ApplicationComponent::class)
+@InstallIn(SingletonComponent::class)
 class TestModule {
     @Provides
     @Singleton
@@ -37,6 +39,13 @@ class TestModule {
         return TestUtilities.newPreferences(context)
     }
 
+    @Provides
+    @Singleton
+    fun getMockLocationManager(): MockLocationManager = MockLocationManager()
+
+    @Provides
+    fun getLocationManager(locationManager: MockLocationManager): LocationManager = locationManager
+
     @Provides
     fun getWorkManager(): WorkManager = mock(WorkManager::class.java)
 }

app/src/androidTest/java/org/tasks/location/MockLocationManager.kt

@@ -0,0 +1,16 @@
+package org.tasks.location
+
+import android.location.Location
+
+class MockLocationManager : LocationManager {
+    private val mockLocations = ArrayList<Location>()
+
+    fun addLocations(vararg locations: Location) {
+        mockLocations.addAll(locations)
+    }
+
+    fun clearLocations() = mockLocations.clear()
+
+    override val lastKnownLocations: List<Location>
+        get() = mockLocations
+}

app/src/androidTestGeneric/java/org/tasks/location/AndroidLocationProviderTest.kt

@@ -0,0 +1,76 @@
+package org.tasks.location
+
+import android.location.Location
+import android.location.LocationManager.GPS_PROVIDER
+import android.location.LocationManager.NETWORK_PROVIDER
+import dagger.hilt.android.testing.HiltAndroidTest
+import dagger.hilt.android.testing.UninstallModules
+import kotlinx.coroutines.runBlocking
+import org.junit.Assert.assertEquals
+import org.junit.Assert.assertNull
+import org.junit.Test
+import org.tasks.injection.InjectingTestCase
+import org.tasks.injection.ProductionModule
+import org.tasks.time.DateTime
+import javax.inject.Inject
+
+@UninstallModules(ProductionModule::class)
+@HiltAndroidTest
+class AndroidLocationProviderTest : InjectingTestCase() {
+    @Inject lateinit var provider: AndroidLocationProvider
+    @Inject lateinit var locationManager: MockLocationManager
+
+    @Test
+    fun sortByAccuracy() = runBlocking {
+        newLocation(NETWORK_PROVIDER, 45.0, 46.0, 50f, DateTime(2021, 2, 4, 13, 35, 45, 121))
+        newLocation(GPS_PROVIDER, 45.1, 46.1, 30f, DateTime(2021, 2, 4, 13, 33, 45, 121))
+
+        assertEquals(MapPosition(45.1, 46.1), provider.currentLocation())
+    }
+
+    @Test
+    fun sortWithStaleLocation() = runBlocking {
+        newLocation(GPS_PROVIDER, 45.1, 46.1, 30f, DateTime(2021, 2, 4, 13, 33, 44, 121))
+        newLocation(NETWORK_PROVIDER, 45.0, 46.0, 50f, DateTime(2021, 2, 4, 13, 35, 45, 121))
+
+        assertEquals(MapPosition(45.0, 46.0), provider.currentLocation())
+    }
+
+    @Test
+    fun useNewerUpdateWhenAccuracySame() = runBlocking {
+        newLocation(GPS_PROVIDER, 45.1, 46.1, 50f, DateTime(2021, 2, 4, 13, 35, 45, 100))
+        newLocation(NETWORK_PROVIDER, 45.0, 46.0, 50f, DateTime(2021, 2, 4, 13, 35, 45, 121))
+
+        assertEquals(MapPosition(45.0, 46.0), provider.currentLocation())
+    }
+
+    @Test
+    fun returnCachedLocation() = runBlocking {
+        newLocation(GPS_PROVIDER, 45.1, 46.1, 50f, DateTime(2021, 2, 4, 13, 35, 45, 100))
+
+        provider.currentLocation()
+
+        locationManager.clearLocations()
+
+        assertEquals(MapPosition(45.1, 46.1), provider.currentLocation())
+    }
+
+    @Test
+    fun nullWhenNoPosition() = runBlocking {
+        assertNull(provider.currentLocation())
+    }
+
+    private fun newLocation(
+            provider: String,
+            latitude: Double,
+            longitude: Double,
+            accuracy: Float,
+            time: DateTime) {
+        locationManager.addLocations(Location(provider).apply {
+            this.latitude = latitude
+            this.longitude = longitude
+            this.accuracy = accuracy
+            this.time = time.millis
+        })
+    }
+}

app/src/generic/java/org/tasks/injection/LocationModule.kt

@@ -22,9 +22,7 @@ class LocationModule {
 
     @Provides
     @ActivityScoped
-    fun getLocationProvider(@ApplicationContext context: Context): LocationProvider {
+    fun getLocationProvider(provider: AndroidLocationProvider): LocationProvider = provider
-        return MapboxLocationProvider(context)
-    }
 
     @Provides
     @ActivityScoped

app/src/generic/java/org/tasks/location/AndroidLocationProvider.kt

@@ -0,0 +1,47 @@
+package org.tasks.location
+
+import android.location.Location
+import org.tasks.preferences.PermissionChecker
+import java.util.concurrent.TimeUnit
+import javax.inject.Inject
+
+class AndroidLocationProvider @Inject constructor(
+        internal val locationManager: LocationManager,
+        private val permissionChecker: PermissionChecker,
+) : LocationProvider {
+
+    private var cached: Location? = null
+
+    override suspend fun currentLocation() = if (permissionChecker.canAccessForegroundLocation()) {
+        locationManager
+                .lastKnownLocations
+                .plus(cached)
+                .filterNotNull()
+                .sortedWith(COMPARATOR)
+                .firstOrNull()
+                ?.let {
+                    cached = it
+                    MapPosition(it.latitude, it.longitude)
+                }
+    } else {
+        null
+    }
+
+    companion object {
+        private val TWO_MINUTES = TimeUnit.MINUTES.toMillis(2)
+
+        internal val COMPARATOR = Comparator<Location> { l1, l2 ->
+            val timeDelta = l1.time - l2.time
+            val accuracyDelta = l1.accuracy - l2.accuracy
+            when {
+                timeDelta > TWO_MINUTES -> -1
+                timeDelta < -TWO_MINUTES -> 1
+                accuracyDelta < 0 -> -1
+                accuracyDelta > 0 -> 1
+                timeDelta > 0 -> -1
+                timeDelta < 0 -> 1
+                else -> 0
+            }
+        }
+    }
+}

app/src/generic/java/org/tasks/location/MapboxLocationProvider.kt

@@ -1,30 +0,0 @@
-package org.tasks.location
-
-import android.annotation.SuppressLint
-import android.content.Context
-import com.mapbox.android.core.location.LocationEngineCallback
-import com.mapbox.android.core.location.LocationEngineProvider
-import com.mapbox.android.core.location.LocationEngineResult
-import kotlinx.coroutines.Dispatchers
-import kotlinx.coroutines.withContext
-import kotlin.coroutines.suspendCoroutine
-
-class MapboxLocationProvider(private val context: Context) : LocationProvider {
-    @SuppressLint("MissingPermission")
-    override suspend fun currentLocation(): MapPosition = withContext(Dispatchers.IO) {
-        suspendCoroutine { cont ->
-            LocationEngineProvider.getBestLocationEngine(context)
-                    .getLastLocation(
-                            object : LocationEngineCallback<LocationEngineResult> {
-                                override fun onSuccess(result: LocationEngineResult) {
-                                    val location = result.lastLocation!!
-                                    cont.resumeWith(Result.success(MapPosition(location.latitude, location.longitude)))
-                                }
-
-                                override fun onFailure(exception: Exception) {
-                                    cont.resumeWith(Result.failure(exception))
-                                }
-                            })
-        }
-    }
-}

app/src/main/java/org/tasks/injection/ProductionModule.kt

@@ -16,6 +16,8 @@ import org.tasks.data.OpenTaskDao
 import org.tasks.db.Migrations
 import org.tasks.jobs.WorkManager
 import org.tasks.jobs.WorkManagerImpl
+import org.tasks.location.AndroidLocationManager
+import org.tasks.location.LocationManager
 import org.tasks.preferences.Preferences
 import javax.inject.Singleton
 
@@ -36,6 +38,9 @@ internal class ProductionModule {
     @Provides
     fun getPreferences(@ApplicationContext context: Context): Preferences = Preferences(context)
 
+    @Provides
+    fun locationManager(locationManager: AndroidLocationManager): LocationManager = locationManager
+
     @Provides
     @Singleton
     fun getWorkManager(

app/src/main/java/org/tasks/location/AndroidLocationManager.kt

@@ -0,0 +1,32 @@
+package org.tasks.location
+
+import android.annotation.SuppressLint
+import android.content.Context
+import android.location.Location
+import dagger.hilt.android.qualifiers.ApplicationContext
+import timber.log.Timber
+import javax.inject.Inject
+
+class AndroidLocationManager @Inject constructor(
+        @ApplicationContext context: Context,
+) : LocationManager {
+
+    private val locationManager =
+            context.getSystemService(Context.LOCATION_SERVICE) as android.location.LocationManager
+
+    override val lastKnownLocations: List<Location>
+        get() = locationManager.allProviders.mapNotNull {
+            locationManager.getLastKnownLocationOrNull(it)
+        }
+
+    companion object {
+        @SuppressLint("MissingPermission")
+        private fun android.location.LocationManager.getLastKnownLocationOrNull(provider: String) =
+                try {
+                    getLastKnownLocation(provider)
+                } catch (e: Exception) {
+                    Timber.e(e)
+                    null
+                }
+    }
+}

app/src/main/java/org/tasks/location/LocationManager.kt

@@ -0,0 +1,7 @@
+package org.tasks.location
+
+import android.location.Location
+
+interface LocationManager {
+    val lastKnownLocations: List<Location>
+}

app/src/main/java/org/tasks/location/LocationPickerActivity.kt

@@ -254,7 +254,7 @@ class LocationPickerActivity : InjectingAppCompatActivity(), Toolbar.OnMenuItemC
         }
         lifecycleScope.launch {
             try {
-                map.movePosition(locationProvider.currentLocation(), animate)
+                locationProvider.currentLocation()?.let { map.movePosition(it, animate) }
             } catch (e: Exception) {
                 toaster.longToast(e.message)
             }

app/src/main/java/org/tasks/location/LocationProvider.kt

@@ -1,5 +1,5 @@
 package org.tasks.location
 
 interface LocationProvider {
-    suspend fun currentLocation(): MapPosition
+    suspend fun currentLocation(): MapPosition?
 }

deps_fdroid.txt

@@ -15,7 +15,6 @@
 ++--- androidx.databinding:databinding-adapters:4.1.2
 +|    +--- androidx.databinding:databinding-common:4.1.2
 +|    \--- androidx.databinding:databinding-runtime:4.1.2 (*)
-++--- com.mapbox.mapboxsdk:mapbox-android-core:3.1.1
 ++--- org.osmdroid:osmdroid-android:6.1.10
 ++--- com.gitlab.bitfireAT:dav4jvm:2.1.1
 +|    +--- org.jetbrains.kotlin:kotlin-stdlib:1.3.72 -> 1.4.30