From eb5bf01fca7a17d3b4fbbe14fa9552edb86aab93 Mon Sep 17 00:00:00 2001 From: Roman Tsisyk Date: Sat, 12 Feb 2022 16:28:20 +0300 Subject: [PATCH] [android] Simplify location code - Remove unnecessary abstractions and duplicated code - Fix "Ask every time" location for all cases - Fix "Continue detecting your current location" - "No" - Improve error handling for both providers - Fix logical problems with isActive()/start()/stop() - Remove dangling @Nullable members from LocationProviders - Remove circular dependency between LocationProvider and LocationHelper - Remove annoying stack trace from the log Signed-off-by: Roman Tsisyk --- .../location/LocationProviderFactory.java | 4 +- .../location/GoogleFusedLocationProvider.java | 71 +++--- .../location/LocationProviderFactory.java | 12 +- .../maps/DownloadResourcesLegacyActivity.java | 2 +- .../src/com/mapswithme/maps/MwmActivity.java | 9 +- .../maps/location/AndroidNativeProvider.java | 142 +++++------ .../maps/location/BaseLocationListener.java | 68 ------ .../maps/location/BaseLocationProvider.java | 39 ++-- .../location/DefaultLocationFixChecker.java | 53 ----- .../location/FusedLocationFixChecker.java | 23 -- .../maps/location/LocationFixChecker.java | 11 - .../maps/location/LocationHelper.java | 221 ++++++------------ ...LocationPermissionNotGrantedException.java | 11 - .../maps/search/SearchFragment.java | 2 +- .../widget/placepage/DirectionFragment.java | 2 +- .../com/mapswithme/util/LocationUtils.java | 74 +++--- 16 files changed, 226 insertions(+), 518 deletions(-) delete mode 100644 android/src/com/mapswithme/maps/location/BaseLocationListener.java delete mode 100644 android/src/com/mapswithme/maps/location/DefaultLocationFixChecker.java delete mode 100644 android/src/com/mapswithme/maps/location/FusedLocationFixChecker.java delete mode 100644 android/src/com/mapswithme/maps/location/LocationFixChecker.java delete mode 100644 android/src/com/mapswithme/maps/location/LocationPermissionNotGrantedException.java diff --git a/android/flavors/gms-disabled/com/mapswithme/maps/location/LocationProviderFactory.java b/android/flavors/gms-disabled/com/mapswithme/maps/location/LocationProviderFactory.java index 4bdc46f9f0..f761a4a087 100644 --- a/android/flavors/gms-disabled/com/mapswithme/maps/location/LocationProviderFactory.java +++ b/android/flavors/gms-disabled/com/mapswithme/maps/location/LocationProviderFactory.java @@ -11,8 +11,8 @@ public class LocationProviderFactory return false; } - public static BaseLocationProvider getProvider(@NonNull Context context) + public static BaseLocationProvider getProvider(@NonNull Context context, @NonNull BaseLocationProvider.Listener listener) { - return new AndroidNativeProvider(new DefaultLocationFixChecker(), context); + return new AndroidNativeProvider(context, listener); } } diff --git a/android/flavors/gms-enabled/com/mapswithme/maps/location/GoogleFusedLocationProvider.java b/android/flavors/gms-enabled/com/mapswithme/maps/location/GoogleFusedLocationProvider.java index 3eed438c18..1d80541812 100644 --- a/android/flavors/gms-enabled/com/mapswithme/maps/location/GoogleFusedLocationProvider.java +++ b/android/flavors/gms-enabled/com/mapswithme/maps/location/GoogleFusedLocationProvider.java @@ -1,10 +1,14 @@ package com.mapswithme.maps.location; +import static com.mapswithme.maps.location.LocationHelper.ERROR_GPS_OFF; +import static com.mapswithme.maps.location.LocationHelper.ERROR_NOT_SUPPORTED; + import android.content.Context; import android.location.Location; import android.os.Looper; import androidx.annotation.NonNull; +import androidx.annotation.Nullable; import com.google.android.gms.location.FusedLocationProviderClient; import com.google.android.gms.location.LocationAvailability; @@ -18,6 +22,7 @@ import com.google.android.gms.location.SettingsClient; class GoogleFusedLocationProvider extends BaseLocationProvider { private final static String TAG = GoogleFusedLocationProvider.class.getSimpleName(); + @NonNull private final FusedLocationProviderClient mFusedLocationClient; @NonNull @@ -29,27 +34,30 @@ class GoogleFusedLocationProvider extends BaseLocationProvider public void onLocationResult(@NonNull LocationResult result) { final Location location = result.getLastLocation(); - LOGGER.d(TAG, "onLocationResult, location = " + location); + // Documentation is inconsistent with the code: "returns null if no locations are available". + // https://developers.google.com/android/reference/com/google/android/gms/location/LocationResult#getLastLocation() + //noinspection ConstantConditions if (location == null) return; - onLocationChanged(location); + mListener.onLocationChanged(location); } @Override public void onLocationAvailability(@NonNull LocationAvailability availability) { - LOGGER.d(TAG, "Location is " + (availability.isLocationAvailable() ? "available" : "unavailable")); - setActive(availability.isLocationAvailable()); + if (!availability.isLocationAvailable()) + mListener.onLocationError(ERROR_GPS_OFF); } } - @NonNull - private final GoogleLocationCallback mCallback; + @Nullable + private final GoogleLocationCallback mCallback = new GoogleLocationCallback(); - GoogleFusedLocationProvider(@NonNull LocationFixChecker locationFixChecker, @NonNull Context context) + private boolean mActive = false; + + GoogleFusedLocationProvider(@NonNull Context context, @NonNull BaseLocationProvider.Listener listener) { - super(locationFixChecker); - mCallback = new GoogleLocationCallback(); + super(listener); mFusedLocationClient = LocationServices.getFusedLocationProviderClient(context); mSettingsClient = LocationServices.getSettingsClient(context); } @@ -57,17 +65,15 @@ class GoogleFusedLocationProvider extends BaseLocationProvider @SuppressWarnings("MissingPermission") // A permission is checked externally @Override - protected void start() + public void start(long interval) { - if (isActive()) - return; - - LOGGER.d(TAG, "Starting"); - setActive(true); + LOGGER.d(TAG, "start()"); + if (mActive) + throw new IllegalStateException("Already subscribed"); + mActive = true; final LocationRequest locationRequest = LocationRequest.create(); locationRequest.setPriority(LocationRequest.PRIORITY_HIGH_ACCURACY); - long interval = LocationHelper.INSTANCE.getInterval(); locationRequest.setInterval(interval); LOGGER.d(TAG, "Request Google fused provider to provide locations at this interval = " + interval + " ms"); @@ -80,12 +86,9 @@ class GoogleFusedLocationProvider extends BaseLocationProvider mSettingsClient.checkLocationSettings(locationSettingsRequest).addOnSuccessListener(locationSettingsResponse -> { LOGGER.d(TAG, "Service is available"); mFusedLocationClient.requestLocationUpdates(locationRequest, mCallback, Looper.myLooper()); - LocationHelper.INSTANCE.startSensors(); }).addOnFailureListener(e -> { - setActive(false); LOGGER.e(TAG, "Service is not available: " + e); - LocationHelper.INSTANCE.initNativeProvider(); - LocationHelper.INSTANCE.start(); + mListener.onLocationError(ERROR_NOT_SUPPORTED); }); // onLocationResult() may not always be called regularly, however the device location is known. @@ -93,37 +96,15 @@ class GoogleFusedLocationProvider extends BaseLocationProvider LOGGER.d(TAG, "onLastLocation, location = " + location); if (location == null) return; - GoogleFusedLocationProvider.this.onLocationChanged(location); + mListener.onLocationChanged(location); }); } @Override protected void stop() { - LOGGER.d(TAG, "Stopping"); + LOGGER.d(TAG, "stop()"); mFusedLocationClient.removeLocationUpdates(mCallback); - - setActive(false); - } - - private void onLocationChanged(@NonNull Location location) - { - if (!mLocationFixChecker.isAccuracySatisfied(location)) - return; - - if (mLocationFixChecker.isLocationBetterThanLast(location)) - { - LocationHelper.INSTANCE.onLocationUpdated(location); - LocationHelper.INSTANCE.notifyLocationUpdated(); - } - else - { - Location last = LocationHelper.INSTANCE.getSavedLocation(); - if (last != null) - { - LOGGER.d(TAG, "The new location from '" + location.getProvider() - + "' is worse than the last one from '" + last.getProvider() + "'"); - } - } + mActive = false; } } diff --git a/android/flavors/gms-enabled/com/mapswithme/maps/location/LocationProviderFactory.java b/android/flavors/gms-enabled/com/mapswithme/maps/location/LocationProviderFactory.java index bea5fcd6d2..dbd5508039 100644 --- a/android/flavors/gms-enabled/com/mapswithme/maps/location/LocationProviderFactory.java +++ b/android/flavors/gms-enabled/com/mapswithme/maps/location/LocationProviderFactory.java @@ -2,14 +2,14 @@ package com.mapswithme.maps.location; import android.content.Context; +import androidx.annotation.NonNull; + import com.google.android.gms.common.ConnectionResult; import com.google.android.gms.common.GoogleApiAvailability; import com.mapswithme.util.Config; import com.mapswithme.util.log.Logger; import com.mapswithme.util.log.LoggerFactory; -import androidx.annotation.NonNull; - public class LocationProviderFactory { private final static String TAG = LocationProviderFactory.class.getSimpleName(); @@ -20,17 +20,17 @@ public class LocationProviderFactory return GoogleApiAvailability.getInstance().isGooglePlayServicesAvailable(context) == ConnectionResult.SUCCESS; } - public static BaseLocationProvider getProvider(@NonNull Context context) + public static BaseLocationProvider getProvider(@NonNull Context context, @NonNull BaseLocationProvider.Listener listener) { if (isGoogleLocationAvailable(context) && Config.useGoogleServices()) { - mLogger.d(TAG, "Use fused provider."); - return new GoogleFusedLocationProvider(new FusedLocationFixChecker(), context); + mLogger.d(TAG, "Use google provider."); + return new GoogleFusedLocationProvider(context, listener); } else { mLogger.d(TAG, "Use native provider"); - return new AndroidNativeProvider(new DefaultLocationFixChecker(), context); + return new AndroidNativeProvider(context, listener); } } } diff --git a/android/src/com/mapswithme/maps/DownloadResourcesLegacyActivity.java b/android/src/com/mapswithme/maps/DownloadResourcesLegacyActivity.java index 1495789459..ea4339bf07 100644 --- a/android/src/com/mapswithme/maps/DownloadResourcesLegacyActivity.java +++ b/android/src/com/mapswithme/maps/DownloadResourcesLegacyActivity.java @@ -235,7 +235,7 @@ public class DownloadResourcesLegacyActivity extends BaseMwmFragmentActivity imp { super.onResume(); if (!isFinishing()) - LocationHelper.INSTANCE.addListener(mLocationListener, true); + LocationHelper.INSTANCE.addListener(mLocationListener); } @Override diff --git a/android/src/com/mapswithme/maps/MwmActivity.java b/android/src/com/mapswithme/maps/MwmActivity.java index 98841b3606..1f0d9e0184 100644 --- a/android/src/com/mapswithme/maps/MwmActivity.java +++ b/android/src/com/mapswithme/maps/MwmActivity.java @@ -1848,8 +1848,14 @@ public class MwmActivity extends BaseMwmFragmentActivity } @Override - public void onLocationError() + public void onLocationError(int errorCode) { + if (errorCode == LocationHelper.ERROR_DENIED) + { + PermissionsUtils.requestLocationPermission(MwmActivity.this, REQ_CODE_LOCATION_PERMISSION); + return; + } + if (mLocationErrorDialogAnnoying || (mLocationErrorDialog != null && mLocationErrorDialog.isShowing())) return; @@ -1913,6 +1919,7 @@ public class MwmActivity extends BaseMwmFragmentActivity DialogInterface.OnClickListener stopClickListener = (dialog, which) -> { LocationHelper.INSTANCE.setStopLocationUpdateByUser(true); + LocationHelper.INSTANCE.stop(); }; DialogInterface.OnClickListener continueClickListener = (dialog, which) -> diff --git a/android/src/com/mapswithme/maps/location/AndroidNativeProvider.java b/android/src/com/mapswithme/maps/location/AndroidNativeProvider.java index 0a109dc893..cc1cd23e79 100644 --- a/android/src/com/mapswithme/maps/location/AndroidNativeProvider.java +++ b/android/src/com/mapswithme/maps/location/AndroidNativeProvider.java @@ -6,120 +6,96 @@ import android.location.LocationListener; import android.location.LocationManager; import androidx.annotation.NonNull; -import androidx.annotation.Nullable; import com.mapswithme.maps.MwmApplication; -import java.util.ArrayList; +import org.jetbrains.annotations.NotNull; + import java.util.List; -import java.util.ListIterator; -import java.util.Objects; class AndroidNativeProvider extends BaseLocationProvider { - private final static String TAG = AndroidNativeProvider.class.getSimpleName(); + protected String TAG = AndroidNativeProvider.class.getSimpleName(); + + private class NativeLocationListener implements LocationListener { + @Override + public void onLocationChanged(@NonNull Location location) + { + mListener.onLocationChanged(location); + } + + @Override + public void onProviderDisabled(@NonNull String provider) + { + LOGGER.d(TAG, "Disabled location provider: " + provider); + mProviderCount--; + if (mProviderCount < MIN_PROVIDER_COUNT) + mListener.onLocationError(LocationHelper.ERROR_GPS_OFF); + } + + @Override + public void onProviderEnabled(@NonNull String provider) + { + LOGGER.d(TAG, "Enabled location provider: " + provider); + mProviderCount++; + } + }; @NonNull private final LocationManager mLocationManager; - @NonNull - private final List mListeners = new ArrayList<>(); + private int mProviderCount = 0; + private boolean mActive = false; + static private int MIN_PROVIDER_COUNT = 2; // PASSIVE is always available - AndroidNativeProvider(@NonNull LocationFixChecker locationFixChecker, @NonNull Context context) + @NotNull + final private NativeLocationListener mNativeLocationListener = new NativeLocationListener(); + + AndroidNativeProvider(@NonNull Context context, @NonNull BaseLocationProvider.Listener listener) { - super(locationFixChecker); - Objects.requireNonNull(context, "Context should be passed!"); + super(listener); mLocationManager = (LocationManager) MwmApplication.from(context).getSystemService(Context.LOCATION_SERVICE); + // This service is always available on all versions of Android + if (mLocationManager == null) + throw new IllegalStateException("Can't get LOCATION_SERVICE"); } @SuppressWarnings("MissingPermission") // A permission is checked externally @Override - protected void start() + public void start(long interval) { - LOGGER.d(TAG, "Android native provider is started"); - if (isActive()) - return; + LOGGER.d(TAG, "start()"); + if (mActive) + throw new IllegalStateException("Already started"); + mActive = true; - List providers = mLocationManager.getProviders(true); - if (providers.isEmpty()) + final List providers = mLocationManager.getProviders(true); + mProviderCount = providers.size(); + if (mProviderCount < MIN_PROVIDER_COUNT) { - setActive(false); - return; + mListener.onLocationError(LocationHelper.ERROR_GPS_OFF); } - setActive(true); for (String provider : providers) { - LocationListener listener = new BaseLocationListener(getLocationFixChecker()); - long interval = LocationHelper.INSTANCE.getInterval(); LOGGER.d(TAG, "Request Android native provider '" + provider + "' to get locations at this interval = " + interval + " ms"); - mLocationManager.requestLocationUpdates(provider, interval, 0, listener); - mListeners.add(listener); + mLocationManager.requestLocationUpdates(provider, interval, 0, mNativeLocationListener); + + final Location location = mLocationManager.getLastKnownLocation(provider); + LOGGER.d(TAG, "provider = '" + provider + "' last location = " + location); + if (location != null) + mListener.onLocationChanged(location); + + mProviderCount++; } - - LocationHelper.INSTANCE.startSensors(); - - Location location = findBestLocation(mLocationManager); - if (location != null && !getLocationFixChecker().isLocationBetterThanLast(location)) - location = LocationHelper.INSTANCE.getSavedLocation(); - - if (location != null) - onLocationChanged(location); - } - - private void onLocationChanged(@NonNull Location location) - { - ListIterator iterator = mListeners.listIterator(); - // All listeners have to be notified only through safe list iterator interface, - // otherwise ConcurrentModificationException will be obtained, because each listener can - // cause 'stop' method calling and modifying the collection during this iteration. - // noinspection WhileLoopReplaceableByForEach - while (iterator.hasNext()) - iterator.next().onLocationChanged(location); } @Override - protected void stop() + public void stop() { - LOGGER.d(TAG, "Android native provider is stopped"); - ListIterator iterator = mListeners.listIterator(); - // noinspection WhileLoopReplaceableByForEach - while (iterator.hasNext()) - mLocationManager.removeUpdates(iterator.next()); - - mListeners.clear(); - setActive(false); - } - - @Nullable - static Location findBestLocation(@NonNull Context context) - { - final LocationManager manager = (LocationManager) MwmApplication.from(context).getSystemService(Context.LOCATION_SERVICE); - return findBestLocation(manager); - } - - @Nullable - private static Location findBestLocation(LocationManager manager) - { - Location res = null; - try - { - for (final String pr : manager.getProviders(true)) - { - final Location last = manager.getLastKnownLocation(pr); - if (last == null) - continue; - - if (res == null || res.getAccuracy() > last.getAccuracy()) - res = last; - } - } - catch (SecurityException e) - { - LOGGER.e(TAG, "Dynamic permission ACCESS_COARSE_LOCATION/ACCESS_FINE_LOCATION is not granted", - e); - } - return res; + LOGGER.d(TAG, "stop()"); + mLocationManager.removeUpdates(mNativeLocationListener); + mActive = false; } } diff --git a/android/src/com/mapswithme/maps/location/BaseLocationListener.java b/android/src/com/mapswithme/maps/location/BaseLocationListener.java deleted file mode 100644 index f982929b9d..0000000000 --- a/android/src/com/mapswithme/maps/location/BaseLocationListener.java +++ /dev/null @@ -1,68 +0,0 @@ -package com.mapswithme.maps.location; - -import android.location.Location; -import android.location.LocationListener; -import android.os.Bundle; - -import androidx.annotation.NonNull; - -import com.mapswithme.util.log.Logger; -import com.mapswithme.util.log.LoggerFactory; - -class BaseLocationListener implements LocationListener -{ - private static final String TAG = BaseLocationListener.class.getSimpleName(); - private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.LOCATION); - @NonNull - private final LocationFixChecker mLocationFixChecker; - - BaseLocationListener(@NonNull LocationFixChecker locationFixChecker) - { - mLocationFixChecker = locationFixChecker; - } - - @Override - public void onLocationChanged(Location location) - { - LOGGER.d(TAG, "onLocationChanged, location = " + location); - - if (location == null) - return; - - if (!mLocationFixChecker.isAccuracySatisfied(location)) - return; - - if (mLocationFixChecker.isLocationBetterThanLast(location)) - { - LocationHelper.INSTANCE.onLocationUpdated(location); - LocationHelper.INSTANCE.notifyLocationUpdated(); - } - else - { - Location last = LocationHelper.INSTANCE.getSavedLocation(); - if (last != null) - { - LOGGER.d(TAG, "The new location from '" + location.getProvider() - + "' is worse than the last one from '" + last.getProvider() + "'"); - } - } - } - - @Override - public void onProviderDisabled(String provider) - { - LOGGER.d(TAG, "Disabled location provider: " + provider); - } - - @Override - public void onProviderEnabled(String provider) - { - LOGGER.d(TAG, "Enabled location provider: " + provider); - } - - @Override - public void onStatusChanged(String provider, int status, Bundle extras) - { - LOGGER.d(TAG, "Status changed for location provider: " + provider + "; new status = " + status); - } -} diff --git a/android/src/com/mapswithme/maps/location/BaseLocationProvider.java b/android/src/com/mapswithme/maps/location/BaseLocationProvider.java index 89b5d07a7a..a17aa4a504 100644 --- a/android/src/com/mapswithme/maps/location/BaseLocationProvider.java +++ b/android/src/com/mapswithme/maps/location/BaseLocationProvider.java @@ -1,5 +1,7 @@ package com.mapswithme.maps.location; +import android.location.Location; + import androidx.annotation.NonNull; import com.mapswithme.util.log.Logger; @@ -7,37 +9,22 @@ import com.mapswithme.util.log.LoggerFactory; abstract class BaseLocationProvider { - static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.LOCATION); - private static final String TAG = BaseLocationProvider.class.getSimpleName(); - @NonNull - protected final LocationFixChecker mLocationFixChecker; - private boolean mActive; - @NonNull - LocationFixChecker getLocationFixChecker() + static protected final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.LOCATION); + + interface Listener { - return mLocationFixChecker; + void onLocationChanged(@NonNull Location location); + void onLocationError(int errorCode); } - BaseLocationProvider(@NonNull LocationFixChecker locationFixChecker) + @NonNull + protected Listener mListener; + + protected BaseLocationProvider(@NonNull Listener listener) { - mLocationFixChecker = locationFixChecker; + mListener = listener; } - protected abstract void start(); + protected abstract void start(long interval); protected abstract void stop(); - - /** - * Indicates whether this provider is providing location updates or not - * @return true - if locations are actively coming from this provider, false - otherwise - */ - public final boolean isActive() - { - return mActive; - } - - final void setActive(boolean active) - { - LOGGER.d(TAG, "setActive active = " + active); - mActive = active; - } } diff --git a/android/src/com/mapswithme/maps/location/DefaultLocationFixChecker.java b/android/src/com/mapswithme/maps/location/DefaultLocationFixChecker.java deleted file mode 100644 index b60e25d6fc..0000000000 --- a/android/src/com/mapswithme/maps/location/DefaultLocationFixChecker.java +++ /dev/null @@ -1,53 +0,0 @@ -package com.mapswithme.maps.location; - -import android.location.Location; - -import androidx.annotation.NonNull; - -import com.mapswithme.util.LocationUtils; - -class DefaultLocationFixChecker implements LocationFixChecker -{ - private static final double DEFAULT_SPEED_MPS = 5; - private static final String GPS_LOCATION_PROVIDER = "gps"; - - @Override - public boolean isAccuracySatisfied(@NonNull Location location) - { - // If it's a gps location then we completely ignore an accuracy checking, - // because there are cases on some devices (https://jira.mail.ru/browse/MAPSME-3789) - // when location is good, but it doesn't contain an accuracy for some reasons - if (isFromGpsProvider(location)) - return true; - - // Completely ignore locations without lat and lon - return location.getAccuracy() > 0.0f; - } - - private static boolean isFromGpsProvider(@NonNull Location location) - { - return GPS_LOCATION_PROVIDER.equals(location.getProvider()); - } - - @Override - public boolean isLocationBetterThanLast(@NonNull Location newLocation) - { - final Location lastLocation = LocationHelper.INSTANCE.getSavedLocation(); - - if (lastLocation == null) - return true; - - //noinspection SimplifiableIfStatement - if (isFromGpsProvider(lastLocation) && lastLocation.getAccuracy() == 0.0f) - return true; - - return isLocationBetterThanLast(newLocation, lastLocation); - } - - boolean isLocationBetterThanLast(@NonNull Location newLocation, @NonNull Location lastLocation) - { - double speed = Math.max(DEFAULT_SPEED_MPS, (newLocation.getSpeed() + lastLocation.getSpeed()) / 2.0); - double lastAccuracy = lastLocation.getAccuracy() + speed * LocationUtils.getDiff(lastLocation, newLocation); - return newLocation.getAccuracy() < lastAccuracy; - } -} diff --git a/android/src/com/mapswithme/maps/location/FusedLocationFixChecker.java b/android/src/com/mapswithme/maps/location/FusedLocationFixChecker.java deleted file mode 100644 index d80ae85b5a..0000000000 --- a/android/src/com/mapswithme/maps/location/FusedLocationFixChecker.java +++ /dev/null @@ -1,23 +0,0 @@ -package com.mapswithme.maps.location; - -import android.location.Location; - -import androidx.annotation.NonNull; - -class FusedLocationFixChecker extends DefaultLocationFixChecker -{ - private static final String GMS_LOCATION_PROVIDER = "fused"; - - @Override - boolean isLocationBetterThanLast(@NonNull Location newLocation, @NonNull Location lastLocation) - { - // We believe that google services always return good locations. - return isFromFusedProvider(newLocation) || - (!isFromFusedProvider(lastLocation) && super.isLocationBetterThanLast(newLocation, lastLocation)); - } - - private static boolean isFromFusedProvider(Location location) - { - return GMS_LOCATION_PROVIDER.equalsIgnoreCase(location.getProvider()); - } -} diff --git a/android/src/com/mapswithme/maps/location/LocationFixChecker.java b/android/src/com/mapswithme/maps/location/LocationFixChecker.java deleted file mode 100644 index 4eb2342240..0000000000 --- a/android/src/com/mapswithme/maps/location/LocationFixChecker.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.mapswithme.maps.location; - -import android.location.Location; - -import androidx.annotation.NonNull; - -interface LocationFixChecker -{ - boolean isLocationBetterThanLast(@NonNull Location newLocation); - boolean isAccuracySatisfied(@NonNull Location location); -} diff --git a/android/src/com/mapswithme/maps/location/LocationHelper.java b/android/src/com/mapswithme/maps/location/LocationHelper.java index fa95403475..b93136ead8 100644 --- a/android/src/com/mapswithme/maps/location/LocationHelper.java +++ b/android/src/com/mapswithme/maps/location/LocationHelper.java @@ -27,7 +27,9 @@ import com.mapswithme.util.Utils; import com.mapswithme.util.log.Logger; import com.mapswithme.util.log.LoggerFactory; -public enum LocationHelper implements Initializable, AppBackgroundTracker.OnTransitionListener +import org.jetbrains.annotations.NotNull; + +public enum LocationHelper implements Initializable, AppBackgroundTracker.OnTransitionListener, BaseLocationProvider.Listener { INSTANCE; @@ -105,7 +107,7 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack mSavedLocation = null; nativeOnLocationError(errorCode); if (mUiCallback != null) - mUiCallback.onLocationError(); + mUiCallback.onLocationError(errorCode); } @Override @@ -123,15 +125,18 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack private Location mSavedLocation; private MapObject mMyPosition; private long mSavedLocationTime; + @SuppressWarnings("NotNullFieldNotInitialized") @NonNull private SensorHelper mSensorHelper; - @Nullable + @SuppressWarnings("NotNullFieldNotInitialized") + @NonNull private BaseLocationProvider mLocationProvider; @Nullable private UiCallback mUiCallback; private long mInterval; private CompassData mCompassData; private boolean mInFirstRun; + private boolean mActive; private boolean mLocationUpdateStoppedByUser; @SuppressWarnings("FieldCanBeLocal") @@ -158,14 +163,15 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack }; @Override - public void initialize(@Nullable Context context) + public void initialize(@NotNull Context context) { mContext = context; mSensorHelper = new SensorHelper(context); - initProvider(); + mLocationProvider = LocationProviderFactory.getProvider(mContext, this); LocationState.nativeSetListener(mMyPositionModeListener); LocationState.nativeSetLocationPendingTimeoutListener(mLocationPendingTimeoutListener); MwmApplication.backgroundTracker(context).addListener(this); + addListener(mCoreLocationListener); } @Override @@ -174,24 +180,6 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack // No op. } - private void initProvider() - { - mLocationProvider = LocationProviderFactory.getProvider(mContext); - } - - void initNativeProvider() - { - mLogger.d(TAG, "Use native provider"); - mLocationProvider = new AndroidNativeProvider(new DefaultLocationFixChecker(), mContext); - } - - public void onLocationUpdated(@NonNull Location location) - { - mSavedLocation = location; - mMyPosition = null; - mSavedLocationTime = System.currentTimeMillis(); - } - /** * @return MapObject.MY_POSITION, null if location is not yet determined or "My position" button is switched off. */ @@ -232,11 +220,10 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack /** * Indicates about whether a location provider is polling location updates right now or not. - * @see BaseLocationProvider#isActive() */ public boolean isActive() { - return mLocationProvider != null && mLocationProvider.isActive(); + return mActive; } public void setStopLocationUpdateByUser(boolean isStopped) @@ -290,13 +277,10 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack mListeners.finishIterate(); } - void notifyLocationUpdated() + private void notifyLocationUpdated() { if (mSavedLocation == null) - { - mLogger.d(TAG, "No saved location - skip"); - return; - } + throw new IllegalStateException("No saved location"); for (LocationListener listener : mListeners) listener.onLocationUpdated(mSavedLocation); @@ -316,22 +300,37 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack } } - private void notifyLocationUpdated(LocationListener listener) + @Override + public void onLocationChanged(@NonNull Location location) { - mLogger.d(TAG, "notifyLocationUpdated(), listener: " + listener); + mLogger.d(TAG, "onLocationChanged, location = " + location); - if (mSavedLocation == null) + if (!LocationUtils.isAccuracySatisfied(location)) + return; + + if (mSavedLocation != null && !LocationUtils.isLocationBetterThanLast(location, mSavedLocation)) { - mLogger.d(TAG, "No saved location - skip"); + mLogger.d(TAG, "The new " + location + " is worse than the last " + mSavedLocation); return; } - listener.onLocationUpdated(mSavedLocation); + mSavedLocation = location; + mMyPosition = null; + mSavedLocationTime = System.currentTimeMillis(); + notifyLocationUpdated(); } - private void notifyLocationError(int errCode) + @Override + public void onLocationError(int errCode) { - mLogger.d(TAG, "notifyLocationError(): " + errCode); + mLogger.d(TAG, "onLocationError(): " + errCode); + if (errCode == ERROR_NOT_SUPPORTED && !(mLocationProvider instanceof AndroidNativeProvider)) + { + // Try to downgrade to native provider first before notifying the user. + mLogger.d(TAG, "Downgrading to use native provider"); + mLocationProvider = new AndroidNativeProvider(mContext, this); + return; + } for (LocationListener listener : mListeners) listener.onLocationError(errCode); @@ -357,29 +356,16 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack * Registers listener to obtain location updates. * * @param listener listener to be registered. - * @param forceUpdate instantly notify given listener about available location, if any. - */ - @UiThread - public void addListener(@NonNull LocationListener listener, boolean forceUpdate) - { - mLogger.d(TAG, "addListener(): " + listener + ", forceUpdate: " + forceUpdate); - mLogger.d(TAG, " - listener count was: " + mListeners.getSize()); - - mListeners.register(listener); - - if (forceUpdate) - notifyLocationUpdated(listener); - } - - /** - * Registers listener to obtain location updates. - * - * @param listener listener to be registered. */ @UiThread public void addListener(@NonNull LocationListener listener) { - addListener(listener, false); + mLogger.d(TAG, "addListener(): " + listener); + mLogger.d(TAG, " - listener count was: " + mListeners.getSize()); + + mListeners.register(listener); + if (mSavedLocation != null) + listener.onLocationUpdated(mSavedLocation); } @UiThread @@ -394,11 +380,6 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack mListeners.unregister(listener); } - void startSensors() - { - mSensorHelper.start(); - } - private void calcLocationUpdatesInterval() { mLogger.d(TAG, "calcLocationUpdatesInterval()"); @@ -449,11 +430,6 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack } } - long getInterval() - { - return mInterval; - } - /** * Stops the current provider. Then initialize the location provider again, * because location settings could be changed and a new location provider can be used, @@ -465,10 +441,8 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack */ public void restart() { - mLogger.d(TAG, "restart()"); - checkProviderInitialization(); - stopInternal(); - initProvider(); + mLogger.i(TAG, "restart()"); + stop(); start(); } @@ -480,98 +454,52 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack */ public void start() { - if (isLocationUpdateStoppedByUser()) - { - mLogger.d(TAG, "Location updates are stopped by the user manually, so skip provider start" - + " until the user starts it manually."); - return; - } - - checkProviderInitialization(); - //noinspection ConstantConditions - if (mLocationProvider.isActive()) + if (mActive) { mLogger.i(TAG, "Provider '" + mLocationProvider + "' is already started"); return; } - addListener(mCoreLocationListener, true); - - if (!LocationUtils.checkProvidersAvailability(mContext)) + if (isLocationUpdateStoppedByUser()) { - // No need to notify about an error in first run mode - if (!mInFirstRun) - notifyLocationError(ERROR_DENIED); + mLogger.d(TAG, "Location updates are stopped by the user manually, so skip provider start" + + " until the user starts it manually."); + onLocationError(ERROR_GPS_OFF); return; } long oldInterval = mInterval; mLogger.d(TAG, "Old time interval (ms): " + oldInterval); calcLocationUpdatesInterval(); - mLogger.d(TAG, "start(), params: " + mInterval); - startInternal(); + if (!PermissionsUtils.isLocationGranted(mContext)) + { + mLogger.w(TAG, "Dynamic permission ACCESS_COARSE_LOCATION/ACCESS_FINE_LOCATION is not granted"); + onLocationError(ERROR_DENIED); + return; + } + mLogger.i(TAG, "start(): interval = " + mInterval + " provider = '" + mLocationProvider + "' mInFirstRun = " + mInFirstRun); + checkForAgpsUpdates(); + mLocationProvider.start(mInterval); + mSensorHelper.start(); + mActive = true; } /** * Stops the polling location updates, i.e. removes the {@link #mCoreLocationListener} and stops * the current active provider. */ - private void stop() + public void stop() { - mLogger.d(TAG, "stop()"); - checkProviderInitialization(); - //noinspection ConstantConditions - if (!mLocationProvider.isActive()) + mLogger.i(TAG, "stop()"); + if (!mActive) { mLogger.i(TAG, "Provider '" + mLocationProvider + "' is already stopped"); return; } - removeListener(mCoreLocationListener); - stopInternal(); - } - - /** - * Actually starts location polling. - */ - private void startInternal() - { - mLogger.d(TAG, "startInternal(), current provider is '" + mLocationProvider - + "' , my position mode = " + LocationState.nameOf(getMyPositionMode()) - + ", mInFirstRun = " + mInFirstRun); - if (!PermissionsUtils.isLocationGranted(mContext)) - { - mLogger.w(TAG, "Dynamic permission ACCESS_COARSE_LOCATION/ACCESS_FINE_LOCATION is not granted", - new Throwable()); - return; - } - checkForAgpsUpdates(); - checkProviderInitialization(); - //noinspection ConstantConditions - mLocationProvider.start(); - mLogger.d(TAG, mLocationProvider.isActive() ? "SUCCESS" : "FAILURE"); - } - - private void checkProviderInitialization() - { - if (mLocationProvider == null) - { - String error = "A location provider must be initialized!"; - mLogger.e(TAG, error, new Throwable()); - throw new AssertionError(error); - } - } - - /** - * Actually stops location polling. - */ - private void stopInternal() - { - mLogger.d(TAG, "stopInternal()"); - checkProviderInitialization(); - //noinspection ConstantConditions mLocationProvider.stop(); mSensorHelper.stop(); + mActive = false; } private void checkForAgpsUpdates() @@ -618,12 +546,11 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack if (mCompassData != null) mUiCallback.onCompassUpdated(mCompassData); - checkProviderInitialization(); - //noinspection ConstantConditions - if (mLocationProvider.isActive()) + if (mActive) { mLogger.d(TAG, "attach() provider '" + mLocationProvider + "' is active, just add the listener"); - addListener(mCoreLocationListener, true); + if (mSavedLocation != null) + mCoreLocationListener.onLocationUpdated(mSavedLocation); } else { @@ -676,8 +603,7 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack // If there is a location we need just to pass it to the listeners, so that // my position state machine will be switched to the FOLLOW state. - Location location = getSavedLocation(); - if (location != null) + if (mSavedLocation != null) { notifyLocationUpdated(); mLogger.d(TAG, "Current location is available, so play the nice zoom animation"); @@ -685,11 +611,9 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack return; } - checkProviderInitialization(); // If the location hasn't been obtained yet we need to switch to the next mode and wait for locations. // Otherwise, try to restart location updates polling. - // noinspection ConstantConditions - if (mLocationProvider.isActive()) + if (mActive) switchToNextMode(); else restart(); @@ -702,10 +626,7 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack @Nullable public Location getLastKnownLocation() { - if (mSavedLocation != null) - return mSavedLocation; - - return AndroidNativeProvider.findBestLocation(mContext); + return mSavedLocation; } @Nullable @@ -730,7 +651,7 @@ public enum LocationHelper implements Initializable, AppBackgroundTrack void onMyPositionModeChanged(int newMode); void onLocationUpdated(@NonNull Location location); void onCompassUpdated(@NonNull CompassData compass); - void onLocationError(); + void onLocationError(int errorCode); void onLocationNotFound(); void onRoutingFinish(); } diff --git a/android/src/com/mapswithme/maps/location/LocationPermissionNotGrantedException.java b/android/src/com/mapswithme/maps/location/LocationPermissionNotGrantedException.java deleted file mode 100644 index 7e644d53e9..0000000000 --- a/android/src/com/mapswithme/maps/location/LocationPermissionNotGrantedException.java +++ /dev/null @@ -1,11 +0,0 @@ -package com.mapswithme.maps.location; - -public class LocationPermissionNotGrantedException extends Exception -{ - private static final long serialVersionUID = -1053815743102694164L; - - public LocationPermissionNotGrantedException() - { - super("Location permission not granted!"); - } -} diff --git a/android/src/com/mapswithme/maps/search/SearchFragment.java b/android/src/com/mapswithme/maps/search/SearchFragment.java index f013aa9749..03535e25a5 100644 --- a/android/src/com/mapswithme/maps/search/SearchFragment.java +++ b/android/src/com/mapswithme/maps/search/SearchFragment.java @@ -346,7 +346,7 @@ public class SearchFragment extends BaseMwmFragment public void onResume() { super.onResume(); - LocationHelper.INSTANCE.addListener(mLocationListener, true); + LocationHelper.INSTANCE.addListener(mLocationListener); mAppBarLayout.addOnOffsetChangedListener(mOffsetListener); } diff --git a/android/src/com/mapswithme/maps/widget/placepage/DirectionFragment.java b/android/src/com/mapswithme/maps/widget/placepage/DirectionFragment.java index c76baf2350..f983b0e401 100644 --- a/android/src/com/mapswithme/maps/widget/placepage/DirectionFragment.java +++ b/android/src/com/mapswithme/maps/widget/placepage/DirectionFragment.java @@ -98,7 +98,7 @@ public class DirectionFragment extends BaseMwmDialogFragment public void onResume() { super.onResume(); - LocationHelper.INSTANCE.addListener(this, true); + LocationHelper.INSTANCE.addListener(this); refreshViews(); } diff --git a/android/src/com/mapswithme/util/LocationUtils.java b/android/src/com/mapswithme/util/LocationUtils.java index e9adeafbfb..cfb8e30b84 100644 --- a/android/src/com/mapswithme/util/LocationUtils.java +++ b/android/src/com/mapswithme/util/LocationUtils.java @@ -16,14 +16,13 @@ import com.mapswithme.maps.location.LocationHelper; import com.mapswithme.util.log.Logger; import com.mapswithme.util.log.LoggerFactory; -import java.util.List; - public class LocationUtils { private LocationUtils() {} private static final Logger LOGGER = LoggerFactory.INSTANCE.getLogger(LoggerFactory.Type.LOCATION); private static final String TAG = LocationUtils.class.getSimpleName(); + private static final double DEFAULT_SPEED_MPS = 5; /** * Correct compass angles due to display orientation. @@ -73,7 +72,7 @@ public class LocationUtils return (timeDiff > expirationMillis); } - public static double getDiff(Location lastLocation, Location newLocation) + public static double getTimeDiff(Location lastLocation, Location newLocation) { if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.JELLY_BEAN_MR1) return (newLocation.getElapsedRealtimeNanos() - lastLocation.getElapsedRealtimeNanos()) * 1.0E-9; @@ -98,6 +97,42 @@ public class LocationUtils return (p1 != null && p1.equals(p2)); } + public static boolean isFromGpsProvider(@NonNull Location location) + { + return LocationManager.GPS_PROVIDER.equals(location.getProvider()); + } + + public static boolean isFromFusedProvider(@NonNull Location location) + { + return LocationManager.FUSED_PROVIDER.equals(location.getProvider()); + } + + public static boolean isAccuracySatisfied(@NonNull Location location) + { + // If it's a gps location then we completely ignore an accuracy checking, + // because there are cases on some devices (https://jira.mail.ru/browse/MAPSME-3789) + // when location is good, but it doesn't contain an accuracy for some reasons. + if (isFromGpsProvider(location)) + return true; + + // Completely ignore locations without lat and lon. + return location.getAccuracy() > 0.0f; + } + + public static boolean isLocationBetterThanLast(@NonNull Location newLocation, @NonNull Location lastLocation) + { + if (isFromFusedProvider(newLocation)) + return true; + + if (isFromGpsProvider(lastLocation) && lastLocation.getAccuracy() == 0.0f) + return true; + + double speed = Math.max(DEFAULT_SPEED_MPS, (newLocation.getSpeed() + lastLocation.getSpeed()) / 2.0); + double lastAccuracy = lastLocation.getAccuracy() + speed * LocationUtils.getTimeDiff(lastLocation, newLocation); + return newLocation.getAccuracy() < lastAccuracy; + } + + @SuppressLint("InlinedApi") @SuppressWarnings("deprecation") public static boolean areLocationServicesTurnedOn(@NonNull Context context) @@ -113,37 +148,4 @@ public class LocationUtils return false; } } - - private static void logAvailableProviders(@NonNull Context context) - { - LocationManager locMngr = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); - List providers = locMngr.getProviders(true); - StringBuilder sb; - if (!providers.isEmpty()) - { - sb = new StringBuilder("Available location providers:"); - for (String provider : providers) - sb.append(" ").append(provider); - } - else - { - sb = new StringBuilder("There are no enabled location providers!"); - } - LOGGER.i(TAG, sb.toString()); - } - - public static boolean checkProvidersAvailability(@NonNull Context context) - { - LocationManager locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); - if (locationManager == null) - { - LOGGER.e(TAG, "This device doesn't support the location service."); - return false; - } - - boolean networkEnabled = locationManager.isProviderEnabled(LocationManager.NETWORK_PROVIDER); - boolean gpsEnabled = locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER); - LocationUtils.logAvailableProviders(context); - return networkEnabled || gpsEnabled; - } }