forked from upstream-mirrors/OpenTracks
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 dfdbc373ff
This commit is contained in:
+47
@@ -10,6 +10,7 @@ import android.os.Looper;
|
|||||||
import androidx.test.core.app.ApplicationProvider;
|
import androidx.test.core.app.ApplicationProvider;
|
||||||
import androidx.test.ext.junit.runners.AndroidJUnit4;
|
import androidx.test.ext.junit.runners.AndroidJUnit4;
|
||||||
import androidx.test.filters.MediumTest;
|
import androidx.test.filters.MediumTest;
|
||||||
|
import androidx.test.rule.GrantPermissionRule;
|
||||||
import androidx.test.rule.ServiceTestRule;
|
import androidx.test.rule.ServiceTestRule;
|
||||||
|
|
||||||
import org.junit.AfterClass;
|
import org.junit.AfterClass;
|
||||||
@@ -20,12 +21,14 @@ import org.junit.Test;
|
|||||||
import org.junit.runner.RunWith;
|
import org.junit.runner.RunWith;
|
||||||
import org.mockito.Mockito;
|
import org.mockito.Mockito;
|
||||||
|
|
||||||
|
import java.time.Duration;
|
||||||
import java.time.Instant;
|
import java.time.Instant;
|
||||||
import java.util.List;
|
import java.util.List;
|
||||||
import java.util.concurrent.TimeUnit;
|
import java.util.concurrent.TimeUnit;
|
||||||
import java.util.concurrent.TimeoutException;
|
import java.util.concurrent.TimeoutException;
|
||||||
|
|
||||||
import de.dennisguse.opentracks.R;
|
import de.dennisguse.opentracks.R;
|
||||||
|
import de.dennisguse.opentracks.TestUtil;
|
||||||
import de.dennisguse.opentracks.content.data.TestDataUtil;
|
import de.dennisguse.opentracks.content.data.TestDataUtil;
|
||||||
import de.dennisguse.opentracks.data.ContentProviderUtils;
|
import de.dennisguse.opentracks.data.ContentProviderUtils;
|
||||||
import de.dennisguse.opentracks.data.models.AltitudeGainLoss;
|
import de.dennisguse.opentracks.data.models.AltitudeGainLoss;
|
||||||
@@ -54,6 +57,9 @@ public class TrackRecordingServiceRecordingTest {
|
|||||||
@Rule
|
@Rule
|
||||||
public final ServiceTestRule mServiceRule = ServiceTestRule.withTimeout(5, TimeUnit.SECONDS);
|
public final ServiceTestRule mServiceRule = ServiceTestRule.withTimeout(5, TimeUnit.SECONDS);
|
||||||
|
|
||||||
|
@Rule
|
||||||
|
public GrantPermissionRule mGrantPermissionRule = TestUtil.createGrantPermissionRule();
|
||||||
|
|
||||||
private final Context context = ApplicationProvider.getApplicationContext();
|
private final Context context = ApplicationProvider.getApplicationContext();
|
||||||
private ContentProviderUtils contentProviderUtils;
|
private ContentProviderUtils contentProviderUtils;
|
||||||
|
|
||||||
@@ -127,6 +133,47 @@ public class TrackRecordingServiceRecordingTest {
|
|||||||
), TestDataUtil.getTrackPoints(contentProviderUtils, trackId));
|
), 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
|
@MediumTest
|
||||||
@Test
|
@Test
|
||||||
public void testRecording_startPauseResume() {
|
public void testRecording_startPauseResume() {
|
||||||
|
|||||||
@@ -44,7 +44,7 @@ public abstract class Aggregator<Input, Output> {
|
|||||||
|
|
||||||
public Output getValue() {
|
public Output getValue() {
|
||||||
if (!hasValue()) {
|
if (!hasValue()) {
|
||||||
return null;
|
return null; //TODO Check if this is a good idea!
|
||||||
}
|
}
|
||||||
if (isRecent()) {
|
if (isRecent()) {
|
||||||
return value;
|
return value;
|
||||||
|
|||||||
@@ -4,7 +4,10 @@ import android.location.Location;
|
|||||||
|
|
||||||
import androidx.annotation.NonNull;
|
import androidx.annotation.NonNull;
|
||||||
|
|
||||||
public class AggregatorGPS extends Aggregator<Location, Location> {
|
import java.util.Optional;
|
||||||
|
|
||||||
|
public class AggregatorGPS extends Aggregator<Location, Optional<Location>> {
|
||||||
|
|
||||||
|
|
||||||
public AggregatorGPS(String sensorAddress) {
|
public AggregatorGPS(String sensorAddress) {
|
||||||
super(sensorAddress);
|
super(sensorAddress);
|
||||||
@@ -12,7 +15,7 @@ public class AggregatorGPS extends Aggregator<Location, Location> {
|
|||||||
|
|
||||||
@Override
|
@Override
|
||||||
protected void computeValue(Raw<Location> current) {
|
protected void computeValue(Raw<Location> current) {
|
||||||
value = current.value();
|
value = Optional.of(current.value());
|
||||||
}
|
}
|
||||||
|
|
||||||
@Override
|
@Override
|
||||||
@@ -22,7 +25,7 @@ public class AggregatorGPS extends Aggregator<Location, Location> {
|
|||||||
|
|
||||||
@NonNull
|
@NonNull
|
||||||
@Override
|
@Override
|
||||||
protected Location getNoneValue() {
|
protected Optional<Location> getNoneValue() {
|
||||||
return new Location("none");
|
return Optional.empty();
|
||||||
}
|
}
|
||||||
}
|
}
|
||||||
|
|||||||
@@ -152,7 +152,8 @@ public final class SensorDataSet {
|
|||||||
|
|
||||||
public void fillTrackPoint(TrackPoint trackPoint) {
|
public void fillTrackPoint(TrackPoint trackPoint) {
|
||||||
if (gps != null && gps.hasValue()) {
|
if (gps != null && gps.hasValue()) {
|
||||||
trackPoint.setLocation(gps.getValue());
|
gps.getValue()
|
||||||
|
.ifPresent(trackPoint::setLocation);
|
||||||
}
|
}
|
||||||
|
|
||||||
if (getHeartRate() != null) {
|
if (getHeartRate() != null) {
|
||||||
|
|||||||
Reference in New Issue
Block a user