Improved user location on Android#2993
Improved user location on Android#2993christopherdro merged 15 commits intoreact-native-maps:masterfrom funkyfourier:locationsource
Conversation
felipecsl
left a comment
felipecsl
left a comment
There was a problem hiding this comment.
Overall looks pretty good with some minor nits
| exclude group: 'com.android.support' | ||
| } | ||
| implementation "com.android.support:appcompat-v7:${safeExtGet('supportLibVersion', '28.0.0')}" | ||
| implementation "com.google.android.gms:play-services-location:16.0.0" |
There was a problem hiding this comment.
I guess this should be using the same ${safeExtGet('playServicesVersion', '16.1.0') mechanism?
There was a problem hiding this comment.
Actually I discovered that play-services-location does not exactly follow the same versions as the rest of Play Services.
For example, play-services-location does not have a version 16.1.0:
https://mvnrepository.com/artifact/com.google.android.gms/play-services-maps
But for example play-services-maps does:
https://mvnrepository.com/artifact/com.google.android.gms/play-services-maps
Or am I missing something? If not, there are two ways to do it:
- Use a constant hard coded version like I did.
- Expose another Gradle variable like "locationServicesVersion" or something like that.
There was a problem hiding this comment.
You're right. I looked directly in the Google maven repo too and it doesnt seem to have version 16.1.0 for play-services-location. That's confusing!
Thanks for explaining, this seems reasonable then!
| import com.google.android.gms.maps.LocationSource; | ||
| import com.google.android.gms.tasks.OnSuccessListener; | ||
|
|
||
| public class FusedLocationSource implements LocationSource { |
There was a problem hiding this comment.
Just for the record:
This file is indented with 4 spaces while the rest of the project is indented with 2 spaces.
It's non blocking, we should create a separate pull request for automatic style checking/formatting.
|
@funkyfourier Could you resolve the conflicts when you get a chance? Here are the changes that cause the conflict. 801614e#diff-ac45350dc94234f6d3c8b856aacbc395 |
|
Was the fixes ok? When will this be merged? Thank you. |
|
@funkyfourier Any thoughts on the issue linked to this PR? |
|
I'll have a look tomorrow. |
* Use custom made LocationSource * Added property to set priority for user location * Property for setting priority of user location * Property for update interval of user location * Changed default update interval for user location to 5000 ms * Property for setting fastest interval for user location updates * Updated documentation * Fixed documentation update * Fixed documentation update * Fixed documentation update * Fixed documentation update * Fixed documentation update * Set fields to final where possible * Merged changes for AndroidX Co-authored-by: jmrhvl <45755981+jmrhvl@users.noreply.github.com>
4 participants
Does any other open PR do the same thing?
Not that I can see.
What issue is this PR fixing?
#2923
Also, it brings some new possibilities for controlling power consumption and accuracy of user location in maps, specifically
userLocationPriority,userLocationUpdateIntervalanduserLocationFastestInterval. This is achieved by setting a custom LocationSource on the GoogleMap object, which exposes these properties and does not cause issue #2923.How did you test this PR?
This PR applies only to Android. I have described in the issue mention above how to recreate the issue, and this PR solves it. It has been tested on a OnePlus 6 and a Sony XZ1.