From b9ed5e28e02d88b5a9b6e3861409fd682ad0f7fa Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 17 Feb 2025 21:17:55 +0100 Subject: [PATCH] Sensor data: separate aggregation and TTL explicitly. Part of #1995. --- .../io/file/importer/ExportImportTest.java | 34 ++++++++++++------ .../sensorData/SensorDataCyclingTest.java | 2 +- .../TrackRecordingServiceRecordingTest.java | 7 ++-- .../sensors/sensorData/Aggregator.java | 35 ++++++++++++------- .../sensorData/AggregatorBarometer.java | 6 +++- .../sensorData/AggregatorCyclingCadence.java | 7 +++- .../AggregatorCyclingDistanceSpeed.java | 19 ++++++---- .../sensorData/AggregatorCyclingPower.java | 7 +++- .../sensors/sensorData/AggregatorGPS.java | 14 +++++++- .../sensorData/AggregatorHeartRate.java | 7 +++- .../sensors/sensorData/AggregatorRunning.java | 13 +++---- .../sensors/sensorData/SensorDataSet.java | 30 ++++++++-------- 12 files changed, 120 insertions(+), 61 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 335af64cb..6181947b4 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 @@ -11,6 +11,7 @@ import android.location.Location; import android.net.Uri; import android.os.Looper; +import androidx.annotation.NonNull; import androidx.preference.PreferenceManager; import androidx.test.core.app.ApplicationProvider; import androidx.test.ext.junit.runners.AndroidJUnit4; @@ -58,12 +59,14 @@ import de.dennisguse.opentracks.data.models.Track; import de.dennisguse.opentracks.data.models.TrackPoint; import de.dennisguse.opentracks.io.file.TrackFileFormat; import de.dennisguse.opentracks.io.file.exporter.TrackExporter; +import de.dennisguse.opentracks.sensors.BluetoothHandlerManagerCyclingPower; import de.dennisguse.opentracks.sensors.sensorData.Aggregator; import de.dennisguse.opentracks.sensors.sensorData.AggregatorBarometer; import de.dennisguse.opentracks.sensors.sensorData.AggregatorCyclingCadence; import de.dennisguse.opentracks.sensors.sensorData.AggregatorCyclingDistanceSpeed; import de.dennisguse.opentracks.sensors.sensorData.AggregatorCyclingPower; import de.dennisguse.opentracks.sensors.sensorData.AggregatorHeartRate; +import de.dennisguse.opentracks.sensors.sensorData.Raw; import de.dennisguse.opentracks.sensors.sensorData.SensorDataSet; import de.dennisguse.opentracks.services.TrackRecordingService; import de.dennisguse.opentracks.services.handlers.TrackPointCreator; @@ -72,7 +75,7 @@ import de.dennisguse.opentracks.stats.TrackStatistics; /** * Export a track to {@link TrackFileFormat} and verify that the import is identical. *

- * Note: those tests are affected by {@link Aggregator}.isRecent(). + * Note: those tests are affected by {@link Aggregator}.isOutdated(). * If the test device is too slow (like in a CI) these are likely to fail as the sensor data will be omitted from actual. */ @RunWith(AndroidJUnit4.class) @@ -548,23 +551,32 @@ public class ExportImportTest { private void mockSensorData(TrackPointCreator trackPointCreator, Float speed, Distance distance, float heartRate, float cadence, Float power, Float altitudeGain) { SensorDataSet sensorDataSet = trackPointCreator.getSensorManager().sensorDataSet; - AggregatorCyclingPower cyclingPower = Mockito.mock(AggregatorCyclingPower.class); - Mockito.when(cyclingPower.hasAggregatedValue()).thenReturn(true); - Mockito.when(cyclingPower.getAggregatedValue(Mockito.any())).thenReturn(Power.of(power)); + AggregatorCyclingPower cyclingPower = new AggregatorCyclingPower("", ""); + cyclingPower.add(new Raw<>(trackPointCreator.createNow(), new BluetoothHandlerManagerCyclingPower.Data(Power.of(power), null))); sensorDataSet.add(cyclingPower); - AggregatorHeartRate avgHeartRate = Mockito.mock(AggregatorHeartRate.class); - Mockito.when(avgHeartRate.getAggregatedValue(Mockito.any())).thenReturn(HeartRate.of(heartRate)); + + AggregatorHeartRate avgHeartRate = new AggregatorHeartRate("", ""); + avgHeartRate.add(new Raw<>(trackPointCreator.createNow(), HeartRate.of(heartRate))); sensorDataSet.add(avgHeartRate); - AggregatorCyclingCadence cyclingCadence = Mockito.mock(AggregatorCyclingCadence.class); - Mockito.when(cyclingCadence.hasAggregatedValue()).thenReturn(true); - Mockito.when(cyclingCadence.getAggregatedValue(Mockito.any())).thenReturn(Cadence.of(cadence)); + AggregatorCyclingCadence cyclingCadence = new AggregatorCyclingCadence("", "") { + @NonNull + @Override + public Cadence getAggregatedValue(Instant now) { + return Cadence.of(cadence); + } + + @Override + public boolean hasReceivedData() { + return true; + } + }; sensorDataSet.add(cyclingCadence); if (distance != null && speed != null) { AggregatorCyclingDistanceSpeed distanceSpeed = Mockito.mock(AggregatorCyclingDistanceSpeed.class); - Mockito.when(distanceSpeed.hasAggregatedValue()).thenReturn(true); + Mockito.when(distanceSpeed.hasReceivedData()).thenReturn(true); Mockito.when(distanceSpeed.getAggregatedValue(Mockito.any())).thenReturn(new AggregatorCyclingDistanceSpeed.Data(null, distance, Speed.of(speed))); sensorDataSet.add(distanceSpeed); } else { @@ -581,7 +593,7 @@ public class ExportImportTest { if (altitudeGain != null) { AggregatorBarometer barometer = Mockito.mock(AggregatorBarometer.class); - Mockito.when(barometer.hasAggregatedValue()).thenReturn(true); + Mockito.when(barometer.hasReceivedData()).thenReturn(true); Mockito.when(barometer.getAggregatedValue(Mockito.any())).thenReturn(new AltitudeGainLoss(altitudeGain, altitudeGain)); sensorDataSet.add(barometer); } else { diff --git a/src/androidTest/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataCyclingTest.java b/src/androidTest/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataCyclingTest.java index e58e32ffc..de6498b0a 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataCyclingTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/sensors/sensorData/SensorDataCyclingTest.java @@ -66,7 +66,7 @@ public class SensorDataCyclingTest { current.add(new Raw<>(Instant.MIN, new BluetoothHandlerCyclingCadence.CrankData(2, 1024))); // then - assertFalse(current.hasAggregatedValue()); //TODO Cadence should be 0? + assertFalse(current.hasReceivedData()); //TODO Cadence should be 0? } @Test diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java index 2acc6d459..6125cb7c9 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceRecordingTest.java @@ -149,12 +149,11 @@ public class TrackRecordingServiceRecordingTest { 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"); + trackPointCreator.setClock(idleTime); Thread.sleep(Duration.ofSeconds(15).toMillis()); // then @@ -722,14 +721,14 @@ public class TrackRecordingServiceRecordingTest { .setLongitude(35) .setHorizontalAccuracy(Distance.of(1)) .setSensorDistance(Distance.of(11)) - .setSpeed(Speed.of(5)) + .setSpeed(Speed.of(0)) //Sensor data is now outdated, but we do not fall back to GPS. .setSensorDistance(Distance.of(0)) ), TestDataUtil.getTrackPoints(contentProviderUtils, trackId)); } private void mockAltitudeChange(TrackPointCreator trackPointCreator, float altitudeGain) { AggregatorBarometer barometer = Mockito.mock(AggregatorBarometer.class); - Mockito.when(barometer.hasAggregatedValue()).thenReturn(true); + Mockito.when(barometer.hasReceivedData()).thenReturn(true); Mockito.when(barometer.getAggregatedValue(Mockito.any())).thenReturn(new AltitudeGainLoss(altitudeGain, altitudeGain)); trackPointCreator.getSensorManager().sensorDataSet.barometer = barometer; diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java index c46d4491a..7f59b79c4 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/Aggregator.java @@ -37,7 +37,11 @@ public abstract class Aggregator { protected abstract void computeValue(Raw current); - public boolean hasAggregatedValue() { + /** + * @return did we process data from a sensor. + * NOTE: for some sensors this may require more than one measurement. + */ + public boolean hasReceivedData() { return aggregatedValue != null; } @@ -46,15 +50,14 @@ public abstract class Aggregator { @NonNull public Output getAggregatedValue(Instant now) { - if (!hasAggregatedValue()) { + if (!hasReceivedData()) { return getNoneValue(); } - //TODO This should only affect measured data (like heartrate), but not aggregated values. - //Remove current measurements, but provide aggregates? - if (isRecent(now)) { - return aggregatedValue; + if (isOutdated(now)) { + resetImmediate(); } - return getNoneValue(); + + return aggregatedValue; } @NonNull @@ -63,20 +66,28 @@ public abstract class Aggregator { } /** - * Reset long term aggregated values (more than derived from previous SensorData). e.g. overall distance. + * Reset short-term (i.e., non-aggregated) values that were directly derived from sensor data. */ - public abstract void reset(); + protected abstract void resetImmediate(); + + /** + * Reset long-term (i.e., aggregated) values (more than derived from previous SensorData) like overall distance. + */ + public abstract void resetAggregated(); /** * Is the data recent considering the current time. */ - private boolean isRecent(Instant now) { + private boolean isOutdated(Instant now) { if (previous == null) { - return false; + return true; } return now - .isBefore(previous.time().plus(BluetoothRemoteSensorManager.MAX_SENSOR_DATE_SET_AGE)); //TODO Per Sensor! + .isAfter( + previous.time() + .plus(BluetoothRemoteSensorManager.MAX_SENSOR_DATE_SET_AGE) + ); } @NonNull diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorBarometer.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorBarometer.java index 89a8562be..bd6b1b017 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorBarometer.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorBarometer.java @@ -32,7 +32,11 @@ public class AggregatorBarometer extends Aggregator { } @Override - public void reset() { + protected void resetImmediate() { + aggregatedValue = Position.empty(); + } + + @Override + public void resetAggregated() { + /* + * GPS data is not an aggregated value, but for now we want to ensure to only save the data once. + * The data is too large to save it more often than needed (i.e., duplicated values). + * TODO: this behavior can be changed if TrackRecordingManager.insertTrackPoint() would strip GPS data if it was already saved. This would simplify TrackPointCreator.createCurrentTrackPoint() + */ + aggregatedValue = Position.empty(); + } @NonNull diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorHeartRate.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorHeartRate.java index fb7cc6c5a..c46434f88 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorHeartRate.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorHeartRate.java @@ -16,7 +16,12 @@ public class AggregatorHeartRate extends Aggregator { } @Override - public void reset() { + protected void resetImmediate() { + aggregatedValue = getNoneValue(); + } + + @Override + public void resetAggregated() { } @NonNull diff --git a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorRunning.java b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorRunning.java index 900c1a7ca..253852fbd 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorRunning.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/sensorData/AggregatorRunning.java @@ -36,7 +36,12 @@ public final class AggregatorRunning extends Aggregator(runningDistanceSpeedCadence.aggregatedValue.cadence(), runningDistanceSpeedCadence.getSensorNameOrAddress()); } @@ -83,11 +83,11 @@ public class SensorDataSet { } public Pair getSpeed() { - if (cyclingDistanceSpeed != null && cyclingDistanceSpeed.hasAggregatedValue() && cyclingDistanceSpeed.getAggregatedValue(trackPointCreator.createNow()).speed() != null) { + if (cyclingDistanceSpeed != null && cyclingDistanceSpeed.hasReceivedData() && cyclingDistanceSpeed.getAggregatedValue(trackPointCreator.createNow()).speed() != null) { return new Pair<>(cyclingDistanceSpeed.getAggregatedValue(trackPointCreator.createNow()).speed(), cyclingDistanceSpeed.getSensorNameOrAddress()); } - if (runningDistanceSpeedCadence != null && runningDistanceSpeedCadence.hasAggregatedValue() && runningDistanceSpeedCadence.getAggregatedValue(trackPointCreator.createNow()).speed() != null) { + if (runningDistanceSpeedCadence != null && runningDistanceSpeedCadence.hasReceivedData() && runningDistanceSpeedCadence.getAggregatedValue(trackPointCreator.createNow()).speed() != null) { return new Pair<>(runningDistanceSpeedCadence.aggregatedValue.speed(), runningDistanceSpeedCadence.getSensorNameOrAddress()); } @@ -156,7 +156,7 @@ public class SensorDataSet { } public void fillTrackPoint(TrackPoint trackPoint) { - if (gps != null && gps.hasAggregatedValue()) { + if (gps != null && gps.hasReceivedData()) { trackPoint.setPosition(gps.getAggregatedValue(trackPointCreator.createNow())); } @@ -172,19 +172,19 @@ public class SensorDataSet { trackPoint.setSpeed(getSpeed().first); } - if (cyclingDistanceSpeed != null && cyclingDistanceSpeed.hasAggregatedValue()) { + if (cyclingDistanceSpeed != null && cyclingDistanceSpeed.hasReceivedData()) { trackPoint.setSensorDistance(cyclingDistanceSpeed.getAggregatedValue(trackPointCreator.createNow()).distanceOverall()); } - if (cyclingPower != null && cyclingPower.hasAggregatedValue()) { + if (cyclingPower != null && cyclingPower.hasReceivedData()) { trackPoint.setPower(cyclingPower.getAggregatedValue(trackPointCreator.createNow())); } - if (runningDistanceSpeedCadence != null && runningDistanceSpeedCadence.hasAggregatedValue()) { + if (runningDistanceSpeedCadence != null && runningDistanceSpeedCadence.hasReceivedData()) { trackPoint.setSensorDistance(runningDistanceSpeedCadence.getAggregatedValue(trackPointCreator.createNow()).distance()); } - if (barometer != null && barometer.hasAggregatedValue()) { + if (barometer != null && barometer.hasReceivedData()) { trackPoint.setAltitudeGain(barometer.getAggregatedValue(trackPointCreator.createNow()).gain_m()); trackPoint.setAltitudeLoss(barometer.getAggregatedValue(trackPointCreator.createNow()).loss_m()); } @@ -193,13 +193,13 @@ public class SensorDataSet { public void reset() { Log.i(TAG, "Resetting data"); - if (heartRate != null) heartRate.reset(); - if (cyclingCadence != null) cyclingCadence.reset(); - if (cyclingDistanceSpeed != null) cyclingDistanceSpeed.reset(); - if (cyclingPower != null) cyclingPower.reset(); - if (runningDistanceSpeedCadence != null) runningDistanceSpeedCadence.reset(); - if (barometer != null) barometer.reset(); - if (gps != null) gps.reset(); + if (heartRate != null) heartRate.resetAggregated(); + if (cyclingCadence != null) cyclingCadence.resetAggregated(); + if (cyclingDistanceSpeed != null) cyclingDistanceSpeed.resetAggregated(); + if (cyclingPower != null) cyclingPower.resetAggregated(); + if (runningDistanceSpeedCadence != null) runningDistanceSpeedCadence.resetAggregated(); + if (barometer != null) barometer.resetAggregated(); + if (gps != null) gps.resetAggregated(); } private void set(@NonNull Aggregator type, @Nullable Aggregator sensorData) {