Merge pull request #5636 from grzesiek2010/COLLECT-5633

Use the same location providers in every map engine + some tweaks
getodk · Jul 7, 2023 · 01d9fc7 · 01d9fc7
2 parents f2edfa5 + 7a13b67
commit 01d9fc7
Show file tree

Hide file tree

Showing 9 changed files with 64 additions and 81 deletions.
diff --git a/collect_app/src/main/java/org/odk/collect/android/application/Collect.java b/collect_app/src/main/java/org/odk/collect/android/application/Collect.java
@@ -53,6 +53,7 @@
 import org.odk.collect.geo.DaggerGeoDependencyComponent;
 import org.odk.collect.geo.GeoDependencyComponent;
 import org.odk.collect.geo.GeoDependencyComponentProvider;
+import org.odk.collect.location.LocationClient;
 import org.odk.collect.maps.layers.ReferenceLayerRepository;
 import org.odk.collect.osmdroid.DaggerOsmDroidDependencyComponent;
 import org.odk.collect.osmdroid.OsmDroidDependencyComponent;
@@ -190,6 +191,7 @@ private void setupDagger() {
         objectProvider.addSupplier(SettingsProvider.class, applicationComponent::settingsProvider);
         objectProvider.addSupplier(NetworkStateProvider.class, applicationComponent::networkStateProvider);
         objectProvider.addSupplier(ReferenceLayerRepository.class, applicationComponent::referenceLayerRepository);
+        objectProvider.addSupplier(LocationClient.class, applicationComponent::locationClient);
     }
 
     @NotNull

diff --git a/collect_app/src/main/java/org/odk/collect/android/geo/GoogleMapFragment.java b/collect_app/src/main/java/org/odk/collect/android/geo/GoogleMapFragment.java
@@ -20,11 +20,9 @@
 
 import android.annotation.SuppressLint;
 import android.content.Context;
-import android.content.Intent;
 import android.location.Location;
 import android.os.Bundle;
 import android.os.Handler;
-import android.provider.Settings;
 
 import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
@@ -49,7 +47,6 @@
 import com.google.android.gms.maps.model.PolylineOptions;
 import com.google.android.gms.maps.model.TileOverlay;
 import com.google.android.gms.maps.model.TileOverlayOptions;
-import com.google.android.material.dialog.MaterialAlertDialogBuilder;
 
 import org.odk.collect.android.R;
 import org.odk.collect.android.geo.GoogleMapConfigurator.GoogleMapTypeOption;
@@ -491,18 +488,12 @@ public boolean hasCenter() {
         lastLocationFix = fromLocation(locationClient.getLastLocation());
         Timber.i("Requesting location updates (to %s)", this);
         locationClient.requestLocationUpdates(this);
-        if (!locationClient.isLocationAvailable()) {
-            showGpsDisabledAlert();
-        }
     }
 
     @Override public void onClientStartFailure() {
-        showGpsDisabledAlert();
     }
 
     @Override public void onClientStop() {
-        Timber.i("Stopping location updates (to %s)", this);
-        locationClient.stopLocationUpdates();
     }
 
     private static @NonNull MapPoint fromLatLng(@NonNull LatLng latLng) {
@@ -703,19 +694,6 @@ private static BitmapDescriptor getBitmapDescriptor(Context context, MarkerIconD
         return BitmapDescriptorCache.getBitmapDescriptor(context, markerIconDescription);
     }
 
-    private void showGpsDisabledAlert() {
-        new MaterialAlertDialogBuilder(getActivity())
-            .setMessage(getString(R.string.gps_enable_message))
-            .setCancelable(false)
-            .setPositiveButton(getString(R.string.enable_gps),
-                (dialog, id) -> startActivityForResult(
-                    new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS), 0))
-            .setNegativeButton(getString(R.string.cancel),
-                (dialog, id) -> dialog.cancel())
-            .create()
-            .show();
-    }
-
     private void onConfigChanged(Bundle config) {
         mapType = config.getInt(KEY_MAP_TYPE, GoogleMap.MAP_TYPE_NORMAL);
         referenceLayerFile = MapFragmentReferenceLayerUtils.getReferenceLayerFile(config, referenceLayerRepository);

diff --git a/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java b/geo/src/main/java/org/odk/collect/geo/geopoint/GeoPointMapActivity.java
@@ -330,25 +330,22 @@ public void onLocationChanged(MapPoint point) {
             placeMarkerButton.setEnabled(true);
         }
 
-        MapPoint previousLocation = this.location;
         this.location = point;
 
         if (point != null) {
-            if (previousLocation != null) {
-                enableZoomButton(true);
-
-                if (!captureLocation && !setClear) {
-                    placeMarker(point);
-                    placeMarkerButton.setEnabled(true);
-                }
+            enableZoomButton(true);
 
-                if (!foundFirstLocation) {
-                    map.zoomToPoint(map.getGpsLocation(), true);
-                    foundFirstLocation = true;
-                }
+            if (!captureLocation && !setClear) {
+                placeMarker(point);
+                placeMarkerButton.setEnabled(true);
+            }
 
-                locationStatus.setText(formatLocationStatus(map.getLocationProvider(), point.accuracy));
+            if (!foundFirstLocation) {
+                map.zoomToPoint(map.getGpsLocation(), true);
+                foundFirstLocation = true;
             }
+
+            locationStatus.setText(formatLocationStatus(map.getLocationProvider(), point.accuracy));
         }
     }
 

diff --git a/geo/src/test/java/org/odk/collect/geo/geopoint/GeoPointMapActivityTest.java b/geo/src/test/java/org/odk/collect/geo/geopoint/GeoPointMapActivityTest.java
@@ -43,7 +43,7 @@ public class GeoPointMapActivityTest {
     public ActivityScenarioLauncherRule launcherRule = new ActivityScenarioLauncherRule();
 
     @Before
-    public void setUp() throws Exception {
+    public void setUp() {
         ShadowApplication shadowApplication = shadowOf(ApplicationProvider.<Application>getApplicationContext());
         shadowApplication.grantPermissions("android.permission.ACCESS_FINE_LOCATION");
         shadowApplication.grantPermissions("android.permission.ACCESS_COARSE_LOCATION");
@@ -68,28 +68,37 @@ public ReferenceLayerSettingsNavigator providesReferenceLayerSettingsNavigator()
     }
 
     @Test
-    public void shouldReturnPointFromSecondLocationFix() {
+    public void whenLocationNotSetShouldDisplayPleaseWaitMessage() {
         ActivityScenario<GeoPointMapActivity> scenario = launcherRule.launchForResult(GeoPointMapActivity.class);
         mapFragment.ready();
 
-        // The very first fix is ignored.
+        scenario.onActivity(activity -> assertEquals(activity.getString(R.string.please_wait_long), activity.getLocationStatus()));
+    }
+
+    @Test
+    public void whenLocationSetShouldDisplayStatusMessage() {
+        ActivityScenario<GeoPointMapActivity> scenario = launcherRule.launchForResult(GeoPointMapActivity.class);
+        mapFragment.ready();
         mapFragment.setLocationProvider("GPS");
         mapFragment.setLocation(new MapPoint(1, 2, 3, 4f));
-        scenario.onActivity(activity -> {
-            assertEquals(activity.getString(R.string.please_wait_long), activity.getLocationStatus());
-        });
 
+        scenario.onActivity(activity -> assertEquals(activity.formatLocationStatus("gps", 4f), activity.getLocationStatus()));
+    }
 
-        // The second fix changes the status message.
+    @Test
+    public void shouldReturnPointFromLastLocationFix() {
+        ActivityScenario<GeoPointMapActivity> scenario = launcherRule.launchForResult(GeoPointMapActivity.class);
+        mapFragment.ready();
+        mapFragment.setLocationProvider("GPS");
+
+        // First location
+        mapFragment.setLocation(new MapPoint(1, 2, 3, 4f));
+
+        // Second location
         mapFragment.setLocation(new MapPoint(5, 6, 7, 8f));
-        scenario.onActivity(activity -> {
-            assertEquals(activity.formatLocationStatus("gps", 8f), activity.getLocationStatus());
-        });
 
         // When the user clicks the "Save" button, the fix location should be returned.
-        scenario.onActivity(activity -> {
-            activity.findViewById(R.id.accept_location).performClick();
-        });
+        scenario.onActivity(activity -> activity.findViewById(R.id.accept_location).performClick());
 
         assertThat(scenario.getResult().getResultCode(), is(RESULT_OK));
         scenario.onActivity(activity -> {

diff --git a/location/build.gradle b/location/build.gradle
@@ -45,6 +45,7 @@ dependencies {
     implementation project(':androidshared')
     implementation project(':icons')
     implementation project(':strings')
+    implementation project(':analytics')
     implementation Dependencies.kotlin_stdlib
     implementation Dependencies.androidx_core_ktx
     implementation Dependencies.play_services_location

diff --git a/location/src/main/java/org/odk/collect/location/GoogleFusedLocationClient.java b/location/src/main/java/org/odk/collect/location/GoogleFusedLocationClient.java
@@ -19,6 +19,8 @@
 import com.google.android.gms.location.LocationRequest;
 import com.google.android.gms.location.LocationServices;
 
+import org.odk.collect.analytics.Analytics;
+
 import timber.log.Timber;
 
 /**
@@ -196,6 +198,7 @@ public void onConnectionSuspended(int cause) {
 
     @Override
     public void onConnectionFailed(@NonNull ConnectionResult connectionResult) {
+        Analytics.log("GoogleFusedLocationClientConnectionFailed");
         if (getListener() != null) {
             getListener().onClientStartFailure();
         }

diff --git a/mapbox/src/main/java/org/odk/collect/mapbox/MapboxMapFragment.kt b/mapbox/src/main/java/org/odk/collect/mapbox/MapboxMapFragment.kt
@@ -12,8 +12,6 @@ import androidx.fragment.app.Fragment
 import androidx.lifecycle.lifecycleScope
 import androidx.startup.AppInitializer
 import com.google.android.gms.location.LocationListener
-import com.mapbox.android.core.location.LocationEngineProvider
-import com.mapbox.android.core.location.LocationEngineRequest
 import com.mapbox.geojson.Point
 import com.mapbox.maps.EdgeInsets
 import com.mapbox.maps.MapView
@@ -50,6 +48,8 @@ import com.mapbox.maps.plugin.scalebar.scalebar
 import kotlinx.coroutines.delay
 import kotlinx.coroutines.launch
 import org.odk.collect.androidshared.utils.ScreenUtils
+import org.odk.collect.location.LocationClient
+import org.odk.collect.location.LocationClient.LocationClientListener
 import org.odk.collect.maps.MapFragment
 import org.odk.collect.maps.MapFragment.ErrorListener
 import org.odk.collect.maps.MapFragment.FeatureListener
@@ -74,7 +74,8 @@ class MapboxMapFragment :
     MapFragment,
     OnMapClickListener,
     OnMapLongClickListener,
-    LocationListener {
+    LocationListener,
+    LocationClientListener {
 
     private lateinit var mapView: MapView
     private lateinit var mapboxMap: MapboxMap
@@ -119,6 +120,10 @@ class MapboxMapFragment :
         (requireActivity().applicationContext as ObjectProviderHost).getObjectProvider().provide(ReferenceLayerRepository::class.java)
     }
 
+    private val locationClient: LocationClient by lazy {
+        (requireActivity().applicationContext as ObjectProviderHost).getObjectProvider().provide(LocationClient::class.java)
+    }
+
     override fun init(readyListener: ReadyListener?, errorListener: ErrorListener?) {
         mapReadyListener = readyListener
 
@@ -490,22 +495,16 @@ class MapboxMapFragment :
 
     @SuppressWarnings("MissingPermission") // permission checks for location services are handled in widgets
     private fun enableLocationUpdates(enabled: Boolean) {
-        val engine = LocationEngineProvider.getBestLocationEngine(requireContext())
+        locationClient.setListener(this)
+
         if (enabled) {
-            Timber.i("Requesting location updates from %s (to %s)", engine, this)
-            engine.requestLocationUpdates(
-                LocationEngineRequest.Builder(1000)
-                    .setPriority(LocationEngineRequest.PRIORITY_HIGH_ACCURACY)
-                    .setMaxWaitTime(5000)
-                    .build(),
-                locationCallback,
-                null
-            )
-            engine.getLastLocation(locationCallback)
+            Timber.i("Starting LocationClient %s (for MapFragment %s)", locationClient, this)
+            locationClient.start()
         } else {
-            Timber.i("Stopping location updates from %s (to %s)", engine, this)
-            engine.removeLocationUpdates(locationCallback)
+            Timber.i("Stopping LocationClient %s (for MapFragment %s)", locationClient, this)
+            locationClient.stop()
         }
+
         mapView.location.enabled = enabled
     }
 
@@ -635,6 +634,17 @@ class MapboxMapFragment :
         }
     }
 
+    override fun onClientStart() {
+        Timber.i("Requesting location updates (to %s)", this)
+        locationClient.requestLocationUpdates(this)
+    }
+
+    override fun onClientStartFailure() {
+    }
+
+    override fun onClientStop() {
+    }
+
     companion object {
         const val KEY_STYLE_URL = "STYLE_URL"
     }

diff --git a/osmdroid/build.gradle b/osmdroid/build.gradle
@@ -52,7 +52,6 @@ dependencies {
     implementation Dependencies.osmdroid
     implementation Dependencies.androidx_fragment_ktx
     implementation Dependencies.androidx_preference_ktx
-    implementation Dependencies.android_material
     implementation Dependencies.timber
     implementation Dependencies.play_services_location
     implementation Dependencies.dagger

diff --git a/osmdroid/src/main/java/org/odk/collect/osmdroid/OsmDroidMapFragment.java b/osmdroid/src/main/java/org/odk/collect/osmdroid/OsmDroidMapFragment.java
@@ -31,7 +31,6 @@
 import android.location.Location;
 import android.os.Bundle;
 import android.os.Handler;
-import android.provider.Settings;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -43,7 +42,6 @@
 import androidx.fragment.app.Fragment;
 
 import com.google.android.gms.location.LocationListener;
-import com.google.android.material.dialog.MaterialAlertDialogBuilder;
 
 import org.odk.collect.androidshared.system.ContextUtils;
 import org.odk.collect.location.LocationClient;
@@ -477,7 +475,6 @@ public void onClientStart() {
 
     @Override
     public void onClientStartFailure() {
-        showGpsDisabledAlert();
     }
 
     @Override
@@ -568,19 +565,6 @@ private void loadReferenceOverlay() {
         map.invalidate();
     }
 
-    private void showGpsDisabledAlert() {
-        new MaterialAlertDialogBuilder(getContext())
-                .setMessage(getString(R.string.gps_enable_message))
-                .setCancelable(false)
-                .setPositiveButton(getString(R.string.enable_gps),
-                        (dialog, id) -> startActivityForResult(
-                                new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS), 0))
-                .setNegativeButton(getString(R.string.cancel),
-                        (dialog, id) -> dialog.cancel())
-                .create()
-                .show();
-    }
-
     /**
      * Adds a listener that keeps track of the map center, and another
      * listener that restores the map center when the MapView's layout changes.