TrackRecordingService: simplify altitude and remoteSensor handling.

src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java

@@ -110,9 +110,6 @@ public class ExportImportTest {
 
 
         trackId = service.startNewTrack();
-        //TODO Workaround as those managers overwrite input data; We need to refactor TrackRecordingService to make it actually testable
-        service.setAltitudeSumManager(null);
-        service.setRemoteSensorManager(null);
 
         Distance sensorDistance = hasSensorDistance ? Distance.of(5) : null;
 
@@ -124,9 +121,6 @@ public class ExportImportTest {
         service.pauseCurrentTrack();
 
         service.resumeCurrentTrack();
-        //TODO Workaround as those managers overwrite input data; We need to refactor TrackRecordingService to make it actually testable
-        service.setAltitudeSumManager(null);
-        service.setRemoteSensorManager(null);
 
         service.newTrackPoint(createTrackPoint(System.currentTimeMillis(), 3, 14.003, 10, 15, 10, 0, 66, 3, 50, sensorDistance), 0);
         service.newTrackPoint(createTrackPoint(System.currentTimeMillis(), 3, 16, 10, 15, 10, 0, 66, 3, 50, sensorDistance), 0);

src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java

@@ -5,6 +5,7 @@ import android.content.Context;
 import android.content.SharedPreferences;
 import android.os.Looper;
 
+import androidx.annotation.NonNull;
 import androidx.test.core.app.ApplicationProvider;
 import androidx.test.ext.junit.runners.AndroidJUnit4;
 import androidx.test.filters.MediumTest;
@@ -32,11 +33,11 @@ import de.dennisguse.opentracks.content.provider.CustomContentProvider;
 import de.dennisguse.opentracks.content.sensor.SensorDataHeartRate;
 import de.dennisguse.opentracks.content.sensor.SensorDataSet;
 import de.dennisguse.opentracks.io.file.importer.TrackPointAssert;
+import de.dennisguse.opentracks.services.sensors.AltitudeSumManager;
 import de.dennisguse.opentracks.services.sensors.BluetoothRemoteSensorManager;
 import de.dennisguse.opentracks.util.PreferencesUtils;
 
 import static org.junit.Assert.assertFalse;
-import static org.junit.Assert.assertNotNull;
 
 /**
  * Tests insert location.
@@ -59,6 +60,14 @@ public class TrackRecordingServiceTestLocation {
 
     private TrackRecordingService service;
 
+    private final AltitudeSumManager altitudeSumManager = new AltitudeSumManager() {
+        @Override
+        public void fill(@NonNull TrackPoint trackPoint) {
+            trackPoint.setAltitudeGain(0f);
+            trackPoint.setAltitudeLoss(0f);
+        }
+    };
+
     @BeforeClass
     public static void preSetUp() {
         // Prepare looper for Android's message queue
@@ -99,6 +108,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_movingAccurate() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
+        service.setAltitudeSumManager(altitudeSumManager);
 
         // when
         TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15);
@@ -122,33 +132,47 @@ public class TrackRecordingServiceTestLocation {
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(1)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0001)
                         .setLongitude(35)
                         .setAccuracy(2)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0001)
                         .setLongitude(35)
                         .setAccuracy(3)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0002)
                         .setLongitude(35)
                         .setAccuracy(4)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0003)
                         .setLongitude(35)
                         .setAccuracy(5)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0004)
                         .setLongitude(35)
                         .setAccuracy(6)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
         ), trackPoints);
     }
 
@@ -157,7 +181,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_slowMovingAccurate() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
-        assertNotNull(trackId);
+        service.setAltitudeSumManager(altitudeSumManager);
 
         // when
         TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15);
@@ -181,13 +205,19 @@ public class TrackRecordingServiceTestLocation {
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(1)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.0005)
                         .setLongitude(35)
                         .setAccuracy(6)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
         ), trackPoints);
     }
 
@@ -196,6 +226,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_idle() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
+        service.setAltitudeSumManager(altitudeSumManager);
 
         // when
         TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0);
@@ -219,18 +250,26 @@ public class TrackRecordingServiceTestLocation {
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(1)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT) //TODO Why is this added? Systems is idle and not moving at all.
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(2)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(6)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
         ), trackPoints);
     }
 
@@ -239,6 +278,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_idle_withMovement() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
+        service.setAltitudeSumManager(altitudeSumManager);
 
         // when
         TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15);
@@ -262,23 +302,33 @@ public class TrackRecordingServiceTestLocation {
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(1)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT) //TODO Check why this trackPoint is inserted.
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(2)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT) //TODO Check why this trackPoint is inserted.
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(5)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(6)
-                        .setSpeed(Speed.of(15)),
+                        .setSpeed(Speed.of(15))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
         ), trackPoints);
     }
 
@@ -287,7 +337,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_idle_withSensorData() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
-
+        service.setAltitudeSumManager(altitudeSumManager);
         service.setRemoteSensorManager(new BluetoothRemoteSensorManager(context) {
 
             @Override
@@ -296,10 +346,10 @@ public class TrackRecordingServiceTestLocation {
             }
 
             @Override
-            public SensorDataSet getSensorData() {
+            public void fill(@NonNull TrackPoint trackPoint) {
                 SensorDataSet sensorDataSet = new SensorDataSet();
                 sensorDataSet.set(new SensorDataHeartRate("sensorName", "sensorAddress", 5f));
-                return sensorDataSet;
+                sensorDataSet.fillTrackPoint(trackPoint);
             }
         });
 
@@ -326,38 +376,52 @@ public class TrackRecordingServiceTestLocation {
                         .setLongitude(35)
                         .setAccuracy(1)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(2)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(3)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(4)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(5)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(6)
                         .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
                         .setHeartRate_bpm(5f)
         ), trackPoints);
     }
@@ -367,6 +431,7 @@ public class TrackRecordingServiceTestLocation {
     public void testOnLocationChangedAsync_segment() throws Exception {
         // given
         Track.Id trackId = service.startNewTrack();
+        service.setAltitudeSumManager(altitudeSumManager);
 
         // when
         TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 0);
@@ -390,30 +455,42 @@ public class TrackRecordingServiceTestLocation {
                         .setLatitude(45)
                         .setLongitude(35)
                         .setAccuracy(1)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
 
                 new TrackPoint(TrackPoint.Type.SEGMENT_START_AUTOMATIC)
                         .setLatitude(45.1)
                         .setLongitude(35)
                         .setAccuracy(2)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.1)
                         .setLongitude(35)
                         .setAccuracy(3)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
 
                 new TrackPoint(TrackPoint.Type.SEGMENT_START_AUTOMATIC)
                         .setLatitude(45.2)
                         .setLongitude(35)
                         .setAccuracy(4)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.TRACKPOINT)
                         .setLatitude(45.2)
                         .setLongitude(35)
                         .setAccuracy(5)
-                        .setSpeed(Speed.of(0)),
+                        .setSpeed(Speed.of(0))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f),
                 new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL)
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
         ), trackPoints);
     }
 }

src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java

@@ -363,11 +363,11 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
         remoteSensorManager = new BluetoothRemoteSensorManager(this);
         remoteSensorManager.start();
 
-        handler.postDelayed(updateRecordingData, RECORDING_DATA_UPDATE_INTERVAL.toMillis());
-
         altitudeSumManager = new AltitudeSumManager();
         altitudeSumManager.start(this);
 
+        handler.postDelayed(updateRecordingData, RECORDING_DATA_UPDATE_INTERVAL.toMillis());
+
         lastTrackPoint = null;
         isIdle = false;
 
@@ -528,7 +528,7 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
             return;
         }
 
-        fillWithSensorDataSet(trackPoint);
+        remoteSensorManager.fill(trackPoint);
 
         correctAltitude(trackPoint);
 
@@ -649,16 +649,14 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
      */
     private void insertTrackPoint(@NonNull Track track, @NonNull TrackPoint trackPoint) {
         try {
-            if (altitudeSumManager != null) {
+            if (!TrackPoint.Type.SEGMENT_START_MANUAL.equals(trackPoint.getType())) {
-                trackPoint.setAltitudeGain(getAltitudeGain_m());
+                altitudeSumManager.fill(trackPoint);
-                trackPoint.setAltitudeLoss(getAltitudeLoss_m());
                 altitudeSumManager.reset();
-            }
 
-            fillWithSensorDataSet(trackPoint);
+                remoteSensorManager.fill(trackPoint);
-            if (remoteSensorManager != null) {
                 remoteSensorManager.reset();
             }
+
             contentProviderUtils.insertTrackPoint(trackPoint, track.getId());
             trackStatisticsUpdater.addTrackPoint(trackPoint, recordingDistanceInterval);
             track.setTrackStatistics(trackStatisticsUpdater.getTrackStatistics());
@@ -674,44 +672,6 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
         voiceExecutor.update();
     }
 
-    private SensorDataSet fillWithSensorDataSet(TrackPoint trackPoint) {
-        if (remoteSensorManager == null) {
-            return null;
-        }
-
-        SensorDataSet sensorData = remoteSensorManager.getSensorData(); //TODO Should return a copy of SensorDataSet
-        if (sensorData == null) {
-            return null;
-        }
-
-        if (trackPoint != null) {
-            sensorData.fillTrackPoint(trackPoint);
-        }
-        return sensorData;
-    }
-
-    /**
-     * Returns the relative altitude gain (since last trackpoint).
-     */
-    private Float getAltitudeGain_m() {
-        if (altitudeSumManager == null || !altitudeSumManager.isConnected()) {
-            return null;
-        }
-
-        return altitudeSumManager.getAltitudeGain_m();
-    }
-
-    /**
-     * Returns the relative altitude loss (since last trackpoint).
-     */
-    private Float getAltitudeLoss_m() {
-        if (altitudeSumManager == null || !altitudeSumManager.isConnected()) {
-            return null;
-        }
-
-        return altitudeSumManager.getAltitudeLoss_m();
-    }
-
     private void showNotification(boolean isGpsStarted) {
         if (isRecording()) {
             Intent intent = IntentUtils.newIntent(this, TrackRecordingActivity.class)
@@ -741,13 +701,17 @@ 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(BluetoothRemoteSensorManager remoteSensorManager) {
+    public void setRemoteSensorManager(@NonNull BluetoothRemoteSensorManager remoteSensorManager) {
         this.remoteSensorManager = remoteSensorManager;
     }
 
+    // This is used to modify the state of this service while testing; must be called after startNewTrack().
+    @Deprecated
     @VisibleForTesting
-    public void setAltitudeSumManager(AltitudeSumManager altitudeSumManager) {
+    public void setAltitudeSumManager(@NonNull AltitudeSumManager altitudeSumManager) {
         this.altitudeSumManager = altitudeSumManager;
     }
 
@@ -778,15 +742,14 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand
             tmpLastTrackPoint.setLongitude(lastTrackPoint.getLongitude());
             tmpLastTrackPoint.setLatitude(lastTrackPoint.getLatitude());
         }
-        tmpLastTrackPoint.setAltitudeGain(getAltitudeGain_m());
-        tmpLastTrackPoint.setAltitudeLoss(getAltitudeLoss_m());
 
-        SensorDataSet sensorDataSet = fillWithSensorDataSet(tmpLastTrackPoint);
+        altitudeSumManager.fill(tmpLastTrackPoint);
+        remoteSensorManager.fill(tmpLastTrackPoint);
 
         tmpTrackStatisticsUpdater.addTrackPoint(tmpLastTrackPoint, recordingDistanceInterval);
         track.setTrackStatistics(tmpTrackStatisticsUpdater.getTrackStatistics());
 
-        recordingDataObservable.postValue(new RecordingData(track, tmpLastTrackPoint, sensorDataSet));
+        recordingDataObservable.postValue(new RecordingData(track, tmpLastTrackPoint, remoteSensorManager.getSensorDataSet()));
     }
 
     public LiveData<RecordingStatus> getRecordingStatusObservable() {

src/main/java/de/dennisguse/opentracks/services/sensors/AltitudeSumManager.java

@@ -7,9 +7,11 @@ import android.hardware.SensorEventListener;
 import android.hardware.SensorManager;
 import android.util.Log;
 
+import androidx.annotation.NonNull;
 import androidx.annotation.Nullable;
 import androidx.annotation.VisibleForTesting;
 
+import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.util.PressureSensorUtils;
 
 /**
@@ -62,6 +64,11 @@ public class AltitudeSumManager implements SensorEventListener {
         this.isConnected = isConnected;
     }
 
+    public void fill(@NonNull TrackPoint trackPoint) {
+        trackPoint.setAltitudeGain(altitudeGain_m);
+        trackPoint.setAltitudeLoss(altitudeLoss_m);
+    }
+
     public @Nullable
     Float getAltitudeGain_m() {
         return isConnected ? altitudeGain_m : null;

src/main/java/de/dennisguse/opentracks/services/sensors/BluetoothRemoteSensorManager.java

@@ -22,10 +22,13 @@ import android.content.Context;
 import android.content.SharedPreferences;
 import android.util.Log;
 
+import androidx.annotation.NonNull;
+
 import java.time.Duration;
 
 import de.dennisguse.opentracks.R;
 import de.dennisguse.opentracks.content.data.Distance;
+import de.dennisguse.opentracks.content.data.TrackPoint;
 import de.dennisguse.opentracks.content.sensor.SensorData;
 import de.dennisguse.opentracks.content.sensor.SensorDataCycling;
 import de.dennisguse.opentracks.content.sensor.SensorDataSet;
@@ -158,7 +161,12 @@ public class BluetoothRemoteSensorManager implements BluetoothConnectionManager.
         }
     }
 
-    public SensorDataSet getSensorData() {
+    public void fill(@NonNull TrackPoint trackPoint) {
+        sensorDataSet.fillTrackPoint(trackPoint);
+    }
+
+    public SensorDataSet getSensorDataSet() {
+        // TODO Should return a copy.
         return sensorDataSet;
     }