From 52a110d998059523fe0c7558a552fb27760365fb Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Thu, 7 Oct 2021 07:46:37 +0200 Subject: [PATCH] Bugfix: reset on SensorDataSet should only take place if the TrackPoint was stored Fixes #969. --- .../io/file/importer/TrackPointAssert.java | 4 +- .../TrackRecordingServiceTestLocation.java | 77 ++++++++++++++++++- .../opentracks/content/data/TrackPoint.java | 9 ++- .../content/sensor/SensorDataSet.java | 1 - .../services/TrackRecordingManager.java | 13 ++-- .../services/TrackRecordingService.java | 6 +- .../services/handlers/TrackPointCreator.java | 23 +++--- 7 files changed, 104 insertions(+), 29 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/TrackPointAssert.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/TrackPointAssert.java index 2bad0ea14..b4e41377e 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/TrackPointAssert.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/TrackPointAssert.java @@ -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()); diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java index be48cd6f0..4b4e38d5b 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestLocation.java @@ -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 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() { diff --git a/src/main/java/de/dennisguse/opentracks/content/data/TrackPoint.java b/src/main/java/de/dennisguse/opentracks/content/data/TrackPoint.java index 363e4886a..bbdfd535f 100644 --- a/src/main/java/de/dennisguse/opentracks/content/data/TrackPoint.java +++ b/src/main/java/de/dennisguse/opentracks/content/data/TrackPoint.java @@ -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 { diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataSet.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataSet.java index c28e2d2f6..b0b3eb3d1 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataSet.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataSet.java @@ -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(); } diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java index 2c2c16060..6f01fda47 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java @@ -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() { diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index db026fa71..05d456608 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -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 diff --git a/src/main/java/de/dennisguse/opentracks/services/handlers/TrackPointCreator.java b/src/main/java/de/dennisguse/opentracks/services/handlers/TrackPointCreator.java index 9e1ac2408..500040ad9 100644 --- a/src/main/java/de/dennisguse/opentracks/services/handlers/TrackPointCreator.java +++ b/src/main/java/de/dennisguse/opentracks/services/handlers/TrackPointCreator.java @@ -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); }