Speculative fix for issue #6468: [Bug]: GPS location does not work (for GMS-less phones)#6763
Open
Jason-Whitmore wants to merge 1 commit intocommons-app:mainfrom
Open
Speculative fix for issue #6468: [Bug]: GPS location does not work (for GMS-less phones)#6763Jason-Whitmore wants to merge 1 commit intocommons-app:mainfrom
Jason-Whitmore wants to merge 1 commit intocommons-app:mainfrom
Conversation
…cuit Before this commit, the logical AND operator was used with the successful/unsuccessful booleans returned from requestLocationUpdatesFromProvider() calls. If the call on the first provider returned false, the logical AND operator would short circuit, therefore not attempting the call on the second provider, even if the second provider was available to use. In this situation, the LocationServiceManager would receive no location updates. This commit changes the logical AND operator to a logical OR operator. Short-circuiting will no longer occur and requestLocationUpdatesFromProvider() for the second provider will be called. If the call for the first provider returned false and call for the second provider returned true, then the LocationServiceManager will receive location updates from the second provider.
Collaborator
Author
|
I apologize for the delay in fixing this bug. I think you should try this PR out since you have a lot of experience with this bug. Let me know if it works! |
|
✅ Generated APK variants! |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Description (required)
Fixes #6468
What changes did you make and why?
This PR changes a logical operator from the logical AND to a logical OR to prevent a short circuit which I believe is causing the issue.
Specifically:
Location updates from providers are requested by calling
LocationServiceManager.requestLocationUpdatesFromProvider()which returns true on success and false on failure.This method is called twice with the logical AND operator. The first call is for the network provider and the second call is for the GPS provider. If the first call fails (network provider), then the AND operator will short circuit, preventing the second call (GPS provider) from occurring.
I believe some non GMS phones do not have a working network location provider, but may still have a working GPS location provider. Therefore, some non GMS phones will not receive location updates despite the ability to. This then causes both the recenter button to center, and the blue dot to appear, at the wrong location.
Replacing the logical AND to a logical OR stops the short circuiting behavior and allows the second call to happen regardless of the return value of the first call.
Tests performed (required)
Tested ProdDebug on Android Studio Emulator with API level 35.
I was able to reproduce the bug on main by creating a new virtual device in the emulator with the "Services" drop down menu set to "Android Open Source". Then, in the Nearby screen, the blue dot would not move despite changing the location in the emulator. Nor does the recenter button work.
On this PR, the bug appears fixed and Nearby works as intended. Since I cannot confirm that the "Android Open Source" setting actually removes GMS from the virtual device, this fix is speculative.