From 86a0054c243a47e11990e23b1a643187957c00b2 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Fri, 16 Jul 2021 00:03:31 +0200 Subject: [PATCH] Bugfix: TrackStatistics' total time was not restored correctly, when having pause/stop and then resume. --- .../io/file/importer/ExportImportTest.java | 145 +++++++----------- .../services/TrackRecordingService.java | 5 +- .../opentracks/stats/TrackStatistics.java | 4 + .../stats/TrackStatisticsUpdater.java | 16 +- 4 files changed, 69 insertions(+), 101 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java index c7a5f5a03..8e6d79faa 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/ExportImportTest.java @@ -24,6 +24,7 @@ import java.io.File; import java.io.IOException; import java.io.InputStream; import java.time.Clock; +import java.time.Duration; import java.time.Instant; import java.time.ZoneId; import java.util.ArrayList; @@ -50,7 +51,6 @@ import de.dennisguse.opentracks.stats.TrackStatistics; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertNotNull; import static org.junit.Assert.assertNull; -import static org.junit.Assert.assertTrue; /** * Export a track to {@link TrackFileFormat} and verify that the import is identical. @@ -161,47 +161,6 @@ public class ExportImportTest { } } - @LargeTest - @Test - public void kml_with_trackdetail_and_sensordata() throws TimeoutException, IOException { - setUp(true); - - // given - Track track = contentProviderUtils.getTrack(trackId); - - TrackExporter trackExporter = TrackFileFormat.KML_WITH_TRACKDETAIL_AND_SENSORDATA.createTrackExporter(context); - - // when - // 1. export - trackExporter.writeTrack(track, context.getContentResolver().openOutputStream(tmpFileUri)); - contentProviderUtils.deleteTrack(context, trackId); - - // 2. import - InputStream inputStream = context.getContentResolver().openInputStream(tmpFileUri); - XMLImporter importer = new XMLImporter(new KmlTrackImporter(context, trackImporter)); - importTrackId = importer.importFile(inputStream).get(0); - - // then - // 1. track - Track importedTrack = contentProviderUtils.getTrack(importTrackId); - assertNotNull(importedTrack); - assertEquals(track.getCategory(), importedTrack.getCategory()); - assertEquals(track.getDescription(), importedTrack.getDescription()); - assertEquals(track.getName(), importedTrack.getName()); - assertEquals(track.getIcon(), importedTrack.getIcon()); - - // 2. trackpoints - TrackPointAssert a = new TrackPointAssert() - .noAccuracy(); - a.assertEquals(trackPoints, TestDataUtil.getTrackPoints(contentProviderUtils, importTrackId)); - - // 2. trackstatistics - assertTrackStatistics(false, true); - - // 4. markers - assertMarkers(); - } - //TODO Does not test images @LargeTest @Test @@ -236,8 +195,30 @@ public class ExportImportTest { .noAccuracy(); a.assertEquals(trackPoints, TestDataUtil.getTrackPoints(contentProviderUtils, importTrackId)); - // 2. trackstatistics - assertTrackStatistics(false, true); + // 3. trackstatistics + TrackStatistics importedTrackStatistics = importedTrack.getTrackStatistics(); + + // Time + assertEquals(Instant.parse("2020-02-02T02:02:02Z"), importedTrackStatistics.getStartTime()); + assertEquals(Instant.parse("2020-02-02T02:02:24Z"), importedTrackStatistics.getStopTime()); + + assertEquals(track.getTrackStatistics().getTotalTime(), importedTrackStatistics.getTotalTime()); + assertEquals(Duration.ofSeconds(8), importedTrackStatistics.getTotalTime()); + assertEquals(Duration.ofSeconds(4), importedTrackStatistics.getMovingTime()); + + // Distance + assertEquals(Distance.of(30), importedTrackStatistics.getTotalDistance()); + + // Speed + assertEquals(Speed.of(15), importedTrackStatistics.getMaxSpeed()); + assertEquals(Speed.of(3.75), importedTrackStatistics.getAverageSpeed()); + assertEquals(Speed.of(7.5), importedTrackStatistics.getAverageMovingSpeed()); + + // Altitude + assertEquals(10, importedTrackStatistics.getMinAltitude(), 0.01); + assertEquals(10, importedTrackStatistics.getMaxAltitude(), 0.01); + assertEquals(1, importedTrackStatistics.getTotalAltitudeGain(), 0.01); + assertEquals(1, importedTrackStatistics.getTotalAltitudeLoss(), 0.01); // 4. markers assertMarkers(); @@ -291,14 +272,14 @@ public class ExportImportTest { // then // 1. track - Track trackImported = contentProviderUtils.getTrack(importTrackId); - assertNotNull(trackImported); - assertEquals(track.getCategory(), trackImported.getCategory()); - assertEquals(track.getDescription(), trackImported.getDescription()); - assertEquals(track.getName(), trackImported.getName()); + Track importedTrack = contentProviderUtils.getTrack(importTrackId); + assertNotNull(importedTrack); + assertEquals(track.getCategory(), importedTrack.getCategory()); + assertEquals(track.getDescription(), importedTrack.getDescription()); + assertEquals(track.getName(), importedTrack.getName()); //TODO exporting and importing a track icon is not yet supported by GpxTrackWriter. - //assertEquals(track.getIcon(), trackImported.getIcon()); + //assertEquals(track.getIcon(), importedTrack.getIcon()); // 2. trackpoints // The GPX exporter does not support exporting TrackPoints without lat/lng. @@ -313,7 +294,29 @@ public class ExportImportTest { a.assertEquals(trackPointsWithCoordinates, TestDataUtil.getTrackPoints(contentProviderUtils, importTrackId)); // 3. trackstatistics - assertTrackStatistics(true, false); + TrackStatistics trackStatistics = track.getTrackStatistics(); + TrackStatistics importedTrackStatistics = importedTrack.getTrackStatistics(); + + // Time + assertEquals(Instant.parse("2020-02-02T02:02:03Z"), importedTrackStatistics.getStartTime()); + assertEquals(Instant.parse("2020-02-02T02:02:23Z"), importedTrackStatistics.getStopTime()); + + assertEquals(Duration.ofSeconds(20), importedTrackStatistics.getTotalTime()); + assertEquals(Duration.ofSeconds(19), importedTrackStatistics.getMovingTime()); + + // Distance + assertEquals(Distance.of(30), importedTrackStatistics.getTotalDistance()); + + // Speed + assertEquals(Speed.of(15), importedTrackStatistics.getMaxSpeed()); + assertEquals(Speed.of(1.5), importedTrackStatistics.getAverageSpeed()); + assertEquals(Speed.of(1.5789473684210527), importedTrackStatistics.getAverageMovingSpeed()); + + // Altitude + assertEquals(10, importedTrackStatistics.getMinAltitude(), 0.01); + assertEquals(10, importedTrackStatistics.getMaxAltitude(), 0.01); + assertEquals(1, importedTrackStatistics.getTotalAltitudeGain(), 0.01); + assertEquals(1, importedTrackStatistics.getTotalAltitudeLoss(), 0.01); // 4. markers assertMarkers(); @@ -366,46 +369,6 @@ public class ExportImportTest { } } - private void assertTrackStatistics(boolean isGpx, boolean verifyDistance) { - double delta = isGpx ? 0.1 : 0.01; - Track importedTrack = contentProviderUtils.getTrack(importTrackId); - - assertNotNull(importedTrack.getTrackStatistics()); - - TrackStatistics trackStatistics = track.getTrackStatistics(); - TrackStatistics importedTrackStatistics = importedTrack.getTrackStatistics(); - - // Time - assertTrue(trackStatistics.getStartTime().isBefore(trackStatistics.getStopTime())); //Just to be sure. - if (!isGpx) { - assertEquals(trackStatistics.getStartTime(), importedTrackStatistics.getStartTime()); - assertEquals(trackStatistics.getStopTime(), importedTrackStatistics.getStopTime()); - - assertEquals(trackStatistics.getTotalTime(), importedTrackStatistics.getTotalTime()); - assertEquals(trackStatistics.getMovingTime(), importedTrackStatistics.getMovingTime()); - - // Distance - if (verifyDistance) { - assertEquals(trackStatistics.getTotalDistance(), importedTrackStatistics.getTotalDistance()); - } - - // Speed - assertEquals(trackStatistics.getMaxSpeed(), importedTrackStatistics.getMaxSpeed()); - assertEquals(trackStatistics.getAverageSpeed(), importedTrackStatistics.getAverageSpeed()); - assertEquals(trackStatistics.getAverageMovingSpeed(), importedTrackStatistics.getAverageMovingSpeed()); - } - - // Altitude - assertEquals(trackStatistics.getMinAltitude(), importedTrackStatistics.getMinAltitude(), delta); - if (isGpx) { - assertEquals(trackStatistics.getMaxAltitude(), importedTrackStatistics.getMaxAltitude(), 2); - } else { - assertEquals(trackStatistics.getMaxAltitude(), importedTrackStatistics.getMaxAltitude(), delta); - } - assertEquals(trackStatistics.getTotalAltitudeGain(), importedTrackStatistics.getTotalAltitudeGain(), delta); - assertEquals(trackStatistics.getTotalAltitudeLoss(), importedTrackStatistics.getTotalAltitudeLoss(), delta); - } - private static TrackPoint createTrackPoint(Instant time, double latitude, double longitude, float accuracy, float speed, float altitude, float altitudeGain, float heartRate, float cyclingCadence, float power, Distance distance) { TrackPoint tp = new TrackPoint(latitude, longitude, Altitude.WGS84.of(altitude), time); tp.setHorizontalAccuracy(Distance.of(accuracy)); diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index e3593019e..ff8e7b843 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -336,9 +336,8 @@ public class TrackRecordingService extends Service implements HandlerServer.Hand // Update database Track track = contentProviderUtils.getTrack(getRecordingTrackId()); - if (track != null) { - insertTrackPoint(track, handlerServer.createSegmentStartManual()); - } + trackStatisticsUpdater = new TrackStatisticsUpdater(track.getTrackStatistics()); + insertTrackPoint(track, handlerServer.createSegmentStartManual()); startRecording(); } diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java index 66e0d4e12..112284bf6 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java @@ -121,6 +121,10 @@ public class TrackStatistics { } } + public boolean isInitialized() { + return startTime != null; + } + public void reset() { startTime = null; stopTime = null; diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index 9014bf75e..38fd4142a 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -116,7 +116,12 @@ public class TrackStatisticsUpdater { * @param minGPSDistance the min recording distance */ public void addTrackPoint(TrackPoint trackPoint, Distance minGPSDistance) { - if (currentSegment.getStartTime() == null) { + if (trackPoint.getType() == TrackPoint.Type.SEGMENT_START_MANUAL) { + reset(trackPoint); + return; + } + + if (!currentSegment.isInitialized()) { currentSegment.setStartTime(trackPoint.getTime()); } @@ -124,11 +129,6 @@ public class TrackStatisticsUpdater { currentSegment.setStopTime(trackPoint.getTime()); currentSegment.setTotalTime(Duration.between(currentSegment.getStartTime(), trackPoint.getTime())); - if (trackPoint.getType() == TrackPoint.Type.SEGMENT_START_MANUAL) { - reset(trackPoint); - return; - } - // Process sensor data if (trackPoint.hasAltitudeGain()) { currentSegment.addTotalAltitudeGain(trackPoint.getAltitudeGain()); @@ -195,7 +195,9 @@ public class TrackStatisticsUpdater { } private void reset(TrackPoint trackPoint) { - trackStatistics.merge(currentSegment); + if (currentSegment.isInitialized()) { + trackStatistics.merge(currentSegment); + } currentSegment.reset(trackPoint.getTime()); lastTrackPoint = null;