Bugfix: reset on SensorDataSet should only take place if the TrackPoint was stored

Fixes #969.
2021-10-07 07:46:37 +02:00
parent 12e18b0bcd
commit 52a110d998
7 changed files with 104 additions and 29 deletions
@@ -83,14 +83,14 @@ public class TrackPointAssert {
        try {
            Assert.assertEquals(expected.size(), actual.size());
        } catch (AssertionError e) {
-            throw new AssertionError("Expected: " + expected + " actual: " + actual);
+            throw new AssertionError("Expected: " + expected + "\n actual: " + actual);
        }

        for (int i = 0; i < expected.size(); i++) {
            try {
                assertEquals(expected.get(i), actual.get(i));
            } catch (AssertionError e) {
-                throw new AssertionError("Expected: " + expected.get(i) + " actual: " + actual.get(i), e);
+                throw new AssertionError("Expected: " + expected.get(i) + "\n actual: " + actual.get(i), e);
            }
        }
        Assert.assertEquals(expected.size(), actual.size());
@@ -1,5 +1,7 @@
 package de.dennisguse.opentracks.services;

+import static org.junit.Assert.assertFalse;
+
 import android.content.ContentProvider;
 import android.content.Context;
 import android.os.Looper;
@@ -31,14 +33,13 @@ 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.content.sensor.SensorDataHeartRate;
+import de.dennisguse.opentracks.content.sensor.SensorDataRunning;
 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.settings.PreferencesUtils;

-import static org.junit.Assert.assertFalse;
-
 /**
 * Tests insert location.
 */
@@ -425,6 +426,78 @@ public class TrackRecordingServiceTestLocation {
        ), trackPoints);
    }

+    @MediumTest
+    @Test
+    public void testOnLocationChangedAsync_idle_withSensorDistance() {
+        BluetoothRemoteSensorManager remoteSensorManager = new BluetoothRemoteSensorManager(context) {
+
+            @Override
+            public boolean isEnabled() {
+                return true;
+            }
+        };
+
+        // given
+        Track.Id trackId = service.startNewTrack();
+        service.getTrackPointCreator().setRemoteSensorManager(remoteSensorManager);
+        service.getTrackPointCreator().setAltitudeSumManager(altitudeSumManager);
+
+        // when
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(0)));
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(2)));
+        TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 1, 15);
+
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(12)));
+        TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 2, 15);
+
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(13)));
+        TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 3, 15);
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(14)));
+        TrackRecordingServiceTest.newTrackPoint(service, 45.0, 35.0, 4, 15);
+
+        remoteSensorManager.onChanged(new SensorDataRunning("", "", Speed.of(5), null, Distance.of(16)));
+        service.endCurrentTrack();
+
+        // then
+        assertFalse(service.isRecording());
+
+        List<TrackPoint> trackPoints = TestDataUtil.getTrackPoints(contentProviderUtils, trackId);
+        TrackPointAssert a = new TrackPointAssert()
+                .ignoreTime();
+        a.assertEquals(List.of(
+                new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, null),
+                new TrackPoint(TrackPoint.Type.TRACKPOINT, null)
+                        .setLatitude(45)
+                        .setLongitude(35)
+                        .setHorizontalAccuracy(Distance.of(1))
+                        .setSpeed(Speed.of(5))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
+                        .setSensorDistance(Distance.of(2)),
+                new TrackPoint(TrackPoint.Type.TRACKPOINT, null)
+                        .setLatitude(45)
+                        .setLongitude(35)
+                        .setHorizontalAccuracy(Distance.of(2))
+                        .setSpeed(Speed.of(5))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
+                        .setSensorDistance(Distance.of(10)),
+                new TrackPoint(TrackPoint.Type.TRACKPOINT, null)
+                        .setLatitude(45)
+                        .setLongitude(35)
+                        .setHorizontalAccuracy(Distance.of(4))
+                        .setSpeed(Speed.of(5))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
+                        .setSensorDistance(Distance.of(2)),
+                new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL, null)
+                        .setSensorDistance(Distance.of(11))
+                        .setAltitudeGain(0f)
+                        .setAltitudeLoss(0f)
+                        .setSensorDistance(Distance.of(2))
+        ), trackPoints);
+    }
+
    @MediumTest
    @Test
    public void testOnLocationChangedAsync_segment() {
@@ -400,11 +400,14 @@ public class TrackPoint {
            return result;
        }
        result += ": lat=" + getLatitude() + " lng=" + getLongitude();
-        if (!hasHorizontalAccuracy()) {
-            return result;
+        if (hasHorizontalAccuracy()) {
+            result += " acc=" + getHorizontalAccuracy();
+        }
+        if (hasSensorDistance()) {
+            result += " distance=" + getSensorDistance();
        }

-        return result + " acc=" + getHorizontalAccuracy();
+        return result;
    }

    public static class Id {
@@ -93,7 +93,6 @@ public final class SensorDataSet {
        if (cyclingCadence != null) cyclingCadence.reset();
        if (cyclingDistanceSpeed != null) cyclingDistanceSpeed.reset();
        if (cyclingPower != null) cyclingPower.reset();
-
        if (runningDistanceSpeedCadence != null) runningDistanceSpeedCadence.reset();
    }

@@ -149,7 +149,7 @@ class TrackRecordingManager {
        return new Marker.Id(ContentUris.parseId(uri));
    }

-    void onNewTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy) {
+    boolean onNewTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy) {
        //TODO Figure out how to avoid loading the lastValidTrackPoint from the database
        TrackPoint lastValidTrackPoint = getLastValidTrackPointInCurrentSegment(trackId);

@@ -159,7 +159,7 @@ class TrackRecordingManager {
        if (!currentSegmentHasTrackPoint()) {
            insertTrackPoint(trackId, trackPoint);
            lastTrackPoint = trackPoint;
-            return;
+            return true;
        }

        Distance distanceToLastTrackLocation = trackPoint.distanceToPrevious(lastValidTrackPoint);
@@ -173,7 +173,7 @@ class TrackRecordingManager {
                isIdle = false;

                lastTrackPoint = trackPoint;
-                return;
+                return true;
            }

            if (trackPoint.hasSensorData() || distanceToLastTrackLocation.greaterOrEqualThan(recordingDistanceInterval)) {
@@ -184,7 +184,7 @@ class TrackRecordingManager {
                isIdle = false;

                lastTrackPoint = trackPoint;
-                return;
+                return true;
            }
        }

@@ -196,7 +196,7 @@ class TrackRecordingManager {
            isIdle = true;

            lastTrackPoint = trackPoint;
-            return;
+            return true;
        }

        if (isIdle && trackPoint.isMoving()) {
@@ -207,11 +207,12 @@ class TrackRecordingManager {
            isIdle = false;

            lastTrackPoint = trackPoint;
-            return;
+            return true;
        }

        Log.d(TAG, "Not recording TrackPoint, idle");
        lastTrackPoint = trackPoint;
+        return false;
    }

    Track getTrack() {
@@ -355,14 +355,14 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
    }

    @Override
-    public void newTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy) {
+    public boolean newTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy) {
        if (!isRecording() || isPaused()) {
            Log.w(TAG, "Ignore newTrackPoint. Not recording or paused.");
-            return;
+            return false;
        }

-        trackRecordingManager.onNewTrackPoint(trackPoint, thresholdHorizontalAccuracy);
        notificationManager.updateTrackPoint(this, trackRecordingManager.getTrackStatistics(), trackPoint, thresholdHorizontalAccuracy);
+        return trackRecordingManager.onNewTrackPoint(trackPoint, thresholdHorizontalAccuracy);
    }

    @Override
@@ -83,14 +83,6 @@ public class TrackPointCreator {
        return sensorDataSet;
    }

-    public SensorDataSet fillAndReset(TrackPoint trackPoint) {
-        SensorDataSet sensorDataSet = fill(trackPoint);
-        resetSensorData();
-
-        return sensorDataSet;
-    }
-
-
    public void stop() {
        locationHandler.onStop();

@@ -112,9 +104,12 @@ public class TrackPointCreator {
    }

    public void onNewTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy) {
-        fillAndReset(trackPoint);
+        fill(trackPoint);

-        service.newTrackPoint(trackPoint, thresholdHorizontalAccuracy);
+        boolean stored = service.newTrackPoint(trackPoint, thresholdHorizontalAccuracy);
+        if (stored) {
+            resetSensorData();
+        }
    }

    public TrackPoint createSegmentStartManual() {
@@ -123,7 +118,8 @@ public class TrackPointCreator {

    public TrackPoint createSegmentEnd() {
        TrackPoint segmentEnd = TrackPoint.createSegmentEndWithTime(createNow());
-        fillAndReset(segmentEnd);
+        fill(segmentEnd);
+        resetSensorData();
        return segmentEnd;
    }

@@ -181,7 +177,10 @@ public class TrackPointCreator {
    }

    public interface Callback {
-        void newTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy);
+        /**
+         * @return Was TrackPoint stored (not discarded)?
+         */
+        boolean newTrackPoint(TrackPoint trackPoint, Distance thresholdHorizontalAccuracy);

        void newGpsStatus(GpsStatusValue gpsStatusValue);
    }