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.

Sign up for GitHub

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

Use the same location providers in every map engine + some tweaks #5636

Merged
merged 6 commits into from
Jul 7, 2023
Merged

Use the same location providers in every map engine + some tweaks #5636

Show file tree
Hide file tree
Changes from 1 commit
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
092075a
Use the same location providers in all map engines
grzesiek2010 Jun 19, 2023
afeece2
Removed duplicated GPS checking
grzesiek2010 Jun 19, 2023
a2a57b0
Removed duplicated stopLocationUpdates call
grzesiek2010 Jun 19, 2023
d6a63e4
Do not ignore the first received location
grzesiek2010 Jun 19, 2023
92f036c
Track if GPS connection fails and why
grzesiek2010 Jun 19, 2023
7a13b67
Updated logging problems with connection
grzesiek2010 Jul 7, 2023
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 1 addition & 0 deletions location/build.gradle
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@ dependencies {
implementation project(':androidshared')
implementation project(':icons')
implementation project(':strings')
implementation project(':analytics')
implementation Dependencies.kotlin_stdlib
implementation Dependencies.androidx_core_ktx
implementation Dependencies.play_services_location
Expand Down
3 changes: 3 additions & 0 deletions location/src/main/java/org/odk/collect/location/GoogleFusedLocationClient.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,8 @@
import com.google.android.gms.location.LocationRequest;
import com.google.android.gms.location.LocationServices;

import org.odk.collect.analytics.Analytics;

import timber.log.Timber;

/**
Expand Down Expand Up @@ -196,6 +198,7 @@ public void onConnectionSuspended(int cause) {

@Override
public void onConnectionFailed(@NonNull ConnectionResult connectionResult) {
Analytics.log("GoogleFusedLocationClient", "onConnectionFailed", connectionResult.toString());
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

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.

if (getListener() != null) {
getListener().onClientStartFailure();
}
Expand Down