From 81bb6699d89d4dbe5110aedb57e80e16a6fc78bf Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Tue, 21 Jun 2022 20:16:53 +0200 Subject: [PATCH] Statistics: remove speed filtering. Part of #1003. --- .../file/importer/GPXTrackImporterTest.java | 2 +- .../TrackRecordingServiceTestRecording.java | 8 +- .../stats/TrackStatisticsUpdaterTest.java | 65 +++++++++ .../opentracks/chart/ChartFragment.java | 4 +- .../opentracks/data/TrackDataHub.java | 4 +- .../data/models/UnitConversions.java | 1 + .../opentracks/stats/RingBuffer.java | 138 ------------------ .../opentracks/stats/SpeedRingBuffer.java | 27 ---- .../stats/TrackStatisticsUpdater.java | 63 ++------ 9 files changed, 86 insertions(+), 226 deletions(-) delete mode 100644 src/main/java/de/dennisguse/opentracks/stats/RingBuffer.java delete mode 100644 src/main/java/de/dennisguse/opentracks/stats/SpeedRingBuffer.java diff --git a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/GPXTrackImporterTest.java b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/GPXTrackImporterTest.java index f3ac08391..c9d0263cc 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/GPXTrackImporterTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/io/file/importer/GPXTrackImporterTest.java @@ -181,7 +181,7 @@ public class GPXTrackImporterTest { // 3. trackstatistics TrackStatistics trackStatistics = importedTrack.getTrackStatistics(); - assertEquals(4, trackStatistics.getMaxSpeed().toMPS(), 0.01); + assertEquals(0.75, trackStatistics.getMaxSpeed().toMPS(), 0.01); assertEquals(Duration.ofSeconds(101), trackStatistics.getMovingTime()); // 4. trackpoints diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestRecording.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestRecording.java index 079efb0c0..d0305505c 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestRecording.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceTestRecording.java @@ -281,7 +281,7 @@ public class TrackRecordingServiceTestRecording { TrackRecordingServiceTestUtils.sendGPSLocation(trackPointCreator, gps2, 45.0001, 35.0, 1, 15); // then - assertEquals(new TrackStatistics(startTime, gps2, 11.113178253173828f, 4, 3, 15, 0f, 0f) + assertEquals(new TrackStatistics(startTime, gps2, 11.113178253173828f, 4, 3, 3.7f, 0f, 0f) , contentProviderUtils.getTrack(trackId).getTrackStatistics()); // when @@ -289,7 +289,7 @@ public class TrackRecordingServiceTestRecording { TrackRecordingServiceTestUtils.sendGPSLocation(trackPointCreator, gps3, 45.0002, 35.0, 1, 15); // then - assertEquals(new TrackStatistics(startTime, gps3, 22.226356506347656, 6, 5, 15, 0f, 0f) + assertEquals(new TrackStatistics(startTime, gps3, 22.226356506347656, 6, 5, 4.4452714920043945f, 0f, 0f) , contentProviderUtils.getTrack(trackId).getTrackStatistics()); @@ -299,7 +299,7 @@ public class TrackRecordingServiceTestRecording { service.endCurrentTrack(); // then - assertEquals(new TrackStatistics(startTime, stopTime, 22.226356506347656, 10, 5, 15, 0f, 0f) + assertEquals(new TrackStatistics(startTime, stopTime, 22.226356506347656, 10, 5, 4.4452714920043945f, 0f, 0f) , contentProviderUtils.getTrack(trackId).getTrackStatistics()); new TrackPointAssert().assertEquals(List.of( @@ -435,7 +435,7 @@ public class TrackRecordingServiceTestRecording { service.endCurrentTrack(); // then - assertEquals(new TrackStatistics(startTime, stopTime, 2.222635507583618, 10, 5, 15, 0f, 0f) + assertEquals(new TrackStatistics(startTime, stopTime, 2.222635507583618, 10, 5, 0.44452710151672364f, 0f, 0f) , contentProviderUtils.getTrack(trackId).getTrackStatistics()); new TrackPointAssert().assertEquals(List.of( diff --git a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java index 3d106cf0b..a6923e775 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdaterTest.java @@ -11,6 +11,7 @@ import org.junit.runner.RunWith; import java.time.Duration; import java.time.Instant; +import java.util.List; import de.dennisguse.opentracks.content.data.TestDataUtil; import de.dennisguse.opentracks.data.models.Altitude; @@ -245,6 +246,70 @@ public class TrackStatisticsUpdaterTest { public void addTrackPoint_speed_from_GPS_moving_and_sensor_speed() { } + @Test + public void addTrackPoint_maxSpeed_ignore_above_acceleration() { + TrackStatisticsUpdater subject = new TrackStatisticsUpdater(); + assertEquals(Speed.of(0f), subject.getTrackStatistics().getMaxSpeed()); + + subject.addTrackPoint(new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.ofEpochSecond(0))); + assertEquals(Speed.of(0f), subject.getTrackStatistics().getMaxSpeed()); + + // Ignore as we set max speed if two consecutive trackpoints were considered moving + subject.addTrackPoint(new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(1)) + .setSpeed(Speed.of(1f))); + assertEquals(Speed.of(0f), subject.getTrackStatistics().getMaxSpeed()); + + // Update max speed + subject.addTrackPoint(new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(2)) + .setSpeed(Speed.of(1f))); + assertEquals(Speed.of(1f), subject.getTrackStatistics().getMaxSpeed()); + + // Update max speed + subject.addTrackPoint(new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(12)) + .setSpeed(Speed.of(50f))); + assertEquals(Speed.of(50f), subject.getTrackStatistics().getMaxSpeed()); + + // Ignore; we were getting slower + subject.addTrackPoint(new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(13)) + .setSpeed(Speed.of(5f))); + assertEquals(Speed.of(50f), subject.getTrackStatistics().getMaxSpeed()); + + // Ignore acceleration above 2g + subject.addTrackPoint(new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(14)) + .setSpeed(Speed.of(500f))); + assertEquals(Speed.of(50f), subject.getTrackStatistics().getMaxSpeed()); + + } + + @Test + public void addTrackPoint_maxSpeed_multiple_segments() { + TrackStatisticsUpdater subject = new TrackStatisticsUpdater(); + assertEquals(Speed.of(0f), subject.getTrackStatistics().getMaxSpeed()); + + subject.addTrackPoints(List.of( + new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.ofEpochSecond(0)), + new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(1)) + .setSpeed(Speed.of(2f)), + new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(2)) + .setSpeed(Speed.of(2f)), + new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL, Instant.ofEpochSecond(4)) + )); + assertEquals(Speed.of(2f), subject.getTrackStatistics().getMaxSpeed()); + + // when + subject.addTrackPoints(List.of( + new TrackPoint(TrackPoint.Type.SEGMENT_START_MANUAL, Instant.ofEpochSecond(5)), + new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(6)) + .setSpeed(Speed.of(1f)), + new TrackPoint(0, 0, Altitude.WGS84.of(0), Instant.ofEpochSecond(7)) + .setSpeed(Speed.of(1f)), + new TrackPoint(TrackPoint.Type.SEGMENT_END_MANUAL, Instant.ofEpochSecond(8)) + )); + + // then + assertEquals(Speed.of(2f), subject.getTrackStatistics().getMaxSpeed()); + } + @Test public void copy_constructor() { // given diff --git a/src/main/java/de/dennisguse/opentracks/chart/ChartFragment.java b/src/main/java/de/dennisguse/opentracks/chart/ChartFragment.java index d9b4117c5..2b4f7f2a5 100644 --- a/src/main/java/de/dennisguse/opentracks/chart/ChartFragment.java +++ b/src/main/java/de/dennisguse/opentracks/chart/ChartFragment.java @@ -188,9 +188,9 @@ public class ChartFragment extends Fragment implements TrackDataHub.Listener { } } - public void onSampledInTrackPoint(@NonNull TrackPoint trackPoint, @NonNull TrackStatistics trackStatistics, Speed smoothedSpeed) { + public void onSampledInTrackPoint(@NonNull TrackPoint trackPoint, @NonNull TrackStatistics trackStatistics) { if (isResumed()) { - ChartPoint point = new ChartPoint(trackStatistics, trackPoint, smoothedSpeed, chartByDistance, viewBinding.chartView.getUnitSystem()); + ChartPoint point = new ChartPoint(trackStatistics, trackPoint, trackPoint.getSpeed(), chartByDistance, viewBinding.chartView.getUnitSystem()); pendingPoints.add(point); } } diff --git a/src/main/java/de/dennisguse/opentracks/data/TrackDataHub.java b/src/main/java/de/dennisguse/opentracks/data/TrackDataHub.java index def843c74..3b064f9c3 100644 --- a/src/main/java/de/dennisguse/opentracks/data/TrackDataHub.java +++ b/src/main/java/de/dennisguse/opentracks/data/TrackDataHub.java @@ -365,7 +365,7 @@ public class TrackDataHub { // Also include the last point if the selected track is not recording. if ((localNumLoadedTrackPoints % samplingFrequency == 0) || (trackPointId == lastTrackPointId && !isSelectedTrackRecording())) { for (Listener trackDataListener : listeners) { - trackDataListener.onSampledInTrackPoint(trackPoint, currentUpdater.getTrackStatistics(), currentUpdater.getSmoothedSpeed()); + trackDataListener.onSampledInTrackPoint(trackPoint, currentUpdater.getTrackStatistics()); } } else { for (Listener trackDataListener : listeners) { @@ -430,7 +430,7 @@ public class TrackDataHub { * * @param trackPoint the trackPoint */ - default void onSampledInTrackPoint(@NonNull TrackPoint trackPoint, @NonNull TrackStatistics trackStatistics, Speed smoothedSpeed) { + default void onSampledInTrackPoint(@NonNull TrackPoint trackPoint, @NonNull TrackStatistics trackStatistics) { } /** diff --git a/src/main/java/de/dennisguse/opentracks/data/models/UnitConversions.java b/src/main/java/de/dennisguse/opentracks/data/models/UnitConversions.java index eee25ea63..033fcafb7 100644 --- a/src/main/java/de/dennisguse/opentracks/data/models/UnitConversions.java +++ b/src/main/java/de/dennisguse/opentracks/data/models/UnitConversions.java @@ -24,6 +24,7 @@ public class UnitConversions { // Time //TODO Use Duration // multiplication factor to convert seconds to milliseconds + @Deprecated public static final long S_TO_MS = 1000; // multiplication factor to convert milliseconds to seconds diff --git a/src/main/java/de/dennisguse/opentracks/stats/RingBuffer.java b/src/main/java/de/dennisguse/opentracks/stats/RingBuffer.java deleted file mode 100644 index a4081c528..000000000 --- a/src/main/java/de/dennisguse/opentracks/stats/RingBuffer.java +++ /dev/null @@ -1,138 +0,0 @@ -/* - * Copyright 2009 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 androidx.annotation.NonNull; -import androidx.annotation.Nullable; - -import java.util.ArrayList; - -/** - * This class maintains a ring buffer of doubles. - * This buffer is a convenient class for storing a series of doubles and calculating information about them. - * This is a FIFO buffer. - * - * @author Sandor Dornbush - */ -abstract class RingBuffer { - - private final ArrayList buffer; - - // The location that the next write will occur at. - private int index; - - // True if the buffer is full - private boolean isFull; - - /** - * Creates a buffer with a certain size. - * - * @param size the size - */ - RingBuffer(int size) { - if (size < 1) { - throw new IllegalArgumentException("The buffer size must be greater than 1."); - } - - buffer = new ArrayList(size); - reset(); - } - - RingBuffer(RingBuffer toCopy) { - this.buffer = new ArrayList<>(toCopy.buffer); - this.index = toCopy.index; - this.isFull = toCopy.isFull; - } - - /** - * Resets the buffer. - */ - public void reset() { - index = 0; - isFull = false; - } - - /** - * Returns true if the buffer is full. - */ - boolean isFull() { - return isFull; - } - - /** - * Gets the average of the buffer. - */ - public T getAverage() { - int numberOfEntries = isFull ? buffer.size() : index; - if (numberOfEntries == 0) { - return null; - } - Double sum = null; - int numberOfUsedEntries = 0; - for (int i = 0; i < numberOfEntries; i++) { - Number value = from(buffer.get(i)); - if (value != null) { - if (sum == null) { - sum = 0.0; - } - sum += value.doubleValue(); - numberOfUsedEntries++; - } - } - if (sum == null) { - return null; - } else { - return to(sum / numberOfUsedEntries); - } - } - - @Nullable - protected abstract Number from(T object); - - protected abstract T to(double object); - - /** - * Adds a double to the buffer. - * If the buffer is full the oldest element is overwritten. - * - * @param value the double to add - */ - public void setNext(T value) { - if (index == buffer.size()) { - index = 0; - } - buffer.add(index, value); - index++; - if (index == buffer.size()) { - isFull = true; - } - } - - @NonNull - @Override - public String toString() { - StringBuilder builder = new StringBuilder("Full: "); - builder.append(isFull); - builder.append("\n"); - for (int i = 0; i < buffer.size(); i++) { - builder.append((i == index) ? "<<" : "["); - builder.append(buffer.get(i)); - builder.append((i == index) ? ">> " : "] "); - } - return builder.toString(); - } -} diff --git a/src/main/java/de/dennisguse/opentracks/stats/SpeedRingBuffer.java b/src/main/java/de/dennisguse/opentracks/stats/SpeedRingBuffer.java deleted file mode 100644 index 990e87cac..000000000 --- a/src/main/java/de/dennisguse/opentracks/stats/SpeedRingBuffer.java +++ /dev/null @@ -1,27 +0,0 @@ -package de.dennisguse.opentracks.stats; - -import androidx.annotation.Nullable; - -import de.dennisguse.opentracks.data.models.Speed; - -public class SpeedRingBuffer extends RingBuffer { - - SpeedRingBuffer(int size) { - super(size); - } - - SpeedRingBuffer(RingBuffer toCopy) { - super(toCopy); - } - - @Nullable - @Override - protected Number from(Speed object) { - return object.toMPS(); - } - - @Override - protected Speed to(double object) { - return Speed.of(object); - } -} diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index d76d9fc4b..c038cee66 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -19,16 +19,15 @@ package de.dennisguse.opentracks.stats; import android.util.Log; import androidx.annotation.NonNull; -import androidx.annotation.VisibleForTesting; import java.time.Duration; import java.util.List; -import de.dennisguse.opentracks.data.models.Altitude; 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.TrackPoint; +import de.dennisguse.opentracks.data.models.UnitConversions; /** * Updater for {@link TrackStatistics}. @@ -41,23 +40,15 @@ import de.dennisguse.opentracks.data.models.TrackPoint; */ public class TrackStatisticsUpdater { - /** - * The number of speed reading to smooth to get a somewhat accurate signal. - */ - @VisibleForTesting - private static final int SPEED_SMOOTHING_FACTOR = 25; - private static final String TAG = TrackStatisticsUpdater.class.getSimpleName(); /** * Ignore any acceleration faster than this. * Will ignore any speeds that imply acceleration greater than 2g's - * 2g = 19.6 m/s^2 = 0.0002 m/ms^2 = 0.02 m/(m*ms) */ - private static final double MAX_ACCELERATION = 0.02; + private static final double SPEED_MAX_ACCELERATION = 2 * 9.81; private final TrackStatistics trackStatistics; - private final SpeedRingBuffer speedBuffer; private float averageHeartRateBPM; private Duration totalHeartRateDuration = Duration.ZERO; @@ -79,7 +70,6 @@ public class TrackStatisticsUpdater { this.trackStatistics = trackStatistics; this.currentSegment = new TrackStatistics(); - speedBuffer = new SpeedRingBuffer(SPEED_SMOOTHING_FACTOR); resetAverageHeartRate(); } @@ -87,8 +77,6 @@ public class TrackStatisticsUpdater { this.currentSegment = new TrackStatistics(toCopy.currentSegment); this.trackStatistics = new TrackStatistics(toCopy.trackStatistics); - this.speedBuffer = new SpeedRingBuffer(toCopy.speedBuffer); - this.lastTrackPoint = toCopy.lastTrackPoint; resetAverageHeartRate(); } @@ -165,11 +153,8 @@ public class TrackStatisticsUpdater { // Update max speed updateSpeed(trackPoint, lastTrackPoint); - } else { - speedBuffer.reset(); } - if (trackPoint.isSegmentEnd()) { reset(trackPoint); return; @@ -185,7 +170,6 @@ public class TrackStatisticsUpdater { currentSegment.reset(trackPoint.getTime()); lastTrackPoint = null; - speedBuffer.reset(); resetAverageHeartRate(); } @@ -194,22 +178,14 @@ public class TrackStatisticsUpdater { totalHeartRateDuration = Duration.ZERO; } - public Speed getSmoothedSpeed() { - return speedBuffer.getAverage(); - } - /** * Updates a speed reading while assuming the user is moving. */ - @VisibleForTesting private void updateSpeed(@NonNull TrackPoint trackPoint, @NonNull TrackPoint lastTrackPoint) { - if (!trackPoint.isMoving()) { - speedBuffer.reset(); - } else if (isValidSpeed(trackPoint, lastTrackPoint)) { - speedBuffer.setNext(trackPoint.getSpeed()); - Speed average = speedBuffer.getAverage(); - if (average.greaterThan(currentSegment.getMaxSpeed())) { - currentSegment.setMaxSpeed(average); + if (isValidSpeed(trackPoint, lastTrackPoint)) { + Speed currentSpeed = trackPoint.getSpeed(); + if (currentSpeed.greaterThan(currentSegment.getMaxSpeed())) { + currentSegment.setMaxSpeed(currentSpeed); } } else { Log.d(TAG, "Invalid speed. speed: " + trackPoint.getSpeed() + " lastLocationSpeed: " + lastTrackPoint.getSpeed()); @@ -217,30 +193,13 @@ public class TrackStatisticsUpdater { } private boolean isValidSpeed(@NonNull TrackPoint trackPoint, @NonNull TrackPoint lastTrackPoint) { - // There are a lot of noisy speed readings. Do the cheapest checks first, most expensive last. - if (trackPoint.getSpeed().isZero()) { - return false; - } - + // See if the speed seems physically likely. Ignore any speeds that imply acceleration greater than 2g. Duration timeDifference = Duration.between(lastTrackPoint.getTime(), trackPoint.getTime()); - Speed maxAcceleration = Speed.of(MAX_ACCELERATION * timeDifference.toMillis()); - { - // See if the speed seems physically likely. Ignore any speeds that imply acceleration greater than 2g. - Speed speedDifference = Speed.absDiff(lastTrackPoint.getSpeed(), trackPoint.getSpeed()); - if (speedDifference.greaterThan(maxAcceleration)) { - return false; - } - } + Speed maxSpeedDifference = Speed.of(Distance.of(SPEED_MAX_ACCELERATION), Duration.ofMillis(1000)) + .mul(timeDifference.toMillis() / UnitConversions.S_TO_MS); - // Only check if the speed buffer is full. Check that the speed is less than 10X the smoothed average and the speed difference doesn't imply 2g acceleration. - if (speedBuffer.isFull()) { - Speed average = speedBuffer.getAverage(); - Speed speedDifference = Speed.absDiff(average, trackPoint.getSpeed()); - - return trackPoint.getSpeed().lessThan(average.mul(10)) && speedDifference.lessThan(maxAcceleration); - } - - return true; + Speed speedDifference = Speed.absDiff(lastTrackPoint.getSpeed(), trackPoint.getSpeed()); + return speedDifference.lessThan(maxSpeedDifference); } @NonNull