diff --git a/src/main/java/de/dennisguse/opentracks/fragments/StatsFragment.java b/src/main/java/de/dennisguse/opentracks/fragments/StatsFragment.java index 874d0bd64..647d44390 100644 --- a/src/main/java/de/dennisguse/opentracks/fragments/StatsFragment.java +++ b/src/main/java/de/dennisguse/opentracks/fragments/StatsFragment.java @@ -49,6 +49,7 @@ import de.dennisguse.opentracks.util.LocationUtils; import de.dennisguse.opentracks.util.PreferencesUtils; import de.dennisguse.opentracks.util.StringUtils; import de.dennisguse.opentracks.util.TrackIconUtils; +import de.dennisguse.opentracks.util.TrackPointUtils; import de.dennisguse.opentracks.util.UnitConversions; /** @@ -356,7 +357,7 @@ public class StatsFragment extends Fragment implements TrackDataListener { if (lastTrackPoint != null) { boolean hasFix = !LocationUtils.isLocationOld(lastTrackPoint.getLocation()); - boolean hasGoodFix = lastTrackPoint.hasAccuracy() && lastTrackPoint.getAccuracy() < recordingGpsAccuracy; + boolean hasGoodFix = TrackPointUtils.fulfillsAccuracy(lastTrackPoint, recordingGpsAccuracy); if (!hasFix || !hasGoodFix) { lastTrackPoint = null; diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index dbecce960..3d8a0e1a7 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -33,6 +33,7 @@ import android.os.IBinder; import android.os.PowerManager.WakeLock; import android.util.Log; +import androidx.annotation.NonNull; import androidx.core.app.TaskStackBuilder; import java.util.concurrent.ExecutorService; @@ -59,6 +60,7 @@ import de.dennisguse.opentracks.util.PreferencesUtils; import de.dennisguse.opentracks.util.SystemUtils; import de.dennisguse.opentracks.util.TrackIconUtils; import de.dennisguse.opentracks.util.TrackNameUtils; +import de.dennisguse.opentracks.util.TrackPointUtils; import de.dennisguse.opentracks.util.UnitConversions; /** @@ -69,11 +71,8 @@ import de.dennisguse.opentracks.util.UnitConversions; */ public class TrackRecordingService extends Service { - // Anything faster than that (in meters per second) will be considered moving. private static final String TAG = TrackRecordingService.class.getSimpleName(); - public static final double MAX_NO_MOVEMENT_SPEED = 0.224; - // The following variables are set in onCreate: private ExecutorService executorService; private ContentProviderUtils contentProviderUtils; @@ -455,7 +454,9 @@ public class TrackRecordingService extends Service { if (track != null) { // If not paused, add the last location if (!paused) { - insertTrackPoint(track, lastTrackPoint, getLastValidTrackPointInCurrentSegment(trackId)); + if (lastTrackPoint != null) { + insertTrackPoint(track, lastTrackPoint, getLastValidTrackPointInCurrentSegment(trackId)); + } // Update the recording track time updateRecordingTrack(track); @@ -574,28 +575,24 @@ public class TrackRecordingService extends Service { TrackPoint trackPoint = new TrackPoint(location, getSensorDataSet()); notificationManager.updateTrackPoint(this, trackPoint, recordingGpsAccuracy); - if (!location.hasAccuracy() || location.getAccuracy() >= recordingGpsAccuracy) { + if (!TrackPointUtils.fulfillsAccuracy(trackPoint, recordingGpsAccuracy)) { Log.d(TAG, "Ignore onLocationChangedAsync. Poor accuracy."); return; } - //TODO Necessary? - // Fix for phones that do not set the time field - if (location.getTime() == 0L) { - location.setTime(System.currentTimeMillis()); - } + TrackPointUtils.fixTime(trackPoint); + //TODO Figure out how to avoid loading the lastValidTrackPoint from the database TrackPoint lastValidTrackPoint = getLastValidTrackPointInCurrentSegment(track.getId()); long idleTime = 0L; - if (lastValidTrackPoint != null && location.getTime() > lastValidTrackPoint.getLocation().getTime()) { - idleTime = location.getTime() - lastValidTrackPoint.getLocation().getTime(); + if (TrackPointUtils.after(trackPoint, lastValidTrackPoint)) { + idleTime = trackPoint.getTime() - lastValidTrackPoint.getLocation().getTime(); } locationListenerPolicy.updateIdleTime(idleTime); if (currentRecordingInterval != locationListenerPolicy.getDesiredPollingInterval()) { registerLocationListener(); } - // Always insert the first segment location if (!currentSegmentHasLocation) { insertTrackPoint(track, trackPoint, null); @@ -611,7 +608,7 @@ public class TrackRecordingService extends Service { return; } - double distanceToLastTrackLocation = location.distanceTo(lastValidTrackPoint.getLocation()); + double distanceToLastTrackLocation = trackPoint.distanceTo(lastValidTrackPoint); if (distanceToLastTrackLocation > maxRecordingDistance) { insertTrackPoint(track, lastTrackPoint, lastValidTrackPoint); insertTrackPoint(track, TrackPoint.createPause(), null); @@ -622,16 +619,16 @@ public class TrackRecordingService extends Service { insertTrackPoint(track, lastTrackPoint, lastValidTrackPoint); insertTrackPoint(track, trackPoint, null); isIdle = false; - } else if (!isIdle && location.hasSpeed() && location.getSpeed() < MAX_NO_MOVEMENT_SPEED) { + } else if (!isIdle && !TrackPointUtils.isMoving(trackPoint)) { insertTrackPoint(track, lastTrackPoint, lastValidTrackPoint); insertTrackPoint(track, trackPoint, null); isIdle = true; - } else if (isIdle && location.hasSpeed() && location.getSpeed() >= MAX_NO_MOVEMENT_SPEED) { + } else if (isIdle && TrackPointUtils.isMoving(trackPoint)) { insertTrackPoint(track, lastTrackPoint, lastValidTrackPoint); insertTrackPoint(track, trackPoint, null); isIdle = false; } else { - Log.d(TAG, "Not recording location, idle"); + Log.d(TAG, "Not recording TrackPoint, idle"); } lastTrackPoint = trackPoint; } @@ -643,14 +640,10 @@ public class TrackRecordingService extends Service { * @param trackPoint the trackPoint * @param lastValidTrackPoint the last valid track point, can be null */ - private void insertTrackPoint(Track track, TrackPoint trackPoint, TrackPoint lastValidTrackPoint) { - if (trackPoint == null) { - Log.w(TAG, "Ignore insertLocation. trackPoint is null."); - return; - } - // Do not insert if inserted already - if (lastValidTrackPoint != null && lastValidTrackPoint.getTime() == trackPoint.getTime()) { - Log.w(TAG, "Ignore insertLocation. trackPoint time same as last valid track point time."); + private void insertTrackPoint(@NonNull Track track, @NonNull TrackPoint trackPoint, TrackPoint lastValidTrackPoint) { + if (TrackPointUtils.equalTime(trackPoint, lastValidTrackPoint)) { + // Do not insert if inserted already + Log.w(TAG, "Ignore insertTrackPoint. trackPoint time same as last valid track point time."); return; } diff --git a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java index 02b69bad0..1d09881e2 100644 --- a/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java +++ b/src/main/java/de/dennisguse/opentracks/stats/TrackStatisticsUpdater.java @@ -18,14 +18,14 @@ package de.dennisguse.opentracks.stats; import android.util.Log; +import androidx.annotation.NonNull; import androidx.annotation.VisibleForTesting; import de.dennisguse.opentracks.content.data.TrackPoint; import de.dennisguse.opentracks.content.data.TrackPointsColumns; import de.dennisguse.opentracks.content.provider.TrackPointIterator; import de.dennisguse.opentracks.util.LocationUtils; - -import static de.dennisguse.opentracks.services.TrackRecordingService.MAX_NO_MOVEMENT_SPEED; +import de.dennisguse.opentracks.util.TrackPointUtils; /** * Updater for {@link TrackStatistics}. @@ -143,7 +143,7 @@ public class TrackStatisticsUpdater { } double movingDistance = lastMovingTrackPoint.distanceTo(trackPoint); - if (movingDistance < minRecordingDistance && (!trackPoint.hasSpeed() || trackPoint.getSpeed() < MAX_NO_MOVEMENT_SPEED)) { + if (movingDistance < minRecordingDistance && !TrackPointUtils.isMoving(trackPoint)) { speedBuffer_ms.reset(); lastTrackPoint = trackPoint; return; @@ -162,7 +162,7 @@ public class TrackStatisticsUpdater { // Update max speed if (trackPoint.hasSpeed() && lastTrackPoint.hasSpeed()) { - updateSpeed(trackPoint.getTime(), trackPoint.getSpeed(), lastTrackPoint.getTime(), lastTrackPoint.getSpeed()); + updateSpeed(trackPoint, lastTrackPoint); } lastTrackPoint = trackPoint; @@ -190,23 +190,18 @@ public class TrackStatisticsUpdater { /** * Updates a speed reading while assuming the user is moving. - * - * @param time the time - * @param speed the speed - * @param lastLocationTime the last location time - * @param lastLocationSpeed the last location speed */ @VisibleForTesting - private void updateSpeed(long time, double speed, long lastLocationTime, double lastLocationSpeed) { - if (speed < MAX_NO_MOVEMENT_SPEED) { + private void updateSpeed(@NonNull TrackPoint trackPoint, @NonNull TrackPoint lastTrackPoint) { + if (!TrackPointUtils.isMoving(trackPoint)) { speedBuffer_ms.reset(); - } else if (isValidSpeed(time, speed, lastLocationTime, lastLocationSpeed)) { - speedBuffer_ms.setNext(speed); + } else if (isValidSpeed(trackPoint, lastTrackPoint)) { + speedBuffer_ms.setNext(trackPoint.getSpeed()); if (speedBuffer_ms.getAverage() > currentSegment.getMaxSpeed()) { currentSegment.setMaxSpeed(speedBuffer_ms.getAverage()); } } else { - Log.d(TAG, "Invalid speed. speed: " + speed + " lastLocationSpeed: " + lastLocationSpeed); + Log.d(TAG, "Invalid speed. speed: " + trackPoint.getSpeed() + " lastLocationSpeed: " + lastTrackPoint.getSpeed()); } } @@ -239,26 +234,21 @@ public class TrackStatisticsUpdater { /** * Returns true if the speed is valid. - * - * @param time the time - * @param speed the speed - * @param lastLocationTime the last location time - * @param lastLocationSpeed the last location speed */ - private boolean isValidSpeed(long time, double speed, long lastLocationTime, double lastLocationSpeed) { + 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 (speed == 0) { + if (trackPoint.getSpeed() == 0) { return false; } // The following code will ignore unlikely readings. 128 m/s seems to be an internal android error code. - if (Math.abs(speed - 128) < 1) { + if (Math.abs(trackPoint.getSpeed() - 128) < 1) { return false; } // See if the speed seems physically likely. Ignore any speeds that imply acceleration greater than 2g. - long timeDifference = time - lastLocationTime; - double speedDifference = Math.abs(lastLocationSpeed - speed); + long timeDifference = trackPoint.getTime() - lastTrackPoint.getTime(); + double speedDifference = Math.abs(lastTrackPoint.getSpeed() - trackPoint.getSpeed()); if (speedDifference > MAX_ACCELERATION * timeDifference) { return false; } @@ -266,8 +256,8 @@ public class TrackStatisticsUpdater { // 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_ms.isFull()) { double average = speedBuffer_ms.getAverage(); - double diff = Math.abs(average - speed); - return (speed < average * 10) && (diff < MAX_ACCELERATION * timeDifference); + double diff = Math.abs(average - trackPoint.getSpeed()); + return (trackPoint.getSpeed() < average * 10) && (diff < MAX_ACCELERATION * timeDifference); } return true; diff --git a/src/main/java/de/dennisguse/opentracks/util/TrackPointUtils.java b/src/main/java/de/dennisguse/opentracks/util/TrackPointUtils.java new file mode 100644 index 000000000..e7251049c --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/util/TrackPointUtils.java @@ -0,0 +1,56 @@ +package de.dennisguse.opentracks.util; + +import android.util.Log; + +import androidx.annotation.NonNull; + +import de.dennisguse.opentracks.content.data.TrackPoint; + +public class TrackPointUtils { + + // Anything faster than that (in meters per second) will be considered moving. + private static final double MAX_NO_MOVEMENT_SPEED = 0.224; + + private final static String TAG = TrackPointUtils.class.getSimpleName(); + + private TrackPointUtils() { + } + + /** + * Ancient fix for phones that do not set the time in {@link android.location.Location}. + */ + //TODO Necessary? + public static void fixTime(@NonNull TrackPoint trackPoint) { + if (trackPoint.getTime() == 0L) { + Log.w(TAG, "Time of provided location was 0. Using current time."); + trackPoint.setTime(System.currentTimeMillis()); + } + } + + public static boolean isMoving(@NonNull TrackPoint trackPoint) { + return trackPoint.hasSpeed() && trackPoint.getSpeed() >= MAX_NO_MOVEMENT_SPEED; + } + + public static boolean equalTime(TrackPoint t1, TrackPoint t2) { + if (t1 == null || t2 == null) { + return false; + } + + return t1.getTime() == t2.getTime(); + } + + public static boolean after(TrackPoint t1, TrackPoint t2) { + if (t1 == null || t2 == null) { + return false; + } + + return t1.getTime() > t2.getTime(); + } + + /** + * Is accuracy better than threshold? + */ + public static boolean fulfillsAccuracy(@NonNull TrackPoint trackPoint, int poorAccuracy) { + return trackPoint.hasAccuracy() && trackPoint.getAccuracy() < poorAccuracy; + } +}