From 5f23c5306799fad3b49f5a8196c8eeb4b0ee774c Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Sat, 4 May 2024 20:11:19 +0200 Subject: [PATCH] Bugfix: moving time was not computed. Introduced in 6be9d2477cfa9f3c6a3d800b291d68717f6c49ba (first released in v4.9.0) Fixes #1756. --- .../stats/TrackStatisticsUpdaterTest.java | 63 ++++++++++++++++++- .../opentracks/data/models/TrackPoint.java | 4 ++ .../opentracks/stats/TrackStatistics.java | 3 + .../stats/TrackStatisticsUpdater.java | 22 +++---- 4 files changed, 78 insertions(+), 14 deletions(-) diff --git a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java index 59c9425a9..b52dcefbf 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java @@ -1,7 +1,9 @@ package de.dennisguse.opentracks.stats; import static org.junit.Assert.assertEquals; +import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNull; +import static org.junit.Assert.assertTrue; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -231,7 +233,7 @@ public void addTrackPoint_idle_withoutDistance() { )); // then - assertEquals(Duration.ofSeconds(40), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(35), subject.getTrackStatistics().getMovingTime()); } @Test @@ -261,10 +263,67 @@ public void addTrackPoint_idle_withDistance() { )); // then - assertEquals(Duration.ofSeconds(45), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(40), subject.getTrackStatistics().getMovingTime()); assertEquals(Distance.of(1040), subject.getTrackStatistics().getTotalDistance()); } + @Test + public void addTrackPoint_idle_remain_idle() { + TrackStatisticsUpdater subject = new TrackStatisticsUpdater(); + + // when + subject.addTrackPoint(new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.ofEpochSecond(0))); + subject.addTrackPoint( + new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(10)) + .setSensorDistance(Distance.of(10))); + + subject.addTrackPoint(new TrackPoint(TrackPoint.Type.IDLE, Instant.ofEpochSecond(30))); + // then + assertTrue(subject.getTrackStatistics().isIdle()); + assertEquals(Duration.ofSeconds(30), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(30), subject.getTrackStatistics().getTotalTime()); + assertEquals(Distance.of(10), subject.getTrackStatistics().getTotalDistance()); + + // when + subject.addTrackPoint( + new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.ofEpochSecond(40)) + .setSensorDistance(Distance.of(0))); + // then + assertTrue(subject.getTrackStatistics().isIdle()); + assertEquals(Duration.ofSeconds(30), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(40), subject.getTrackStatistics().getTotalTime()); + assertEquals(Distance.of(10), subject.getTrackStatistics().getTotalDistance()); + + // when + subject.addTrackPoint( + new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.ofEpochSecond(45)) + .setSensorDistance(Distance.of(1))); + // then + assertTrue(subject.getTrackStatistics().isIdle()); + assertEquals(Duration.ofSeconds(30), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(45), subject.getTrackStatistics().getTotalTime()); + assertEquals(Distance.of(11), subject.getTrackStatistics().getTotalDistance()); + + // when + subject.addTrackPoint( + new TrackPoint(TrackPoint.Type.TRACKPOINT, Instant.ofEpochSecond(50)) + .setSensorDistance(Distance.of(10))); + // then + assertFalse(subject.getTrackStatistics().isIdle()); + assertEquals(Duration.ofSeconds(30), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(50), subject.getTrackStatistics().getTotalTime()); + assertEquals(Distance.of(21), subject.getTrackStatistics().getTotalDistance()); + + // when + subject.addTrackPoint(new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL, Instant.ofEpochSecond(60))); + + // then + assertFalse(subject.getTrackStatistics().isIdle()); + assertEquals(Duration.ofSeconds(40), subject.getTrackStatistics().getMovingTime()); + assertEquals(Duration.ofSeconds(60), subject.getTrackStatistics().getTotalTime()); + assertEquals(Distance.of(21), subject.getTrackStatistics().getTotalDistance()); + } + @Test public void copy_constructor() { // given diff --git a/src/main/java/de/dennisguse/opentracks/data/models/TrackPoint.java b/src/main/java/de/dennisguse/opentracks/data/models/TrackPoint.java index 03eaf6c73..8e68fdc13 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/TrackPoint.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/TrackPoint.java @@ -135,6 +135,10 @@ public TrackPoint setType(@NonNull Type type) { return this; } + public boolean isIdleTriggered() { + return type == Type.IDLE; + } + public boolean isSegmentManualStart() { return type == Type.SEGMENT_START_MANUAL; } diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java index 976f9b618..015ee5db5 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatistics.java @@ -107,6 +107,7 @@ public TrackStatistics(String startTime, String stopTime, double totalDistance_m * * @param other another statistics data object */ + //TODO Should be refactored to append only [mainly due to isIdle] (NOTE: This requires to use a custom value object for AggregatedStatistics; this is anyhow recommended). public void merge(TrackStatistics other) { if (startTime == null) { startTime = other.startTime; @@ -119,6 +120,8 @@ public void merge(TrackStatistics other) { stopTime = stopTime.isAfter(other.stopTime) ? stopTime : other.stopTime; } + isIdle = other.isIdle; //TODO This implicitly assumes append mode. + if (avgHeartRate == null) { avgHeartRate = other.avgHeartRate; } else { diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index 36fb7fae0..4c6310480 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -26,6 +26,7 @@ import de.dennisguse.opentracks.data.models.Power; import de.dennisguse.opentracks.data.models.Speed; import de.dennisguse.opentracks.data.models.TrackPoint; +import de.dennisguse.opentracks.settings.PreferencesUtils; /** * Updater for {@link TrackStatistics}. @@ -56,11 +57,6 @@ public TrackStatisticsUpdater() { this(new TrackStatistics()); } - /** - * Creates a new{@link TrackStatisticsUpdater} with a {@link TrackStatisticsUpdater} already existed. - * - * @param trackStatistics a {@link TrackStatisticsUpdater} - */ public TrackStatisticsUpdater(TrackStatistics trackStatistics) { this.trackStatistics = trackStatistics; this.currentSegment = new TrackStatistics(); @@ -87,9 +83,6 @@ public void addTrackPoints(List trackPoints) { trackPoints.stream().forEachOrdered(this::addTrackPoint); } - /** - * - */ public void addTrackPoint(TrackPoint trackPoint) { if (trackPoint.isSegmentManualStart()) { reset(trackPoint); @@ -151,18 +144,23 @@ public void addTrackPoint(TrackPoint trackPoint) { movingDistance = trackPoint.distanceToPrevious(lastTrackPoint); } if (movingDistance != null) { - currentSegment.setIdle(false); currentSegment.addTotalDistance(movingDistance); } - if (!currentSegment.isIdle() && !trackPoint.isSegmentManualStart()) { - if (lastTrackPoint != null) { + if (!currentSegment.isIdle()) { + if (!trackPoint.isSegmentManualStart() && lastTrackPoint != null) { currentSegment.addMovingTime(trackPoint, lastTrackPoint); } } - if (trackPoint.getType() == TrackPoint.Type.IDLE) { + if (trackPoint.isIdleTriggered()) { currentSegment.setIdle(true); + } else if (currentSegment.isIdle()) { + // Shall we switch to non-idle? + if (movingDistance != null + && movingDistance.greaterOrEqualThan(PreferencesUtils.getRecordingDistanceInterval())) { + currentSegment.setIdle(false); + } } if (trackPoint.hasSpeed()) {