From 150cbee95b4aeeb0eeb175bc37a79c17b0998cc6 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 18:48:14 +0100 Subject: [PATCH 1/5] BarometerInternal: if there is no internal barometer - don't register a listener. --- .../dennisguse/opentracks/sensors/driver/BarometerInternal.java | 1 + 1 file changed, 1 insertion(+) diff --git a/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java b/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java index a2c98e07d..ea223e6f0 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java @@ -42,6 +42,7 @@ public class BarometerInternal implements SensorEventListener { if (pressureSensor == null) { Log.w(TAG, "No pressure sensor available."); this.observer = null; + return; } if (sensorManager.registerListener(this, pressureSensor, SAMPLING_PERIOD, handler)) { From 0ee4a4e72fd5ca977c9bcec4239ed350e11bc563 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 20:49:49 +0100 Subject: [PATCH 2/5] Bugfix: TrackListActivity triggered TrackRecordingService twice (startSensors and startRecording). Part of #1780. --- .../opentracks/TrackListActivity.java | 29 +++++++++---------- .../TrackRecordingServiceConnection.java | 5 +--- 2 files changed, 15 insertions(+), 19 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java index ad8e78b27..d279df769 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java @@ -25,6 +25,7 @@ import android.graphics.drawable.AnimatedVectorDrawable; 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 class TrackListActivity extends AbstractTrackDeleteActivity implements Co .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 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co 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,13 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co startActivity(new Intent(Settings.ACTION_LOCATION_SOURCE_SETTINGS)); } else { if (gpsStatusValue.isGpsStarted()) { - trackRecordingServiceConnection.unbindAndStop(this); + recordingStatusConnection.unbindAndStop(this); } else { - trackRecordingServiceConnection.startAndBindWithCallback(this); + new TrackRecordingServiceConnection((service, connection) -> { + service.tryStartSensors(); + + connection.unbind(this); + }).startAndBindWithCallback(this); } } }); @@ -182,6 +180,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co } // Not Recording -> Recording + Log.i(TAG, "Starting recording"); updateGpsMenuItem(false, true); new TrackRecordingServiceConnection((service, connection) -> { Track.Id trackId = service.startNewTrack(); @@ -201,7 +200,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co // 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 +219,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co super.onStart(); PreferencesUtils.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); - trackRecordingServiceConnection.startConnection(this); + recordingStatusConnection.startConnection(this); } @Override @@ -240,14 +239,14 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co 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; } diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java index 15dc2bc28..706bdbbd8 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceConnection.java @@ -127,9 +127,6 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death 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 @@ public class TrackRecordingServiceConnection implements ServiceConnection, Death @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) { From 6cd627e83234da9cb8f883bf470bfeaff64abdad Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 20:55:32 +0100 Subject: [PATCH 3/5] Bugfix: TrackRecordingService can only start sensors once. Fixes #1780. --- .../services/TrackRecordingService.java | 17 +++++++++++++++-- 1 file changed, 15 insertions(+), 2 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 734b7b088..35566dcc9 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -163,6 +163,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator. Log.w(TAG, "Ignore startNewTrack. Already recording."); return null; } + Log.i(TAG, "startNewTrack"); // Set recording status Track.Id trackId = trackRecordingManager.startNewTrack(); @@ -177,6 +178,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator. Log.w(TAG, "Cannot resume a non-existing track."); return; } + Log.i(TAG, "resumeTrack"); updateRecordingStatus(RecordingStatus.record(trackId)); @@ -193,12 +195,19 @@ public class TrackRecordingService extends Service implements TrackPointCreator. } 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) { @@ -336,6 +345,10 @@ public class TrackRecordingService extends Service implements TrackPointCreator. return recordingStatus.isRecording(); } + private boolean isSensorStarted() { + return wakeLock != null; + } + @Override public void onSharedPreferenceChanged(SharedPreferences sharedPreferences, @Nullable String key) { voiceAnnouncementManager.onSharedPreferenceChanged(sharedPreferences, key); From 826cf0d468608fcbf7055e03b735aad4fc4e81cf Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 21:07:22 +0100 Subject: [PATCH 4/5] Bugfix: TrackListActivity couldn't stop and restart sensors. Part of #1780. --- src/main/java/de/dennisguse/opentracks/TrackListActivity.java | 1 + .../dennisguse/opentracks/services/TrackRecordingService.java | 4 ++++ 2 files changed, 5 insertions(+) diff --git a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java index d279df769..5e7a53348 100644 --- a/src/main/java/de/dennisguse/opentracks/TrackListActivity.java +++ b/src/main/java/de/dennisguse/opentracks/TrackListActivity.java @@ -158,6 +158,7 @@ public class TrackListActivity extends AbstractTrackDeleteActivity implements Co } else { if (gpsStatusValue.isGpsStarted()) { recordingStatusConnection.unbindAndStop(this); + recordingStatusConnection.startConnection(this); //TODO We need to stay listening! } else { new TrackRecordingServiceConnection((service, connection) -> { service.tryStartSensors(); diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 35566dcc9..6c24c7de9 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -125,6 +125,9 @@ public class TrackRecordingService extends Service implements TrackPointCreator. if (isRecording()) { endCurrentTrack(); } + if (isSensorStarted()) { + stopSensors(); + } PreferencesUtils.unregisterOnSharedPreferenceChangeListener(this); @@ -248,6 +251,7 @@ public class TrackRecordingService extends Service implements TrackPointCreator. stopForeground(true); notificationManager.cancelNotification(); wakeLock = SystemUtils.releaseWakeLock(wakeLock); + gpsStatusObservable.postValue(STATUS_GPS_DEFAULT); } @Override From 2fe452c77cddae3d3830770e219fd200c0c8a71a Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Mon, 25 Dec 2023 21:40:22 +0100 Subject: [PATCH 5/5] Revert "fix NPE at null barometer after unregister listener" This reverts commit 5dbd17f3bbd65df835cb6e635164f3695499503f. --- .../sensors/driver/BarometerInternal.java | 32 ++++++++++--------- 1 file changed, 17 insertions(+), 15 deletions(-) diff --git a/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java b/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java index ea223e6f0..9032010a5 100644 --- a/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java +++ b/src/main/java/de/dennisguse/opentracks/sensors/driver/BarometerInternal.java @@ -13,7 +13,7 @@ import java.util.concurrent.TimeUnit; 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(); @@ -21,20 +21,22 @@ public class BarometerInternal implements SensorEventListener { 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); @@ -45,7 +47,7 @@ public class BarometerInternal implements SensorEventListener { return; } - if (sensorManager.registerListener(this, pressureSensor, SAMPLING_PERIOD, handler)) { + if (sensorManager.registerListener(listener, pressureSensor, SAMPLING_PERIOD, handler)) { this.observer = observer; return; } @@ -55,7 +57,7 @@ public class BarometerInternal implements SensorEventListener { public void disconnect(Context context) { SensorManager sensorManager = (SensorManager) context.getSystemService(Context.SENSOR_SERVICE); - sensorManager.unregisterListener(this); + sensorManager.unregisterListener(listener); observer = null; }