diff --git a/src/androidTest/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingTest.java b/src/androidTest/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingTest.java index 63ba862e8..003630fc4 100644 --- a/src/androidTest/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingTest.java +++ b/src/androidTest/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingTest.java @@ -2,6 +2,7 @@ package de.dennisguse.opentracks.content.sensor; import androidx.test.ext.junit.runners.AndroidJUnit4; +import org.junit.Ignore; import org.junit.Test; import org.junit.runner.RunWith; @@ -11,6 +12,7 @@ import de.dennisguse.opentracks.util.UintUtils; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertFalse; import static org.junit.Assert.assertNotEquals; +import static org.junit.Assert.assertNull; @RunWith(AndroidJUnit4.class) public class SensorDataCyclingTest { @@ -81,7 +83,9 @@ public class SensorDataCyclingTest { assertEquals(60, current.getValue(), 0.01); } + @Ignore("Disabled from #953") @Test + @Deprecated public void compute_cadence_rollOverCount() { // given SensorDataCycling.Cadence previous = new SensorDataCycling.Cadence("sensorAddress", "sensorName", UintUtils.UINT32_MAX - 1, 1024); @@ -94,6 +98,19 @@ public class SensorDataCyclingTest { assertEquals(60, current.getValue(), 0.01); } + @Test + public void compute_cadence_overflow() { + // given + SensorDataCycling.Cadence previous = new SensorDataCycling.Cadence("sensorAddress", "sensorName", UintUtils.UINT32_MAX - 1, 1024); + SensorDataCycling.Cadence current = new SensorDataCycling.Cadence("sensorAddress", "sensorName", 0, 2048); + + // when + current.compute(previous); + + // then + assertNull(current.getValue()); + } + @Test public void compute_speed() { // given @@ -108,7 +125,9 @@ public class SensorDataCyclingTest { assertEquals(1.20, current.getValue().getSpeed().toMPS(), 0.01); } + @Ignore("Disabled from #953") @Test + @Deprecated public void compute_speed_rollOverCount() { // given SensorDataCycling.DistanceSpeed previous = new SensorDataCycling.DistanceSpeed("sensorAddress", "sensorName", UintUtils.UINT32_MAX - 1, 1024); @@ -122,6 +141,19 @@ public class SensorDataCyclingTest { assertEquals(2, current.getValue().getSpeed().toMPS(), 0.01); } + @Test + public void compute_speed_overflow() { + // given + SensorDataCycling.DistanceSpeed previous = new SensorDataCycling.DistanceSpeed("sensorAddress", "sensorName", UintUtils.UINT32_MAX - 1, 1024); + SensorDataCycling.DistanceSpeed current = new SensorDataCycling.DistanceSpeed("sensorAddress", "sensorName", 0, 2048); + + // when + current.compute(previous, Distance.ofMM(2000)); + + // then + assertNull(current.getValue()); + } + @Test public void equals_speed_with_no_data() { // given diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorData.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorData.java index a5e110e01..5efe7189a 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorData.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorData.java @@ -50,8 +50,14 @@ public abstract class SensorData { return value != null; } + @NonNull + protected abstract T getNoneValue(); + public T getValue() { - return value; + if (isRecent()) { + return value; + } + return getNoneValue(); } /** @@ -63,7 +69,7 @@ public abstract class SensorData { /** * Is the data recent considering the current time. */ - public boolean isRecent() { + private boolean isRecent() { return Instant.now() .isBefore(time.plus(BluetoothRemoteSensorManager.MAX_SENSOR_DATE_SET_AGE_MS)); } diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCycling.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCycling.java index 6c067a7c4..aa0c338bc 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCycling.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCycling.java @@ -59,17 +59,30 @@ public final class SensorDataCycling { return crankRevolutionsTime; } + @NonNull + @Override + protected Float getNoneValue() { + return 0f; + } + public void compute(Cadence previous) { if (hasData() && previous != null && previous.hasData()) { float timeDiff_ms = UintUtils.diff(crankRevolutionsTime, previous.crankRevolutionsTime, UintUtils.UINT16_MAX) / 1024f * UnitConversions.S_TO_MS; if (timeDiff_ms <= 0) { Log.e(TAG, "Timestamps difference is invalid: cannot compute cadence."); value = null; - } else { - long crankDiff = UintUtils.diff(crankRevolutionsCount, previous.crankRevolutionsCount, UintUtils.UINT32_MAX); - float cadence_ms = crankDiff / timeDiff_ms; - value = (float) (cadence_ms / UnitConversions.MS_TO_S / UnitConversions.S_TO_MIN); + return; } + + // TODO We have to treat with overflow according to the documentation: read https://github.com/OpenTracksApp/OpenTracks/pull/953#discussion_r711625268 + if (crankRevolutionsCount < previous.crankRevolutionsCount) { + Log.e(TAG, "Crank revolutions count difference is invalid: cannot compute cadence."); + return; + } + + long crankDiff = UintUtils.diff(crankRevolutionsCount, previous.crankRevolutionsCount, UintUtils.UINT32_MAX); + float cadence_ms = crankDiff / timeDiff_ms; + value = (float) (cadence_ms / UnitConversions.MS_TO_S / UnitConversions.S_TO_MIN); } } @@ -121,6 +134,16 @@ public final class SensorDataCycling { return wheelRevolutionsTime; } + @NonNull + @Override + protected Data getNoneValue() { + if (value != null) { + return new Data(value.distance, value.distanceOverall, Speed.zero()); + } else { + return new Data(Distance.of(0), Distance.of(0), Speed.zero()); + } + } + public void compute(DistanceSpeed previous, Distance wheelCircumference) { if (hasData() && previous != null && previous.hasData()) { float timeDiff_ms = UintUtils.diff(wheelRevolutionsTime, previous.wheelRevolutionsTime, UintUtils.UINT16_MAX) / 1024f * UnitConversions.S_TO_MS; @@ -131,6 +154,10 @@ public final class SensorDataCycling { return; } + if (wheelRevolutionsCount < previous.wheelRevolutionsCount) { + Log.e(TAG, "Wheel revolutions count difference is invalid: cannot compute speed."); + return; + } long wheelDiff = UintUtils.diff(wheelRevolutionsCount, previous.wheelRevolutionsCount, UintUtils.UINT32_MAX); Distance distance = wheelCircumference.multipliedBy(wheelDiff); @@ -217,6 +244,12 @@ public final class SensorDataCycling { public DistanceSpeed getDistanceSpeed() { return this.value != null ? this.value.second : null; } + + @NonNull + @Override + protected Pair getNoneValue() { + return new Pair<>(null, null); + } } } diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingPower.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingPower.java index 3662bcd3d..637c7a39b 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingPower.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataCyclingPower.java @@ -18,4 +18,10 @@ public class SensorDataCyclingPower extends SensorData { public String toString() { return super.toString() + " power=" + value; } + + @NonNull + @Override + protected Float getNoneValue() { + return 0f; + } } diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataHeartRate.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataHeartRate.java index d1e7c1604..8682c6ae2 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataHeartRate.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataHeartRate.java @@ -18,4 +18,10 @@ public class SensorDataHeartRate extends SensorData { public String toString() { return super.toString() + " heart=" + value; } + + @NonNull + @Override + protected Float getNoneValue() { + return 0f; + } } diff --git a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataRunning.java b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataRunning.java index 8fa35fb78..b9416e163 100644 --- a/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataRunning.java +++ b/src/main/java/de/dennisguse/opentracks/content/sensor/SensorDataRunning.java @@ -53,6 +53,16 @@ public final class SensorDataRunning extends SensorData return totalDistance; } + @NonNull + @Override + protected Data getNoneValue() { + if (value != null) { + return new Data(Speed.zero(), 0f, ((Data) value).distance); + } else { + return new Data(Speed.zero(), 0f, Distance.of(0)); + } + } + public void compute(SensorDataRunning previous) { Distance overallDistance = null; if (hasTotalDistance() && previous != null && previous.hasTotalDistance()) { diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java index 19cc6a029..520fdf90d 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java @@ -172,6 +172,7 @@ class TrackRecordingManager { insertTrackPoint(trackId, trackPoint); isIdle = false; + lastTrackPoint = trackPoint; return; } diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index 04a1329db..d7af6eb5c 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -178,7 +178,9 @@ public class TrackStatisticsUpdater { } // Update moving time - currentSegment.addMovingTime(movingTime); + if (lastTrackPoint.isMoving()) { + currentSegment.addMovingTime(movingTime); + } // Update max speed if (trackPoint.hasSpeed() && lastTrackPoint.hasSpeed()) { diff --git a/src/main/java/de/dennisguse/opentracks/viewmodels/StatisticDataBuilder.java b/src/main/java/de/dennisguse/opentracks/viewmodels/StatisticDataBuilder.java index eca0ecfb4..98c407d5c 100644 --- a/src/main/java/de/dennisguse/opentracks/viewmodels/StatisticDataBuilder.java +++ b/src/main/java/de/dennisguse/opentracks/viewmodels/StatisticDataBuilder.java @@ -53,7 +53,7 @@ public class StatisticDataBuilder { boolean reportSpeed = fieldKey.equals("speed"); title = fieldKey.equals("speed") ? context.getString(R.string.stats_speed) : context.getString(R.string.stats_pace); Speed speed = latestTrackPoint != null && latestTrackPoint.hasSpeed() ? latestTrackPoint.getSpeed() : null; - if (sensorDataSet != null && sensorDataSet.getCyclingDistanceSpeed() != null && sensorDataSet.getCyclingDistanceSpeed().hasValue() && sensorDataSet.getCyclingDistanceSpeed().isRecent()) { + if (sensorDataSet != null && sensorDataSet.getCyclingDistanceSpeed() != null && sensorDataSet.getCyclingDistanceSpeed().hasValue()) { valueAndUnit = StringUtils.getSpeedParts(context, sensorDataSet.getCyclingDistanceSpeed().getValue().getSpeed(), metricUnits, reportSpeed); description = context.getString(R.string.description_speed_source_sensor, sensorDataSet.getCyclingDistanceSpeed().getSensorNameOrAddress()); } else { @@ -99,7 +99,7 @@ public class StatisticDataBuilder { } } else if (fieldKey.equals(context.getString(R.string.stats_custom_layout_heart_rate_key))) { title = context.getString(R.string.stats_sensors_heart_rate); - if (sensorDataSet != null && sensorDataSet.getHeartRate() != null && sensorDataSet.getHeartRate().hasValue() && sensorDataSet.getHeartRate().isRecent()) { + if (sensorDataSet != null && sensorDataSet.getHeartRate() != null && sensorDataSet.getHeartRate().hasValue()) { valueAndUnit = StringUtils.getHeartRateParts(context, sensorDataSet.getHeartRate().getValue()); description = sensorDataSet.getHeartRate().getSensorNameOrAddress(); } else { @@ -108,7 +108,7 @@ public class StatisticDataBuilder { } } else if (fieldKey.equals(context.getString(R.string.stats_custom_layout_cadence_key))) { title = context.getString(R.string.stats_sensors_cadence); - if (sensorDataSet != null && sensorDataSet.getCyclingCadence() != null && sensorDataSet.getCyclingCadence().hasValue() && sensorDataSet.getCyclingCadence().isRecent()) { + if (sensorDataSet != null && sensorDataSet.getCyclingCadence() != null && sensorDataSet.getCyclingCadence().hasValue()) { valueAndUnit = StringUtils.getCadenceParts(context, sensorDataSet.getCyclingCadence().getValue()); description = sensorDataSet.getCyclingCadence().getSensorNameOrAddress(); } else { @@ -117,7 +117,7 @@ public class StatisticDataBuilder { } } else if (fieldKey.equals(context.getString(R.string.stats_custom_layout_power_key))) { title = context.getString(R.string.stats_sensors_power); - if (sensorDataSet != null && sensorDataSet.getCyclingPower() != null && sensorDataSet.getCyclingPower().hasValue() && sensorDataSet.getCyclingPower().isRecent()) { + if (sensorDataSet != null && sensorDataSet.getCyclingPower() != null && sensorDataSet.getCyclingPower().hasValue()) { valueAndUnit = StringUtils.getPowerParts(context, sensorDataSet.getCyclingPower().getValue()); description = sensorDataSet.getCyclingPower().getSensorNameOrAddress(); } else { @@ -142,13 +142,13 @@ public class StatisticDataBuilder { if (sensorDataSet == null) { return sensorDataList; } - if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_heart_rate))) && sensorDataSet.getHeartRate() != null && sensorDataSet.getHeartRate().hasValue() && sensorDataSet.getHeartRate().isRecent()) { + if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_heart_rate))) && sensorDataSet.getHeartRate() != null && sensorDataSet.getHeartRate().hasValue()) { sensorDataList.add(build(context, recordingData, "heart_rate", true, metricUnits)); } - if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_cadence))) && sensorDataSet.getCyclingCadence() != null && sensorDataSet.getCyclingCadence().hasValue() && sensorDataSet.getCyclingCadence().isRecent()) { + if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_cadence))) && sensorDataSet.getCyclingCadence() != null && sensorDataSet.getCyclingCadence().hasValue()) { sensorDataList.add(build(context, recordingData, "cadence", true, metricUnits)); } - if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_power))) && sensorDataSet.getCyclingPower() != null && sensorDataSet.getCyclingPower().hasValue() && sensorDataSet.getCyclingPower().isRecent()) { + if (statisticDataList.stream().noneMatch(i -> i.getField().getTitle().equals(context.getString(R.string.stats_sensors_power))) && sensorDataSet.getCyclingPower() != null && sensorDataSet.getCyclingPower().hasValue()) { sensorDataList.add(build(context, recordingData, "power", true, metricUnits)); }