Merge pull request #1793 from OpenTracksApp/crash#1708

Bugfix: sensors were started twice.

Author: dennisguse

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

@@ -25,6 +25,7 @@
 import android.location.LocationManager;
 import android.os.Bundle;
 import android.provider.Settings;
+import android.util.Log;
 import android.view.KeyEvent;
 import android.view.Menu;
 import android.view.MenuItem;
@@ -73,7 +74,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co
     private static final String TAG = TrackListActivity.class.getSimpleName();
 
     // The following are set in onCreate
-    private TrackRecordingServiceConnection trackRecordingServiceConnection;
+    private TrackRecordingServiceConnection recordingStatusConnection;
     private TrackListAdapter adapter;
 
     private TrackListBinding viewBinding;
@@ -138,13 +139,6 @@ public void onDestroy() {
                 .observe(TrackListActivity.this, this::onGpsStatusChanged);
 
         updateGpsMenuItem(true, recordingStatus.isRecording());
-
-        if (service.getGpsStatusObservable().getValue().isGpsStarted()) {
-            return;
-        }
-
-        //TODO Not cool to do this in a callback that might be called more than once!
-        service.tryStartSensors();
     };
 
     @Override
@@ -154,7 +148,7 @@ protected void onCreate(Bundle savedInstanceState) {
 
         requestRequiredPermissions();
 
-        trackRecordingServiceConnection = new TrackRecordingServiceConnection(bindChangedCallback);
+        recordingStatusConnection = new TrackRecordingServiceConnection(bindChangedCallback);
 
         viewBinding.aggregatedStatsButton.setOnClickListener((view) -> startActivity(IntentUtils.newIntent(this, AggregatedStatisticsActivity.class)));
         viewBinding.sensorStartButton.setOnClickListener((view) -> {
@@ -163,9 +157,14 @@ protected void onCreate(Bundle savedInstanceState) {
                 startActivity(new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS));
             } else {
                 if (gpsStatusValue.isGpsStarted()) {
-                    trackRecordingServiceConnection.unbindAndStop(this);
+                    recordingStatusConnection.unbindAndStop(this);
+                    recordingStatusConnection.startConnection(this); //TODO We need to stay listening!
                 } else {
-                    trackRecordingServiceConnection.startAndBindWithCallback(this);
+                    new TrackRecordingServiceConnection((service, connection) -> {
+                        service.tryStartSensors();
+
+                        connection.unbind(this);
+                    }).startAndBindWithCallback(this);
                 }
             }
         });
@@ -182,6 +181,7 @@ protected void onCreate(Bundle savedInstanceState) {
             }
 
             // Not Recording -> Recording
+            Log.i(TAG, "Starting recording");
             updateGpsMenuItem(false, true);
             new TrackRecordingServiceConnection((service, connection) -> {
                 Track.Id trackId = service.startNewTrack();
@@ -201,7 +201,7 @@ protected void onCreate(Bundle savedInstanceState) {
             // Recording -> Stop
             ActivityUtils.vibrate(this, 1000);
             updateGpsMenuItem(false, false);
-            trackRecordingServiceConnection.stopRecording(TrackListActivity.this);
+            recordingStatusConnection.stopRecording(TrackListActivity.this);
             viewBinding.trackListFabAction.setImageResource(R.drawable.ic_baseline_record_24);
             viewBinding.trackListFabAction.setBackgroundTintList(ContextCompat.getColorStateList(this, R.color.red_dark));
             return true;
@@ -220,7 +220,7 @@ protected void onStart() {
         super.onStart();
 
         PreferencesUtils.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener);
-        trackRecordingServiceConnection.startConnection(this);
+        recordingStatusConnection.startConnection(this);
     }
 
     @Override
@@ -240,14 +240,14 @@ protected void onStop() {
         super.onStop();
 
         PreferencesUtils.unregisterOnSharedPreferenceChangeListener(sharedPreferenceChangeListener);
-        trackRecordingServiceConnection.unbind(this);
+        recordingStatusConnection.unbind(this);
     }
 
     @Override
     protected void onDestroy() {
         super.onDestroy();
         viewBinding = null;
-        trackRecordingServiceConnection = null;
+        recordingStatusConnection = null;
         adapter = null;
     }
 

src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java

@@ -13,38 +13,41 @@
 import de.dennisguse.opentracks.data.models.AtmosphericPressure;
 import de.dennisguse.opentracks.sensors.GainManager;
 
-public class BarometerInternal implements SensorEventListener {
+public class BarometerInternal {
 
     private static final String TAG = BarometerInternal.class.getSimpleName();
 
     private static final int SAMPLING_PERIOD = (int) TimeUnit.SECONDS.toMicros(5);
 
     private GainManager observer;
 
-    @Override
-    public void onSensorChanged(SensorEvent event) {
-        if (!isConnected()) {
-            Log.w(TAG, "Not connected to sensor, cannot process data.");
-            return;
-        }
+    private final SensorEventListener listener = new SensorEventListener() {
+        @Override
+        public void onSensorChanged(SensorEvent event) {
+            if (!isConnected()) {
+                Log.w(TAG, "Not connected to sensor, cannot process data.");
+                return;
+            }
 
-        observer.onSensorValueChanged(AtmosphericPressure.ofHPA(event.values[0]));
-    }
+            observer.onSensorValueChanged(AtmosphericPressure.ofHPA(event.values[0]));
+        }
 
-    @Override
-    public void onAccuracyChanged(Sensor sensor, int accuracy) {
-        Log.w(TAG, "Sensor accuracy changes are (currently) ignored.");
-    }
+        @Override
+        public void onAccuracyChanged(Sensor sensor, int accuracy) {
+            Log.w(TAG, "Sensor accuracy changes are (currently) ignored.");
+        }
+    };
 
     public void connect(Context context, Handler handler, GainManager observer) {
         SensorManager sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE);
         Sensor pressureSensor = sensorManager.getDefaultSensor(Sensor.TYPE_PRESSURE);
         if (pressureSensor == null) {
             Log.w(TAG, "No pressure sensor available.");
             this.observer = null;
+            return;
         }
 
-        if (sensorManager.registerListener(this, pressureSensor, SAMPLING_PERIOD, handler)) {
+        if (sensorManager.registerListener(listener, pressureSensor, SAMPLING_PERIOD, handler)) {
             this.observer = observer;
             return;
         }
@@ -54,7 +57,7 @@ public void connect(Context context, Handler handler, GainManager observer) {
 
     public void disconnect(Context context) {
         SensorManager sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE);
-        sensorManager.unregisterListener(this);
+        sensorManager.unregisterListener(listener);
         observer = null;
     }
 

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

@@ -125,6 +125,9 @@ public void onDestroy() {
         if (isRecording()) {
             endCurrentTrack();
         }
+        if (isSensorStarted()) {
+            stopSensors();
+        }
 
         PreferencesUtils.unregisterOnSharedPreferenceChangeListener(this);
 
@@ -163,6 +166,7 @@ public Track.Id startNewTrack() {
             Log.w(TAG, "Ignore startNewTrack. Already recording.");
             return null;
         }
+        Log.i(TAG, "startNewTrack");
 
         // Set recording status
         Track.Id trackId = trackRecordingManager.startNewTrack();
@@ -177,6 +181,7 @@ public void resumeTrack(Track.Id trackId) {
             Log.w(TAG, "Cannot resume a non-existing track.");
             return;
         }
+        Log.i(TAG, "resumeTrack");
 
         updateRecordingStatus(RecordingStatus.record(trackId));
 
@@ -193,12 +198,19 @@ private void startRecording() {
     }
 
     public void tryStartSensors() {
-        if (isRecording()) return;
+        if (isSensorStarted()) return;
+
+        Log.i(TAG, "tryStartSensors");
 
         startSensors();
     }
 
-    private void startSensors() {
+    private synchronized void startSensors() {
+        if (isSensorStarted()) {
+            Log.i(TAG, "sensors already started; skipping");
+            return;
+        }
+        Log.i(TAG, "startSensors");
         wakeLock = SystemUtils.acquireWakeLock(this, wakeLock);
         trackPointCreator.start(this, handler);
         if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) {
@@ -239,6 +251,7 @@ void stopSensors() {
         stopForeground(true);
         notificationManager.cancelNotification();
         wakeLock = SystemUtils.releaseWakeLock(wakeLock);
+        gpsStatusObservable.postValue(STATUS_GPS_DEFAULT);
     }
 
     @Override
@@ -336,6 +349,10 @@ public boolean isRecording() {
         return recordingStatus.isRecording();
     }
 
+    private boolean isSensorStarted() {
+        return wakeLock != null;
+    }
+
     @Override
     public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, @Nullable String key) {
         voiceAnnouncementManager.onSharedPreferenceChanged(sharedPreferences, key);

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

@@ -127,9 +127,6 @@ public void unbind(Context context) {
         setTrackRecordingService(null);
     }
 
-    /**
-     * Unbinds and stops the service.
-     */
     public void unbindAndStop(Context context) {
         unbind(context);
         context.stopService(new Intent(context, TrackRecordingService.class));
@@ -153,7 +150,7 @@ private void setTrackRecordingService(TrackRecordingService value) {
 
     @Override
     public void onServiceConnected(ComponentName className, IBinder service) {
-        Log.i(TAG, "Connected to the service.");
+        Log.i(TAG, "Connected to the service: " + service);
         try {
             service.linkToDeath(this, 0);
         } catch (RemoteException e) {