TrackRecordingServiceConnection: use a callback instead of a Runnable.

src/main/java/de/dennisguse/opentracks/ControllerFragment.java

@@ -60,21 +60,11 @@ public class ControllerFragment extends Fragment implements View.OnTouchListener
     private TrackRecordingService.RecordingStatus recordingStatus;
     private TrackRecordingService.RecordingData recordingData;
 
-    private final Runnable bindChangedCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> {
-        @Override
+        service.getRecordingStatusObservable()
-        public void run() {
+                .observe(ControllerFragment.this, this::onRecordingStatusChanged);
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
+        service.getRecordingDataObservable()
-            if (service == null) {
+                .observe(ControllerFragment.this, this::onTotalTimeChanged);
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
-
-            service.getRecordingStatusObservable()
-                    .observe(ControllerFragment.this, status -> onRecordingStatusChanged(status));
-
-            service.getRecordingDataObservable()
-                    .observe(ControllerFragment.this, recordingData -> onTotalTimeChanged(recordingData));
-        }
     };
 
     @Override

src/main/java/de/dennisguse/opentracks/MarkerListActivity.java

@@ -20,7 +20,6 @@ import android.app.SearchManager;
 import android.content.Intent;
 import android.database.Cursor;
 import android.os.Bundle;
-import android.util.Log;
 import android.view.KeyEvent;
 import android.view.Menu;
 import android.view.MenuItem;
@@ -70,19 +69,8 @@ public class MarkerListActivity extends AbstractActivity implements DeleteMarker
 
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
 
-    private final Runnable bindCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindCallback = service -> service.getRecordingStatusObservable()
-        @Override
+            .observe(MarkerListActivity.this, this::onRecordingStatusChanged);
-        public void run() {
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
-
-            service.getRecordingStatusObservable()
-                    .observe(MarkerListActivity.this, status -> onRecordingStatusChanged(status));
-        }
-    };
 
     // Callback when an item is selected in the contextual action mode
     private final ContextualActionModeCallback contextualActionModeCallback = new ContextualActionModeCallback() {

src/main/java/de/dennisguse/opentracks/TrackListActivity.java

@@ -28,7 +28,6 @@ import android.graphics.drawable.AnimatedVectorDrawable;
 import android.location.LocationManager;
 import android.os.Bundle;
 import android.provider.Settings;
-import android.util.Log;
 import android.util.Pair;
 import android.view.KeyEvent;
 import android.view.Menu;
@@ -128,32 +127,21 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
     private MenuItem searchMenuItem;
     private MenuItem startGpsMenuItem;
 
-    // Callback when the trackRecordingServiceConnection binding changes.
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> {
-    private final Runnable bindChangedCallback = new Runnable() {
+        service.getRecordingStatusObservable()
-        @Override
+                .observe(TrackListActivity.this, this::onRecordingStatusChanged);
-        public void run() {
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.e(TAG, "service not available to start gps or a new recording");
-                return;
-            }
 
-            service.getRecordingStatusObservable()
+        service.getGpsStatusObservable()
-                    .observe(TrackListActivity.this, status -> onRecordingStatusChanged(status));
+                .observe(TrackListActivity.this, this::onGpsStatusChanged);
 
-            // Get GPS status and listen GPS status changes.
+        updateGpsMenuItem(true, recordingStatus.isRecording());
-            service.getGpsStatusObservable()
-                    .observe(TrackListActivity.this, status -> onGpsStatusChanged(status));
 
-            updateGpsMenuItem(true, recordingStatus.isRecording());
+        if (service.getGpsStatusObservable().getValue().isGpsStarted()) {
-
+            return;
-            if (service.getGpsStatusObservable().getValue().isGpsStarted()) {
-                return;
-            }
-
-            //TODO Not cool to do this in a callback that might be called more than once!
-            service.tryStartGps();
         }
+
+        //TODO Not cool to do this in a callback that might be called more than once!
+        service.tryStartGps();
     };
 
     @Override
@@ -305,12 +293,11 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
             } else {
                 // Invoke trackRecordingService
                 if (!gpsStatusValue.isGpsStarted()) {
-                    trackRecordingServiceConnection.startAndBind(this);
+                    trackRecordingServiceConnection.startAndBindWithCallback(this);
-                    bindChangedCallback.run();
                 } else {
                     TrackRecordingService trackRecordingService = trackRecordingServiceConnection.getServiceIfBound();
                     if (trackRecordingService != null) {
-                        trackRecordingService.stopGpsAndShutdown();
+                        trackRecordingService.stopGpsAndShutdown(); //TODO Handle this in TrackRecordingServiceConnection
                     }
                     trackRecordingServiceConnection.unbindAndStop(this);
                 }

src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java

@@ -77,20 +77,8 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement
 
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
 
-    private final Runnable bindCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindCallback = service -> service.getRecordingStatusObservable()
-        @Override
+            .observe(TrackRecordedActivity.this, this::onRecordingStatusChanged);
-        public void run() {
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
-
-            service.getRecordingStatusObservable()
-                    .observe(TrackRecordedActivity.this, status -> onRecordingStatusChanged(status));
-
-        }
-    };
 
     private final SharedPreferences.OnSharedPreferenceChangeListener sharedPreferenceChangeListener = new SharedPreferences.OnSharedPreferenceChangeListener() {
         @Override

src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java

@@ -67,14 +67,9 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
 
     private TrackRecordingService.RecordingStatus recordingStatus = TrackRecordingService.STATUS_DEFAULT;
 
-    private final Runnable bindChangedCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = new TrackRecordingServiceConnection.Callback() {
         @Override
-        public void run() {
+        public void onConnected(TrackRecordingService service) {
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
 
             service.getRecordingStatusObservable()
                     .observe(TrackRecordingActivity.this, status -> onRecordingStatusChanged(status));
@@ -211,13 +206,7 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
             trackDataHub.setRecordingStatus(recordingStatus);
         }
 
-        /*
+        trackRecordingServiceConnection.startAndBindWithCallback(this);
-         * If the binding has happened, then invoke the callback to start a new recording.
-         * If the binding hasn't happened, then invoking the callback will have no effect.
-         * But when the binding occurs, the callback will get invoked.
-         */
-        trackRecordingServiceConnection.startAndBind(this);
-        bindChangedCallback.run();
     }
 
     @Override

src/main/java/de/dennisguse/opentracks/fragments/StatisticsRecordingFragment.java

@@ -2,7 +2,6 @@ package de.dennisguse.opentracks.fragments;
 
 import android.content.SharedPreferences;
 import android.os.Bundle;
-import android.util.Log;
 import android.view.LayoutInflater;
 import android.view.View;
 import android.view.ViewGroup;
@@ -76,19 +75,8 @@ public class StatisticsRecordingFragment extends Fragment {
         }
     };
 
-    private final Runnable bindChangedCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> service.getRecordingDataObservable()
-        @Override
+            .observe(StatisticsRecordingFragment.this, this::onRecordingDataChanged);
-        public void run() {
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
-
-            service.getRecordingDataObservable()
-                    .observe(StatisticsRecordingFragment.this, recordingData -> onRecordingDataChanged(recordingData));
-        }
-    };
 
     @Override
     public void onCreate(@Nullable Bundle savedInstanceState) {

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

@@ -27,6 +27,7 @@ import android.util.Log;
 import android.widget.Toast;
 
 import androidx.annotation.NonNull;
+import androidx.annotation.Nullable;
 
 import de.dennisguse.opentracks.BuildConfig;
 import de.dennisguse.opentracks.R;
@@ -44,8 +45,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
 
     private static final String TAG = TrackRecordingServiceConnection.class.getSimpleName();
 
-    @Deprecated //TODO Implement proper interface that passes the current TrackRecordingService
+    private final Callback callback;
-    private final Runnable callback;
 
     private TrackRecordingService trackRecordingService;
 
@@ -53,10 +53,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         callback = null;
     }
 
-    /**
+    public TrackRecordingServiceConnection(@NonNull Callback callback) {
-     * @param callback the callback to invoke when the service binding changes
-     */
-    public TrackRecordingServiceConnection(@NonNull Runnable callback) {
         this.callback = callback;
     }
 
@@ -82,6 +79,23 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         startConnection(context);
     }
 
+    //TODO There should be a better way to implement this.
+
+    /**
+     * Triggers the onConnected() callback even if already connected.
+     */
+    //TODO Check if this is actually needed as it is used to re-connect from Activities in onResume by using a LiveData; might be obsolete. If not, there should be a better way to implement this.
+    @Deprecated
+    public void startAndBindWithCallback(Context context) {
+        if (trackRecordingService == null) {
+            startAndBind(context);
+            return;
+        }
+        if (callback != null) {
+            callback.onConnected(trackRecordingService);
+        }
+    }
+
     /**
      * Resumes the track recording service connection.
      *
@@ -118,22 +132,19 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         context.stopService(new Intent(context, TrackRecordingService.class));
     }
 
-    /**
+    @Nullable
-     * Gets the track recording service if bound. Returns null otherwise
-     */
     public TrackRecordingService getServiceIfBound() {
         return trackRecordingService;
     }
 
-    /**
-     * Sets the trackRecordingService.
-     *
-     * @param value the value
-     */
     private void setTrackRecordingService(TrackRecordingService value) {
         trackRecordingService = value;
         if (callback != null) {
-            callback.run();
+            if (value != null) {
+                callback.onConnected(value);
+            } else {
+                callback.onDisconnected();
+            }
         }
     }
 
@@ -160,31 +171,18 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         setTrackRecordingService(null);
     }
 
-    /**
-     * Resumes the recording track.
-     */
     public void resumeTrack() {
-        TrackRecordingService service = getServiceIfBound();
+        if (trackRecordingService != null) {
-        if (service != null) {
+            trackRecordingService.resumeCurrentTrack();
-            service.resumeCurrentTrack();
         }
     }
-
-    /**
-     * Pauses the recording track.
-     */
     public void pauseTrack() {
-        TrackRecordingService service = getServiceIfBound();
+        if (trackRecordingService != null) {
-        if (service != null) {
+            trackRecordingService.pauseCurrentTrack();
-            service.pauseCurrentTrack();
         }
     }
 
-    /**
+    @Nullable
-     * Adds a marker.
-     *
-     * @return the id of the marker or null if none could be created.
-     */
     public Marker.Id addMarker(Context context, String name, String category, String description, String photoUrl) {
         TrackRecordingService trackRecordingService = getServiceIfBound();
         if (trackRecordingService == null) {
@@ -214,4 +212,11 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         }
         unbindAndStop(context);
     }
+
+    public interface Callback {
+        void onConnected(TrackRecordingService service);
+
+        default void onDisconnected() {
+        }
+    }
 }

src/main/java/de/dennisguse/opentracks/settings/MainSettingsFragment.java

@@ -1,7 +1,6 @@
 package de.dennisguse.opentracks.settings;
 
 import android.os.Bundle;
-import android.util.Log;
 
 import androidx.fragment.app.DialogFragment;
 import androidx.preference.Preference;
@@ -17,20 +16,9 @@ public class MainSettingsFragment extends PreferenceFragmentCompat {
 
     private TrackRecordingService.RecordingStatus recordingStatus = TrackRecordingService.STATUS_DEFAULT;
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
-    private final Runnable bindServiceCallback = new Runnable() {
+    private final TrackRecordingServiceConnection.Callback bindServiceCallback =
-        @Override
+            service -> service.getRecordingStatusObservable()
-        public void run() {
+                    .observe(MainSettingsFragment.this, this::onRecordingStatusChanged);
-            TrackRecordingService service = trackRecordingServiceConnection.getServiceIfBound();
-            if (service == null) {
-                Log.w(TAG, "could not get TrackRecordingService");
-                return;
-            }
-
-            service.getRecordingStatusObservable()
-                    .observe(MainSettingsFragment.this, status -> onRecordingStatusChanged(status));
-
-        }
-    };
 
     @Override
     public void onCreatePreferences(Bundle savedInstanceState, String rootKey) {