Closed ericafenyo closed 4 years ago
Can one of the admins verify this patch?
@ericafenyo this looks good for the upgrade to API 28. Can you outline the testing done, including:
config.xml
@ericafenyo I am a bit surprised that this was so easy :) As I said in https://github.com/e-mission/e-mission-docs/issues/465, the main change appeared to be to support location permission changes (https://source.android.com/devices/tech/config/tristate-perms). Note that:
Tristate location permissions are applied to apps in Android 10 irrespective of an app’s target SDK.
I do see that there is an exception
For details if your app targets Android 9 and lower, see Continuation of user-initiated action.
which uses foreground services to get around the tristate checks and use the "when in use" permission for background updates.
But in that case, you would also need the
To retain access to the device's location in this specific type of use case, start a foreground service that you've declared as having a foreground service type of "location" in your app's manifest:
And I don't see that in this PR.
So I would really like to see the results of testing this change on Android 10 as well 😄
@ericafenyo any updates on the testing result with android 10?
@shankari Sorry for the late response.
changes to config.xml
- We added the following lines to the android platform section within the
config.xml
.<platform name="android"> ...
<preference name="android-targetSdkVersion" value="28" />
<config-file parent="/manifest/application" target="AndroidManifest.xml">
<uses-library android:name="org.apache.http.legacy" android:required="false" />
</config-file>
For more details on supporting legacy apache http client which is no more supported from API 6.
https://developer.android.com/about/versions/pie/android-9.0-changes-28#apache-p
> set of android versions tested against
In the meantime, we have only tested the application on Samsung galaxy A50 and S10 with android versions 9 and 10 respectively. Though it was stated in [Location in Android 10 code lab](https://codelabs.developers.google.com/codelabs/while-in-use-location/index.html?index=..%2F..index#1) that and I quote;
>" If your app has a foreground service (along with a notification), then you don't have to request
background location access, but you will still need to declare that your foreground service has a
[foreground service type](https://developer.android.com/reference/android/R.attr.html#foregroundServiceType) of "location " ,
the `android:foregroundServiceType="location"` field wasn't added to our foreground service to be able to run on S10 with android 10.
I tried adding it but got an error.
I have this in the manifest file
```xml
<service
android:enabled="true"
android:exported="false"
android:name="<package>.TripDiaryStateMachineForegroundService"
android:foregroundServiceType="location"
/>
<!-- NB: <package> = edu.berkeley.eecs.emission.cordova.tracker.location-->
and got this error. I haven't found a solution yet.
AAPT: error: attribute android:foregroundServiceType not found.
We might have to target API 29 and update our android library versions.
set of operations performed
Currently, we are targeting API 28 and we were able to record and display trip details. Based on these details, we added our own functionalities. We were able to sync data between the client and our e-mission server.
@ericafenyo
AAPT: error: attribute android:foregroundServiceType not found.
Yes, it looks like this is due to targeting API 28 instead of API 29.
Currently, we are targeting API 28 and we were able to record and display trip details.
So just to double confirm:
attribute android:foregroundServiceType
andIf so, I am happy to merge this change.
@ericafenyo
We added the following lines to the android platform section within the config.xml. For more details on supporting legacy apache http client which is no more supported from API 6. https://developer.android.com/about/versions/pie/android-9.0-changes-28#apache-p
Hmmm, the legacy apache http client has been deprecated for a long time, and there is already code to address that https://github.com/e-mission/cordova-server-communication/blob/master/src/android/httplib.gradle
However, I see that now there is also a manifest update required now. If this is really required (e.g. app does not compile without it), can you submit a PR for it as well? Since this is only relevant while compiling the app from scratch, you can just change https://github.com/e-mission/e-mission-phone/blob/master/config.cordovabuild.xml
@shankari
If so, I am happy to merge this change.
Yea, we are able to record trips correctly on the Samsung galaxy with android 10.
However, I see that now there is also a manifest update required now. If this is really required (e.g. app does not compile without it), can you submit a PR for it as well? Since this is only relevant while compiling the app from scratch, you can just change
Yea, we do have a runtime error when targeting API 28 and above but not API 27 and below. I created a pull request https://github.com/e-mission/e-mission-phone/pull/616
LGTM!
@ericafenyo thanks so much for the contribution!
Add the permission required for apps targeting Android 9 (API level 28) and above.
https://developer.android.com/about/versions/pie/android-9.0-migration#tya