TrackRecordingService: move BluetoothRemoteSensorManager and AltitudeSumManager to HandlerServer.

Part of #822.

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);
     }
 }

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.getHandlerServer().setAltitudeSumManager(altitudeSumManager);
-        service.setRemoteSensorManager(new BluetoothRemoteSensorManager(context) {
+        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);

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
+    @Ignore("ServiceExecutor disabled for #822")
-    public void onSharedPreferenceChanged() {
-        // when
-        subject.onSharedPreferenceChanged(context, sharedPreferences, null);
-
-        // then
-        verify(locationHandler).onSharedPreferenceChanged(context, sharedPreferences, null);
-    }
-
     @Test
     public void sendTrackPoint() throws InterruptedException {
         // given

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) {
+    public HandlerServer getHandlerServer() {
-        this.remoteSensorManager = remoteSensorManager;
+        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) {
+    public void stopProcessingGPS() {
-        this.altitudeSumManager = altitudeSumManager;
+        handlerServer.stopGPS();
     }
 
     public LiveData<GpsStatusValue> getGpsStatusObservable() {
@@ -723,16 +689,14 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
             tmpLastTrackPoint.setLatitude(lastTrackPoint.getLatitude());
         }
 
-        BluetoothRemoteSensorManager localRemoteSensorManager = this.remoteSensorManager;
+        HandlerServer localHandlerServer = this.handlerServer;
-        AltitudeSumManager localAltitudeSumManager = this.altitudeSumManager;
+        if (localHandlerServer == null) {
-        if (localAltitudeSumManager == null || localRemoteSensorManager == 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 sensorDataSet = localHandlerServer.fill(tmpLastTrackPoint);
-        sensorDataSet.fillTrackPoint(tmpLastTrackPoint);
         tmpTrackStatisticsUpdater.addTrackPoint(tmpLastTrackPoint, recordingDistanceInterval);
         track.setTrackStatistics(tmpTrackStatisticsUpdater.getTrackStatistics());
 

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.onStart(context, sharedPreferences);
-        locationHandler.onSharedPreferenceChanged(context, sharedPreferences, null);
+
+        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);
     }
 }

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) {

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);
     }