diff --git a/src/androidTest/java/de/dennisguse/opentracks/data/models/StatisticsTest.java b/src/androidTest/java/de/dennisguse/opentracks/data/models/StatisticsTest.java new file mode 100644 index 000000000..89a1871a5 --- /dev/null +++ b/src/androidTest/java/de/dennisguse/opentracks/data/models/StatisticsTest.java @@ -0,0 +1,65 @@ +package de.dennisguse.opentracks.data.models; + +import static org.junit.Assert.assertEquals; + +import androidx.test.ext.junit.runners.AndroidJUnit4; + +import org.junit.Test; +import org.junit.runner.RunWith; + +import java.time.Duration; +import java.time.Instant; + +@RunWith(AndroidJUnit4.class) +public class StatisticsTest { + + @Test + public void testMerge() { + // given + Statistics subject = new Statistics( + Instant.ofEpochMilli(1000), + Instant.ofEpochMilli(2500), + Duration.ofMillis(1500), + Duration.ofMillis(700), + Distance.of(750.0), + Speed.of(60.0), + new AltitudeExtremities(1200, 1250), + new AltitudeGainLoss(50, 0), + HeartRate.of(100), + null + ); + + Statistics append = new Statistics( + Instant.ofEpochMilli(3000), + Instant.ofEpochMilli(4000), + Duration.ofMillis(1000), + Duration.ofMillis(600), + Distance.of(350.0), + Speed.of(30.0), + new AltitudeExtremities(2800.0, 3575.0), + new AltitudeGainLoss(850, 0), + HeartRate.of(200), + null + ); + + // when + Statistics result = subject.merge(append); + + // then + assertEquals( + new Statistics( + Instant.ofEpochMilli(1000), + Instant.ofEpochMilli(4000), + Duration.ofMillis(2500), + Duration.ofMillis(1300), + Distance.of(1100), + Speed.of(60), + new AltitudeExtremities(1200, 3575), + new AltitudeGainLoss(900, 0), + HeartRate.of(150), + null + ) + , result + ); + } +} \ No newline at end of file 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 2c1d9d370..f1ad9a91b 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 @@ -216,7 +216,7 @@ public class ExportImportTest { assertEquals(222049.34375, trackStatistics.totalDistance().toM(), 0.01); //TODO Too low // Speed - assertEquals(8540.359, trackStatistics.maxSpeed().toMPS(), 0.01); + assertEquals(22203.7421875, trackStatistics.maxSpeed().toMPS(), 0.01); assertEquals(3965.166, trackStatistics.getAverageSpeed().toMPS(), 0.01); assertEquals(8540.359, trackStatistics.getAverageMovingSpeed().toMPS(), 0.01); @@ -340,7 +340,7 @@ public class ExportImportTest { assertEquals(222049.421, importedTrackStatistics.totalDistance().toM(), 0.01); // Speed - assertEquals(8540.362, importedTrackStatistics.maxSpeed().toMPS(), 0.01); + assertEquals(22203.7421875, importedTrackStatistics.maxSpeed().toMPS(), 0.01); assertEquals(3965.168, importedTrackStatistics.getAverageSpeed().toMPS(), 0.01); assertEquals(8540.362, importedTrackStatistics.getAverageMovingSpeed().toMPS(), 0.01); diff --git a/src/androidTest/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdaterTest.java b/src/androidTest/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdaterTest.java deleted file mode 100644 index e90ffe442..000000000 --- a/src/androidTest/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdaterTest.java +++ /dev/null @@ -1,127 +0,0 @@ -/* - * Copyright 2010 Google Inc. - * - * Licensed under the Apache License, Version 2.0 (the "License"); you may not - * use this file except in compliance with the License. You may obtain a copy of - * the License at - * - * http://www.apache.org/licenses/LICENSE-2.0 - * - * Unless required by applicable law or agreed to in writing, software - * distributed under the License is distributed on an "AS IS" BASIS, WITHOUT - * WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. See the - * License for the specific language governing permissions and limitations under - * the License. - */ -package de.dennisguse.opentracks.stats; - -import static org.junit.Assert.assertEquals; - -import androidx.test.ext.junit.runners.AndroidJUnit4; - -import org.junit.Test; -import org.junit.runner.RunWith; - -import java.time.Duration; -import java.time.Instant; - -import de.dennisguse.opentracks.data.models.AltitudeExtremities; -import de.dennisguse.opentracks.data.models.AltitudeGainLoss; -import de.dennisguse.opentracks.data.models.Distance; -import de.dennisguse.opentracks.data.models.HeartRate; -import de.dennisguse.opentracks.data.models.Speed; -import de.dennisguse.opentracks.data.models.Statistics; - -/** - * Tests for {@link SegmentStatisticUpdater}. - * This only tests non-trivial pieces of that class. - * - * @author Rodrigo Damazio - */ -@RunWith(AndroidJUnit4.class) -public class SegmentStatisticUpdaterTest { - - @Deprecated //TODO SegmentStatisticsUpdater should always have data, right? - @Test - public void testMerge_no_data() { - // given - SegmentStatisticUpdater subject = new SegmentStatisticUpdater(); - SegmentStatisticUpdater append = new SegmentStatisticUpdater(); - - // when - subject.merge(append); - Statistics result = subject.getStatistics(); - - // then - assertEquals( - new Statistics( - null, - null, - Duration.ZERO, - Duration.ZERO, - Distance.ZERO, - Speed.ZERO, - null, - null, - null, - null - ), - result - ); - } - - @Test - public void testMerge() { - // given - SegmentStatisticUpdater subject = new SegmentStatisticUpdater( - new Statistics( - Instant.ofEpochMilli(1000), - Instant.ofEpochMilli(2500), - Duration.ofMillis(1500), - Duration.ofMillis(700), - Distance.of(750.0), - Speed.of(60.0), - new AltitudeExtremities(1200, 1250), - new AltitudeGainLoss(50, 0), - HeartRate.of(100), - null - ) - ); - - SegmentStatisticUpdater append = new SegmentStatisticUpdater( - new Statistics( - Instant.ofEpochMilli(3000), - Instant.ofEpochMilli(4000), - Duration.ofMillis(1000), - Duration.ofMillis(600), - Distance.of(350.0), - Speed.of(30.0), - new AltitudeExtremities(2800.0, 3575.0), - new AltitudeGainLoss(850, 0), - HeartRate.of(200), - null - ) - ); - - // when - subject.merge(append); - Statistics result = subject.getStatistics(); - - // then - assertEquals( - new Statistics( - Instant.ofEpochMilli(1000), - Instant.ofEpochMilli(4000), - Duration.ofMillis(2500), - Duration.ofMillis(1300), - Distance.of(1100), - Speed.of(846.1538461538461), //TODO Check why this is not 60 - new AltitudeExtremities(1200, 3575), - new AltitudeGainLoss(900, 0), - HeartRate.of(150), - null - ) - , result - ); - } -} diff --git a/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java b/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java index 20134cae3..ce4d00863 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/Statistics.java @@ -1,9 +1,11 @@ package de.dennisguse.opentracks.data.models; +import androidx.annotation.NonNull; import androidx.annotation.Nullable; import java.time.Duration; import java.time.Instant; +import java.util.Objects; //TODO Add @NonNull to attributes public record Statistics( @@ -54,4 +56,73 @@ public record Statistics( if (altitudeExtremities == null) return 0; return altitudeExtremities.max_m(); } + + /** + * Combines these statistics with those from another object. + * This assumes that the time periods covered by each do not intersect. + */ + //TODO Should be refactored to append only + @NonNull + public Statistics merge(@NonNull Statistics other) { + HeartRate newAvgHeartRate = avgHeartRate; + if (avgHeartRate == null) { + newAvgHeartRate = other.avgHeartRate; + } else { + if (other.avgHeartRate != null) { + // Using total time as weights for the averaging. + // Important to do this before total time is updated + newAvgHeartRate = HeartRate.of( + (totalDuration.getSeconds() * avgHeartRate.getBPM() + other.totalDuration.getSeconds() * other.avgHeartRate.getBPM()) + / (totalDuration.getSeconds() + other.totalDuration.getSeconds()) + ); + } + } + + Power newAvgPower = avgPower; + if (avgPower == null) { + newAvgPower = other.avgPower; + } else { + if (other.avgPower != null) { + // Using total time as weights for the averaging. + // Important to do this before total time is updated + newAvgPower = Power.of( + (totalDuration.getSeconds() * avgPower.getW() + other.totalDuration.getSeconds() * other.avgPower.getW()) + / (totalDuration.getSeconds() + other.totalDuration.getSeconds()) + ); + } + } + + AltitudeExtremities newAltitudeExtremities; + if (altitudeExtremities != null && other.altitudeExtremities != null) { + newAltitudeExtremities = new AltitudeExtremities( + Math.min(altitudeExtremities.min_m(), other.altitudeExtremities.min_m()), + Math.max(altitudeExtremities.max_m(), other.altitudeExtremities.max_m()) + ); + } else { + newAltitudeExtremities = altitudeExtremities != null ? altitudeExtremities : other.altitudeExtremities; + } + + AltitudeGainLoss newAltitudeGainLoss; + if (altitudeGainLoss != null && other.altitudeGainLoss != null) { + newAltitudeGainLoss = new AltitudeGainLoss( + altitudeGainLoss.gain_m() + other.altitudeGainLoss.gain_m(), + altitudeGainLoss.loss_m() + other.altitudeGainLoss.loss_m() + ); + } else { + newAltitudeGainLoss = altitudeGainLoss != null ? altitudeGainLoss : other.altitudeGainLoss; + } + + return new Statistics( + startTime == null ? other.startTime : startTime.isBefore(other.startTime) ? startTime : other.startTime, + stopTime == null ? other.stopTime : stopTime.isAfter(other.stopTime) ? stopTime : other.stopTime, + totalDuration.plus(other.totalDuration), + movingDuration.plus(other.movingDuration), + totalDistance.plus(other.totalDistance), + Speed.max(maxSpeed, other.maxSpeed), + newAltitudeExtremities, + newAltitudeGainLoss, + newAvgHeartRate, + newAvgPower + ); + } } diff --git a/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java index 097f4ef43..e6851320b 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/SegmentStatisticUpdater.java @@ -71,6 +71,10 @@ public class SegmentStatisticUpdater { reset(); } + public SegmentStatisticUpdater(Instant startTime) { + reset(startTime); + } + /** * Copy constructor. * @@ -98,81 +102,8 @@ public class SegmentStatisticUpdater { avgPower = statistics.avgPower(); } - public Statistics aggregate(Statistics statistics) { - SegmentStatisticUpdater intermediate = new SegmentStatisticUpdater(statistics); - intermediate.merge(this); - return intermediate.getStatistics(); - } - - /** - * Combines these statistics with those from another object. - * This assumes that the time periods covered by each do not intersect. - */ - //TODO Should be refactored to append only [mainly due to isIdle] - public void merge(SegmentStatisticUpdater other) { - if (startTime == null) { - startTime = other.startTime; - } else { - startTime = startTime.isBefore(other.startTime) ? startTime : other.startTime; - } - if (stopTime == null) { - stopTime = other.stopTime; - } else { - stopTime = stopTime.isAfter(other.stopTime) ? stopTime : other.stopTime; - } - - if (avgHeartRate == null) { - avgHeartRate = other.avgHeartRate; - } else { - if (other.avgHeartRate != null) { - // Using total time as weights for the averaging. - // Important to do this before total time is updated - avgHeartRate = HeartRate.of( - (totalDuration.getSeconds() * avgHeartRate.getBPM() + other.totalDuration.getSeconds() * other.avgHeartRate.getBPM()) - / (totalDuration.getSeconds() + other.totalDuration.getSeconds()) - ); - } - } - - if (avgPower == null) { - avgPower = other.avgPower; - } else { - if (other.avgPower != null) { - // Using total time as weights for the averaging. - // Important to do this before total time is updated - avgPower = Power.of( - (totalDuration.getSeconds() * avgPower.getW() + other.totalDuration.getSeconds() * other.avgPower.getW()) - / (totalDuration.getSeconds() + other.totalDuration.getSeconds()) - ); - } - } - - totalDistance = totalDistance.plus(other.totalDistance); - totalDuration = totalDuration.plus(other.totalDuration); - movingDuration = movingDuration.plus(other.movingDuration); - maxSpeed = Speed.max(maxSpeed, other.maxSpeed); - if (other.altitudeExtremities.hasData()) { - altitudeExtremities.update(other.altitudeExtremities.getMin()); - altitudeExtremities.update(other.altitudeExtremities.getMax()); - } - if (totalAltitudeGain_m == null) { - if (other.totalAltitudeGain_m != null) { - totalAltitudeGain_m = other.totalAltitudeGain_m; - } - } else { - if (other.totalAltitudeGain_m != null) { - totalAltitudeGain_m += other.totalAltitudeGain_m; - } - } - if (totalAltitudeLoss_m == null) { - if (other.totalAltitudeLoss_m != null) { - totalAltitudeLoss_m = other.totalAltitudeLoss_m; - } - } else { - if (other.totalAltitudeLoss_m != null) { - totalAltitudeLoss_m += other.totalAltitudeLoss_m; - } - } + public Statistics merge(Statistics statistics) { + return statistics.merge(getStatistics()); } public boolean isInitialized() { diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index b85ca896a..a0cca7182 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -94,7 +94,7 @@ public class TrackStatisticsUpdater { * Compute TrackStatistics. */ public Statistics getTrackStatistics() { - return currentSegment.aggregate(statisticsWithoutCurrentSegment); + return currentSegment.merge(statisticsWithoutCurrentSegment); } public boolean isIdle() { @@ -200,7 +200,7 @@ public class TrackStatisticsUpdater { private void reset(TrackPoint trackPoint) { if (currentSegment.isInitialized()) { - statisticsWithoutCurrentSegment = currentSegment.aggregate(statisticsWithoutCurrentSegment); + statisticsWithoutCurrentSegment = currentSegment.merge(statisticsWithoutCurrentSegment); } currentSegment.reset(trackPoint.getTime());