From 1ef4c8f8f2d1534018d4c7ccc9824d8ffff363a3 Mon Sep 17 00:00:00 2001 From: Shankari Date: Fri, 4 Jun 2021 23:01:28 -0700 Subject: [PATCH 1/7] Update android notifications to be more meaningful + less code-y Particularly, the "In state %s" message is now changed to localizable strings such as "Ready to start trip". @PatGendre, you might want to localize these as well to remove the remaining english text in your app :) --- res/android/values/dc_strings.xml | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/res/android/values/dc_strings.xml b/res/android/values/dc_strings.xml index 4b76e9d..360a5da 100644 --- a/res/android/values/dc_strings.xml +++ b/res/android/values/dc_strings.xml @@ -29,4 +29,9 @@ Error reading stored tracking config, reset to defaults Unrecoverable error in tracking. Profile -> Email Log for further investigation + + Ready for your next trip + Yay! You are on a trip, keep going! + Can't start app, see next pop up + Stopped tracking; remember to re-enable From e24462e56015883eb8fefda51d9db8f8ab29e73e Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 07:45:02 -0700 Subject: [PATCH 2/7] Popup the app settings for android 11+ instead of requesting for location permission This makes it easier to set the permissions instead of having people select "when in use" first and then have to go to the app settings later to fix it. This fixes https://github.com/e-mission/e-mission-docs/issues/608 at least for now --- .../SensorControlForegroundDelegate.java | 21 +++++++++++++++++++ 1 file changed, 21 insertions(+) diff --git a/src/android/verification/SensorControlForegroundDelegate.java b/src/android/verification/SensorControlForegroundDelegate.java index b66dffc..694ca10 100644 --- a/src/android/verification/SensorControlForegroundDelegate.java +++ b/src/android/verification/SensorControlForegroundDelegate.java @@ -52,6 +52,27 @@ private void checkAndPromptLocationPermissions() { SensorControlBackgroundChecker.restartFSMIfStartState(cordova.getActivity()); return; } + // If this is android 11 (API 30), we want to launch the app settings instead of prompting for permission + // because the default permission prompting does not offer "always" as an option + // https://github.com/e-mission/e-mission-docs/issues/608 + // we don't really care about which level of permission is missing since the prompt doesn't + // do anything anyway. If either permission is missing, we just open the app settings + // Note also that we should actually check for VERSION_CODES.R + // but since we are not targeting API 30 yet, we can't do that + // so we use Q (29) + 1 instead. I think that is more readable than 30 + if ((Build.VERSION.SDK_INT >= (Build.VERSION_CODES.Q + 1)) && + (!cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) || + !cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION))) { + Context mAct = cordova.getActivity(); + String msgString = " LOC = "+cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION)+ + " BACKGROUND LOC "+ cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)+ + " Android R+, so opening app settings anyway"; + Log.i(cordova.getActivity(), TAG, msgString); + Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS); + intent.setData(Uri.fromParts("package", mAct.getPackageName(), null)); + mAct.startActivity(intent); + return; + } if(!cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) && (Build.VERSION.SDK_INT >= Build.VERSION_CODES.Q) && !cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)) { From 2f6f5d4bf869635910005eb953178f71119edc55 Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 08:14:56 -0700 Subject: [PATCH 3/7] Humanize the messages commonly generated by the foreground service The most common messages displayed by the foreground service are the "In state XXX" message. But the state is too jargon-y - e.g. "In state local.state.waiting_for_trip_start". It could also not be easily internationalized. Based on feedback from the Smart Commute program, we changed this to more meaningful messages. I'd been putting this off because it is a bit tricky to do. We have the state as a string. But the i18n solution maps from an id to the humanized string. So how do I convert a string to an ID? It turned out to not be that hard. https://stackoverflow.com/a/29902630/4040267 https://stackoverflow.com/a/19093447/4040267 --- .../location/ForegroundServiceComm.java | 14 +++++----- ...ripDiaryStateMachineForegroundService.java | 26 ++++++++++++++++--- .../TripDiaryStateMachineService.java | 2 +- .../TripDiaryStateMachineServiceOngoing.java | 2 +- 4 files changed, 32 insertions(+), 12 deletions(-) diff --git a/src/android/location/ForegroundServiceComm.java b/src/android/location/ForegroundServiceComm.java index 3a15c68..bce42e3 100644 --- a/src/android/location/ForegroundServiceComm.java +++ b/src/android/location/ForegroundServiceComm.java @@ -20,27 +20,27 @@ public class ForegroundServiceComm { private TripDiaryStateMachineForegroundService mService; private boolean mBound = false; private int nRecursion = 0; - private String pendingMsg = null; + private String pendingMsgState = null; public ForegroundServiceComm(Context context) { mCtxt = context; mCtxt.bindService(getForegroundServiceIntent(), connection, 0); } - public void setMessage(String msg) { + public void setNewState(String newState) { nRecursion++; if (mBound) { Log.d(mCtxt, TAG, "Service successfully bound, setting state"); - mService.setStateMessage(msg); + mService.setStateMessage(newState); } else { Intent fsi = getForegroundServiceIntent(); if (nRecursion < 5) { Log.d(mCtxt, TAG, "nRecursion = "+nRecursion+" rebinding "); - pendingMsg = msg; + pendingMsgState = newState; mCtxt.bindService(fsi, connection, 0); } else if (nRecursion < 10) { Log.d(mCtxt, TAG, "nRecursion = "+nRecursion+" restarting before rebind "); - pendingMsg = msg; + pendingMsgState = newState; TripDiaryStateMachineForegroundService.startProperly(mCtxt); mCtxt.bindService(fsi, connection, 0); } else { @@ -67,8 +67,8 @@ public void onServiceConnected(ComponentName className, mService = binder.getService(); Log.e(mCtxt, TAG, "Successfully bound to service "+ mService); mBound = true; - if (pendingMsg != null) { - setMessage(pendingMsg); + if (pendingMsgState != null) { + setNewState(pendingMsgState); } } diff --git a/src/android/location/TripDiaryStateMachineForegroundService.java b/src/android/location/TripDiaryStateMachineForegroundService.java index c618ed5..8ce73e9 100644 --- a/src/android/location/TripDiaryStateMachineForegroundService.java +++ b/src/android/location/TripDiaryStateMachineForegroundService.java @@ -46,14 +46,33 @@ public void onCreate() { Log.d(this, TAG, "onCreate called"); } + /* + * Convert the current state, which is a string (e.g. + * waiting_for_trip_start) into a more human-friendly string (e.g. Ready to + * go). Since the string mappings expect an id, we need to look up the ID + * first using getResources and then the string. + * https://stackoverflow.com/a/19093447/4040267 + * Fallback to the old "In state XXX" if there is no mapping because + * otherwise the app will crash. + */ + public static String humanizeState(Context ctxt, String state) { + try { + int resId = ctxt.getResources().getIdentifier(state, "string", ctxt.getPackageName()); + return ctxt.getString(resId); + } catch (android.content.res.Resources.NotFoundException e) { + Log.e(ctxt, TAG, "ResourcesNotFoundException while humanizing message, falling back to auto-generated"); + return ctxt.getString(R.string.notify_curr_state, state); + } + } + @Override public int onStartCommand(Intent intent, int flags, int startId) { Log.d(this, TAG, "onStartCommand called with intent = "+intent+ " flags = " + flags + " and startId = " + startId); - String message = this.getString(R.string.notify_curr_state, TripDiaryStateMachineService.getState(this)); + String message = humanizeState(this, TripDiaryStateMachineService.getState(this)); if (intent == null) { SensorControlBackgroundChecker.checkLocationSettingsAndPermissions(this); - message = this.getString(R.string.notify_curr_state, TripDiaryStateMachineService.getState(this)); + message = humanizeState(this, TripDiaryStateMachineService.getState(this)); } handleStart(message, intent, flags, startId); // We want this service to continue running until it is explicitly @@ -103,7 +122,8 @@ public IBinder onBind(Intent intent) { return mBinder; } - public void setStateMessage(String message) { + public void setStateMessage(String newState) { + String message = humanizeState(this, newState); NotificationManager nMgr = (NotificationManager)this.getSystemService(Context.NOTIFICATION_SERVICE); nMgr.notify(ONGOING_TRIP_ID, getNotification(message)); } diff --git a/src/android/location/TripDiaryStateMachineService.java b/src/android/location/TripDiaryStateMachineService.java index 5838b0f..b65ee62 100644 --- a/src/android/location/TripDiaryStateMachineService.java +++ b/src/android/location/TripDiaryStateMachineService.java @@ -110,7 +110,7 @@ public void setNewState(String newState, boolean doChecks) { Log.d(this, TAG, "newState saved in prefManager is "+ PreferenceManager.getDefaultSharedPreferences(this).getString( this.getString(R.string.curr_state_key), "not found")); - mComm.setMessage(this.getString(R.string.notify_curr_state, newState)); + mComm.setNewState(newState); // Let's check the location settings every time we change the state instead of only on failure // This makes the rest of the code much simpler, allows us to catch issues as quickly as possible, // and diff --git a/src/android/location/TripDiaryStateMachineServiceOngoing.java b/src/android/location/TripDiaryStateMachineServiceOngoing.java index 22d2137..af2019a 100644 --- a/src/android/location/TripDiaryStateMachineServiceOngoing.java +++ b/src/android/location/TripDiaryStateMachineServiceOngoing.java @@ -114,7 +114,7 @@ public void setNewState(String newState) { Log.d(this, TAG, "newState saved in prefManager is "+ PreferenceManager.getDefaultSharedPreferences(this).getString( this.getString(R.string.curr_state_key), "not found")); - mComm.setMessage(this.getString(R.string.notify_curr_state, newState)); + mComm.setNewState(newState); stopSelf(); } From ed7aa9c08d32c5c4602a9273de33d15109f8fe13 Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 08:22:04 -0700 Subject: [PATCH 4/7] Fix some more of the android strings Remove all references to "email" since we now "upload" logs + shorten some strings --- res/android/values/dc_strings.xml | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/res/android/values/dc_strings.xml b/res/android/values/dc_strings.xml index 360a5da..ffb0085 100644 --- a/res/android/values/dc_strings.xml +++ b/res/android/values/dc_strings.xml @@ -24,14 +24,14 @@ Unknown error while reading location, please check your settings In state %1$s - Foreground service killed, Profile -> Email Log for debugging + Foreground service killed, report at Profile -> Upload Log Error reading stored tracking config, reset to defaults - Unrecoverable error in tracking. Profile -> Email Log for further investigation + Unrecoverable error in tracking, report at Profile -> Upload Log Ready for your next trip Yay! You are on a trip, keep going! - Can't start app, see next pop up + Cannot start app, see next pop up Stopped tracking; remember to re-enable From db176d60be3eaec883b1a06f52340fd118e79ead Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 08:30:48 -0700 Subject: [PATCH 5/7] Add additional imports to the sensor foreground delegate Follow on to e24462e56015883eb8fefda51d9db8f8ab29e73e Was ignored initially because I removed all imports to avoid the package renames --- src/android/verification/SensorControlForegroundDelegate.java | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/android/verification/SensorControlForegroundDelegate.java b/src/android/verification/SensorControlForegroundDelegate.java index 694ca10..59388ea 100644 --- a/src/android/verification/SensorControlForegroundDelegate.java +++ b/src/android/verification/SensorControlForegroundDelegate.java @@ -23,7 +23,10 @@ import android.content.Intent; import android.content.IntentSender; import android.content.pm.PackageManager; +import android.net.Uri; import android.os.Build; +import android.provider.Settings; + import com.google.android.gms.location.LocationSettingsStates; From 0e47d500503c8034af8c207b9e0288e8106072fa Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 10:19:18 -0700 Subject: [PATCH 6/7] Ensure that we get a callback after the app settings page is closed So that we can re-check and ensure that the user used the appropriate settings Testing done: - on android 29, ensured that the original in-line prompt was displayed - on android 30, - launch the settings, user doesn't change anything, come back to app, error about location permission, click - launch the settings again, user switches to "when in use", come back to app, error about background permission, click - launch the settings again, user switches to "always", come back to app, no additional errors, now in waiting_for_trip_start This is a follow on to https://github.com/e-mission/e-mission-data-collection/pull/192/commits/e24462e56015883eb8fefda51d9db8f8ab29e73e --- .../SensorControlForegroundDelegate.java | 25 ++++++++++++++++--- 1 file changed, 21 insertions(+), 4 deletions(-) diff --git a/src/android/verification/SensorControlForegroundDelegate.java b/src/android/verification/SensorControlForegroundDelegate.java index 59388ea..ff31723 100644 --- a/src/android/verification/SensorControlForegroundDelegate.java +++ b/src/android/verification/SensorControlForegroundDelegate.java @@ -51,7 +51,8 @@ public void checkAndPromptPermissions() { } private void checkAndPromptLocationPermissions() { - if(cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) && cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)) { + if(cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) && + cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)) { SensorControlBackgroundChecker.restartFSMIfStartState(cordova.getActivity()); return; } @@ -66,14 +67,16 @@ private void checkAndPromptLocationPermissions() { if ((Build.VERSION.SDK_INT >= (Build.VERSION_CODES.Q + 1)) && (!cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) || !cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION))) { - Context mAct = cordova.getActivity(); + Activity mAct = cordova.getActivity(); String msgString = " LOC = "+cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION)+ " BACKGROUND LOC "+ cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)+ " Android R+, so opening app settings anyway"; Log.i(cordova.getActivity(), TAG, msgString); + // These are to hopefully help us get a callback once the settings are changed Intent intent = new Intent(Settings.ACTION_APPLICATION_DETAILS_SETTINGS); intent.setData(Uri.fromParts("package", mAct.getPackageName(), null)); - mAct.startActivity(intent); + cordova.setActivityResultCallback(plugin); + mAct.startActivityForResult(intent, SensorControlConstants.ENABLE_BOTH_PERMISSION); return; } if(!cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION) && @@ -198,9 +201,9 @@ public void onRequestPermissionResult(int requestCode, String[] permissions, public void onActivityResult(int requestCode, int resultCode, Intent data) { + Activity mAct = cordova.getActivity(); switch (requestCode) { case SensorControlConstants.ENABLE_LOCATION_SETTINGS: - Activity mAct = cordova.getActivity(); Log.d(mAct, TAG, requestCode + " is our code, handling callback"); cordova.setActivityResultCallback(null); final LocationSettingsStates states = LocationSettingsStates.fromIntent(data); @@ -220,6 +223,20 @@ public void onActivityResult(int requestCode, int resultCode, Intent data) { Log.e(cordova.getActivity(), TAG, "Unknown result code while enabling location " + resultCode); break; } + case SensorControlConstants.ENABLE_BOTH_PERMISSION: + Log.d(mAct, TAG, requestCode + " is our code, handling callback"); + cordova.setActivityResultCallback(null); + Log.d(mAct, TAG, "Got permission callback from launching app settings"); + if (cordova.hasPermission(SensorControlConstants.LOCATION_PERMISSION)) { + // location permission enabled, cancelling notification + NotificationHelper.cancelNotification(cordova.getActivity(), SensorControlConstants.ENABLE_LOCATION_PERMISSION); + } + if (cordova.hasPermission(SensorControlConstants.BACKGROUND_LOC_PERMISSION)) { + // background location permission enabled, cancelling notification + NotificationHelper.cancelNotification(cordova.getActivity(), SensorControlConstants.ENABLE_BACKGROUND_LOC_PERMISSION); + } + SensorControlBackgroundChecker.restartFSMIfStartState(cordova.getActivity()); + break; default: Log.d(cordova.getActivity(), TAG, "Got unsupported request code " + requestCode + " , ignoring..."); } From a726584663a2b315eea02183b4d4294710a3cb16 Mon Sep 17 00:00:00 2001 From: Shankari Date: Sat, 5 Jun 2021 10:22:35 -0700 Subject: [PATCH 7/7] Add null check for "data" in the callback Since the app settings screen does not have an output --- src/android/DataCollectionPlugin.java | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/android/DataCollectionPlugin.java b/src/android/DataCollectionPlugin.java index 5330401..d399f91 100644 --- a/src/android/DataCollectionPlugin.java +++ b/src/android/DataCollectionPlugin.java @@ -161,8 +161,13 @@ public void onRequestPermissionResult(int requestCode, String[] permissions, @Override public void onActivityResult(int requestCode, int resultCode, Intent data) { + if (data == null) { + Log.d(cordova.getActivity(), TAG, "received onActivityResult(" + requestCode + "," + + resultCode + "," + "null data" + ")"); + } else { Log.d(cordova.getActivity(), TAG, "received onActivityResult("+requestCode+","+ resultCode+","+data.getDataString()+")"); + } // This will be a NOP if we are not handling the correct activity intent mControlDelegate.onActivityResult(requestCode, resultCode, data); /*