diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicyTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicyTest.java index d6e5a6e92..b7c677a0a 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicyTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicyTest.java @@ -21,6 +21,8 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.junit.runners.JUnit4; +import de.dennisguse.opentracks.services.handlers.AdaptiveLocationListenerPolicy; + /** * Tests the {@link AdaptiveLocationListenerPolicy}. * diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java index ffe0ecb2b..50e7f1b73 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java @@ -172,7 +172,7 @@ public class TrackRecordingServiceTest { // given createDummyTrack(trackId, -1L, true); TrackRecordingServiceInterface service = ((TrackRecordingServiceInterface) mServiceRule.bindService(createStartIntent(context))); - insertLocation(service); + newTrackPoint(service); // when service.pauseCurrentTrack(); @@ -182,7 +182,7 @@ public class TrackRecordingServiceTest { //when service.resumeTrack(trackId); - insertLocation(service); + newTrackPoint(service); // then Assert.assertTrue(service.isRecording()); @@ -201,14 +201,14 @@ public class TrackRecordingServiceTest { // given createDummyTrack(trackId, -1L, true); TrackRecordingServiceInterface service = ((TrackRecordingServiceInterface) mServiceRule.bindService(createStartIntent(context))); - insertLocation(service); + newTrackPoint(service); service.endCurrentTrack(); Assert.assertEquals(1, contentProviderUtils.getTrackPoints(trackId).size()); // when service.resumeTrack(trackId); - insertLocation(service); + newTrackPoint(service); // then Assert.assertTrue(service.isRecording()); @@ -294,7 +294,7 @@ public class TrackRecordingServiceTest { TrackRecordingServiceInterface service = ((TrackRecordingServiceInterface) mServiceRule.bindService(createStartIntent(context))); service.startNewTrack(); Assert.assertTrue(service.isRecording()); - insertLocation(service); + newTrackPoint(service); long trackId = service.getRecordingTrackId(); // when @@ -331,18 +331,18 @@ public class TrackRecordingServiceTest { addTrack(dummyTrack, isRecording); } - private static void insertLocation(TrackRecordingServiceInterface trackRecordingService) throws InterruptedException { - insertLocation(trackRecordingService, 45.0f, 35f, 5, 10, System.currentTimeMillis()); + private static void newTrackPoint(TrackRecordingServiceInterface trackRecordingService) throws InterruptedException { + newTrackPoint(trackRecordingService, 45.0f, 35f, 5, 10, System.currentTimeMillis()); } - static void insertLocation(TrackRecordingServiceInterface trackRecordingService, double latitude, double longitude, float accuracy, long speed) throws InterruptedException { - insertLocation(trackRecordingService, latitude, longitude, accuracy, speed, System.currentTimeMillis()); + static void newTrackPoint(TrackRecordingServiceInterface trackRecordingService, double latitude, double longitude, float accuracy, long speed) throws InterruptedException { + newTrackPoint(trackRecordingService, latitude, longitude, accuracy, speed, System.currentTimeMillis()); } /** * Inserts a location and waits for 200ms. */ - private static void insertLocation(TrackRecordingServiceInterface trackRecordingService, double latitude, double longitude, float accuracy, long speed, long time) throws InterruptedException { + private static void newTrackPoint(TrackRecordingServiceInterface trackRecordingService, double latitude, double longitude, float accuracy, long speed, long time) throws InterruptedException { Location location = new Location("gps"); location.setLongitude(longitude); location.setLatitude(latitude); @@ -350,7 +350,9 @@ public class TrackRecordingServiceTest { location.setSpeed(speed); location.setTime(time); location.setBearing(3.0f); - trackRecordingService.insertLocation(location); + TrackPoint trackPoint = new TrackPoint(location); + int prefAccuracy = PreferencesUtils.getRecordingGPSAccuracy(ApplicationProvider.getApplicationContext()); + trackRecordingService.newTrackPoint(trackPoint, prefAccuracy); //TODO Needed? Thread.sleep(200); diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java index 3a07aab8e..0d761d892 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java @@ -72,14 +72,11 @@ public class TrackRecordingServiceTestLocation { sharedPreferences.edit().clear().commit(); service = ((TrackRecordingServiceInterface) mServiceRule.bindService(TrackRecordingServiceTest.createStartIntent(context))); - //Disable executorService to not insert locations from GPS via LocationManager - service.enableLocationExecutor(false); } @After public void tearDown() { // Reset service (if some previous test failed) - service.enableLocationExecutor(true); if (service.isRecording() || service.isPaused()) { service.endCurrentTrack(); } @@ -95,12 +92,12 @@ public class TrackRecordingServiceTestLocation { long trackId = service.startNewTrack(); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0001, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0002, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0003, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0004, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0005, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0001, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0002, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0003, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0004, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0005, 35.0, 5, 15); service.endCurrentTrack(); @@ -112,29 +109,6 @@ public class TrackRecordingServiceTestLocation { Assert.assertEquals(45.0005, trackPoints.get(5).getLatitude(), 0.01); } - @MediumTest - @Test - public void testOnLocationChangedAsync_movingInaccurate() throws Exception { - // given - long trackId = service.startNewTrack(); - - // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.1, 35.0, Long.MAX_VALUE, 15); - TrackRecordingServiceTest.insertLocation(service, 45.2, 35.0, Long.MAX_VALUE, 15); - TrackRecordingServiceTest.insertLocation(service, 45.3, 35.0, Long.MAX_VALUE, 15); - TrackRecordingServiceTest.insertLocation(service, 99.0, 35.0, Long.MAX_VALUE, 15); - - service.endCurrentTrack(); - - // then - Assert.assertFalse(service.isRecording()); - - List trackPoints = contentProviderUtils.getTrackPoints(trackId); - Assert.assertEquals(1, trackPoints.size()); - Assert.assertEquals(45.0, trackPoints.get(0).getLatitude(), 0.01); - } - @MediumTest @Test public void testOnLocationChangedAsync_slowMovingAccurate() throws Exception { @@ -142,12 +116,12 @@ public class TrackRecordingServiceTestLocation { long trackId = service.startNewTrack(); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.000001, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.000002, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.000003, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.000004, 35.0, 5, 15); - TrackRecordingServiceTest.insertLocation(service, 45.000005, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.000001, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.000002, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.000003, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.000004, 35.0, 5, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.000005, 35.0, 5, 15); service.endCurrentTrack(); @@ -184,12 +158,12 @@ public class TrackRecordingServiceTestLocation { long trackId = service.startNewTrack(); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 1, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 2, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 3, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 4, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 6, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 2, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 3, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 4, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 5, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 6, 0); service.endCurrentTrack(); @@ -210,12 +184,12 @@ public class TrackRecordingServiceTestLocation { long trackId = service.startNewTrack(); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 1, 15); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 2, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 3, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 4, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 6, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 2, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 3, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 4, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 5, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 6, 15); service.endCurrentTrack(); @@ -253,12 +227,12 @@ public class TrackRecordingServiceTestLocation { }); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 0, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 1, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 2, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 3, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 4, 0); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 5, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 0, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 2, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 3, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 4, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 5, 0); service.endCurrentTrack(); @@ -282,11 +256,11 @@ public class TrackRecordingServiceTestLocation { long trackId = service.startNewTrack(); // when - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 1, 0); - TrackRecordingServiceTest.insertLocation(service, 45.1, 35.0, 2, 0); - TrackRecordingServiceTest.insertLocation(service, 45.1, 35.0, 3, 0); - TrackRecordingServiceTest.insertLocation(service, 45.2, 35.0, 4, 0); - TrackRecordingServiceTest.insertLocation(service, 45.2, 35.0, 5, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.1, 35.0, 2, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.1, 35.0, 3, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.2, 35.0, 4, 0); + TrackRecordingServiceTest.newTrackPoint(service, 45.2, 35.0, 5, 0); service.endCurrentTrack(); @@ -303,25 +277,4 @@ public class TrackRecordingServiceTestLocation { Assert.assertEquals(4, trackPoints.get(5).getAccuracy(), 0.01); Assert.assertEquals(5, trackPoints.get(6).getAccuracy(), 0.01); } - - @MediumTest - @Test - public void testOnLocationChangedAsync_firstTrackPointInvalid() throws Exception { - // given - long trackId = service.startNewTrack(); - - // when - service.insertLocation(TrackPoint.createPause().getLocation()); - TrackRecordingServiceTest.insertLocation(service, 45.0, 35.0, 0, 0); - service.insertLocation(TrackPoint.createPause().getLocation()); - - service.endCurrentTrack(); - - // then - Assert.assertFalse(service.isRecording()); - - List trackPoints = contentProviderUtils.getTrackPoints(trackId); - Assert.assertEquals(1, trackPoints.size()); - Assert.assertEquals(0, trackPoints.get(0).getAccuracy(), 0.01); - } } diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLooper.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLooper.java index 872988780..dbe04bc3b 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLooper.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLooper.java @@ -26,6 +26,7 @@ import java.util.concurrent.TimeoutException; import de.dennisguse.opentracks.R; import de.dennisguse.opentracks.content.data.Track; +import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.provider.ContentProviderUtils; import de.dennisguse.opentracks.content.provider.CustomContentProvider; import de.dennisguse.opentracks.stats.TrackStatistics; @@ -215,7 +216,9 @@ public class TrackRecordingServiceTestLooper { location.setSpeed(10); location.setTime(startTime + i * 10000); location.setBearing(3.0f); - service.insertLocation(location); + TrackPoint trackPoint = new TrackPoint(location); + int prefAccuracy = PreferencesUtils.getRecordingGPSAccuracy(context); + service.newTrackPoint(trackPoint, prefAccuracy); if (i % 7 == 0) { service.insertWaypoint(null, null, null, null); diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/handlers/LocationHandlerTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/LocationHandlerTest.java new file mode 100644 index 000000000..6ba081ff3 --- /dev/null +++ b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/LocationHandlerTest.java @@ -0,0 +1,128 @@ +package de.dennisguse.opentracks.services.handlers; + +import android.content.Context; +import android.content.SharedPreferences; +import android.location.Location; + +import androidx.test.core.app.ApplicationProvider; +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.junit.After; +import org.junit.Before; +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mockito; + +import de.dennisguse.opentracks.R; +import de.dennisguse.opentracks.content.data.TrackPoint; +import de.dennisguse.opentracks.services.TrackRecordingService; +import de.dennisguse.opentracks.util.PreferencesUtils; + +import static org.mockito.ArgumentMatchers.any; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; + +@RunWith(AndroidJUnit4.class) +public class LocationHandlerTest { + private final Context context = ApplicationProvider.getApplicationContext(); + private HandlerServer handlerServer; + private LocationHandler locationHandler; + private TrackRecordingService mockService; + + @Before + public void setUp() { + // Let's use default values. + SharedPreferences sharedPreferences = PreferencesUtils.getSharedPreferences(context); + sharedPreferences.edit().clear().commit(); + + mockService = Mockito.mock(TrackRecordingService.class); + handlerServer = new HandlerServer(mockService); + locationHandler = new LocationHandler(handlerServer); + locationHandler.onSharedPreferenceChanged(context, PreferencesUtils.getSharedPreferences(context), context.getString(R.string.recording_gps_accuracy_key)); + locationHandler.onSharedPreferenceChanged(context, PreferencesUtils.getSharedPreferences(context), context.getString(R.string.min_recording_interval_key)); + } + + @After + public void tearDown() { + mockService = null; + handlerServer = null; + locationHandler = null; + } + + /** + * When a valid location changed in LocationHandler -> newTrackPoint service method is called. + */ + @Test + public void testOnLocationChanged_okay() { + // when + locationHandler.onLocationChanged(createLocation(45f, 35f, 3, 5, System.currentTimeMillis())); + + // then + verify(mockService, times(1)).newTrackPoint(any(TrackPoint.class), any(Integer.class)); + } + + /** + * When location changed in LocationHandler with bad location -> newTrackPoint service method is not called. + */ + @Test + public void testOnLocationChanged_badLocation() { + // given + float latitude = 91f; + + // when + // bad latitude + locationHandler.onLocationChanged(createLocation(latitude, 35f, 3, 5, System.currentTimeMillis())); + + // then + verify(mockService, times(0)).newTrackPoint(any(TrackPoint.class), any(Integer.class)); + } + + /** + * When location changed in LocationHandler with poor accuracy -> newTrackPoint service method is not called. + */ + @Test + public void testOnLocationChanged_poorAccuracy() { + // given + int prefAccuracy = PreferencesUtils.getRecordingGPSAccuracy(context); + + // when + // poor latitude + locationHandler.onLocationChanged(createLocation(45f, 35f, prefAccuracy + 1, 5, System.currentTimeMillis())); + + // then + // no newTrackPoint called + verify(mockService, times(0)).newTrackPoint(any(TrackPoint.class), any(Integer.class)); + } + + @Test + public void testOnLocationChanged_movingInaccurate() throws Exception { + // when + locationHandler.onLocationChanged( + createLocation(45.0, 35.0, 5, 15, System.currentTimeMillis())); + locationHandler.onLocationChanged( + createLocation(45.1, 35.0, Long.MAX_VALUE, 15, System.currentTimeMillis())); + locationHandler.onLocationChanged( + createLocation(45.2, 35.0, Long.MAX_VALUE, 15, System.currentTimeMillis())); + locationHandler.onLocationChanged( + createLocation(45.3, 35.0, Long.MAX_VALUE, 15, System.currentTimeMillis())); + locationHandler.onLocationChanged( + createLocation(99.0, 35.0, Long.MAX_VALUE, 15, System.currentTimeMillis())); + + // then + verify(mockService, times(1)).newTrackPoint(any(TrackPoint.class), any(Integer.class)); + } + + /** + * Creates a location with parameters and returns the Location object. + */ + private static Location createLocation(double latitude, double longitude, float accuracy, long speed, long time) { + Location location = new Location("gps"); + location.setLongitude(longitude); + location.setLatitude(latitude); + location.setAccuracy(accuracy); + location.setSpeed(speed); + location.setTime(time); + location.setBearing(3.0f); + return location; + } +} \ No newline at end of file diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 80f58c82d..4dffac656 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -24,11 +24,7 @@ import android.content.Intent; import android.content.SharedPreferences; import android.content.SharedPreferences.OnSharedPreferenceChangeListener; import android.database.sqlite.SQLiteException; -import android.location.Location; -import android.location.LocationListener; -import android.location.LocationManager; import android.net.Uri; -import android.os.Bundle; import android.os.IBinder; import android.os.PowerManager.WakeLock; import android.util.Log; @@ -37,9 +33,6 @@ import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import androidx.core.app.TaskStackBuilder; -import java.util.concurrent.ExecutorService; -import java.util.concurrent.Executors; - import de.dennisguse.opentracks.R; import de.dennisguse.opentracks.TrackListActivity; import de.dennisguse.opentracks.TrackRecordingActivity; @@ -50,6 +43,7 @@ import de.dennisguse.opentracks.content.provider.ContentProviderUtils; import de.dennisguse.opentracks.content.provider.CustomContentProvider; import de.dennisguse.opentracks.content.provider.TrackPointIterator; import de.dennisguse.opentracks.content.sensor.SensorDataSet; +import de.dennisguse.opentracks.services.handlers.HandlerServer; import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; import de.dennisguse.opentracks.services.tasks.AnnouncementPeriodicTaskFactory; import de.dennisguse.opentracks.services.tasks.PeriodicTaskExecutor; @@ -62,7 +56,6 @@ import de.dennisguse.opentracks.util.SystemUtils; import de.dennisguse.opentracks.util.TrackIconUtils; import de.dennisguse.opentracks.util.TrackNameUtils; import de.dennisguse.opentracks.util.TrackPointUtils; -import de.dennisguse.opentracks.util.UnitConversions; /** * A background service that registers a location listener and records track points. @@ -70,25 +63,19 @@ import de.dennisguse.opentracks.util.UnitConversions; * * @author Leif Hendrik Wilden */ -public class TrackRecordingService extends Service { +public class TrackRecordingService extends Service implements HandlerServer.HandlerServerInterface { private static final String TAG = TrackRecordingService.class.getSimpleName(); // The following variables are set in onCreate: - @Deprecated //TODO Should not be necessary - private ExecutorService locationExecutorService; // Enforces order of location changes. private ContentProviderUtils contentProviderUtils; - private LocationManager locationManager; private PeriodicTaskExecutor voiceExecutor; private TrackRecordingServiceNotificationManager notificationManager; - private LocationListenerPolicy locationListenerPolicy; private long recordingTrackId; private boolean recordingTrackPaused; private int recordingDistanceInterval; private int maxRecordingDistance; - private int recordingGpsAccuracy; - private long currentRecordingInterval; private final OnSharedPreferenceChangeListener sharedPreferenceChangeListener = new OnSharedPreferenceChangeListener() { @Override @@ -110,27 +97,14 @@ public class TrackRecordingService extends Service { if (PreferencesUtils.isKey(context, R.string.voice_frequency_key, key)) { voiceExecutor.setTaskFrequency(PreferencesUtils.getVoiceFrequency(context)); } - if (PreferencesUtils.isKey(context, R.string.min_recording_interval_key, key)) { - int minRecordingInterval = PreferencesUtils.getMinRecordingInterval(context); - if (minRecordingInterval == PreferencesUtils.getMinRecordingIntervalAdaptBatteryLife(context)) { - // Choose battery life over moving time accuracy. - locationListenerPolicy = new AdaptiveLocationListenerPolicy(30 * UnitConversions.ONE_SECOND_MS, 5 * UnitConversions.ONE_MINUTE_MS, 5); - } else if (minRecordingInterval == PreferencesUtils.getMinRecordingIntervalAdaptAccuracy(context)) { - // Get all the updates. - locationListenerPolicy = new AdaptiveLocationListenerPolicy(UnitConversions.ONE_SECOND_MS, 30 * UnitConversions.ONE_SECOND_MS, 0); - } else { - locationListenerPolicy = new AbsoluteLocationListenerPolicy(minRecordingInterval * UnitConversions.ONE_SECOND_MS); - } - } if (PreferencesUtils.isKey(context, R.string.recording_distance_interval_key, key)) { recordingDistanceInterval = PreferencesUtils.getRecordingDistanceInterval(context); } if (PreferencesUtils.isKey(context, R.string.max_recording_distance_key, key)) { maxRecordingDistance = PreferencesUtils.getMaxRecordingDistance(context); } - if (PreferencesUtils.isKey(context, R.string.recording_gps_accuracy_key, key)) { - recordingGpsAccuracy = PreferencesUtils.getRecordingGPSAccuracy(context); - } + + handlerServer.onSharedPreferenceChanged(context, preferences, key); } }; @@ -143,38 +117,16 @@ public class TrackRecordingService extends Service { private boolean isIdle; private TrackRecordingServiceBinder binder = new TrackRecordingServiceBinder(this); - private final LocationListener locationListener = new LocationListener() { - @Override - public void onLocationChanged(final Location location) { - if (locationExecutorService == null || locationExecutorService.isShutdown() || locationExecutorService.isTerminated()) { - return; - } - locationExecutorService.submit(() -> onLocationChangedAsync(location)); - } - - @Override - public void onStatusChanged(String provider, int status, Bundle extras) { - Log.w(TAG, "LocationListener.onStatusChanged(): is not implemented."); - } - - @Override - public void onProviderEnabled(String provider) { - Log.w(TAG, "LocationListener.onProviderEnabled(): is not implemented."); - } - - @Override - public void onProviderDisabled(String provider) { - Log.w(TAG, "LocationListener.onProviderDisabled(): is not implemented."); - } - }; + private HandlerServer handlerServer; @Override public void onCreate() { super.onCreate(); - locationExecutorService = Executors.newSingleThreadExecutor(); + + handlerServer = new HandlerServer(this); + contentProviderUtils = new ContentProviderUtils(this); - locationManager = (LocationManager) getSystemService(Context.LOCATION_SERVICE); voiceExecutor = new PeriodicTaskExecutor(this, new AnnouncementPeriodicTaskFactory()); notificationManager = new TrackRecordingServiceNotificationManager(this); @@ -200,6 +152,8 @@ public class TrackRecordingService extends Service { @Override public void onDestroy() { + handlerServer.stop(this); + if (remoteSensorManager != null) { remoteSensorManager.stop(); remoteSensorManager = null; @@ -208,9 +162,6 @@ public class TrackRecordingService extends Service { // Reverse order from onCreate showNotification(false); //TODO Why? - unregisterLocationListener(); - locationManager = null; - PreferencesUtils.unregister(this, sharedPreferenceChangeListener); try { @@ -227,8 +178,6 @@ public class TrackRecordingService extends Service { // This should be the next to last operation wakeLock = SystemUtils.releaseWakeLock(wakeLock); - // Shutdown the executorService last to avoid sending events to a dead executor. - locationExecutorService.shutdown(); super.onDestroy(); } @@ -428,7 +377,7 @@ public class TrackRecordingService extends Service { private void startGps() { wakeLock = SystemUtils.acquireWakeLock(this, wakeLock); - registerLocationListener(); + handlerServer.start(this); showNotification(true); } @@ -498,6 +447,8 @@ public class TrackRecordingService extends Service { } lastTrackPoint = null; + handlerServer.stop(this); + stopGps(trackStopped); } @@ -509,7 +460,7 @@ public class TrackRecordingService extends Service { void stopGps(boolean shutdown) { if (!isRecording()) return; - unregisterLocationListener(); + handlerServer.stop(this); showNotification(false); wakeLock = SystemUtils.releaseWakeLock(wakeLock); if (shutdown) { @@ -548,46 +499,27 @@ public class TrackRecordingService extends Service { PreferencesUtils.setBoolean(this, R.string.recording_track_paused_key, recordingTrackPaused); } - void onLocationChangedAsync(Location location) { + @Override + public void newTrackPoint(TrackPoint trackPoint, int recordingGpsAccuracy) { if (!isRecording() || isPaused()) { - Log.w(TAG, "Ignore onLocationChangedAsync. Not recording or paused."); + Log.w(TAG, "Ignore newTrackPoint. Not recording or paused."); return; } Track track = contentProviderUtils.getTrack(recordingTrackId); if (track == null) { - Log.w(TAG, "Ignore onLocationChangedAsync. No track."); + Log.w(TAG, "Ignore newTrackPoint. No track."); return; } - if (!LocationUtils.isValidLocation(location)) { - Log.w(TAG, "Ignore onLocationChangedAsync. location is invalid."); - return; - } - - TrackPoint trackPoint = new TrackPoint(location); fillWithSensorDataSet(trackPoint); notificationManager.updateTrackPoint(this, trackPoint, recordingGpsAccuracy); - if (!TrackPointUtils.fulfillsAccuracy(trackPoint, recordingGpsAccuracy)) { - Log.d(TAG, "Ignore onLocationChangedAsync. Poor accuracy."); - return; - } - TrackPointUtils.fixTime(trackPoint); //TODO Figure out how to avoid loading the lastValidTrackPoint from the database TrackPoint lastValidTrackPoint = getLastValidTrackPointInCurrentSegment(track.getId()); - long idleTime = 0L; - if (TrackPointUtils.after(trackPoint, lastValidTrackPoint)) { - idleTime = trackPoint.getTime() - lastValidTrackPoint.getTime(); - } - - locationListenerPolicy.updateIdleTime(idleTime); - if (currentRecordingInterval != locationListenerPolicy.getDesiredPollingInterval()) { - registerLocationListener(); - } //Storing trackPoint @@ -598,7 +530,7 @@ public class TrackRecordingService extends Service { return; } - if (!LocationUtils.isValidLocation(lastValidTrackPoint.getLocation())) { + if (lastValidTrackPoint == null || !LocationUtils.isValidLocation(lastValidTrackPoint.getLocation())) { // For some reason the previous first trackPoint was not stored, but currentSegmentHasLocation set true. // Should not happen. The current segment should have a location. Just insert the current location. insertTrackPoint(track, trackPoint); @@ -714,28 +646,6 @@ public class TrackRecordingService extends Service { } } - private void registerLocationListener() { - if (locationManager == null) { - Log.e(TAG, "locationManager is null."); - return; - } - try { - long interval = locationListenerPolicy.getDesiredPollingInterval(); - locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, interval, locationListenerPolicy.getMinDistance_m(), locationListener); - currentRecordingInterval = interval; - } catch (SecurityException e) { - Log.e(TAG, "Could not register location listener; permissions not granted.", e); - } - } - - private void unregisterLocationListener() { - if (locationManager == null) { - Log.e(TAG, "locationManager is null."); - return; - } - locationManager.removeUpdates(locationListener); - } - private void showNotification(boolean isGpsStarted) { if (isRecording()) { Intent intent = IntentUtils.newIntent(this, TrackRecordingActivity.class) @@ -765,19 +675,6 @@ public class TrackRecordingService extends Service { } } - /** - * Disables processing of location updates from {@link android.location.LocationManager}. - */ - @VisibleForTesting - public void enableLocationExecutor(boolean enable) { - if (enable) { - locationExecutorService = Executors.newSingleThreadExecutor(); - } else { - locationExecutorService.shutdownNow(); - locationExecutorService = null; - } - } - @VisibleForTesting public void setRemoteSensorManager(BluetoothRemoteSensorManager remoteSensorManager) { this.remoteSensorManager = remoteSensorManager; diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceBinder.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceBinder.java index f5ec8d004..b6e9ea49d 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceBinder.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceBinder.java @@ -1,9 +1,8 @@ package de.dennisguse.opentracks.services; -import android.location.Location; - import androidx.annotation.VisibleForTesting; +import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.sensor.SensorDataSet; import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; @@ -82,23 +81,11 @@ class TrackRecordingServiceBinder extends android.os.Binder implements TrackReco return trackRecordingService.insertWaypoint(name, category, description, photoUrl); } - @VisibleForTesting - @Override - public void insertLocation(Location location) { - trackRecordingService.onLocationChangedAsync(location); - } - @Override public SensorDataSet getSensorData() { return trackRecordingService.getSensorDataSet(); } - @VisibleForTesting - @Override - public void enableLocationExecutor(boolean enable) { - trackRecordingService.enableLocationExecutor(enable); - } - @VisibleForTesting @Override public void setRemoteSensorManager(BluetoothRemoteSensorManager remoteSensorManager) { @@ -112,4 +99,10 @@ class TrackRecordingServiceBinder extends android.os.Binder implements TrackReco void detachFromService() { trackRecordingService = null; } + + @VisibleForTesting + @Override + public void newTrackPoint(TrackPoint trackPoint, int recordingGpsAccuracy) { + trackRecordingService.newTrackPoint(trackPoint, recordingGpsAccuracy); + } } diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceInterface.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceInterface.java index 08edfc89c..6fa847e52 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceInterface.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceInterface.java @@ -15,10 +15,9 @@ */ package de.dennisguse.opentracks.services; -import android.location.Location; - import androidx.annotation.VisibleForTesting; +import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.sensor.SensorDataSet; import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; @@ -99,23 +98,16 @@ public interface TrackRecordingServiceInterface { */ SensorDataSet getSensorData(); - /** - * Inserts a location in the current recording track. - *

- * When recording a track, GPS locations are automatically inserted. - * This is used for inserting special track points or for testing. - * - * @param location the location to be inserted - */ - @VisibleForTesting - void insertLocation(Location location); - - /** - * Disables processing of location updates from {@link android.location.LocationManager}. - */ - @VisibleForTesting - void enableLocationExecutor(boolean enable); - @VisibleForTesting void setRemoteSensorManager(BluetoothRemoteSensorManager remoteSensorManager); + + /** + * Inserts a track point in the current recording track. + * This is used for inserting special track points or for testing. + * + * @param trackPoint the track point object to be inserted. + * @param recordingGpsAccuracy recording GPS accuracy. + */ + @VisibleForTesting + void newTrackPoint(TrackPoint trackPoint, int recordingGpsAccuracy); } diff --git a/src/main/java/de/dennisguse/opentracks/services/AbsoluteLocationListenerPolicy.java b/src/main/java/de/dennisguse/opentracks/services/handlers/AbsoluteLocationListenerPolicy.java similarity index 96% rename from src/main/java/de/dennisguse/opentracks/services/AbsoluteLocationListenerPolicy.java rename to src/main/java/de/dennisguse/opentracks/services/handlers/AbsoluteLocationListenerPolicy.java index bd1460f8b..17ba96b89 100644 --- a/src/main/java/de/dennisguse/opentracks/services/AbsoluteLocationListenerPolicy.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/AbsoluteLocationListenerPolicy.java @@ -14,7 +14,7 @@ * the License. */ -package de.dennisguse.opentracks.services; +package de.dennisguse.opentracks.services.handlers; /** * This is a simple location listener policy that will always dictate the same polling interval. diff --git a/src/main/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicy.java b/src/main/java/de/dennisguse/opentracks/services/handlers/AdaptiveLocationListenerPolicy.java similarity index 97% rename from src/main/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicy.java rename to src/main/java/de/dennisguse/opentracks/services/handlers/AdaptiveLocationListenerPolicy.java index 3174d5582..03a5c3261 100644 --- a/src/main/java/de/dennisguse/opentracks/services/AdaptiveLocationListenerPolicy.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/AdaptiveLocationListenerPolicy.java @@ -14,7 +14,7 @@ * the License. */ -package de.dennisguse.opentracks.services; +package de.dennisguse.opentracks.services.handlers; /** * A {@link LocationListenerPolicy} that will change based on how long the user has been stationary. diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java b/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java new file mode 100644 index 000000000..34839cc59 --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java @@ -0,0 +1,47 @@ +package de.dennisguse.opentracks.services.handlers; + +import android.content.Context; +import android.content.SharedPreferences; + +import de.dennisguse.opentracks.content.data.TrackPoint; + +public class HandlerServer { + private String TAG = HandlerServer.class.getSimpleName(); + + private LocationHandler locationHandler; + private HandlerServerInterface service; + + public HandlerServer(HandlerServerInterface service) { + this.locationHandler = new LocationHandler(this); + this.service = service; + } + + public void start(Context context) { + locationHandler.onStart(context); + locationHandler.onSharedPreferenceChanged(context, null, null); + } + + public void stop(Context context) { + locationHandler.onStop(context); + } + + public void onSharedPreferenceChanged(Context context, SharedPreferences preferences, String key) { + locationHandler.onSharedPreferenceChanged(context, preferences, key); + } + + public void sendTrackPoint(TrackPoint trackPoint, int recordingGpsAccuracy) { + service.newTrackPoint(trackPoint, recordingGpsAccuracy); + } + + public interface HandlerServerInterface { + void newTrackPoint(TrackPoint trackPoint, int gpsAccuracy); + } + + public interface Handler { + void onStart(Context context); + + void onStop(Context context); + + void onSharedPreferenceChanged(Context context, SharedPreferences preferences, String key); + } +} diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java new file mode 100644 index 000000000..25153990d --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java @@ -0,0 +1,143 @@ +package de.dennisguse.opentracks.services.handlers; + +import android.content.Context; +import android.content.SharedPreferences; +import android.location.Location; +import android.location.LocationListener; +import android.location.LocationManager; +import android.os.Bundle; +import android.util.Log; + +import androidx.annotation.NonNull; + +import de.dennisguse.opentracks.R; +import de.dennisguse.opentracks.content.data.TrackPoint; +import de.dennisguse.opentracks.util.LocationUtils; +import de.dennisguse.opentracks.util.PreferencesUtils; +import de.dennisguse.opentracks.util.TrackPointUtils; +import de.dennisguse.opentracks.util.UnitConversions; + +class LocationHandler implements HandlerServer.Handler, LocationListener { + + private String TAG = LocationHandler.class.getSimpleName(); + + private LocationManager locationManager; + private HandlerServer handlerServer; + private LocationListenerPolicy locationListenerPolicy; + private long currentRecordingInterval; + private int recordingGpsAccuracy; + private TrackPoint lastValidTrackPoint; + + public LocationHandler(HandlerServer handlerServer) { + this.handlerServer = handlerServer; + } + + @Override + public void onStart(Context context) { + locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); + registerLocationListener(); + } + + @Override + public void onStop(Context context) { + locationManager = null; + unregisterLocationListener(); + } + + @Override + public void onSharedPreferenceChanged(Context context, SharedPreferences preferences, String key) { + if (PreferencesUtils.isKey(context, R.string.min_recording_interval_key, key)) { + int minRecordingInterval = PreferencesUtils.getMinRecordingInterval(context); + if (minRecordingInterval == PreferencesUtils.getMinRecordingIntervalAdaptBatteryLife(context)) { + // Choose battery life over moving time accuracy. + locationListenerPolicy = new AdaptiveLocationListenerPolicy(30 * UnitConversions.ONE_SECOND_MS, 5 * UnitConversions.ONE_MINUTE_MS, 5); + } else if (minRecordingInterval == PreferencesUtils.getMinRecordingIntervalAdaptAccuracy(context)) { + // Get all the updates. + locationListenerPolicy = new AdaptiveLocationListenerPolicy(UnitConversions.ONE_SECOND_MS, 30 * UnitConversions.ONE_SECOND_MS, 0); + } else { + locationListenerPolicy = new AbsoluteLocationListenerPolicy(minRecordingInterval * UnitConversions.ONE_SECOND_MS); + } + + if (locationManager != null) { + registerLocationListener(); + } + } + if (PreferencesUtils.isKey(context, R.string.recording_gps_accuracy_key, key)) { + recordingGpsAccuracy = PreferencesUtils.getRecordingGPSAccuracy(context); + } + } + + @Override + public void onLocationChanged(@NonNull Location location) { + // TODO do we still need to process the location processing in an asynchronous manner? Let's go to check it out. + computeLocation(location); + } + + @Override + public void onStatusChanged(String provider, int status, Bundle extras) { + + } + + @Override + public void onProviderEnabled(@NonNull String provider) { + } + + @Override + public void onProviderDisabled(@NonNull String provider) { + } + + /** + * Checks if location is valid and builds a track point that will be send through HandlerServer. + * + * @param location {@link Location} object. + */ + private void computeLocation(Location location) { + if (!LocationUtils.isValidLocation(location)) { + Log.w(TAG, "Ignore newTrackPoint. location is invalid."); + return; + } + + TrackPoint trackPoint = new TrackPoint(location); + + if (!TrackPointUtils.fulfillsAccuracy(trackPoint, recordingGpsAccuracy)) { + Log.d(TAG, "Ignore newTrackPoint. Poor accuracy."); + return; + } + + long idleTime = 0L; + if (TrackPointUtils.after(trackPoint, lastValidTrackPoint)) { + idleTime = trackPoint.getTime() - lastValidTrackPoint.getTime(); + } + + locationListenerPolicy.updateIdleTime(idleTime); + if (currentRecordingInterval != locationListenerPolicy.getDesiredPollingInterval()) { + registerLocationListener(); + } + + lastValidTrackPoint = trackPoint; + handlerServer.sendTrackPoint(trackPoint, recordingGpsAccuracy); + } + + private void registerLocationListener() { + if (locationManager == null) { + Log.e(TAG, "locationManager is null."); + return; + } + try { + long interval = locationListenerPolicy.getDesiredPollingInterval(); + currentRecordingInterval = interval; + locationManager.requestLocationUpdates(LocationManager.GPS_PROVIDER, interval, locationListenerPolicy.getMinDistance_m(), this); + } catch (SecurityException e) { + Log.e(TAG, "Could not register location listener; permissions not granted.", e); + } + } + + private void unregisterLocationListener() { + if (locationManager == null) { + Log.e(TAG, "locationManager is null."); + return; + } + locationManager.removeUpdates(this); + locationManager = null; + } +} diff --git a/src/main/java/de/dennisguse/opentracks/services/LocationListenerPolicy.java b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationListenerPolicy.java similarity index 96% rename from src/main/java/de/dennisguse/opentracks/services/LocationListenerPolicy.java rename to src/main/java/de/dennisguse/opentracks/services/handlers/LocationListenerPolicy.java index e48975e25..beebeda4e 100644 --- a/src/main/java/de/dennisguse/opentracks/services/LocationListenerPolicy.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationListenerPolicy.java @@ -14,7 +14,7 @@ * the License. */ -package de.dennisguse.opentracks.services; +package de.dennisguse.opentracks.services.handlers; /** * This is an interface for classes that will manage the location listener policy.