Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Migrate from android.location API. #6

Closed
michalchudziak opened this issue Apr 2, 2019 · 26 comments
Closed

Migrate from android.location API. #6

michalchudziak opened this issue Apr 2, 2019 · 26 comments
Labels
help wanted Extra attention is needed question Further information is requested

Comments

@michalchudziak
Copy link
Owner

michalchudziak commented Apr 2, 2019

Ask your Question

As discussed, we should migrate from android.location API. I'd like to use this issue to start the discussion on potential alternatives.

Reference libraries:

@michalchudziak michalchudziak added help wanted Extra attention is needed question Further information is requested labels Apr 2, 2019
@matt-oakes
Copy link
Collaborator

This is defiantly a good idea, however, I think we should still allow developers to use the current android.location API if they want as there are some good reasons to use it:

  • Not all devices have Google Play, especially in China where none do.
  • Not all devices have up-to-date versions of Google Play and users who are signed in to enable the API. I'm not sure if location is affected by this, but it can be a problem for some Google Play Services APIs.

We might also need to alter the API to properly support this and other settings.

@dulmandakh
Copy link
Contributor

how is it easy to support multiple backends, like plain android and google play services?

@matt-oakes
Copy link
Collaborator

I have implemented it in react-native-location by having an abstract interface which each provider can implement and then implementing it for each provider:

There is then a piece of code which selects which one to use based on the app dependencies and the configuration setting which the user has requested.

I've not really dug too much into this libraries code yet, but we can likely use a similar method.

@tijs
Copy link

tijs commented Apr 3, 2019

Isn’t fused location the preferred way to get location on android these days. As implemented by https://github.com/MustansirZia/react-native-fused-location for instance...

@matt-oakes
Copy link
Collaborator

@tijs It is, and that's why this library should support it, however, as noted above it's not always available on all devices so we need to support both.

@tijs
Copy link

tijs commented Apr 3, 2019

Right that seems like a sensible path. So the main package would provide a stable API while the actual location service is provided by a 'plugin' or 'driver', with some defaults out of the box. I've found the way they did this in the LocalForage package very practical. Actually makes it quite easy to provide a custom driver as well. Maybe interesting inspiration? https://localforage.github.io/localForage/#settings-api-setdriver

@matt-oakes
Copy link
Collaborator

Exactly. We need to discuss how this will work exactly, but I think that we will try to use the best API possible by default and let the developer override that if they know they want to use a specific one.

Thanks for the link to LocalForge 👍

@michalchudziak
Copy link
Owner Author

@matt-oakes @tijs I love the ideas of a native interface to be implemented by different providers as well as the driver system. I wonder if we should enforce users to pick one of the available services via setDriver method, or would we have a default one and make it options if someone wants to use a different service? If we'll have a default one - which one it should be?

@matt-oakes
Copy link
Collaborator

I think the default should be "auto". It should try to use the Google Play one, but if one of these conditions is not met, it would automatically fallback to the android.location API:

  • App does not have the Google Play Services dependencies linked.
  • Device does not have Google Play services installed.
  • Google Play Services responds with a non-resolvable error (see here for how error handling works).

That should "do the right thing" with no configuration for most users and avoid the need for users to understand the differences and handle checking for the presence of Google Play Services themselves.

@dohard-ma

This comment has been minimized.

@nicomontanari
Copy link

nicomontanari commented May 20, 2019

Hi! Are there any updates?

@matt-oakes
Copy link
Collaborator

@nicklockwood No, there are no updates. Contributions are always welcome though.

@dulmandakh
Copy link
Contributor

dulmandakh commented May 21, 2019

let's make it work for most cases, or use Google Services. Maybe add other backends later on.

@giacomocerquone
Copy link

why not to try to ask to one of the developers of those modules to join this repo and put here their code in order to help them too maintaining their libs?
I know it may be problematic for a lot of reasons but something must be done on trying to avoid to waste more time on writing implementations of things that we already have out in the public.

Do you have any ideas that would let us write less code to implement this feature (that I find really of core importance)?

@huszzsolt
Copy link

I've just seen react-native-geolocation-service is now using react-native-geolocation
https://github.com/Agontuk/react-native-geolocation-service/blob/master/CHANGELOG.md
which is not great, i feel

@vomchik
Copy link

vomchik commented Oct 30, 2019

I've just seen react-native-geolocation-service is now using react-native-geolocation
https://github.com/Agontuk/react-native-geolocation-service/blob/master/CHANGELOG.md
which is not great, i feel

This is for iOS only.

@ravirajn22
Copy link

I will try to work on this change, @matt-oakes @dulmandakh @michalchudziak any suggestion and ideas related to the API.
Need help and suggestions regarding

  1. Permission handling.
  2. Should we explicitly inform the users that the geolocation updates when in background is not possible using javascript? or is it possible?
  3. Testing? Can we skip automated tests for now and later we can add it.

Is some other team or person working on this change already?

@giacomocerquone
Copy link

giacomocerquone commented Dec 23, 2019

@ravirajn22

I have a good background on geolocation features in react native.
I'd say to use as a guideline this package here: https://github.com/transistorsoft/react-native-background-geolocation
It's a fantastic lib that should be taken as inspiration when working on this.
Let's go for points:

  1. For this I wouldn't expose too much. As I have just said, getting inspiration from that package, I'd expose just a "requestPermission" function to request geolocation permission, but it should never be called manually by the developer since the module should request it automatically. And just for info, you shouldn't create a structure as the one created by react-native-permissions and you shouldn't import that too.
  2. This is a very important decision. That same module I talked before, it's very complex and allows you to track an user in various way, in background (and in killed state too) both on iOS and Android.
    If you ask me, I'd say to absolutely avoid any kind of background implementation and give to the user just the possibility to track a device getting a position once or doing a "watchPosition" as long as the app is running in foreground.
  3. yes sure, but I'd not publish without testing it.

@ravirajn22
Copy link

ravirajn22 commented Dec 26, 2019

Hi, I went through other geolocation libraries and feel react-native-geolocation-service seems to have a stable code and almost supports web API spec and react-native-location has implemented a way of using both google play services and android location api but does not match web API spec . I feel better copy android code from react-native-geolocation-service and add a way to use android.location API for non google services devices like they have done it in react-native-location. So that we can keep the current API as it is without making any breaking changes? This requires copying code from other libraries instead of reimplementing here again. What do you say folks, is this approach ok?

@Return-1
Copy link

Just out of curiosity, are there still plans to maintain this lib?

@lucasbento lucasbento pinned this issue Mar 20, 2020
@vorasudh
Copy link

vorasudh commented May 7, 2020

@ravirajn22, were you able to work on the changes?

@vinithreddy3
Copy link

vinithreddy3 commented Jun 2, 2020

Any updates whether this package is planning to implement location updates using play services api ? Looking into the discussion 'react-native-geolocation-service' package for android is using FusedLocationProviderClient's requestLocationUpdates task with a callback for which official doc says 'This method is suited for the foreground use cases' . So any plan for implementing background location updates with this package ?

@scousino
Copy link

Just want to hop in here and see if there has been any updates on this issue?

@hondem
Copy link

hondem commented Apr 23, 2021

Hello guys. Were there any updates on this topic recently?

@ravirajn22
Copy link

Moved to https://github.com/Agontuk/react-native-geolocation-service long back. Very stable.

@AlgorithmeCM
Copy link

Moved to https://github.com/Agontuk/react-native-geolocation-service long back. Very stable.

I'm stuck on the geolocation access authorization problem on Android with this library.
Can you share how you were able to deal with this problem?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed question Further information is requested
Projects
None yet
Development

No branches or pull requests