From e590e4e07e33ffce65a834310d49a79f42951e04 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Thu, 25 Apr 2024 22:51:51 +0200 Subject: [PATCH] Bugfix: overestimated distance. A combination of idle trackpoint and stale GPS data, resulted in recording lat=0.0 and lng=0.0; and thus inflating distance as well as GPS-based speed. Fixes #1898. Introduced in dfdbc373ff90c37c8fb5fab396f3c470eee65772 --- .../TrackRecordingServiceRecordingTest.java | 47 +++++++++++++++++++ .../sensors/sensorData/Aggregator.java | 2 +- .../sensors/sensorData/AggregatorGPS.java | 11 +++-- .../sensors/sensorData/SensorDataSet.java | 3 +- 4 files changed, 57 insertions(+), 6 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java index 09d041464..831ac6622 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java @@ -10,6 +10,7 @@ import android.os.Looper; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; import androidx.test.filters.MediumTest; +import androidx.test.rule.GrantPermissionRule; import androidx.test.rule.ServiceTestRule; import org.junit.AfterClass; @@ -20,12 +21,14 @@ import org.junit.Test; import org.junit.runner.RunWith; import org.mockito.Mockito; +import java.time.Duration; import java.time.Instant; import java.util.List; import java.util.concurrent.TimeUnit; import java.util.concurrent.TimeoutException; import de.dennisguse.opentracks.R; +import de.dennisguse.opentracks.TestUtil; import de.dennisguse.opentracks.content.data.TestDataUtil; import de.dennisguse.opentracks.data.ContentProviderUtils; import de.dennisguse.opentracks.data.models.AltitudeGainLoss; @@ -54,6 +57,9 @@ public class TrackRecordingServiceRecordingTest { @Rule public final ServiceTestRule mServiceRule = ServiceTestRule.withTimeout(5, TimeUnit.SECONDS); + @Rule + public GrantPermissionRule mGrantPermissionRule = TestUtil.createGrantPermissionRule(); + private final Context context = ApplicationProvider.getApplicationContext(); private ContentProviderUtils contentProviderUtils; @@ -127,6 +133,47 @@ public class TrackRecordingServiceRecordingTest { ), TestDataUtil.getTrackPoints(contentProviderUtils, trackId)); } + + /** + * Test that an IDLE event, doesn't store invalid GPS-provided data. + */ + @MediumTest + @Test + public void recording_startIdle() throws InterruptedException { + + // given + TrackPointCreator trackPointCreator = service.getTrackPointCreator(); + String startTime = "2020-02-02T02:02:02Z"; + trackPointCreator.setClock(startTime); + Track.Id trackId = service.startNewTrack(); + String gps1 = "2020-02-02T02:02:03Z"; + TrackRecordingServiceTestUtils.sendGPSLocation(trackPointCreator, gps1, 45.0, 35.0, 1, 15); + String gps2 = "2020-02-02T02:02:04Z"; + + TrackRecordingServiceTestUtils.sendGPSLocation(trackPointCreator, gps2, 45.0, 35.0, 1, 15); + + // when + String idleTime = "2020-02-02T02:02:17Z"; + trackPointCreator.setClock("2020-02-02T02:02:17Z"); + Thread.sleep(Duration.ofSeconds(15).toMillis()); + + // then + new TrackPointAssert().assertEquals(List.of( + new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.parse(startTime)), + new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.parse(gps1)) + .setLatitude(45) + .setLongitude(35) + .setHorizontalAccuracy(Distance.of(1)) + .setSpeed(Speed.of(15)), + new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.parse(gps2)) + .setLatitude(45) + .setLongitude(35) + .setHorizontalAccuracy(Distance.of(1)) + .setSpeed(Speed.of(15)), + new TrackPoint(TrackPoint.Type.IDLE, Instant.parse(idleTime)) + ), TestDataUtil.getTrackPoints(contentProviderUtils, trackId)); + } + @MediumTest @Test public void testRecording_startPauseResume() { diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java index 91be573e1..0858226eb 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java @@ -44,7 +44,7 @@ public abstract class Aggregator { public Output getValue() { if (!hasValue()) { - return null; + return null; //TODO Check if this is a good idea! } if (isRecent()) { return value; diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorGPS.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorGPS.java index 71c2aca87..6463d9c6c 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorGPS.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorGPS.java @@ -4,7 +4,10 @@ import android.location.Location; import androidx.annotation.NonNull; -public class AggregatorGPS extends Aggregator { +import java.util.Optional; + +public class AggregatorGPS extends Aggregator> { + public AggregatorGPS(String sensorAddress) { super(sensorAddress); @@ -12,7 +15,7 @@ public class AggregatorGPS extends Aggregator { @Override protected void computeValue(Raw current) { - value = current.value(); + value = Optional.of(current.value()); } @Override @@ -22,7 +25,7 @@ public class AggregatorGPS extends Aggregator { @NonNull @Override - protected Location getNoneValue() { - return new Location("none"); + protected Optional getNoneValue() { + return Optional.empty(); } } diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataSet.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataSet.java index 8b423fdba..626442cdd 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataSet.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataSet.java @@ -152,7 +152,8 @@ public final class SensorDataSet { public void fillTrackPoint(TrackPoint trackPoint) { if (gps != null && gps.hasValue()) { - trackPoint.setLocation(gps.getValue()); + gps.getValue() + .ifPresent(trackPoint::setLocation); } if (getHeartRate() != null) {