From 223ad70d3618af9e207380e5aeaa2b0226e5aefd Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 12 Jul 2021 21:22:28 +0200 Subject: [PATCH] TrackRecordingService: move BluetoothRemoteSensorManager and AltitudeSumManager to HandlerServer. Part of #822. --- .../services/TrackRecordingServiceTest.java | 5 +- .../TrackRecordingServiceTestLocation.java | 19 ++--- .../services/handlers/HandlerServerTest.java | 11 +-- .../services/TrackRecordingService.java | 70 +++++-------------- .../services/handlers/HandlerServer.java | 68 ++++++++++++++++-- .../services/handlers/LocationHandler.java | 5 +- .../sensors/BluetoothRemoteSensorManager.java | 5 +- 7 files changed, 101 insertions(+), 82 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java index 5c18c827b..28024c2e1 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTest.java @@ -406,13 +406,14 @@ public class TrackRecordingServiceTest { /** * Inserts a location and waits for 200ms. */ - private static void newTrackPoint(TrackRecordingService trackRecordingService, double latitude, double longitude, float accuracy, long speed, long time) { + private static void newTrackPoint(TrackRecordingService trackRecordingService, double latitude, double longitude, float accuracy, long speed, long time) throws InterruptedException { TrackPoint trackPoint = new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.ofEpochMilli(time)) .setLongitude(longitude) .setLatitude(latitude) .setAccuracy(accuracy) .setSpeed(Speed.of(speed)) .setBearing(3.0f); - trackRecordingService.newTrackPoint(trackPoint, 50); + + trackRecordingService.getHandlerServer().onNewTrackPoint(trackPoint, 50); } } diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java index d7f9343d4..1ceecd98e 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java @@ -95,6 +95,7 @@ public class TrackRecordingServiceTestLocation { service = ((TrackRecordingService.Binder) mServiceRule.bindService(TrackRecordingServiceTest.createStartIntent(context))) .getService(); + service.stopProcessingGPS(); } @After @@ -108,7 +109,7 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_movingAccurate() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); // when TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15); @@ -181,7 +182,7 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_slowMovingAccurate() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); // when TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15); @@ -226,7 +227,7 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_idle() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); // when TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0); @@ -278,7 +279,8 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_idle_withMovement() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().stopGPS(); // when TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15); @@ -337,8 +339,8 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_idle_withSensorData() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); - service.setRemoteSensorManager(new BluetoothRemoteSensorManager(context) { + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setRemoteSensorManager(new BluetoothRemoteSensorManager(context) { @Override public boolean isEnabled() { @@ -346,10 +348,11 @@ public class TrackRecordingServiceTestLocation { } @Override - public void fill(@NonNull TrackPoint trackPoint) { + public SensorDataSet fill(@NonNull TrackPoint trackPoint) { SensorDataSet sensorDataSet = new SensorDataSet(); sensorDataSet.set(new SensorDataHeartRate("sensorName", "sensorAddress", 5f)); sensorDataSet.fillTrackPoint(trackPoint); + return sensorDataSet; } }); @@ -431,7 +434,7 @@ public class TrackRecordingServiceTestLocation { public void testOnLocationChangedAsync_segment() throws Exception { // given Track.Id trackId = service.startNewTrack(); - service.setAltitudeSumManager(altitudeSumManager); + service.getHandlerServer().setAltitudeSumManager(altitudeSumManager); // when TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0); diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/handlers/HandlerServerTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/HandlerServerTest.java index 096a9d685..47de3aa45 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/handlers/HandlerServerTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/handlers/HandlerServerTest.java @@ -5,6 +5,7 @@ import android.content.SharedPreferences; import org.junit.After; import org.junit.Before; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mock; @@ -42,15 +43,7 @@ public class HandlerServerTest { subject.stop(); } - @Test - public void onSharedPreferenceChanged() { - // when - subject.onSharedPreferenceChanged(context, sharedPreferences, null); - - // then - verify(locationHandler).onSharedPreferenceChanged(context, sharedPreferences, null); - } - + @Ignore("ServiceExecutor disabled for #822") @Test public void sendTrackPoint() throws InterruptedException { // given diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index aa29401c1..d0e32762a 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -52,8 +52,6 @@ import de.dennisguse.opentracks.content.sensor.SensorDataSet; import de.dennisguse.opentracks.io.file.exporter.ExportServiceResultReceiver; import de.dennisguse.opentracks.services.handlers.GpsStatusValue; import de.dennisguse.opentracks.services.handlers.HandlerServer; -import de.dennisguse.opentracks.services.sensors.AltitudeSumManager; -import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; import de.dennisguse.opentracks.services.tasks.AnnouncementPeriodicTask; import de.dennisguse.opentracks.services.tasks.PeriodicTaskExecutor; import de.dennisguse.opentracks.settings.SettingsActivity; @@ -125,14 +123,12 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand maxRecordingDistance = PreferencesUtils.getMaxRecordingDistance(sharedPreferences, context); } - handlerServer.onSharedPreferenceChanged(context, sharedPreferences, key); + handlerServer.onSharedPreferenceChanged(sharedPreferences, key); } }; // The following variables are set when recording: private WakeLock wakeLock; - private BluetoothRemoteSensorManager remoteSensorManager; - private AltitudeSumManager altitudeSumManager; private TrackStatisticsUpdater trackStatisticsUpdater; private TrackPoint lastTrackPoint; @@ -187,16 +183,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand handlerServer.stop(); handlerServer = null; - if (remoteSensorManager != null) { - remoteSensorManager.stop(); - remoteSensorManager = null; - } - - if (altitudeSumManager != null) { - altitudeSumManager.stop(this); - altitudeSumManager = null; - } - // Reverse order from onCreate showNotification(false); //TODO Why? @@ -361,12 +347,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand */ private void startRecording() { // Update instance variables - remoteSensorManager = new BluetoothRemoteSensorManager(this); - remoteSensorManager.start(); - - altitudeSumManager = new AltitudeSumManager(); - altitudeSumManager.start(this); - handler.postDelayed(updateRecordingData, RECORDING_DATA_UPDATE_INTERVAL.toMillis()); lastTrackPoint = null; @@ -374,6 +354,8 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand startGps(); + handlerServer.resetSensorData(); + // Restore periodic tasks voiceExecutor.restore(); } @@ -412,7 +394,9 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand insertTrackPointIfNewer(track, lastTrackPoint); } - insertTrackPoint(track, TrackPoint.createSegmentEnd()); + TrackPoint segmentEnd = TrackPoint.createSegmentEnd(); + handlerServer.fillAndReset(segmentEnd); + insertTrackPoint(track, segmentEnd); } } @@ -463,15 +447,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand voiceExecutor.shutdown(); // Update instance variables - if (remoteSensorManager != null) { - remoteSensorManager.stop(); - remoteSensorManager = null; - } - if (altitudeSumManager != null) { - altitudeSumManager.stop(this); - altitudeSumManager = null; - } - lastTrackPoint = null; handlerServer.stop(); @@ -529,8 +504,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand return; } - remoteSensorManager.fill(trackPoint); - notificationManager.updateTrackPoint(this, track.getTrackStatistics(), trackPoint, recordingGpsAccuracy); //TODO Figure out how to avoid loading the lastValidTrackPoint from the database @@ -629,14 +602,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand */ private void insertTrackPoint(@NonNull Track track, @NonNull TrackPoint trackPoint) { try { - if (!TrackPoint.Type.SEGMENT_START_MANUAL.equals(trackPoint.getType())) { - altitudeSumManager.fill(trackPoint); - altitudeSumManager.reset(); - - remoteSensorManager.fill(trackPoint); - remoteSensorManager.reset(); - } - contentProviderUtils.insertTrackPoint(trackPoint, track.getId()); trackStatisticsUpdater.addTrackPoint(trackPoint, recordingDistanceInterval); track.setTrackStatistics(trackStatisticsUpdater.getTrackStatistics()); @@ -681,18 +646,19 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand } } - // This is used to modify the state of this service while testing; must be called after startNewTrack(). @Deprecated @VisibleForTesting - public void setRemoteSensorManager(@NonNull BluetoothRemoteSensorManager remoteSensorManager) { - this.remoteSensorManager = remoteSensorManager; + public HandlerServer getHandlerServer() { + return handlerServer; } - // This is used to modify the state of this service while testing; must be called after startNewTrack(). + /** + * To mock locations (to not get it from GPS). + */ @Deprecated @VisibleForTesting - public void setAltitudeSumManager(@NonNull AltitudeSumManager altitudeSumManager) { - this.altitudeSumManager = altitudeSumManager; + public void stopProcessingGPS() { + handlerServer.stopGPS(); } public LiveData getGpsStatusObservable() { @@ -723,16 +689,14 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand tmpLastTrackPoint.setLatitude(lastTrackPoint.getLatitude()); } - BluetoothRemoteSensorManager localRemoteSensorManager = this.remoteSensorManager; - AltitudeSumManager localAltitudeSumManager = this.altitudeSumManager; - if (localAltitudeSumManager == null || localRemoteSensorManager == null) { + HandlerServer localHandlerServer = this.handlerServer; + if (localHandlerServer == null) { // when this happens, no recording is running and we should not send any notifications. //TODO This implementation is not a good idea; rather solve the issue for this properly return; } - localAltitudeSumManager.fill(tmpLastTrackPoint); - SensorDataSet sensorDataSet = localRemoteSensorManager.getSensorDataSet(); - sensorDataSet.fillTrackPoint(tmpLastTrackPoint); + + SensorDataSet sensorDataSet = localHandlerServer.fill(tmpLastTrackPoint); tmpTrackStatisticsUpdater.addTrackPoint(tmpLastTrackPoint, recordingDistanceInterval); track.setTrackStatistics(tmpTrackStatisticsUpdater.getTrackStatistics()); diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java b/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java index 32817ab80..8eb6ea72d 100644 --- a/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/HandlerServer.java @@ -2,11 +2,15 @@ package de.dennisguse.opentracks.services.handlers; import android.content.Context; import android.content.SharedPreferences; +import android.util.Log; import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import de.dennisguse.opentracks.content.data.TrackPoint; +import de.dennisguse.opentracks.content.sensor.SensorDataSet; +import de.dennisguse.opentracks.services.sensors.AltitudeSumManager; +import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager; import de.dennisguse.opentracks.util.PreferencesUtils; public class HandlerServer { @@ -21,6 +25,8 @@ public class HandlerServer { private final LocationHandler locationHandler; private final EGM2008CorrectionManager egm2008CorrectionManager = new EGM2008CorrectionManager(); + private BluetoothRemoteSensorManager remoteSensorManager; + private AltitudeSumManager altitudeSumManager; public HandlerServer(HandlerServerInterface service) { this.service = service; @@ -38,12 +44,46 @@ public class HandlerServer { // serviceExecutor = Executors.newSingleThreadExecutor(); SharedPreferences sharedPreferences = PreferencesUtils.getSharedPreferences(context); - locationHandler.onStart(context); - locationHandler.onSharedPreferenceChanged(context, sharedPreferences, null); + locationHandler.onStart(context, sharedPreferences); + + remoteSensorManager = new BluetoothRemoteSensorManager(context); + remoteSensorManager.start(); + + altitudeSumManager = new AltitudeSumManager(); + altitudeSumManager.start(context); } + @Deprecated + //There should be a cooler way to do this; we want to send fake locations without getting affected by real GPS data. + @VisibleForTesting + public void stopGPS() { + locationHandler.onStop(); + } + + public void resetSensorData() { + remoteSensorManager.reset(); + altitudeSumManager.reset(); + } + + //TODO TrackPoint should be created by HandlerServer; instead of in the TrackRecordingService. + @Deprecated + public SensorDataSet fill(TrackPoint trackPoint) { + SensorDataSet sensorDataSet = remoteSensorManager.fill(trackPoint); + altitudeSumManager.fill(trackPoint); + + return sensorDataSet; + } + + public SensorDataSet fillAndReset(TrackPoint trackPoint) { + SensorDataSet sensorDataSet = fill(trackPoint); + resetSensorData(); + + return sensorDataSet; + } + + public void stop() { - locationHandler.onStop(context); + locationHandler.onStop(); // if (serviceExecutor != null) { // serviceExecutor.shutdownNow(); @@ -53,7 +93,12 @@ public class HandlerServer { this.context = null; } - public void onSharedPreferenceChanged(@NonNull Context context, @NonNull SharedPreferences preferences, String key) { + public void onSharedPreferenceChanged(@NonNull SharedPreferences preferences, String key) { + if (context == null) { + Log.w(TAG, "not started yet."); + return; + } + locationHandler.onSharedPreferenceChanged(context, preferences, key); } @@ -63,17 +108,32 @@ public class HandlerServer { // } egm2008CorrectionManager.correctAltitude(context, trackPoint); + fillAndReset(trackPoint); + // serviceExecutor.execute(() -> service.newTrackPoint(trackPoint, recordingGpsAccuracy)); service.newTrackPoint(trackPoint, recordingGpsAccuracy); } + @Deprecated + @VisibleForTesting + public void setAltitudeSumManager(AltitudeSumManager altitudeSumManager) { + this.altitudeSumManager = altitudeSumManager; + } + + @Deprecated + @VisibleForTesting + public void setRemoteSensorManager(BluetoothRemoteSensorManager remoteSensorManager) { + this.remoteSensorManager = remoteSensorManager; + } + void sendGpsStatus(GpsStatusValue gpsStatusValue) { service.newGpsStatus(gpsStatusValue); } public interface HandlerServerInterface { void newTrackPoint(TrackPoint trackPoint, int gpsAccuracy); + void newGpsStatus(GpsStatusValue gpsStatusValue); } } diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java index c07dc7f12..0683a026e 100644 --- a/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/LocationHandler.java @@ -33,14 +33,15 @@ class LocationHandler implements LocationListener, GpsStatus.GpsStatusListener { this.handlerServer = handlerServer; } - public void onStart(@NonNull Context context) { + public void onStart(@NonNull Context context, SharedPreferences sharedPreferences) { + onSharedPreferenceChanged(context, sharedPreferences, null); gpsStatus = new GpsStatus(context, this); locationManager = (LocationManager) context.getSystemService(Context.LOCATION_SERVICE); registerLocationListener(); gpsStatus.start(); } - public void onStop(@NonNull Context context) { + public void onStop() { unregisterLocationListener(); locationManager = null; if (gpsStatus != null) { diff --git a/src/main/java/de/dennisguse/opentracks/services/sensors/BluetoothRemoteSensorManager.java b/src/main/java/de/dennisguse/opentracks/services/sensors/BluetoothRemoteSensorManager.java index e306549b9..ae9d17c41 100644 --- a/src/main/java/de/dennisguse/opentracks/services/sensors/BluetoothRemoteSensorManager.java +++ b/src/main/java/de/dennisguse/opentracks/services/sensors/BluetoothRemoteSensorManager.java @@ -161,11 +161,8 @@ public class BluetoothRemoteSensorManager implements BluetoothConnectionManager. } } - public void fill(@NonNull TrackPoint trackPoint) { + public SensorDataSet fill(@NonNull TrackPoint trackPoint) { sensorDataSet.fillTrackPoint(trackPoint); - } - - public SensorDataSet getSensorDataSet() { return new SensorDataSet(sensorDataSet); }