Public API: implemented STOP properly.

Fixes #1103.

README.md

@@ -130,6 +130,7 @@ The API can be invoked by sending an explicit Intent to start an activity.
 
 `Class`:
 * **Start a recording:**  `de.dennisguse.opentracks.publicapi.StartRecording`
+* **Start a recording:**  `de.dennisguse.opentracks.publicapi.StopRecording`
 
 For testing via adb: `adb shell am start -n "package/class"`
 

src/main/AndroidManifest.xml

@@ -77,14 +77,14 @@ limitations under the License.
                 <category android:name="android.intent.category.DEFAULT" />
             </intent-filter>
         </activity>
-        <!--        <activity-->
+        <activity
-        <!--            android:name=".publicapi.StopRecording"-->
+            android:name=".publicapi.StopRecording"
-        <!--            android:exported="true"-->
+            android:exported="true"
-        <!--            android:theme="@style/SplashTheme">-->
+            android:theme="@style/SplashTheme">
-        <!--            <intent-filter>-->
+            <intent-filter>
-        <!--                <category android:name="android.intent.category.DEFAULT" />-->
+                <category android:name="android.intent.category.DEFAULT" />
-        <!--            </intent-filter>-->
+            </intent-filter>
-        <!--        </activity>-->
+        </activity>
 
         <activity android:name=".AboutActivity" />
 

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

@@ -22,7 +22,6 @@ import android.content.Context;
 import android.content.Intent;
 import android.content.SharedPreferences.OnSharedPreferenceChangeListener;
 import android.database.Cursor;
-import android.graphics.Color;
 import android.graphics.drawable.AnimatedVectorDrawable;
 import android.location.LocationManager;
 import android.os.Bundle;
@@ -139,7 +138,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
     private MenuItem searchMenuItem;
     private MenuItem startGpsMenuItem;
 
-    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = (service, unused) -> {
         service.getRecordingStatusObservable()
                 .observe(TrackListActivity.this, this::onRecordingStatusChanged);
 
@@ -167,16 +166,15 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
         trackRecordingServiceConnection = new TrackRecordingServiceConnection(bindChangedCallback);
 
         viewBinding.trackList.setEmptyView(viewBinding.trackListEmptyView);
-        viewBinding.trackList.setOnItemClickListener((parent, view, position, trackId) -> {
+        viewBinding.trackList.setOnItemClickListener((parent, view, position, trackIdId) -> {
-            if (recordingStatus.isRecording() && trackId == recordingStatus.getTrackId().getId()) {
+            Track.Id trackId = new Track.Id(trackIdId);
+            Intent newIntent = IntentUtils.newIntent(TrackListActivity.this, TrackRecordedActivity.class)
+                    .putExtra(TrackRecordedActivity.EXTRA_TRACK_ID, trackId);
+            if (recordingStatus.isRecording() && trackId.equals(recordingStatus.getTrackId())) {
                 // Is recording -> open record activity.
-                Intent newIntent = IntentUtils.newIntent(TrackListActivity.this, TrackRecordingActivity.class)
-                        .putExtra(TrackRecordedActivity.EXTRA_TRACK_ID, new Track.Id(trackId));
                 startActivity(newIntent);
             } else {
                 // Not recording -> open detail activity.
-                Intent newIntent = IntentUtils.newIntent(TrackListActivity.this, TrackRecordedActivity.class)
-                        .putExtra(TrackRecordedActivity.EXTRA_TRACK_ID, new Track.Id(trackId));
                 ActivityOptions activityOptions = ActivityOptions.makeSceneTransitionAnimation(
                         this,
                         new Pair<>(view.findViewById(R.id.list_item_icon), TrackRecordedActivity.VIEW_TRACK_ICON));
@@ -230,8 +228,15 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
 
             // Not Recording -> Recording
             updateGpsMenuItem(false, true);
-            Intent newIntent = IntentUtils.newIntent(TrackListActivity.this, TrackRecordingActivity.class);
+            new TrackRecordingServiceConnection((service, connection) -> {
-            startActivity(newIntent);
+                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()) {

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

@@ -79,7 +79,7 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement
 
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
 
-    private final TrackRecordingServiceConnection.Callback bindCallback = service -> service.getRecordingStatusObservable()
+    private final TrackRecordingServiceConnection.Callback bindCallback = (service, unused) -> service.getRecordingStatusObservable()
             .observe(TrackRecordedActivity.this, this::onRecordingStatusChanged);
 
     @Override
@@ -203,11 +203,17 @@ public class TrackRecordedActivity extends AbstractTrackDeleteActivity implement
         }
 
         if (item.getItemId() == R.id.track_detail_resume_track) {
-            Intent newIntent = IntentUtils.newIntent(TrackRecordedActivity.this, TrackRecordingActivity.class)
+            new TrackRecordingServiceConnection((service, connection) -> {
-                    .putExtra(TrackRecordingActivity.EXTRA_TRACK_ID, trackId);
+                service.resumeTrack(trackId);
-            startActivity(newIntent);
+
-            overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out);
+                Intent newIntent = IntentUtils.newIntent(TrackRecordedActivity.this, TrackRecordingActivity.class)
-            finish();
+                        .putExtra(TrackRecordingActivity.EXTRA_TRACK_ID, trackId);
+                startActivity(newIntent);
+                overridePendingTransition(android.R.anim.fade_in, android.R.anim.fade_out);
+
+                connection.unbind(this);
+                finish();
+            }).startAndBind(this);
             return true;
         }
 

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

@@ -57,8 +57,6 @@ import de.dennisguse.opentracks.util.TrackUtils;
  * @author Leif Hendrik Wilden
  * @author Rodrigo Damazio
  */
-//NOTE: This activity does NOT react to preference changes of R.string.recording_track_id_key.
-//This mode of communication should be removed anyhow.
 public class TrackRecordingActivity extends AbstractActivity implements ChooseActivityTypeDialogFragment.ChooseActivityTypeCaller, TrackDataHubInterface {
 
     public static final String EXTRA_TRACK_ID = "track_id";
@@ -76,13 +74,11 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
 
     private TrackRecordingBinding viewBinding;
 
-    // Initialized from Intent; if a new track recording is started, a new TrackId will be provided by TrackRecordingService
-    @Deprecated
     private Track.Id trackId;
 
     private RecordingStatus recordingStatus = TrackRecordingService.STATUS_DEFAULT;
 
-    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = (service, unused) -> {
         service.getRecordingStatusObservable()
                 .observe(TrackRecordingActivity.this, this::onRecordingStatusChanged);
 
@@ -90,18 +86,12 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
                 .observe(TrackRecordingActivity.this, this::onGpsStatusChanged);
 
         if (!service.isRecording()) {
-            if (trackId == null) {
+            finish();
-                // trackId isn't initialized -> leads a new recording.
+            return;
-                trackId = service.startNewTrack();
-            } else {
-                // trackId is initialized -> resumes the track.
-                service.resumeTrack(trackId);
-            }
-
-            // A recording track is on.
-            trackDataHub.loadTrack(trackId);
-            trackDataHub.setRecordingStatus(recordingStatus);
         }
+
+        trackDataHub.loadTrack(trackId);
+        trackDataHub.setRecordingStatus(recordingStatus);
     };
 
     private final OnSharedPreferenceChangeListener sharedPreferenceChangeListener = (sharedPreferences, key) -> {
@@ -123,17 +113,14 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
     protected void onCreate(Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         contentProviderUtils = new ContentProviderUtils(this);
-        trackId = null;
+
-        if (savedInstanceState != null) {
+        trackId = getIntent().getParcelableExtra(EXTRA_TRACK_ID);
-            //Activity was recreated.
+        if (trackId == null) {
-            trackId = savedInstanceState.getParcelable(EXTRA_TRACK_ID);
+            throw new RuntimeException("TrackId is mandatory");
-        } else {
+        }
-            // Resume a track
+        if (contentProviderUtils.getTrack(trackId) == null) {
-            trackId = getIntent().getParcelableExtra(EXTRA_TRACK_ID);
+            Log.w(TAG, "TrackId does not exists; cannot continue the recording.");
-            if (trackId != null && contentProviderUtils.getTrack(trackId) == null) {
+            finish();
-                Log.w(TAG, "TrackId does not exists; cannot continue the recording.");
-                finish();
-            }
         }
 
         trackRecordingServiceConnection = new TrackRecordingServiceConnection(bindChangedCallback);
@@ -240,7 +227,6 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
     protected void onSaveInstanceState(@NonNull Bundle outState) {
         super.onSaveInstanceState(outState);
         outState.putInt(CURRENT_TAB_TAG_KEY, viewBinding.trackDetailActivityViewPager.getCurrentItem());
-        outState.putParcelable(EXTRA_TRACK_ID, trackId);
     }
 
     @Override
@@ -377,6 +363,9 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
     }
 
     private void onRecordingStatusChanged(RecordingStatus status) {
+        if (!status.isRecording()) {
+            finish();
+        }
         recordingStatus = status;
 
         trackDataHub.setRecordingStatus(recordingStatus);
@@ -415,7 +404,8 @@ public class TrackRecordingActivity extends AbstractActivity implements ChooseAc
                 .make(viewBinding.trackRecordingCoordinatorLayout,
                         getString(R.string.gps_recording_status, getString(gpsStatusValue.message), getString(R.string.gps_recording_without_signal)),
                         Snackbar.LENGTH_INDEFINITE)
-                .setAction(getString(R.string.generic_dismiss), v -> {});
+                .setAction(getString(R.string.generic_dismiss), v -> {
+                });
         snackbar.show();
     }
 }

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

@@ -77,7 +77,7 @@ public class StatisticsRecordingFragment extends Fragment {
         }
     };
 
-    private final TrackRecordingServiceConnection.Callback bindChangedCallback = service -> {
+    private final TrackRecordingServiceConnection.Callback bindChangedCallback = (service, unused) -> {
         service.getRecordingDataObservable()
                 .observe(StatisticsRecordingFragment.this, this::onRecordingDataChanged);
     };

src/main/java/de/dennisguse/opentracks/publicapi/AbstractAPIActivity.java

@@ -16,27 +16,25 @@ public abstract class AbstractAPIActivity extends AppCompatActivity {
 
     private final String TAG = AbstractAPIActivity.class.getSimpleName();
 
-    private final TrackRecordingServiceConnection.Callback serviceConnectedCallback = service -> {
+    private final TrackRecordingServiceConnection.Callback serviceConnectedCallback = (service, connection) -> {
         if (!isFinishing() && !isDestroyed()) {
             execute(service);
         }
         if (isPostExecuteStopService()) {
-            AbstractAPIActivity.this.trackRecordingServiceConnection.unbindAndStop(AbstractAPIActivity.this);
+            connection.unbindAndStop(AbstractAPIActivity.this);
         } else {
-            AbstractAPIActivity.this.trackRecordingServiceConnection.unbind(AbstractAPIActivity.this);
+            connection.unbind(AbstractAPIActivity.this);
         }
         finish();
     };
 
-    private TrackRecordingServiceConnection trackRecordingServiceConnection;
-
     @Override
     protected void onCreate(@Nullable Bundle savedInstanceState) {
         super.onCreate(savedInstanceState);
         if (PreferencesUtils.isPublicAPIenabled()) {
             Log.i(TAG, "Received and trying to execute requested action.");
-            trackRecordingServiceConnection = new TrackRecordingServiceConnection(serviceConnectedCallback);
+            new TrackRecordingServiceConnection(serviceConnectedCallback)
-            trackRecordingServiceConnection.startAndBind(this);
+                    .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.");
@@ -44,12 +42,6 @@ public abstract class AbstractAPIActivity extends AppCompatActivity {
         }
     }
 
-    @Override
-    protected void onDestroy() {
-        super.onDestroy();
-        trackRecordingServiceConnection = null;
-    }
-
     protected abstract void execute(TrackRecordingService service);
 
     protected abstract boolean isPostExecuteStopService();

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

@@ -92,15 +92,10 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
             return;
         }
         if (callback != null) {
-            callback.onConnected(trackRecordingService);
+            callback.onConnected(trackRecordingService, this);
         }
     }
 
-    /**
-     * Resumes the track recording service connection.
-     *
-     * @param context the context
-     */
     public void startConnection(@NonNull Context context) {
         if (trackRecordingService != null) {
             // Service is already started and bound.
@@ -115,6 +110,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
     /**
      * Unbinds the service (but leave it running).
      */
+    //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);
@@ -141,7 +137,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
         trackRecordingService = value;
         if (callback != null) {
             if (value != null) {
-                callback.onConnected(value);
+                callback.onConnected(value, this);
             } else {
                 callback.onDisconnected();
             }
@@ -214,7 +210,7 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death
     }
 
     public interface Callback {
-        void onConnected(TrackRecordingService service);
+        void onConnected(TrackRecordingService service, TrackRecordingServiceConnection connection);
 
         default void onDisconnected() {
         }

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

@@ -18,7 +18,7 @@ public class MainSettingsFragment extends PreferenceFragmentCompat {
     private RecordingStatus recordingStatus = TrackRecordingService.STATUS_DEFAULT;
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
     private final TrackRecordingServiceConnection.Callback bindServiceCallback =
-            service -> service.getRecordingStatusObservable()
+            (service, unused) -> service.getRecordingStatusObservable()
                     .observe(MainSettingsFragment.this, this::onRecordingStatusChanged);
 
     @Override

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

@@ -71,7 +71,7 @@ public class MarkerListActivity extends AbstractActivity implements DeleteMarker
 
     private TrackRecordingServiceConnection trackRecordingServiceConnection;
 
-    private final TrackRecordingServiceConnection.Callback bindCallback = service -> service.getRecordingStatusObservable()
+    private final TrackRecordingServiceConnection.Callback bindCallback = (service, unused) -> service.getRecordingStatusObservable()
             .observe(MarkerListActivity.this, this::onRecordingStatusChanged);
 
     // Callback when an item is selected in the contextual action mode