From 41da2f5b5399fc761605e6418842381e5d86ba74 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 21:36:19 +0100 Subject: [PATCH 1/7] Cleanup: simplify TrackServiceConnection. --- .../opentracks/TrackRecordedActivity.java | 2 +- .../TrackRecordingServiceConnection.java | 35 ++++++------------- .../ui/markers/MarkerListActivity.java | 2 +- 3 files changed, 13 insertions(+), 26 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java index d63cfe8ed..c6d9364e2 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java @@ -126,7 +126,7 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement trackDataHub.loadTrack(trackId); } - trackRecordingServiceConnection.bind(this); + trackRecordingServiceConnection.startConnection(this); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index 706bdbbd8..6bdca8947 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -58,13 +58,6 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death this.callback = callback; } - public void bind(@NonNull Context context) { - if (trackRecordingService != null) { - return; - } - context.bindService(new Intent(context, TrackRecordingService.class), this, 0); - } - /** * Starts and binds the service. * @@ -132,11 +125,6 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death context.stopService(new Intent(context, TrackRecordingService.class)); } - @Nullable - public TrackRecordingService getServiceIfBound() { - return trackRecordingService; - } - private void setTrackRecordingService(TrackRecordingService value) { trackRecordingService = value; if (callback != null) { @@ -173,19 +161,19 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death @Nullable public Marker.Id addMarker(Context context, String name, String category, String description, String photoUrl) { - TrackRecordingService trackRecordingService = getServiceIfBound(); if (trackRecordingService == null) { Log.d(TAG, "Unable to add marker, no track recording service"); - } else { - try { - Marker.Id marker = trackRecordingService.insertMarker(name, category, description, photoUrl); - if (marker != null) { - Toast.makeText(context, R.string.marker_add_success, Toast.LENGTH_SHORT).show(); - return marker; - } - } catch (IllegalStateException e) { - Log.e(TAG, "Unable to add marker.", e); + return null; + } + + try { + Marker.Id marker = trackRecordingService.insertMarker(name, category, description, photoUrl); + if (marker != null) { + Toast.makeText(context, R.string.marker_add_success, Toast.LENGTH_SHORT).show(); + return marker; } + } catch (IllegalStateException e) { + Log.e(TAG, "Unable to add marker.", e); } Toast.makeText(context, R.string.marker_add_error, Toast.LENGTH_LONG).show(); @@ -193,7 +181,6 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death } public void stopRecording(@NonNull Context context) { - TrackRecordingService trackRecordingService = getServiceIfBound(); if (trackRecordingService == null) { Log.e(TAG, "TrackRecordingService not connected."); } else { @@ -203,7 +190,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death } public interface Callback { - void onConnected(TrackRecordingService service, TrackRecordingServiceConnection connection); + void onConnected(TrackRecordingService service, TrackRecordingServiceConnection self); default void onDisconnected() { } diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java index b72a52f09..6ad125a44 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java @@ -125,7 +125,7 @@ public class MarkerListActivity extends AbstractActivity { @Override protected void onResume() { super.onResume(); - trackRecordingServiceConnection.bind(this); + trackRecordingServiceConnection.startConnection(this); this.invalidateOptionsMenu(); loadData(); } From e2f598b2824edd3c1fc9764f8f5d20761699f188 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 21:38:53 +0100 Subject: [PATCH 2/7] Cleanup: simplify TrackServiceConnection. --- .../TrackRecordingServiceConnection.java | 58 +++++++++---------- 1 file changed, 27 insertions(+), 31 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index 6bdca8947..43cbf4ed1 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -42,7 +42,7 @@ import de.dennisguse.opentracks.data.models.Marker; * * @author Rodrigo Damazio */ -public class TrackRecordingServiceConnection implements ServiceConnection, DeathRecipient { +public class TrackRecordingServiceConnection { private static final String TAG = TrackRecordingServiceConnection.class.getSimpleName(); @@ -50,6 +50,30 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death private TrackRecordingService trackRecordingService; + private final ServiceConnection serviceConnection = new ServiceConnection() { + @Override + public void onServiceConnected(ComponentName className, IBinder service) { + Log.i(TAG, "Connected to the service: " + service); + try { + service.linkToDeath(deathRecipient, 0); + } catch (RemoteException e) { + Log.e(TAG, "Failed to bind a death recipient.", e); + } + setTrackRecordingService(((TrackRecordingService.Binder) service).getService()); + } + + @Override + public void onServiceDisconnected(ComponentName className) { + Log.i(TAG, "Disconnected from the service."); + setTrackRecordingService(null); + } + }; + + private final DeathRecipient deathRecipient = () -> { + Log.d(TAG, "Service died."); + setTrackRecordingService(null); + }; + public TrackRecordingServiceConnection() { callback = null; } @@ -104,7 +128,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death Log.i(TAG, "Binding the service."); int flags = BuildConfig.DEBUG ? Context.BIND_DEBUG_UNBIND : 0; - context.bindService(new Intent(context, TrackRecordingService.class), this, flags); + context.bindService(new Intent(context, TrackRecordingService.class), serviceConnection, flags); } /** @@ -113,7 +137,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death //TODO This is often called for one-shot operations and should be refactored as unbinding is required. public void unbind(Context context) { try { - context.unbindService(this); + context.unbindService(serviceConnection); } catch (IllegalArgumentException e) { // Means not bound to the service. OK to ignore. } @@ -130,35 +154,10 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death if (callback != null) { if (value != null) { callback.onConnected(value, this); - } else { - callback.onDisconnected(); } } } - @Override - public void onServiceConnected(ComponentName className, IBinder service) { - Log.i(TAG, "Connected to the service: " + service); - try { - service.linkToDeath(this, 0); - } catch (RemoteException e) { - Log.e(TAG, "Failed to bind a death recipient.", e); - } - setTrackRecordingService(((TrackRecordingService.Binder) service).getService()); - } - - @Override - public void onServiceDisconnected(ComponentName className) { - Log.i(TAG, "Disconnected from the service."); - setTrackRecordingService(null); - } - - @Override - public void binderDied() { - Log.d(TAG, "Service died."); - setTrackRecordingService(null); - } - @Nullable public Marker.Id addMarker(Context context, String name, String category, String description, String photoUrl) { if (trackRecordingService == null) { @@ -191,8 +190,5 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death public interface Callback { void onConnected(TrackRecordingService service, TrackRecordingServiceConnection self); - - default void onDisconnected() { - } } } From abbc61d4193eb1e0aaf96b789ee274cad2858d1f Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 21:50:55 +0100 Subject: [PATCH 3/7] TrackRecordingService: start via ServiceCompat. --- build.gradle | 1 + .../services/TrackRecordingService.java | 19 ++++++++----------- 2 files changed, 9 insertions(+), 11 deletions(-) diff --git a/build.gradle b/build.gradle index 4b7ec7406..37778f45d 100644 --- a/build.gradle +++ b/build.gradle @@ -133,6 +133,7 @@ dependencies { implementation 'androidx.gridlayout:gridlayout:1.0.0' implementation 'com.google.android.material:material:1.11.0' implementation 'androidx.constraintlayout:constraintlayout:2.1.4' + implementation 'androidx.core:core:1.12.0' implementation 'androidx.core:core-splashscreen:1.0.1' implementation 'androidx.mediarouter:mediarouter:1.6.0' diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 6c24c7de9..29cedf294 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -30,6 +30,7 @@ import android.widget.Toast; import androidx.annotation.Nullable; import androidx.annotation.VisibleForTesting; +import androidx.core.app.ServiceCompat; import androidx.lifecycle.LiveData; import androidx.lifecycle.MutableLiveData; @@ -111,7 +112,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator. recordingDataObservable = new MutableLiveData<>(NOT_RECORDING); trackPointCreator = new TrackPointCreator(this); - trackRecordingManager = new TrackRecordingManager(this, trackPointCreator, this , handler); + trackRecordingManager = new TrackRecordingManager(this, trackPointCreator, this, handler); voiceAnnouncementManager = new VoiceAnnouncementManager(this); notificationManager = new TrackRecordingServiceNotificationManager(this); @@ -213,19 +214,15 @@ public class TrackRecordingService extends Service implements TrackPointCreator. Log.i(TAG, "startSensors"); wakeLock = SystemUtils.acquireWakeLock(this, wakeLock); trackPointCreator.start(this, handler); - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { - if (!PermissionRequester.RECORDING.hasPermission(this)) { - Toast.makeText(this, R.string.permission_recording_failed, Toast.LENGTH_LONG).show(); - return; - } + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.UPSIDE_DOWN_CAKE) { + if (!PermissionRequester.RECORDING.hasPermission(this)) { + Toast.makeText(this, R.string.permission_recording_failed, Toast.LENGTH_LONG).show(); + return; } - - startForeground(TrackRecordingServiceNotificationManager.NOTIFICATION_ID, notificationManager.setGPSonlyStarted(this), ServiceInfo.FOREGROUND_SERVICE_TYPE_LOCATION + ServiceInfo.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE); - } else { - startForeground(TrackRecordingServiceNotificationManager.NOTIFICATION_ID, notificationManager.setGPSonlyStarted(this)); } + + ServiceCompat.startForeground(this, TrackRecordingServiceNotificationManager.NOTIFICATION_ID, notificationManager.setGPSonlyStarted(this), ServiceInfo.FOREGROUND_SERVICE_TYPE_LOCATION + ServiceInfo.FOREGROUND_SERVICE_TYPE_CONNECTED_DEVICE); } public void endCurrentTrack() { From 5c6a4a71a41d0dbdf2853a225c5f361b454af4f1 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Tue, 26 Dec 2023 08:30:40 +0100 Subject: [PATCH 4/7] TrackRecordingService: always starts as foreground service. --- .../opentracks/TrackListActivity.java | 2 +- .../opentracks/TrackRecordedActivity.java | 2 +- .../opentracks/TrackStoppedActivity.java | 2 +- .../publicapi/AbstractAPIActivity.java | 6 +----- .../opentracks/publicapi/StartRecording.java | 5 ----- .../TrackRecordingServiceConnection.java | 21 +++++-------------- 6 files changed, 9 insertions(+), 29 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java index 5e7a53348..f269a763a 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java @@ -191,7 +191,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co startActivity(newIntent); connection.unbind(this); - }).startAndBind(this, true); + }).startAndBind(this); }); viewBinding.trackListFabAction.setOnLongClickListener((view) -> { if (!recordingStatus.isRecording()) { diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java index c6d9364e2..5127f6e47 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java @@ -211,7 +211,7 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement connection.unbind(this); finish(); - }).startAndBind(this, true); + }).startAndBind(this); return true; } diff --git a/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java index 03ddd5fc4..6cfce7898 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java @@ -146,7 +146,7 @@ public class TrackStoppedActivity extends AbstractTrackDeleteActivity implements connection.unbind(this); finish(); - }).startAndBind(this, true); + }).startAndBind(this); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java b/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java index 5684e0bcf..98b6b83ec 100644 --- a/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java +++ b/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java @@ -38,7 +38,7 @@ public abstract class AbstractAPIActivity extends AppCompatActivity { if (PreferencesUtils.isPublicAPIenabled()) { Log.i(TAG, "Received and trying to execute requested action."); new TrackRecordingServiceConnection(serviceConnectedCallback) - .startAndBind(this, isStartServiceForeground()); + .startAndBind(this); } else { Toast.makeText(this, getString(R.string.settings_public_api_disabled_toast), Toast.LENGTH_LONG).show(); Log.w(TAG, "Public API is disabled; ignoring request."); @@ -46,10 +46,6 @@ public abstract class AbstractAPIActivity extends AppCompatActivity { } } - protected boolean isStartServiceForeground() { - return false; - } - protected abstract void execute(TrackRecordingService service); protected abstract boolean isPostExecuteStopService(); diff --git a/src/main/java/de/dennisguse/opentracks/publicapi/StartRecording.java b/src/main/java/de/dennisguse/opentracks/publicapi/StartRecording.java index a40d83ae5..ff778a94a 100644 --- a/src/main/java/de/dennisguse/opentracks/publicapi/StartRecording.java +++ b/src/main/java/de/dennisguse/opentracks/publicapi/StartRecording.java @@ -62,9 +62,4 @@ public class StartRecording extends AbstractAPIActivity { protected boolean isPostExecuteStopService() { return false; } - - @Override - protected boolean isStartServiceForeground() { - return true; - } } diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index 43cbf4ed1..d2a631de8 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -82,23 +82,13 @@ public class TrackRecordingServiceConnection { this.callback = callback; } - /** - * Starts and binds the service. - * - * @param foreground is the service expected to call `startForeground()`? - */ - public void startAndBind(Context context, boolean foreground) { + public void startAndBind(Context context) { if (trackRecordingService != null) { // Service is already started and bound. return; } - Log.i(TAG, "Starting the service."); - if (foreground) { - ContextCompat.startForegroundService(context, new Intent(context, TrackRecordingService.class)); - } else { - context.startService(new Intent(context, TrackRecordingService.class)); - } + ContextCompat.startForegroundService(context, new Intent(context, TrackRecordingService.class)); startConnection(context); } @@ -112,7 +102,7 @@ public class TrackRecordingServiceConnection { @Deprecated public void startAndBindWithCallback(Context context) { if (trackRecordingService == null) { - startAndBind(context, false); + startAndBind(context); return; } if (callback != null) { @@ -151,13 +141,12 @@ public class TrackRecordingServiceConnection { private void setTrackRecordingService(TrackRecordingService value) { trackRecordingService = value; - if (callback != null) { - if (value != null) { + if (callback != null && value != null) { callback.onConnected(value, this); - } } } + //TODO Move to some other place; not needed here. @Nullable public Marker.Id addMarker(Context context, String name, String category, String description, String photoUrl) { if (trackRecordingService == null) { From df8306c2f8c37033d5bc288c6d0068855fdc268e Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Tue, 26 Dec 2023 08:33:03 +0100 Subject: [PATCH 5/7] Cleanup: TrackEditActivity doesn't need a TrackRecordingServiceConnection. --- .../dennisguse/opentracks/TrackEditActivity.java | 15 --------------- 1 file changed, 15 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackEditActivity.java b/src/main/java/de/dennisguse/opentracks/TrackEditActivity.java index e1ac6612a..bb0f90303 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackEditActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackEditActivity.java @@ -28,7 +28,6 @@ import de.dennisguse.opentracks.data.models.ActivityType; import de.dennisguse.opentracks.data.models.Track; import de.dennisguse.opentracks.databinding.TrackEditBinding; import de.dennisguse.opentracks.fragments.ChooseActivityTypeDialogFragment; -import de.dennisguse.opentracks.services.TrackRecordingServiceConnection; import de.dennisguse.opentracks.util.TrackUtils; /** @@ -44,7 +43,6 @@ public class TrackEditActivity extends AbstractActivity implements ChooseActivit private static final String ICON_VALUE_KEY = "icon_value_key"; - private TrackRecordingServiceConnection trackRecordingServiceConnection; private ContentProviderUtils contentProviderUtils; private Track track; private ActivityType activityType; @@ -55,7 +53,6 @@ public class TrackEditActivity extends AbstractActivity implements ChooseActivit protected void onCreate(Bundle bundle) { super.onCreate(bundle); - trackRecordingServiceConnection = new TrackRecordingServiceConnection(); Track.Id trackId = getIntent().getParcelableExtra(EXTRA_TRACK_ID); if (trackId == null) { Log.e(TAG, "invalid trackId"); @@ -114,18 +111,6 @@ public class TrackEditActivity extends AbstractActivity implements ChooseActivit setSupportActionBar(viewBinding.bottomAppBarLayout.bottomAppBar); } - @Override - protected void onStart() { - super.onStart(); - trackRecordingServiceConnection.startConnection(this); - } - - @Override - protected void onStop() { - super.onStop(); - trackRecordingServiceConnection.unbind(this); - } - @Override public void onSaveInstanceState(@NonNull Bundle outState) { super.onSaveInstanceState(outState); From 59aeec4fbc9335f62a23ab6ef569075e5bfcca01 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Tue, 26 Dec 2023 11:30:50 +0100 Subject: [PATCH 6/7] Cleanup: TrackRecordingServiceConnection. --- .../opentracks/TrackListActivity.java | 5 +-- .../opentracks/TrackRecordedActivity.java | 2 +- .../opentracks/TrackRecordingActivity.java | 2 +- .../StatisticsRecordingFragment.java | 2 +- .../TrackRecordingServiceConnection.java | 42 +++++++++---------- .../ui/markers/MarkerEditViewModel.java | 4 +- .../ui/markers/MarkerListActivity.java | 4 +- 7 files changed, 29 insertions(+), 32 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java index f269a763a..2a9e2d8c9 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java @@ -157,8 +157,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co startActivity(new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS)); } else { if (gpsStatusValue.isGpsStarted()) { - recordingStatusConnection.unbindAndStop(this); - recordingStatusConnection.startConnection(this); //TODO We need to stay listening! + recordingStatusConnection.stopService(this); } else { new TrackRecordingServiceConnection((service, connection) -> { service.tryStartSensors(); @@ -220,7 +219,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co super.onStart(); PreferencesUtils.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); - recordingStatusConnection.startConnection(this); + recordingStatusConnection.bind(this); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java index 5127f6e47..257c4dbab 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java @@ -126,7 +126,7 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement trackDataHub.loadTrack(trackId); } - trackRecordingServiceConnection.startConnection(this); + trackRecordingServiceConnection.bind(this); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java index 6c307ec6a..432b086c3 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java @@ -207,7 +207,7 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc PreferencesUtils.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); - trackRecordingServiceConnection.startConnection(this); + trackRecordingServiceConnection.bind(this); trackDataHub.start(); } diff --git a/src/main/java/de/dennisguse/opentracks/fragments/StatisticsRecordingFragment.java b/src/main/java/de/dennisguse/opentracks/fragments/StatisticsRecordingFragment.java index a0d015790..5f9c6ede3 100644 --- a/src/main/java/de/dennisguse/opentracks/fragments/StatisticsRecordingFragment.java +++ b/src/main/java/de/dennisguse/opentracks/fragments/StatisticsRecordingFragment.java @@ -88,7 +88,7 @@ public class StatisticsRecordingFragment extends Fragment { PreferencesUtils.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); - trackRecordingServiceConnection.startConnection(getContext()); + trackRecordingServiceConnection.bind(getContext()); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index d2a631de8..d34fe1b77 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -74,14 +74,21 @@ public class TrackRecordingServiceConnection { setTrackRecordingService(null); }; - public TrackRecordingServiceConnection() { - callback = null; - } - public TrackRecordingServiceConnection(@NonNull Callback callback) { this.callback = callback; } + public void bind(@NonNull Context context) { + if (trackRecordingService != null) { + // Service is already started and bound. + return; + } + + Log.i(TAG, "Binding the service."); + int flags = BuildConfig.DEBUG ? Context.BIND_DEBUG_UNBIND : 0; + context.bindService(new Intent(context, TrackRecordingService.class), serviceConnection, flags); + } + public void startAndBind(Context context) { if (trackRecordingService != null) { // Service is already started and bound. @@ -90,7 +97,7 @@ public class TrackRecordingServiceConnection { ContextCompat.startForegroundService(context, new Intent(context, TrackRecordingService.class)); - startConnection(context); + bind(context); } //TODO There should be a better way to implement this. @@ -105,20 +112,7 @@ public class TrackRecordingServiceConnection { startAndBind(context); return; } - if (callback != null) { - callback.onConnected(trackRecordingService, this); - } - } - - public void startConnection(@NonNull Context context) { - if (trackRecordingService != null) { - // Service is already started and bound. - return; - } - - Log.i(TAG, "Binding the service."); - int flags = BuildConfig.DEBUG ? Context.BIND_DEBUG_UNBIND : 0; - context.bindService(new Intent(context, TrackRecordingService.class), serviceConnection, flags); + callback.onConnected(trackRecordingService, this); } /** @@ -134,15 +128,19 @@ public class TrackRecordingServiceConnection { setTrackRecordingService(null); } + public void stopService(Context context) { + context.stopService(new Intent(context, TrackRecordingService.class)); + } + public void unbindAndStop(Context context) { unbind(context); - context.stopService(new Intent(context, TrackRecordingService.class)); + stopService(context); } private void setTrackRecordingService(TrackRecordingService value) { trackRecordingService = value; - if (callback != null && value != null) { - callback.onConnected(value, this); + if (value != null) { + callback.onConnected(value, this); } } diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java index ee58ccfcb..40f83c784 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerEditViewModel.java @@ -32,7 +32,7 @@ public class MarkerEditViewModel extends AndroidViewModel { private MutableLiveData markerData; private boolean isNewMarker; private Uri photoOriginalUri; - private final TrackRecordingServiceConnection trackRecordingServiceConnection = new TrackRecordingServiceConnection(); + private final TrackRecordingServiceConnection trackRecordingServiceConnection = new TrackRecordingServiceConnection((service, connection) -> {}); public MarkerEditViewModel(@NonNull Application application) { super(application); @@ -41,7 +41,7 @@ public class MarkerEditViewModel extends AndroidViewModel { public LiveData getMarkerData(@NonNull Track.Id trackId, @Nullable Marker.Id markerId) { if (markerData == null) { markerData = new MutableLiveData<>(); - trackRecordingServiceConnection.startConnection(getApplication()); + trackRecordingServiceConnection.bind(getApplication()); loadData(trackId, markerId); } return markerData; diff --git a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java index 6ad125a44..48e4c064f 100644 --- a/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/ui/markers/MarkerListActivity.java @@ -119,13 +119,13 @@ public class MarkerListActivity extends AbstractActivity { @Override protected void onStart() { super.onStart(); - trackRecordingServiceConnection.startConnection(this); + trackRecordingServiceConnection.bind(this); } @Override protected void onResume() { super.onResume(); - trackRecordingServiceConnection.startConnection(this); + trackRecordingServiceConnection.bind(this); this.invalidateOptionsMenu(); loadData(); } From 9a0f9271820d37dfe86d0c42cd1fd55773099ef0 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Tue, 26 Dec 2023 11:54:38 +0100 Subject: [PATCH 7/7] Cleanup: TrackRecordingServiceConnection has now an execute method (avoid instantiation). --- .../opentracks/TrackListActivity.java | 12 ++---- .../opentracks/TrackRecordedActivity.java | 5 +-- .../opentracks/TrackRecordingActivity.java | 2 +- .../opentracks/TrackStoppedActivity.java | 5 +-- .../publicapi/AbstractAPIActivity.java | 3 +- .../TrackRecordingServiceConnection.java | 39 ++++++------------- 6 files changed, 21 insertions(+), 45 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java index 2a9e2d8c9..dda4cb355 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java @@ -159,11 +159,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co if (gpsStatusValue.isGpsStarted()) { recordingStatusConnection.stopService(this); } else { - new TrackRecordingServiceConnection((service, connection) -> { - service.tryStartSensors(); - - connection.unbind(this); - }).startAndBindWithCallback(this); + TrackRecordingServiceConnection.execute(this, (service, connection) -> service.tryStartSensors()); } } }); @@ -182,15 +178,13 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co // Not Recording -> Recording Log.i(TAG, "Starting recording"); updateGpsMenuItem(false, true); - new TrackRecordingServiceConnection((service, connection) -> { + TrackRecordingServiceConnection.execute(this, (service, connection) -> { Track.Id trackId = service.startNewTrack(); Intent newIntent = IntentUtils.newIntent(TrackListActivity.this, TrackRecordingActivity.class); newIntent.putExtra(TrackRecordingActivity.EXTRA_TRACK_ID, trackId); startActivity(newIntent); - - connection.unbind(this); - }).startAndBind(this); + }); }); viewBinding.trackListFabAction.setOnLongClickListener((view) -> { if (!recordingStatus.isRecording()) { diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java index 257c4dbab..856a04a53 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordedActivity.java @@ -201,7 +201,7 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement } if (item.getItemId() == R.id.track_detail_resume_track) { - new TrackRecordingServiceConnection((service, connection) -> { + TrackRecordingServiceConnection.execute(this, (service, connection) -> { service.resumeTrack(trackId); Intent newIntent = IntentUtils.newIntent(TrackRecordedActivity.this, TrackRecordingActivity.class) @@ -209,9 +209,8 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement startActivity(newIntent); overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out); - connection.unbind(this); finish(); - }).startAndBind(this); + }); return true; } diff --git a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java index 432b086c3..3390484d1 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackRecordingActivity.java @@ -224,7 +224,7 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc trackDataHub.setRecordingStatus(recordingStatus); } - trackRecordingServiceConnection.startAndBindWithCallback(this); + trackRecordingServiceConnection.bind(this); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java b/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java index 6cfce7898..ddb916775 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackStoppedActivity.java @@ -136,7 +136,7 @@ public class TrackStoppedActivity extends AbstractTrackDeleteActivity implements } private void resumeTrackAndFinish() { - new TrackRecordingServiceConnection((service, connection) -> { + TrackRecordingServiceConnection.execute(this, (service, connection) -> { service.resumeTrack(trackId); Intent newIntent = IntentUtils.newIntent(TrackStoppedActivity.this, TrackRecordingActivity.class) @@ -144,9 +144,8 @@ public class TrackStoppedActivity extends AbstractTrackDeleteActivity implements startActivity(newIntent); overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out); - connection.unbind(this); finish(); - }).startAndBind(this); + }); } @Override diff --git a/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java b/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java index 98b6b83ec..3cc4352be 100644 --- a/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java +++ b/src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java @@ -37,8 +37,7 @@ public abstract class AbstractAPIActivity extends AppCompatActivity { if (PreferencesUtils.isPublicAPIenabled()) { Log.i(TAG, "Received and trying to execute requested action."); - new TrackRecordingServiceConnection(serviceConnectedCallback) - .startAndBind(this); + TrackRecordingServiceConnection.execute(this, serviceConnectedCallback); } else { Toast.makeText(this, getString(R.string.settings_public_api_disabled_toast), Toast.LENGTH_LONG).show(); Log.w(TAG, "Public API is disabled; ignoring request."); diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index d34fe1b77..97c3bc725 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -80,7 +80,7 @@ public class TrackRecordingServiceConnection { public void bind(@NonNull Context context) { if (trackRecordingService != null) { - // Service is already started and bound. + callback.onConnected(trackRecordingService, this); return; } @@ -89,32 +89,6 @@ public class TrackRecordingServiceConnection { context.bindService(new Intent(context, TrackRecordingService.class), serviceConnection, flags); } - public void startAndBind(Context context) { - if (trackRecordingService != null) { - // Service is already started and bound. - return; - } - - ContextCompat.startForegroundService(context, new Intent(context, TrackRecordingService.class)); - - bind(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; - } - callback.onConnected(trackRecordingService, this); - } - /** * Unbinds the service (but leave it running). */ @@ -178,4 +152,15 @@ public class TrackRecordingServiceConnection { public interface Callback { void onConnected(TrackRecordingService service, TrackRecordingServiceConnection self); } + + public static void execute(Context context, Callback callback) { + Callback withUnbind = (service, connection) -> { + callback.onConnected(service, connection); + connection.unbind(context); + }; + new TrackRecordingServiceConnection(withUnbind) + .bind(context); + + ContextCompat.startForegroundService(context, new Intent(context, TrackRecordingService.class)); + } }