ttnmapper / ttnmapper-android-v3

The 3rd rewrite of the Android mapping app
GNU General Public License v3.0
8 stars 8 forks source link

Adding ACCESS_BACKGROUND_LOCATION feature for android 10 & 11 #25

Closed Ravi0li closed 3 years ago

Ravi0li commented 3 years ago

For Android 10 & 11 it is very usefull to get the permission ACCESS_BACKGROUND_LOCATION. I test the code with an Android 9, 10 and 11 AVD.

For more informations see: https://developer.android.com/training/location/permissions

jpmeijers commented 3 years ago

Are you sure we really need this? I have been avoiding this, as it requires special permission filing when publishing via the Play Store.

As far as I understand this is that we are using a foreground service. ie, there is a persistent notification that says the app is running. For that reason we are obtaining the location in the foreground, not in the background.

jpmeijers commented 3 years ago

I read through the document you linked, and my understanding is still the same. Because we are using a Foreground Service, we do not need the ACCESS_BACKGROUND_LOCATION permission. What I did pick up is that we need to declare the service type as location. I've added that to the manifest:

https://github.com/ttnmapper/ttnmapper-android-v3/commit/7e9fb5b8a3011299fe1df74dabfc4f45a2492374

Do you agree with me on this, or am I missing something that requires us to add ACCESS_BACKGROUND_LOCATION?

Ravi0li commented 3 years ago

Sry I’m not a professional Android programmer. I overlooked that this is a foreground service and not a background service. The problem in Android 10 & 11 was when the app was closed and the service received a MQTT messages: But no circles was painted on the map, possibly because he couldn’t read the current location.

My text with “I test the code with an Android 9, 10 and 11 AVD” was maybe a little bit confusing. I only tested the fine work from the permission messages. Not if the location service work fine.

You are absolutely right with your solution. Now I test the commit 7e9fb5b with Android 11 and this solution works fine. The foreground service could read the current position and paint the circle on the map.

Thanks for your fast respond 👍

jpmeijers commented 3 years ago

Thanks for you input. It's good to have another set of eyes checking the code and interpreting the documentation. I'm also not a professional Android dev, but rather a full stack engineer. Maintaining this app is a lot of effort and requires a lot of reading and updates every couple of months just to keep it working on all new and old Android versions.

Thanks to your PR I realised that we needed that extra location type service tag.

Please feel free to file more PRs if you make any other improvements.