Merge pull request #1660 from OpenTracksApp/cleanup

Cleanup

src/main/java/de/dennisguse/opentracks/io/file/exporter/CSVTrackExporter.java

@@ -153,25 +153,19 @@ public class CSVTrackExporter implements TrackExporter {
     }
 
     public void close() {
-        if (printWriter != null) {
         printWriter.flush();
         printWriter = null;
     }
-    }
 
     public void writeHeader(List<Column> columns) {
-        if (printWriter != null) {
         String columnNames = columns.stream().map(c -> c.columnName).reduce((s, s2) -> s + "," + s2).orElseThrow(() -> new RuntimeException("No columns defined"));
         printWriter.println("#" + columnNames);
     }
-    }
 
     public void writeTrackPoint(List<Column> columns, TrackPoint trackPoint) {
-        if (printWriter != null) {
         String columnNames = columns.stream().map(c -> c.extractor.apply(trackPoint)).reduce((s, s2) -> s + "," + s2).orElseThrow(() -> new RuntimeException("No columns defined"));
         printWriter.println(columnNames);
     }
-    }
 
     private static class Column {
         final String columnName;

src/main/java/de/dennisguse/opentracks/io/file/exporter/GPXTrackExporter.java

@@ -199,15 +199,12 @@ public class GPXTrackExporter implements TrackExporter {
     }
 
     public void close() {
-        if (printWriter != null) {
         printWriter.flush();
         printWriter = null;
     }
-    }
 
 
     public void writeHeader() {
-        if (printWriter != null) {
         printWriter.println("<?xml version=\"1.0\" encoding=\"UTF-8\"?>");
         printWriter.println("<gpx");
         printWriter.println("version=\"1.1\"");
@@ -229,13 +226,10 @@ public class GPXTrackExporter implements TrackExporter {
                 + " http://www.cluetrust.com/Schemas http://www.cluetrust.com/Schemas/gpxdata10.xsd"
                 + " http://opentracksapp.com/xmlschemas/v1 http://opentracksapp.com/xmlschemas/OpenTracks_v1.xsd\">");
     }
-    }
 
     public void writeFooter() {
-        if (printWriter != null) {
         printWriter.println("</gpx>");
     }
-    }
 
     private void writeMarkers(Track track) throws InterruptedException {
         try (Cursor cursor = contentProviderUtils.getMarkerCursor(track.getId(), null, -1)) {
@@ -254,7 +248,6 @@ public class GPXTrackExporter implements TrackExporter {
     }
 
     public void writeMarker(ZoneOffset zoneOffset, Marker marker) {
-        if (printWriter != null) {
         printWriter.println("<wpt " + formatLocation(marker.getLatitude(), marker.getLongitude()) + ">");
         if (marker.hasAltitude()) {
             printWriter.println("<ele>" + ALTITUDE_FORMAT.format(marker.getAltitude().toM()) + "</ele>");
@@ -265,10 +258,8 @@ public class GPXTrackExporter implements TrackExporter {
         printWriter.println("<type>" + StringUtils.formatCData(marker.getCategory()) + "</type>");
         printWriter.println("</wpt>");
     }
-    }
 
     public void writeBeginTrack(Track track) {
-        if (printWriter != null) {
         printWriter.println("<trk>");
         printWriter.println("<name>" + StringUtils.formatCData(track.getName()) + "</name>");
         printWriter.println("<desc>" + StringUtils.formatCData(track.getDescription()) + "</desc>");
@@ -291,13 +282,10 @@ public class GPXTrackExporter implements TrackExporter {
 
         printWriter.println("</extensions>");
     }
-    }
 
     public void writeEndTrack() {
-        if (printWriter != null) {
         printWriter.println("</trk>");
     }
-    }
 
     public void writeOpenSegment() {
         printWriter.println("<trkseg>");
@@ -310,8 +298,6 @@ public class GPXTrackExporter implements TrackExporter {
     public Distance writeTrackPoint(ZoneOffset zoneOffset, TrackPoint trackPoint, List<TrackPoint> sensorPoints, Distance trackDistance) {
         Distance cumulativeDistance = null;
 
-        if (printWriter != null) {
-
         printWriter.println("<trkpt " + formatLocation(trackPoint.getLatitude(), trackPoint.getLongitude()) + ">");
 
         if (trackPoint.hasAltitude()) {
@@ -370,7 +356,7 @@ public class GPXTrackExporter implements TrackExporter {
         }
 
         printWriter.println("</trkpt>");
-        }
+
         if (cumulativeDistance != null) {
             return cumulativeDistance;
         }

src/main/java/de/dennisguse/opentracks/io/file/exporter/KMLTrackExporter.java

@@ -212,14 +212,11 @@ public class KMLTrackExporter implements TrackExporter {
 
     @VisibleForTesting
     void close() {
-        if (printWriter != null) {
         printWriter.flush();
         printWriter = null;
     }
-    }
 
     private void writeHeader(List<Track> tracks) {
-        if (printWriter != null) {
         printWriter.println(
                 """
                         <?xml version="1.0" encoding="UTF-8"?>
@@ -252,25 +249,19 @@ public class KMLTrackExporter implements TrackExporter {
 
         printWriter.println("</Schema>");
     }
-    }
 
     private void writeFooter() {
-        if (printWriter != null) {
         printWriter.println("</Document>");
         printWriter.println("</kml>");
     }
-    }
 
     private void writeBeginMarkers(Track track) {
-        if (printWriter != null) {
         printWriter.println("<Folder>");
         printWriter.println("<name>" + StringUtils.formatCData(context.getString(R.string.track_markers, track.getName())) + "</name>");
         printWriter.println("<open>1</open>");
     }
-    }
 
     private void writeMarker(Marker marker, ZoneOffset zoneOffset) {
-        if (printWriter != null) {
         boolean existsPhoto = MarkerUtils.buildInternalPhotoFile(context, marker.getTrackId(), marker.getPhotoURI()) != null;
         if (marker.hasPhoto() && exportPhotos && existsPhoto) {
             float heading = getHeading(marker.getTrackId(), marker.getLocation());
@@ -279,30 +270,22 @@ public class KMLTrackExporter implements TrackExporter {
             writePlacemark(marker.getName(), marker.getCategory(), marker.getDescription(), marker.getLocation(), zoneOffset);
         }
     }
-    }
 
     private void writeEndMarkers() {
-        if (printWriter != null) {
         printWriter.println("</Folder>");
     }
-    }
 
     private void writeMultiTrackBegin() {
-        if (printWriter != null) {
         printWriter.println("<Folder id=\"tracks\">");
         printWriter.println("<name>" + context.getString(R.string.generic_tracks) + "</name>");
         printWriter.println("<open>1</open>");
     }
-    }
 
     private void writeMultiTrackEnd() {
-        if (printWriter != null) {
         printWriter.println("</Folder>");
     }
-    }
 
     private void writeBeginTrack(Track track) {
-        if (printWriter != null) {
         printWriter.println("<Placemark>");
 
         printWriter.println("<name>" + StringUtils.formatCData(track.getName()) + "</name>");
@@ -316,19 +299,15 @@ public class KMLTrackExporter implements TrackExporter {
         printWriter.println("<altitudeMode>absolute</altitudeMode>");
         printWriter.println("<interpolate>1</interpolate>");
     }
-    }
 
 
     private void writeEndTrack() {
-        if (printWriter != null) {
         printWriter.println("</MultiTrack>");
         printWriter.println("</Placemark>");
     }
-    }
 
     @VisibleForTesting
     void writeOpenSegment() {
-        if (printWriter != null) {
         printWriter.println("<Track>");
         speedList.clear();
         distanceList.clear();
@@ -340,11 +319,9 @@ public class KMLTrackExporter implements TrackExporter {
         accuracyHorizontal.clear();
         accuracyVertical.clear();
     }
-    }
 
     @VisibleForTesting
     void writeCloseSegment() {
-        if (printWriter != null) {
         printWriter.println("<ExtendedData>");
         printWriter.println("<SchemaData schemaUrl=\"#" + SCHEMA_ID + "\">");
         if (speedList.stream().anyMatch(Objects::nonNull)) {
@@ -378,11 +355,9 @@ public class KMLTrackExporter implements TrackExporter {
         printWriter.println("</ExtendedData>");
         printWriter.println("</Track>");
     }
-    }
 
     @VisibleForTesting
     void writeTrackPoint(ZoneOffset zoneOffset, TrackPoint trackPoint) {
-        if (printWriter != null) {
         printWriter.println("<when>" + getTime(zoneOffset, trackPoint.getLocation()) + "</when>");
 
         if (trackPoint.hasLocation()) {
@@ -402,7 +377,6 @@ public class KMLTrackExporter implements TrackExporter {
         accuracyHorizontal.add(trackPoint.hasHorizontalAccuracy() ? (float) trackPoint.getHorizontalAccuracy().toM() : null);
         accuracyVertical.add(trackPoint.hasVerticalAccuracy() ? (float) trackPoint.getVerticalAccuracy().toM() : null);
     }
-    }
 
     /**
      * Writes the simple array data.

src/main/java/de/dennisguse/opentracks/services/TrackRecordingManager.java

@@ -21,6 +21,7 @@ import de.dennisguse.opentracks.data.models.Marker;
 import de.dennisguse.opentracks.data.models.Track;
 import de.dennisguse.opentracks.data.models.TrackPoint;
 import de.dennisguse.opentracks.sensors.sensorData.SensorDataSet;
+import de.dennisguse.opentracks.services.handlers.AltitudeCorrectionManager;
 import de.dennisguse.opentracks.services.handlers.TrackPointCreator;
 import de.dennisguse.opentracks.settings.PreferencesUtils;
 import de.dennisguse.opentracks.stats.TrackStatistics;
@@ -31,9 +32,13 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
 
     private static final String TAG = TrackRecordingManager.class.getSimpleName();
 
+    private static final AltitudeCorrectionManager ALTITUDE_CORRECTION_MANAGER = new AltitudeCorrectionManager();
+
     private final ContentProviderUtils contentProviderUtils;
     private final Context context;
 
+    private final TrackPointCreator trackPointCreator;
+
     private Distance recordingDistanceInterval;
     private Distance maxRecordingDistance;
 
@@ -47,8 +52,9 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
     private TrackPoint lastStoredTrackPoint;
     private TrackPoint lastStoredTrackPointWithLocation;
 
-    TrackRecordingManager(Context context) {
+    TrackRecordingManager(Context context, TrackPointCreator trackPointCreator) {
         this.context = context;
+        this.trackPointCreator = trackPointCreator;
         contentProviderUtils = new ContentProviderUtils(context);
     }
 
@@ -60,7 +66,7 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
         PreferencesUtils.unregisterOnSharedPreferenceChangeListener(this);
     }
 
-    Track.Id startNewTrack(TrackPointCreator trackPointCreator) {
+    Track.Id startNewTrack() {
         TrackPoint segmentStartTrackPoint = trackPointCreator.createSegmentStartManual();
         // Create new track
         ZoneOffset zoneOffset = ZoneOffset.systemDefault().getRules().getOffset(segmentStartTrackPoint.getTime());
@@ -86,7 +92,7 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
     /**
      * @return if the recording could be started.
      */
-    boolean resumeExistingTrack(@NonNull Track.Id resumeTrackId, @NonNull TrackPointCreator trackPointCreator) {
+    boolean resumeExistingTrack(@NonNull Track.Id resumeTrackId) {
         trackId = resumeTrackId;
         Track track = contentProviderUtils.getTrack(trackId);
         if (track == null) {
@@ -102,7 +108,7 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
         return true;
     }
 
-    void end(TrackPointCreator trackPointCreator) {
+    void end() {
         TrackPoint segmentEnd = trackPointCreator.createSegmentEnd();
         insertTrackPoint(segmentEnd, true);
 
@@ -112,15 +118,14 @@ class TrackRecordingManager implements SharedPreferences.OnSharedPreferenceChang
         reset();
     }
 
-    Pair<Track, Pair<TrackPoint, SensorDataSet>> getDataForUI(TrackPointCreator trackPointCreator) {
+    Pair<Track, Pair<TrackPoint, SensorDataSet>> getDataForUI() {
-        if (trackPointCreator == null) {
-            return null;
-        }
         TrackStatisticsUpdater tmpTrackStatisticsUpdater = new TrackStatisticsUpdater(trackStatisticsUpdater);
         Pair<TrackPoint, SensorDataSet> current = trackPointCreator.createCurrentTrackPoint(lastTrackPointUIWithSpeed, lastTrackPointUIWithAltitude, lastStoredTrackPointWithLocation);
 
         tmpTrackStatisticsUpdater.addTrackPoint(current.first);
 
+        ALTITUDE_CORRECTION_MANAGER.correctAltitude(context, current.first);
+
         Track track = contentProviderUtils.getTrack(trackId); //Get copy
         if (track == null) {
             Log.w(TAG, "Requesting data if not recording is taking place, should not be done.");

src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java

@@ -38,7 +38,6 @@ import de.dennisguse.opentracks.data.models.Track;
 import de.dennisguse.opentracks.data.models.TrackPoint;
 import de.dennisguse.opentracks.sensors.sensorData.SensorDataSet;
 import de.dennisguse.opentracks.services.announcement.VoiceAnnouncementManager;
-import de.dennisguse.opentracks.services.handlers.AltitudeCorrectionManager;
 import de.dennisguse.opentracks.services.handlers.GpsStatusValue;
 import de.dennisguse.opentracks.services.handlers.TrackPointCreator;
 import de.dennisguse.opentracks.util.SystemUtils;
@@ -71,13 +70,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         public void run() {
             updateRecordingDataWhileRecording();
 
-            Handler localHandler = TrackRecordingService.this.handler;
+            TrackRecordingService.this.handler.postDelayed(this, RECORDING_DATA_UPDATE_INTERVAL.toMillis());
-            if (localHandler == null) {
-                // when this happens, no recording is running and we should not send any notifications.
-                //TODO This implementation is not a good idea; rather solve the issue for this properly
-                return;
-            }
-            localHandler.postDelayed(this, RECORDING_DATA_UPDATE_INTERVAL.toMillis());
         }
     };
 
@@ -97,8 +90,6 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
     private VoiceAnnouncementManager voiceAnnouncementManager;
     private TrackRecordingServiceNotificationManager notificationManager;
 
-    private AltitudeCorrectionManager egm2008CorrectionManager;
-
     @Override
     public void onCreate() {
         super.onCreate();
@@ -110,10 +101,9 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         gpsStatusObservable = new MutableLiveData<>(STATUS_GPS_DEFAULT);
         recordingDataObservable = new MutableLiveData<>(NOT_RECORDING);
 
-        egm2008CorrectionManager = new AltitudeCorrectionManager();
-        trackRecordingManager = new TrackRecordingManager(this);
-        trackRecordingManager.start();
         trackPointCreator = new TrackPointCreator(this, this, handler);
+        trackRecordingManager = new TrackRecordingManager(this, trackPointCreator);
+        trackRecordingManager.start();
 
         voiceAnnouncementManager = new VoiceAnnouncementManager(this);
         notificationManager = new TrackRecordingServiceNotificationManager(this);
@@ -136,12 +126,8 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         notificationManager.stop();
         notificationManager = null;
 
-        egm2008CorrectionManager = null;
-        try {
         voiceAnnouncementManager.stop();
-        } finally {
         voiceAnnouncementManager = null;
-        }
 
         // This should be the next to last operation
         wakeLock = SystemUtils.releaseWakeLock(wakeLock);
@@ -171,7 +157,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         }
 
         // Set recording status
-        Track.Id trackId = trackRecordingManager.startNewTrack(trackPointCreator);
+        Track.Id trackId = trackRecordingManager.startNewTrack();
         updateRecordingStatus(RecordingStatus.record(trackId));
 
         startRecording();
@@ -179,7 +165,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
     }
 
     public void resumeTrack(Track.Id trackId) {
-        if (!trackRecordingManager.resumeExistingTrack(trackId, trackPointCreator)) {
+        if (!trackRecordingManager.resumeExistingTrack(trackId)) {
             Log.w(TAG, "Cannot resume a non-existing track.");
             return;
         }
@@ -223,7 +209,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         // Set recording status
         updateRecordingStatus(STATUS_DEFAULT);
 
-        trackRecordingManager.end(trackPointCreator);
+        trackRecordingManager.end();
         endRecording();
 
         stopSelf();
@@ -272,14 +258,9 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
 
     @Override
     public void newGpsStatus(GpsStatusValue gpsStatusValue) {
-
-        //TODO This check should not be necessary, but prevents a crash; somehow the shutdown is not working correctly as we should not receive a notification then.
-        // It is likely a race condition as the LocationManager provides location updates without using the Handler.
-        if (gpsStatusObservable != null) {
         notificationManager.updateContent(getString(gpsStatusValue.message));
         gpsStatusObservable.postValue(gpsStatusValue);
     }
-    }
 
     public Marker.Id insertMarker(String name, String category, String description, String photoUrl) {
         if (!isRecording()) {
@@ -316,26 +297,11 @@ public class TrackRecordingService extends Service implements TrackPointCreator.
         }
 
         // Compute temporary track statistics using sensorData and update time.
+        Pair<Track, Pair<TrackPoint, SensorDataSet>> data = trackRecordingManager.getDataForUI();
 
-        TrackPointCreator localTrackPointCreator = this.trackPointCreator;
+        voiceAnnouncementManager.update(this, data.first);
-        VoiceAnnouncementManager localVoiceAnnouncementManager = this.voiceAnnouncementManager;
-        if (localTrackPointCreator == null || localVoiceAnnouncementManager == null) {
-            // when this happens, no recording is running and we should not send any notifications.
-            //TODO This implementation is not a good idea; rather solve the issue for this properly
-            return;
-        }
 
-        Pair<Track, Pair<TrackPoint, SensorDataSet>> data = trackRecordingManager.getDataForUI(trackPointCreator);
+        recordingDataObservable.postValue(new RecordingData(data.first, data.second.first, data.second.second));
-        if (data == null) {
-            Log.w(TAG, "Requesting data if not recording is taking place, should not be done.");
-            return;
-        }
-        TrackPoint trackPoint = data.second.first;
-        egm2008CorrectionManager.correctAltitude(this, trackPoint);
-
-        localVoiceAnnouncementManager.update(this, data.first);
-
-        recordingDataObservable.postValue(new RecordingData(data.first, trackPoint, data.second.second));
     }
 
     @VisibleForTesting