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

fix(geolocation)!: use getCurrentLocation for single location #582

Closed
wants to merge 3 commits into from

Conversation

jcesarmobile
Copy link
Member

BREAKING CHANGE:
getCurrentPosition ignores the timeout
sendLocation removes int timeout and boolean getCurrentPosition parameters.
requestLocationUpdates removes boolean getCurrentPosition parameter, use sendLocation for requesting a single location.

closes #571

@jcesarmobile jcesarmobile requested a review from carlpoole August 31, 2021 10:26
@KoenLav
Copy link

KoenLav commented Sep 1, 2021

I've tested these changes and they seem to resolve the issue for getCurentPosition, but watchPosition is still producing 'location unavailable'.

It seems like fusedLocationClient.requestLocationUpdates is simply not providing us with any valid updates?

@jcesarmobile
Copy link
Member Author

yeah, it will only fix getCurrentPosition, watchPosition code didn't change (other than removing a few parts used only by getCurrentPosition)
The problem is that isLocationAvailable() always returns false now, I've reported it to google. We can remove that code and then watchPosition will get the location, but then, when location is disabled the watchPosition will just hang because there won't be a way of knowing if it's disabled.

@KoenLav
Copy link

KoenLav commented Sep 1, 2021

I see! We get two responses to watchPosition; one successful and another failure, so basically both methods (onLocationResult and onLocationAvailability) are called.

        locationCallback =
            new LocationCallback() {
                @Override
                public void onLocationResult(LocationResult locationResult) {
                    Location lastLocation = locationResult.getLastLocation();
                    if (lastLocation == null) {
                        resultCallback.error("location unavailable");
                    } else {
                        resultCallback.success(lastLocation);
                    }
                }

                @Override
                public void onLocationAvailability(LocationAvailability availability) {
                    if (!availability.isLocationAvailable()) {
                        resultCallback.error("location unavailable");
                        clearLocationUpdates();
                    }
                }
            };

@KoenLav
Copy link

KoenLav commented Sep 1, 2021

We currently resolved this for our use-case by removing the onLocationAvailability method from the locationCallback in requestLocationUpdates.

We check the permission before we start watching the location and we don't mind if the watchLocation will continue running even if there is some kind of reason for there to be no location data, for now.

@jcesarmobile
Copy link
Member Author

Sadly the location being enabled/disabled has little to do with the geolocation permission being accepted/rejected, it can be disabled globally, so even if users accept the geolocation permission, the app will still hang if the location is disabled globally and you remove that function.

I've been investigating and it might be possible to detect if the location is disabled, so I might send a PR after this one gets merged that checks if the location is enabled instead of relying on the onLocationAvailability

@jcesarmobile
Copy link
Member Author

We went with #589 for now as it fixes the problem for both watchPosition and getCurrentPosition without introducing breaking changes. We will eventually do this change in the future as it's better to use getCurrentLocation API than the current approach, but no need to do a major release for now, so going to close the PR and create a new one in the future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

replace getCurrentPosition logic
3 participants