From 67d1ac551de71f66d1fe932ed46a8f62dd8676b5 Mon Sep 17 00:00:00 2001 From: Dennis Guse Date: Sat, 23 Nov 2019 22:47:40 +0100 Subject: [PATCH] Show GPS accuracy while recording in the notification of the TrackRecordingService and notify if accuracy goes beyond configured threshold. --- ...cordingServiceNotificationManagerTest.java | 54 +++++++++ .../services/TrackRecordingService.java | 105 ++++++------------ ...ckRecordingServiceNotificationManager.java | 94 ++++++++++++++++ .../opentracks/util/LocationUtils.java | 2 +- src/main/res/values/strings.xml | 3 + 5 files changed, 187 insertions(+), 71 deletions(-) create mode 100644 src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManagerTest.java create mode 100644 src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java diff --git a/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManagerTest.java b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManagerTest.java new file mode 100644 index 000000000..70998fed4 --- /dev/null +++ b/src/androidTest/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManagerTest.java @@ -0,0 +1,54 @@ +package de.dennisguse.opentracks.services; + +import android.app.NotificationManager; +import android.content.Context; +import android.location.Location; + +import androidx.core.app.NotificationCompat; +import androidx.test.core.app.ApplicationProvider; + +import org.junit.Test; +import org.junit.runner.RunWith; +import org.mockito.Mock; +import org.mockito.junit.MockitoJUnitRunner; + +import static org.mockito.ArgumentMatchers.anyBoolean; +import static org.mockito.ArgumentMatchers.anyString; +import static org.mockito.Mockito.times; +import static org.mockito.Mockito.verify; +import static org.mockito.Mockito.when; + +@RunWith(MockitoJUnitRunner.class) +public class TrackRecordingServiceNotificationManagerTest { + + private Context context = ApplicationProvider.getApplicationContext(); + + @Mock + private Location locationMock; + + @Mock + private NotificationCompat.Builder notificationCompatBuilder; + + @Mock + private NotificationManager notificationManager; + + @Test + public void updateLocation_triggersAlertOnlyOnFirstInaccurateLocation() { + when(locationMock.hasAccuracy()).thenReturn(true); + when(locationMock.getAccuracy()).thenReturn(999f); + when(notificationCompatBuilder.setContentText(anyString())).thenReturn(notificationCompatBuilder); + when(notificationCompatBuilder.setOnlyAlertOnce(anyBoolean())).thenReturn(notificationCompatBuilder); + + TrackRecordingServiceNotificationManager subject = new TrackRecordingServiceNotificationManager(notificationManager, notificationCompatBuilder); + + // when + subject.updateLocation(context, locationMock, 100); + subject.updateLocation(context, locationMock, 100); + subject.updateLocation(context, locationMock, 1000); + subject.updateLocation(context, locationMock, 100); + + // then + verify(notificationCompatBuilder, times(6)).setOnlyAlertOnce(true); + verify(notificationCompatBuilder, times(2)).setOnlyAlertOnce(false); + } +} \ No newline at end of file diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java index 3229b2caa..43ebd55fa 100644 --- a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingService.java @@ -16,9 +16,6 @@ package de.dennisguse.opentracks.services; -import android.app.Notification; -import android.app.NotificationChannel; -import android.app.NotificationManager; import android.app.PendingIntent; import android.app.Service; import android.content.Context; @@ -30,7 +27,6 @@ import android.location.Location; import android.location.LocationListener; import android.location.LocationManager; import android.net.Uri; -import android.os.Build; import android.os.Bundle; import android.os.Handler; import android.os.IBinder; @@ -38,7 +34,6 @@ import android.os.PowerManager.WakeLock; import android.util.Log; import androidx.annotation.VisibleForTesting; -import androidx.core.app.NotificationCompat; import androidx.core.app.TaskStackBuilder; import java.util.concurrent.ExecutorService; @@ -102,6 +97,7 @@ public class TrackRecordingService extends Service { private PeriodicTaskExecutor voiceExecutor; private PeriodicTaskExecutor splitExecutor; private SharedPreferences sharedPreferences; + private TrackRecordingServiceNotificationManager notificationManager; private long recordingTrackId; private boolean recordingTrackPaused; private LocationListenerPolicy locationListenerPolicy; @@ -226,6 +222,7 @@ public class TrackRecordingService extends Service { splitExecutor = new PeriodicTaskExecutor(this, new SplitPeriodicTaskFactory()); sharedPreferences = PreferencesUtils.getSharedPreferences(this); sharedPreferences.registerOnSharedPreferenceChangeListener(sharedPreferenceChangeListener); + notificationManager = new TrackRecordingServiceNotificationManager(context); // onSharedPreferenceChanged might not set recordingTrackId. recordingTrackId = PreferencesUtils.RECORDING_TRACK_ID_DEFAULT; @@ -267,7 +264,7 @@ public class TrackRecordingService extends Service { } // Reverse order from onCreate - showNotification(false); + showNotification(false); //TODO Why? handler.removeCallbacks(registerLocationRunnable); unregisterLocationListener(); @@ -400,40 +397,6 @@ public class TrackRecordingService extends Service { return Long.parseLong(uri.getLastPathSegment()); } - /** - * Starts the service as a foreground service. - * - * @param pendingIntent the notification pending intent - * @param messageId the notification message id - */ - @VisibleForTesting - protected void startForegroundService(PendingIntent pendingIntent, int messageId) { - if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { - NotificationChannel notificationChannel = new NotificationChannel(getString(R.string.app_name), getString(R.string.app_name), NotificationManager.IMPORTANCE_DEFAULT); - NotificationManager manager = (NotificationManager) getSystemService(Context.NOTIFICATION_SERVICE); - if (manager != null) { - manager.createNotificationChannel(notificationChannel); - } - } - NotificationCompat.Builder builder = new NotificationCompat.Builder(this, getString(R.string.app_name)) - .setContentIntent(pendingIntent) - .setContentText(getString(messageId)) - .setContentTitle(getString(R.string.app_name)) - .setOngoing(true) - .setSmallIcon(R.drawable.ic_logo_color_24dp) - .setCategory(Notification.CATEGORY_SERVICE) - .setWhen(System.currentTimeMillis()); - startForeground(NOTIFICATION_ID, builder.build()); - } - - /** - * Stops the service as a foreground service. - */ - @VisibleForTesting - protected void stopForegroundService() { - stopForeground(true); - } - /** * Handles start command. * @@ -677,6 +640,8 @@ public class TrackRecordingService extends Service { } endRecording(false, recordingTrackId); + + notificationManager.updateContent(getString(R.string.generic_paused)); } /** @@ -763,6 +728,8 @@ public class TrackRecordingService extends Service { return; } + notificationManager.updateLocation(this, location, recordingGpsAccuracy); + if (!location.hasAccuracy() || location.getAccuracy() >= recordingGpsAccuracy) { Log.d(TAG, "Ignore onLocationChangedAsync. Poor accuracy."); return; @@ -798,10 +765,7 @@ public class TrackRecordingService extends Service { } if (!LocationUtils.isValidLocation(lastValidTrackPoint)) { - /* - * Should not happen. The current segment should have a location. Just - * insert the current location. - */ + // Should not happen. The current segment should have a location. Just insert the current location. insertLocation(track, location, null); lastLocation = location; return; @@ -948,36 +912,37 @@ public class TrackRecordingService extends Service { } } - /** - * Shows the notification. - * - * @param isGpsStarted true if GPS is started - */ private void showNotification(boolean isGpsStarted) { - if (isRecording()) { - if (isPaused()) { - stopForegroundService(); - } else { - Intent intent = IntentUtils.newIntent(this, TrackDetailActivity.class) - .putExtra(TrackDetailActivity.EXTRA_TRACK_ID, recordingTrackId); - PendingIntent pendingIntent = TaskStackBuilder.create(this) - .addParentStack(TrackDetailActivity.class).addNextIntent(intent) - .getPendingIntent(0, PendingIntent.FLAG_UPDATE_CURRENT); - startForegroundService(pendingIntent, R.string.track_record_notification); - } - } else { - // Not recording - if (isGpsStarted) { - Intent intent = IntentUtils.newIntent(this, TrackListActivity.class); - PendingIntent pendingIntent = TaskStackBuilder.create(this) - .addNextIntent(intent).getPendingIntent(0, 0); - startForegroundService(pendingIntent, R.string.gps_starting); - } else { - stopForegroundService(); - } + if ((isRecording() && isPaused()) || (!isRecording() && !isGpsStarted)) { + stopForeground(true); + } + + if (isRecording() && !isPaused()) { + Intent intent = IntentUtils.newIntent(this, TrackDetailActivity.class) + .putExtra(TrackDetailActivity.EXTRA_TRACK_ID, recordingTrackId); + PendingIntent pendingIntent = TaskStackBuilder.create(this) + .addParentStack(TrackDetailActivity.class) + .addNextIntent(intent) + .getPendingIntent(0, PendingIntent.FLAG_UPDATE_CURRENT); + + notificationManager.updatePendingIntent(pendingIntent); + notificationManager.updateContent(getString(R.string.gps_starting)); + startForeground(NOTIFICATION_ID, notificationManager.getNotification()); + } + if (!isRecording() && isGpsStarted) { + Intent intent = IntentUtils.newIntent(this, TrackListActivity.class); + PendingIntent pendingIntent = TaskStackBuilder.create(this) + .addNextIntent(intent) + .getPendingIntent(0, 0); + + notificationManager.updatePendingIntent(pendingIntent); + notificationManager.updateContent(getString(R.string.gps_starting)); + + startForeground(NOTIFICATION_ID, notificationManager.getNotification()); } } + /** * TODO: There is a bug in Android that leaks Binder instances. This bug is * especially visible if we have a non-static class, as there is no way to diff --git a/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java new file mode 100644 index 000000000..064f1e866 --- /dev/null +++ b/src/main/java/de/dennisguse/opentracks/services/TrackRecordingServiceNotificationManager.java @@ -0,0 +1,94 @@ +package de.dennisguse.opentracks.services; + +import android.app.Notification; +import android.app.NotificationChannel; +import android.app.NotificationManager; +import android.app.PendingIntent; +import android.content.Context; +import android.location.Location; +import android.os.Build; + +import androidx.annotation.VisibleForTesting; +import androidx.core.app.NotificationCompat; + +import de.dennisguse.opentracks.R; +import de.dennisguse.opentracks.util.PreferencesUtils; +import de.dennisguse.opentracks.util.StringUtils; + +/** + * Manages the content of the notification shown by {@link TrackRecordingService}. + */ +class TrackRecordingServiceNotificationManager { + + private final static int NOTIFICATION_ID = 123; + + private final static String CHANNEL_ID = TrackRecordingServiceNotificationManager.class.getSimpleName(); + + private NotificationCompat.Builder notificationBuilder; + + private NotificationManager notificationManager; + + private boolean previousLocationWasAccurate = true; + + TrackRecordingServiceNotificationManager(Context context) { + notificationManager = (NotificationManager) context.getSystemService(Context.NOTIFICATION_SERVICE); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.O) { + NotificationChannel notificationChannel = new NotificationChannel(CHANNEL_ID, context.getString(R.string.app_name), NotificationManager.IMPORTANCE_HIGH); + if (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) { + notificationChannel.setAllowBubbles(true); + } + + notificationManager.createNotificationChannel(notificationChannel); + } + + notificationBuilder = new NotificationCompat.Builder(context, CHANNEL_ID); + notificationBuilder + .setDefaults(NotificationCompat.DEFAULT_ALL) + .setPriority(NotificationCompat.PRIORITY_MAX) + .setOnlyAlertOnce(true) + .setOngoing(true) + .setCategory(NotificationCompat.CATEGORY_SERVICE) + .setContentTitle(context.getString(R.string.app_name)) + .setSmallIcon(R.drawable.ic_logo_color_24dp); + } + + @VisibleForTesting + TrackRecordingServiceNotificationManager(NotificationManager notificationManager, NotificationCompat.Builder notificationBuilder) { + this.notificationManager = notificationManager; + this.notificationBuilder = notificationBuilder; + } + + void updateContent(String content) { + notificationBuilder.setContentText(content); + updateNotification(); + } + + void updateLocation(Context context, Location location, int recordingGpsAccuracy) { + String formattedAccuracy = context.getString(R.string.value_none); + if (location.hasAccuracy()) { + formattedAccuracy = StringUtils.formatDistance(context, location.getAccuracy(), PreferencesUtils.isMetricUnits(context)); + + boolean currentLocationWasAccurate = location.getAccuracy() < recordingGpsAccuracy; + boolean shouldAlert = !currentLocationWasAccurate && previousLocationWasAccurate; + notificationBuilder.setOnlyAlertOnce(!shouldAlert); + previousLocationWasAccurate = currentLocationWasAccurate; + } + + notificationBuilder.setContentText(context.getString(R.string.track_recording_notification_accuracy, formattedAccuracy)); + updateNotification(); + notificationBuilder.setOnlyAlertOnce(true); + } + + void updatePendingIntent(PendingIntent pendingIntent) { + notificationBuilder.setContentIntent(pendingIntent); + updateNotification(); + } + + Notification getNotification() { + return notificationBuilder.build(); + } + + private void updateNotification() { + notificationManager.notify(NOTIFICATION_ID, getNotification()); + } +} diff --git a/src/main/java/de/dennisguse/opentracks/util/LocationUtils.java b/src/main/java/de/dennisguse/opentracks/util/LocationUtils.java index b3ffdb398..b94883ba7 100644 --- a/src/main/java/de/dennisguse/opentracks/util/LocationUtils.java +++ b/src/main/java/de/dennisguse/opentracks/util/LocationUtils.java @@ -157,7 +157,7 @@ public class LocationUtils { } /** - * Checks if a given location is a valid (i.e. physically possible) locationon Earth. + * Checks if a given location is a valid (i.e. physically possible) location on Earth. * Note: The special separator locations (which have latitude = 100) will not qualify as valid. * Neither will locations with lat=0 and lng=0 as these are most likely "bad" measurements which often cause trouble. * diff --git a/src/main/res/values/strings.xml b/src/main/res/values/strings.xml index e28271e30..ea462c4ef 100644 --- a/src/main/res/values/strings.xml +++ b/src/main/res/values/strings.xml @@ -1514,6 +1514,9 @@ limitations under the License. Recording your track… + + + Location accuracy: %1$s