diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/handlers/GpsStatusTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/GpsStatusTest.java new file mode 100644 index 000000000..c241b68f2 --- /dev/null +++ b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/GpsStatusTest.java @@ -0,0 +1,146 @@ +package de.dennisguse.opentracks.services.handlers; + +import static org.junit.Assert.assertEquals; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_DISABLED; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_ENABLED; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_NONE; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_SIGNAL_BAD; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_SIGNAL_FIX; +import static de.dennisguse.opentracks.services.handlers.GpsStatusValue.GPS_SIGNAL_LOST; + +import android.content.Context; +import android.location.Location; +import android.os.Handler; +import android.os.HandlerThread; +import android.os.Looper; + +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.junit.BeforeClass; +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.time.Duration; +import java.time.Instant; +import java.util.ArrayList; +import java.util.List; + +import de.dennisguse.opentracks.data.models.Distance; +import de.dennisguse.opentracks.data.models.TrackPoint; + +@RunWith(AndroidJUnit4.class) +public class GpsStatusTest { + + private final Context context = ApplicationProvider.getApplicationContext(); + + @BeforeClass + public static void preSetUp() { + // Prepare looper for Android's message queue + if (Looper.myLooper() == null) Looper.prepare(); + } + + private final static Location badFix = new Location("gps"); + private final static Location ok = new Location("gps"); + + static { + badFix.setAccuracy(50); + + ok.setAccuracy(10); + } + + @Test + public void testStartDisabledEnabledStop() { + ArrayList statusList = new ArrayList<>(); + + // given + GpsStatusManager subject = new GpsStatusManager(context, statusList::add, new Handler()); + + // when / then + subject.start(); + assertEquals(List.of(GPS_ENABLED), statusList); + + subject.onGpsDisabled(); + assertEquals(List.of(GPS_ENABLED, GPS_DISABLED), statusList); + + subject.onGpsEnabled(); + assertEquals(List.of(GPS_ENABLED, GPS_DISABLED, GPS_ENABLED), statusList); + + subject.stop(); + assertEquals(List.of(GPS_ENABLED, GPS_DISABLED, GPS_ENABLED, GPS_NONE), statusList); + } + + @Test + public void testStartBadfixOk() { + ArrayList statusList = new ArrayList<>(); + + // given + GpsStatusManager subject = new GpsStatusManager(context, statusList::add, new Handler()); + subject.onRecordingDistanceChanged(Distance.of(10)); + + // when / then + subject.start(); + subject.onNewTrackPoint(new TrackPoint(badFix, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_BAD), statusList); + + subject.onNewTrackPoint(new TrackPoint(ok, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_BAD, GPS_SIGNAL_FIX), statusList); + + subject.onNewTrackPoint(new TrackPoint(ok, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_BAD, GPS_SIGNAL_FIX), statusList); + + subject.onNewTrackPoint(new TrackPoint(badFix, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_BAD, GPS_SIGNAL_FIX, GPS_SIGNAL_BAD), statusList); + } + + @Test + public void testStartSignalLost() { + ArrayList statusList = new ArrayList<>(); + + // given + GpsStatusManager subject = new GpsStatusManager(context, statusList::add, new Handler()); + subject.onRecordingDistanceChanged(Distance.of(10)); + subject.onMinRecordingIntervalChanged(GpsStatusManager.SIGNAL_LOST_THRESHOLD.multipliedBy(-1)); + + // when / then + subject.start(); + subject.onNewTrackPoint(new TrackPoint(ok, Instant.now().minusMillis(1000))); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX), statusList); + + subject.determineGpsStatusByTime(Instant.now()); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX, GPS_SIGNAL_LOST), statusList); + + subject.onNewTrackPoint(new TrackPoint(ok, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX, GPS_SIGNAL_LOST, GPS_SIGNAL_FIX), statusList); + } + + @Test + public void testStartSignalLostByTimer() throws InterruptedException { + ArrayList statusList = new ArrayList<>(); + + final HandlerThread handlerThread = new HandlerThread("solution!"); + handlerThread.start(); + + // given + GpsStatusManager subject = new GpsStatusManager(context, statusList::add, new Handler(handlerThread.getLooper())); + subject.onRecordingDistanceChanged(Distance.of(10)); + subject.onMinRecordingIntervalChanged(GpsStatusManager.SIGNAL_LOST_THRESHOLD.multipliedBy(-1).plus(Duration.ofMillis(10))); + + // when / then + subject.start(); + Thread.sleep(100); + assertEquals(List.of(GPS_ENABLED), statusList); + + subject.onNewTrackPoint(new TrackPoint(ok, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX), statusList); + + Thread.sleep(100); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX, GPS_SIGNAL_LOST), statusList); + + subject.onNewTrackPoint(new TrackPoint(badFix, Instant.now())); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX, GPS_SIGNAL_LOST, GPS_SIGNAL_BAD), statusList); + + Thread.sleep(100); + assertEquals(List.of(GPS_ENABLED, GPS_SIGNAL_FIX, GPS_SIGNAL_LOST, GPS_SIGNAL_BAD, GPS_SIGNAL_LOST), statusList); + } +} \ No newline at end of file diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/GPSManager.java b/src/main/java/de/dennisguse/opentracks/services/handlers/GPSManager.java index 4a6a56cfe..6d3cb1134 100644 --- a/src/main/java/de/dennisguse/opentracks/services/handlers/GPSManager.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/GPSManager.java @@ -15,7 +15,6 @@ import androidx.core.location.LocationManagerCompat; import androidx.core.location.LocationRequestCompat; import java.time.Duration; -import java.time.Instant; import de.dennisguse.opentracks.R; import de.dennisguse.opentracks.data.models.Distance; @@ -26,7 +25,7 @@ import de.dennisguse.opentracks.util.LocationUtils; import de.dennisguse.opentracks.util.PermissionRequester; @VisibleForTesting(otherwise = VisibleForTesting.PACKAGE_PRIVATE) -public class GPSManager implements SensorConnector, LocationListenerCompat, GpsStatus.GpsStatusListener, SharedPreferences.OnSharedPreferenceChangeListener { +public class GPSManager implements SensorConnector, LocationListenerCompat, GpsStatusManager.GpsStatusListener, SharedPreferences.OnSharedPreferenceChangeListener { private final String TAG = GPSManager.class.getSimpleName(); @@ -37,7 +36,7 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS private Handler handler; private LocationManager locationManager; - private GpsStatus gpsStatus; + private GpsStatusManager gpsStatusManager; private Duration gpsInterval; private Distance thresholdHorizontalAccuracy; @@ -50,10 +49,10 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS this.handler = handler; PreferencesUtils.registerOnSharedPreferenceChangeListener(this); - gpsStatus = new GpsStatus(context, this); + gpsStatusManager = new GpsStatusManager(context, this, handler); locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); registerLocationListener(); - gpsStatus.start(); + gpsStatusManager.start(); } private boolean isStarted() { @@ -72,9 +71,9 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS handler = null; } - if (gpsStatus != null) { - gpsStatus.stop(); - gpsStatus = null; + if (gpsStatusManager != null) { + gpsStatusManager.stop(); + gpsStatusManager = null; } PreferencesUtils.unregisterOnSharedPreferenceChangeListener(this); } @@ -88,8 +87,8 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS gpsInterval = PreferencesUtils.getMinRecordingInterval(); - if (gpsStatus != null) { - gpsStatus.onMinRecordingIntervalChanged(gpsInterval); + if (gpsStatusManager != null) { + gpsStatusManager.onMinRecordingIntervalChanged(gpsInterval); } } if (PreferencesUtils.isKey(R.string.recording_gps_accuracy_key, key)) { @@ -98,9 +97,9 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS if (PreferencesUtils.isKey(R.string.recording_distance_interval_key, key)) { registerListener = true; - if (gpsStatus != null) { + if (gpsStatusManager != null) { Distance gpsMinDistance = PreferencesUtils.getRecordingDistanceInterval(); - gpsStatus.onRecordingDistanceChanged(gpsMinDistance); + gpsStatusManager.onRecordingDistanceChanged(gpsMinDistance); } } @@ -121,10 +120,10 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS return; } - if (gpsStatus != null) { + if (gpsStatusManager != null) { // Send each update to the status; please note that this TrackPoint is not stored. - TrackPoint trackPoint = new TrackPoint(location, Instant.ofEpochMilli(location.getTime())); - gpsStatus.onLocationChanged(trackPoint); + TrackPoint trackPoint = new TrackPoint(location, trackPointCreator.createNow()); + gpsStatusManager.onNewTrackPoint(trackPoint); } if (!LocationUtils.isValidLocation(location)) { @@ -146,15 +145,15 @@ public class GPSManager implements SensorConnector, LocationListenerCompat, GpsS @Override public void onProviderEnabled(@NonNull String provider) { - if (gpsStatus != null) { - gpsStatus.onGpsEnabled(); + if (gpsStatusManager != null) { + gpsStatusManager.onGpsEnabled(); } } @Override public void onProviderDisabled(@NonNull String provider) { - if (gpsStatus != null) { - gpsStatus.onGpsDisabled(); + if (gpsStatusManager != null) { + gpsStatusManager.onGpsDisabled(); } } diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatus.java b/src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatusManager.java similarity index 53% rename from src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatus.java rename to src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatusManager.java index 193143d24..4875641fc 100644 --- a/src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatus.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/GpsStatusManager.java @@ -6,6 +6,7 @@ import android.os.Handler; import androidx.annotation.NonNull; import androidx.annotation.Nullable; +import androidx.annotation.VisibleForTesting; import java.time.Duration; import java.time.Instant; @@ -17,14 +18,15 @@ import de.dennisguse.opentracks.settings.PreferencesUtils; /** * This class handle GPS status according to received locations` and some thresholds. */ -class GpsStatus { +class GpsStatusManager { - private static final String TAG = GpsStatus.class.getSimpleName(); + private static final String TAG = GpsStatusManager.class.getSimpleName(); // The duration that GpsStatus waits from minimal interval to consider GPS lost. - private static final Duration SIGNAL_LOST_THRESHOLD = Duration.ofSeconds(30); + @VisibleForTesting + public static final Duration SIGNAL_LOST_THRESHOLD = Duration.ofSeconds(30); - private Distance thresholdHorizontalAccuracy; + private Distance horizontalAccuracyThreshold; // Threshold for time without points. private Duration signalLostThreshold; @@ -35,40 +37,23 @@ class GpsStatus { @Nullable private TrackPoint lastTrackPoint = null; - @Nullable - // The last valid (not null) location. Null value means that there have not been any location yet. - private TrackPoint lastValidTrackPoint = null; - // Flag to prevent GpsStatus checks two or more locations at the same time. private boolean checking = false; - private class GpsStatusRunner implements Runnable { - private boolean stopped = false; + private Handler handler; - @Override - public void run() { - if (gpsStatus != null && !stopped) { - onLocationChanged(null); - gpsStatusHandler.postDelayed(gpsStatusRunner, getIntervalThreshold().toMillis()); - } - } + public final Runnable gpsStatusTimer = () -> { + determineGpsStatusByTime(Instant.now()); //TODO Get now via TrackPointCreator? + }; - public void stop() { - stopped = true; - } - } - private final Handler gpsStatusHandler; - private GpsStatusRunner gpsStatusRunner = null; - - public GpsStatus(Context context, GpsStatusListener client) { + public GpsStatusManager(Context context, GpsStatusListener client, Handler handler) { this.client = client; this.context = context; + this.handler = handler; onRecordingDistanceChanged(PreferencesUtils.getRecordingDistanceInterval()); onMinRecordingIntervalChanged(PreferencesUtils.getMinRecordingInterval()); - - gpsStatusHandler = new Handler(); } public void start() { @@ -81,10 +66,7 @@ class GpsStatus { public void stop() { client.onGpsStatusChanged(GpsStatusValue.GPS_NONE); client = null; - if (gpsStatusRunner != null) { - gpsStatusRunner.stop(); - gpsStatusRunner = null; - } + handler = null; } /** @@ -93,11 +75,11 @@ class GpsStatus { * @param value New preference value to signalBadThreshold. */ public void onRecordingDistanceChanged(@NonNull Distance value) { - thresholdHorizontalAccuracy = value; + horizontalAccuracyThreshold = value; } public void onMinRecordingIntervalChanged(Duration value) { - signalLostThreshold = SIGNAL_LOST_THRESHOLD.plus(value); + signalLostThreshold = SIGNAL_LOST_THRESHOLD.plus(value); //TODO Reschedule gpsStatusTimer? } /** @@ -105,22 +87,17 @@ class GpsStatus { * Receive new trackPoint and calculate the new status if needed. * It look for GPS changes in lastLocation if it's not null. If it's null then look for in lastValidLocation if any. */ - public void onLocationChanged(final TrackPoint trackPoint) { + //TODO Remove checking; should be synchronized if this is a problem. + public void onNewTrackPoint(@NonNull final TrackPoint trackPoint) { if (checking) { return; } checking = true; - if (lastTrackPoint != null) { - checkStatusFromLastLocation(); - } else if (lastValidTrackPoint != null) { - checkStatusFromLastValidLocation(); - } - - if (trackPoint != null) { - lastValidTrackPoint = trackPoint; - } lastTrackPoint = trackPoint; + + determineGpsStatusOnTrackpoint(trackPoint); + checking = false; } @@ -130,40 +107,35 @@ class GpsStatus { * If there is any change then it does the change. * Also, it'll run the runnable if signal is bad or stop it if the signal is lost. */ - private void checkStatusFromLastLocation() { - if (Duration.between(lastTrackPoint.getTime(), Instant.now()).compareTo(signalLostThreshold) > 0 && gpsStatus != GpsStatusValue.GPS_SIGNAL_LOST) { - // Too much time without receiving signal -> signal lost. - setGpsStatus(GpsStatusValue.GPS_SIGNAL_LOST); - stopStatusRunner(); - return; - } - if (lastTrackPoint.fulfillsAccuracy(thresholdHorizontalAccuracy) && gpsStatus != GpsStatusValue.GPS_SIGNAL_BAD) { - // Too little accuracy -> bad signal. - setGpsStatus(GpsStatusValue.GPS_SIGNAL_BAD); - startStatusRunner(); - return; - } - if (lastTrackPoint.fulfillsAccuracy(thresholdHorizontalAccuracy) && gpsStatus != GpsStatusValue.GPS_SIGNAL_FIX) { - // Gps okay. - setGpsStatus(GpsStatusValue.GPS_SIGNAL_FIX); - startStatusRunner(); - return; + //TODO use MonotonicClock instead of Instant.now() + @VisibleForTesting + void determineGpsStatusOnTrackpoint(@NonNull TrackPoint lastTrackPoint) { + if (lastTrackPoint.fulfillsAccuracy(horizontalAccuracyThreshold)) { + if (gpsStatus != GpsStatusValue.GPS_SIGNAL_FIX) { + setGpsStatus(GpsStatusValue.GPS_SIGNAL_FIX); + scheduleTimer(); //TODO + } + } else { + // GPS signal is to weak; TODO we might need a time-based threshold here as well (i.e., warn after Duration) + if (gpsStatus != GpsStatusValue.GPS_SIGNAL_BAD) { + setGpsStatus(GpsStatusValue.GPS_SIGNAL_BAD); + scheduleTimer(); + } } } - /** - * Checks if lastValidLocation has a new GPS status looking up time. - * It depends on signalLostThreshold. - * If there is any change then it does the change. - */ - private void checkStatusFromLastValidLocation() { - Duration elapsed = Duration.between(lastValidTrackPoint.getTime(), Instant.now()); - if (signalLostThreshold.minus(elapsed).isNegative()) { - // Too much time without locations -> lost signal? (wait signalLostThreshold from last valid location). - setGpsStatus(GpsStatusValue.GPS_SIGNAL_LOST); - stopStatusRunner(); - lastValidTrackPoint = null; + void determineGpsStatusByTime(Instant now) { + if (lastTrackPoint == null) { + return; } + if (signalLostThreshold.minus(Duration.between(lastTrackPoint.getTime(), now)).isNegative()) { + // Too much time without receiving signal -> signal lost. + if (gpsStatus != GpsStatusValue.GPS_SIGNAL_LOST) { + setGpsStatus(GpsStatusValue.GPS_SIGNAL_LOST); + } + return; + } + scheduleTimer(); } /** @@ -177,7 +149,7 @@ class GpsStatus { LocationManager locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); if (locationManager != null && locationManager.isProviderEnabled(LocationManager.GPS_PROVIDER)) { setGpsStatus(GpsStatusValue.GPS_ENABLED); - startStatusRunner(); + scheduleTimer(); } else { onGpsDisabled(); } @@ -194,33 +166,23 @@ class GpsStatus { setGpsStatus(GpsStatusValue.GPS_DISABLED); lastTrackPoint = null; - lastValidTrackPoint = null; - stopStatusRunner(); + stopTimer(); } private void setGpsStatus(GpsStatusValue current) { - gpsStatus = GpsStatusValue.GPS_DISABLED; + gpsStatus = current; if (client != null) { client.onGpsStatusChanged(current); } } - private void startStatusRunner() { - if (gpsStatusRunner == null) { - gpsStatusRunner = new GpsStatusRunner(); - gpsStatusRunner.run(); - } + private void scheduleTimer() { + handler.removeCallbacks(gpsStatusTimer); + handler.postDelayed(gpsStatusTimer, signalLostThreshold.toMillis()); } - private void stopStatusRunner() { - if (gpsStatusRunner != null) { - gpsStatusRunner.stop(); - gpsStatusRunner = null; - } - } - - public Duration getIntervalThreshold() { - return signalLostThreshold; + private void stopTimer() { + handler.removeCallbacks(gpsStatusTimer); } public interface GpsStatusListener {