Use the same location providers in every map engine + some tweaks #5636
Conversation
grzesiek2010
force-pushed
the
COLLECT-5633
branch
from
7593e7d
to
92f036c
Compare
| @@ -196,6 +198,7 @@ public void onConnectionSuspended(int cause) { | |||
|
|
|||
| @Override | |||
| public void onConnectionFailed(@NonNull ConnectionResult connectionResult) { | |||
| Analytics.log("GoogleFusedLocationClient", "onConnectionFailed", connectionResult.toString()); | |||
There was a problem hiding this comment.
We'd need to add onConnectionFailed as a custom dimension to events in Firebase for this. I'm also a little worried about logging the connectionResult in case it contains a device identifier (or anything that could be PII).
Instead, could we just log a simple event like GoogleFustLocationClientConnectionFailed? That said, is this even something we have a question about? I'm not sure we're looking to track this at the moment.
There was a problem hiding this comment.
I'm also a little worried about logging the connectionResult in case it contains a device identifier
Instead, could we just log a simple event like GoogleFustLocationClientConnectionFailed? That said, is this even something we have a question about?
I wanted to know not only how often it happens but if the result contains something important to display to a user.
There was a problem hiding this comment.
but I don't have strong feelings about it so I'm ok with just checking if this takes place at all and then deciding if we need to determine the reason or just display something.
|
Tested with Success! Verified on device with Android 13 Verified cases:
|
|
Tested with Success! Verified on device with Android 10 |
Closes #5633
What has been done to verify that this works as intended?
I've tested manually the map engines we support.
Why is this the best possible solution? Were any other approaches considered?
The fix itself is in the first commit Use the same location providers in all map engines
. As discussed on slack the problem was that mapbox used its own location engine but google and osm our own providers. As a result, the mapbox engine messed up somehow with our providers (internally). Now all map engines use the same mechanism to retrieve the location and thanks to that mapbox works well and the behavior is consistent.
Other commits are just some improvements that I've identified:
stopLocationUpdatesis already called inGoogleFusedLocationClient/AndroidLocationClient#stop()I don't know why the first location was ignored. As a result, zooming to the current location in
GeoPointMapActivitywas slower compared toGeoPolyActivityit was an old fragment of code so I believe it is something we do not need anymore.just wanted to check if there are problems with connections and why. Maybe we need to display something ins such a case? We will see.
How does this change affect users? Describe intentional changes to behavior and behavior that could have accidentally been affected by code changes. In other words, what are the regression risks?
It should solve the problem with switching map engines and also make the behavior across different engines consistent. There is no one particular area of risk but it ould be more about recording locations than drawing lines/shapes/points for example.
Do we need any specific form for testing your changes? If so, please attach one.
No.
Does this change require updates to documentation? If so, please file an issue here and include the link below.
No.
Before submitting this PR, please make sure you have:
./gradlew checkAlland confirmed all checks still pass OR confirm CircleCI build passes and run./gradlew connectedDebugAndroidTestlocally.