Skip to content
This repository has been archived by the owner on May 22, 2023. It is now read-only.

Log location coordinates in the background #28

Closed
yanokwa opened this issue Oct 31, 2018 · 39 comments
Closed

Log location coordinates in the background #28

yanokwa opened this issue Oct 31, 2018 · 39 comments
Projects

Comments

@yanokwa
Copy link
Member

yanokwa commented Oct 31, 2018

The original thread on the forum
The initial specification written in Google Docs

What is the general goal of the feature?

As a supervisor, I would like to collect location coordinates in the background to provide evidence that data was collected in a particular place.

The reason of this feature is to avoid bad data. For example, you might notice an enumerator who as been poorly trained collect location in the morning, then go home and fill in the survey data whenever.

Proposed implementation

The idea is to extend the form audit log to add location to the log if location-mode is set as parameters in the audit row of the XLSForm, we will choose the most accurate point recorded in the age window.

  • location-mode: no-power, low-power, balanced, high-accuracy as defined in LocationRequest
  • location-interval (optional): the desired minimum interval in seconds that the location will be fetched
  • location-age (optional): the time in seconds that will location will be valid

In the ideal case for location-interval=10 and location-age=60, 6 points will be collected and we choose the most accurate one to write the log. But location-interval is not guaranteed and so there may be more points or less points.

If location-interval and location-age are not set, defaults will be set by the application based on the mode. If either is set, the value will override the defaults.

Currently an audit.csv file has the following structure:

event node start end
question /data/name 1523403169208 1523403170733

Location will be recorded in three columns: latitude, longitude, accuracy in the audit log that is attached to Collect's submissions.

New structure of an audit.csv file:

event node start end latitude longitude accuracy
question /data/name 1523403169208 1523403169208 27.1080 11.19574 20

New columns will be added only if the audit row of the XLSForm contains background parameters otherwise they are not needed.

When you have an existing form with audit that did not have location enabled and you change that form to add location, three new columns will be added.

XForms specification

In the XForms spec audit is placed in the meta block under the orx namespace. It uses a binary data type. See getodk/xforms-spec#94

<orx:meta>
	<orx:audit/>
</orx:meta>
<bind nodeset="/orx:meta/orx:audit" type="binary"/>

We will add the following bind attributes in the odk namespace to the specification. Both attributes are required to enable location audits and location-age must be greater than or equal to location-interval.

<bind nodeset="/orx:meta/orx:audit" type="binary" 
           odk:location-mode="balanced" odk:location-interval="10" odk:location-age="60" />

We prepend "location-" to the tags to ensure that there are no future conflicts with mode, interval and age.

The tentative pyxform specification is as follows.

type name parameters
audit audit location-mode=balanced location-age=60

Device behavior

When Collect opens the form, we will request location updates from the location provider every desired interval seconds. Location updates will be started in onCreate() method and paused when the app is in the background (onStop() method) to avoid battery draining.

At the time of the audit event (e.g., view question, form save etc.) occurs, Collect will check the cache for the location.

If a user disables/enables location or revokes/grants the permission after starting a form an appropriate event will be logged:

event node start end latitude longitude accuracy
location disabled 1523403169208
location enabled 1523403269301
location permission revoked 1523403469758
location permission granted 1523403669354

We will not exit a form if a user disables location or revokes the permission.

Data privacy issues

To ensure enumerators are aware that this sensitive data is being gathered...

On first launch of the form, we use a dialog to get consent.

  • Paragraph 1: This form continuously collects your location in the background and includes it with the form data.
    • Android >6
      • Paragraph 2: Revoke Collect's location permissions to fill the form without any location data.
      • Buttons: OK | Revoke location permissions
    • Android <6
      • Paragraph 2: Disable your location provider to fill the form without any location data.
      • Buttons: OK | Disable location provider
  • On OK, fire up the GPS
  • On revoke/disable, go to the location permissions/provider screen

We use the above strategy because if we just ask a yes/no consent question and the user says no, we have no easy way of re-enabling. That is, a yes/no question takes what we expect to be an occasional disabling and turns it into a permanent disable. Using permissions or location providers solves that problem.

The caveat is that this not the easiest thing for users to do. Further, disabling location for the device means no other location apps will work. And revoking permissions means that any required location questions (e.g., geopoint) will not work.

On subsequent launches of the form, we use a snackbar (see image below) to show background location collection status.

  • Paragraph 1: This form collects your location continuously and includes it with the form data.
  • Paragraph 2: This form needs to continuously collect your location, but <reason why it can't be accessed>
    • your location provider has been disabled
    • your location permissions have been revoked

screen shot 2018-10-30 at 17 48 43

This will serve as a reminder to the enumerator that their location is tracked for that specific form. In the case where the enumerator is not the one to open a form for the first time, they will still see the snackbar and have an opportunity to exit the form or turn off location is required for safety or other reasons. Even though Android does show that location is being accessed, it's not clear by which app. The snackbar is clearly associated with Collect.

If location is enabled/disabled during data collection, then we show the snackbar on return to Collect. If this is hard to do, we will show the snackbar on return to Collect (so no need to check enabled/disabled).

Questions

  1. Is this sensible that we use the odk namespace for these new attribute given that that audit is under orx?
  2. Are the tradeoffs we make when a user doesn't want location collected acceptable?
@yanokwa yanokwa added this to Under consideration in Roadmap Oct 31, 2018
@yanokwa
Copy link
Member Author

yanokwa commented Nov 5, 2018

Question 1

I got some feedback from @grzesiek2010:

Wouldn't a new option in General Settings be better? Something like "Allow collecting location coordinates in the background" Then such a dialog would have an option to to enable/disable it. The same with snackbar.

I think this is an improvement we should add. I would put this under General Settings > Form management > Form filling under Allow continuous collection of location in background. This feature would default to enabled and could be hidden/revealed under the Admin settings.

On first launch, we'd use a dialog that says...

This form continuously collects your location in the background and includes it with the form data.
To fill the form without any location data, exit and disable continuous collection in the General Settings – OK | Exit

On second launch, we'd then use a snackbar...

This form collects your location continuously and includes it with the form data.

One of the events would then be

app preference enabled/disabled

And one of the reasons in the snackbar would be

your app preference has disabled it

Question 2

I think folks feel strongly that there should always be a notification (e.g., the snackback), so I don't think we should make an option to remove it.

@lognaturel @MartijnR @aurdipas Do these changes seem reasonable to you?

@adamvert @smoyth does the UX feel good to you?

@admbtlr
Copy link

admbtlr commented Nov 6, 2018

Seems sensible. The only suggestion I would make is to extend the option text to something like Allow continuous background location tracking (for forms that request it) to indicate that we're not always collecting position data, but only when the forms themselves want it.

That's kind of a long setting label, but I can't figure out a way to make it more succinct right now.

@smoyte
Copy link

smoyte commented Nov 7, 2018

This is looking good to me.

How about Allow background location tracking with a hint of You will always be notified when a form is tracking your location.

Does it make sense for this setting to be hide-able by admins? Doesn't that defeat the intent of always having a way to opt out? If we keep this, we should not be displaying instructions for how to disable tracking when the setting is hidden...

@aurdipas
Copy link
Member

aurdipas commented Nov 7, 2018

I think that the setting hide-able by the admins makes sense. I want this setting to be the same on all devices for my data collection by default. The enumerator is informed about that with a dialog the first time and then with a snackbar from the second time onwards.
In any case the enumerato can disable the gps in the device if want to force to not have the background data collection of the coordinates.

@smoyte
Copy link

smoyte commented Nov 7, 2018

@aurdipas fair points. How about if the setting is hidden and set to enable tracking, the instructions we give say to turn off tracking by revoking permission or disabling GPS (as Yaw originally planned) instead of pointing them to a setting they can't see?

@yanokwa
Copy link
Member Author

yanokwa commented Nov 7, 2018

@smoyth If the setting is hidden and set to enable tracking, maybe we give them a hint about revoking permission of disabling GPS.

> To fill the form without any location data, exit and disable Android's location provider. You can also revoke Collect's location permissions.

I think letting enumerators exit on these dialogs is fine. Taking them directly to the settings I think is a bit too much code for what is likely to be an edge case.

Update: I need to think about this a bit more...

@zestyping
Copy link

zestyping commented Nov 15, 2018

The name of the "age" parameter seems confusing to me. Or, at least, I think it would be pretty hard for someone to see the word "age" and figure out what it means in this context.

Can we clarify something about the intended behaviour? Suppose location-interval is 10, and location-age is 60. And suppose, just for the sake of illustration, that we're recording GPS fixes in an array named fixes. After 121 seconds have elapsed, we have recorded 13 fixes:

fixes[0] is recorded at time 0:00:00.
fixes[1] is recorded at time 0:00:10.
fixes[2] is recorded at time 0:00:20.
...
fixes[5] is recorded at time 0:00:50.
fixes[6] is recorded at time 0:01:00.
fixes[7] is recorded at time 0:01:10.
...
fixes[12] is recorded at time 0:02:00.

How many points are stored in the log?

(a) Just two points:

  • The point with the lowest accuracy among fixes[0] through fixes[5] inclusive
  • The point with the lowest accuracy among fixes[6] through fixes[11] inclusive
  • fixes[12] is not used.

(b) 8 points:

  • The point with the lowest accuracy among fixes[0] through fixes[5] inclusive
  • The point with the lowest accuracy among fixes[1] through fixes[6] inclusive
  • The point with the lowest accuracy among fixes[2] through fixes[7] inclusive
  • The point with the lowest accuracy among fixes[3] through fixes[8] inclusive
  • The point with the lowest accuracy among fixes[4] through fixes[9] inclusive
  • The point with the lowest accuracy among fixes[5] through fixes[10] inclusive
  • The point with the lowest accuracy among fixes[6] through fixes[11] inclusive
  • The point with the lowest accuracy among fixes[7] through fixes[12] inclusive

@yanokwa
Copy link
Member Author

yanokwa commented Nov 29, 2018

@grzesiek2010 will be returning in a few days from vacation, so I figure we should have a few things resolved!

@zestyping Instead of age, what word would you prefer? Maybe max-age? My goal was that we communicate the oldest point allowed.

As to the intended behavior, note that we aren't just writing things to a log. We are gathering a set of fixes in a moving window and when an event occurs, we look backwards in that window and pick a point. Let's say an event occurs at 0:02:00, we would look back 60 seconds to fixes[6] through fixes[11] and pick the point with the most accuracy to write to the log. If there are multiple points with matching accuracy, we write the most recent. Does that clarify the intended behavior?

@zestyping
Copy link

zestyping commented Nov 30, 2018

Ohhh, I see now. Thanks for explaining! Sorry, I hadn't understood from the spec that we're only recording locations when particular other audit events occur, not in their own recurring "location" event. Yes, location-max-age makes sense.

Given this behaviour, it almost seems like location-interval hardly matters. I mean, if I were giving a tutorial, it's hard for me to imagine what I would say to someone to explain how to choose the right value of location-interval, and it seems like the configuration choices we ask people to make should meet that bar.

I'd be surprised if a location-interval of 10 would save a noticeable amount of battery power compared to a location-interval of 1, for example; I assume the phone isn't going to turn off the GPS and turn it back on within a short interval, so I would suspect that (in the call to requestLocationUpdates) all values of minTime less than 120 or so have equivalent cost. (I'd want to test this before confidently asserting this is the case.) Also, minTime is just a minimum; Android may decide to return locations with a longer interval. If increasing location-interval doesn't save battery and decreasing it doesn't necessarily improve the results, it seems hard to give clear guidance on what to set for location-interval.

All this makes me wonder—should location-interval should be optional, or does it even need to be a parameter? What promise are we committing to exactly for a given value of location-interval, and is it going to be substantially better in practice than "we'll sample the location as often as Android will give it to us and take the most accurate sample received in the last location-max-age seconds"?

@yanokwa
Copy link
Member Author

yanokwa commented Nov 30, 2018

@zestyping Very good question. I'm not an expert here, but I believe depending on location API, interval has a big impact on battery.

There are two ways to get location. We currently use LocationManager, but might switch to FusedLocationProvider at some point (see getodk/collect#2717). Either way, we'll be able to set some desired interval and that interval does have an impact on battery life. Or to quote the Android docs, choose your interval wisely.

If we could pick a good default, I agree we wouldn't need the location-interval, but some deployments may value accuracy over battery life and it seems reasonable to let them choose that priority. Maybe we could call it location-min-interval to help explain the behavior better.

@admbtlr
Copy link

admbtlr commented Nov 30, 2018

Or maybe it makes sense to make that payoff explicit, call it accuracy with say three levels of "low", "medium" and "high" (or similar), document that this involves a payoff against battery consumption, and then set the values behind the scenes in a way that makes sense based on the library that we're using to retrieve the location?

Do we want users to have to understand how retrieving GPS locations works in order to make a decision about how much they value accuracy over battery life? (I'm asking that question without assuming an answer... )

@yanokwa
Copy link
Member Author

yanokwa commented Dec 3, 2018

@adamvert Here's one approach we can start with. Let's use the FusedProvider and instead of location-interval, we allow four values for location-priority: no-power, low-power, balanced, high-accuracy.

These correspond, respectively, with passive listening, city level accuracy (10km), block level (100m) accuracy, and active polling. More on the API at LocationRequest.

This approach, will let us see empirically how things behave on a few devices and I think @grzesiek2010 can do technical spike around this idea. If it looks good, then we can codify it in the spec.

@smoyth @aurdipas As to the user privacy issues, I think we should put Form background location (for forms that request it) below Form metadata in User and device identity (not in form filling) and default it on.

We should then show the warning in snackbar when the form is collecting that data and add a Disable button that takes you to the setting. If the setting is hidden by admins, we add a Disable this time button on snackbar, that let's a user override for that session.

@grzesiek2010
Copy link
Member

grzesiek2010 commented Dec 12, 2018

I have tested the FusedProvider (only high-accuracy) using three different devices and I can share my insights.
I agree with @zestyping's concerns about the location-interval parameter. We can't ensure that location will be recorded every x seconds. It's just an approximate value so for example, if you use 5s location might be recorded after 4,5,6,7,8 or even 9s as I noticed. It might be confusing for our users and cause problems:
for example, someone uses location-interval 5s and location-age 10s and expects he will have two points for each age what's not always true.

I think we should agree on good default values and of course, we can keep that option to use those parameters but we shouldn't require it or even encourage.

Using the FusedProvider with the mentioned accuracy levels seems like a good way for saving battery.

@yanokwa
Copy link
Member Author

yanokwa commented Dec 12, 2018

@grzesiek2010 So to be precise here, you think we should add location-priority and location-age at the spec level, and you'll pass the priority from the form to the FusedProvider. And we can use the intervals Google recommends at https://developers.google.com/android/reference/com/google/android/gms/location/LocationRequest. Correct?

@grzesiek2010
Copy link
Member

I would prefer location-mode than location-priority since it seems more human-friendly, and that's all.
Both location-interval and location-age should be optional.

I think we can use default intervals recommended by Google and agree on default location-age for each mode based on intervals.

Of course, as I said it's should be possible to pass different values but we shouldn't require that or encourage people. It should be just an option for cases difficult to imagine now which obviously exist.

@yanokwa
Copy link
Member Author

yanokwa commented Dec 12, 2018

I don't understand about why you'd need both mode and interval. If you say interval isn't reliable, why include it at all?

As to age, it seems that should be required because how else does a form designer ensure that a point is recent? Am I missing something?

@grzesiek2010
Copy link
Member

I don't understand about why you'd need both mode and interval. If you say interval isn't reliable, why include it at all?

For example, default interval for PRIORITY_HIGH_ACCURACY is 5s but someone might want to use
PRIORITY_HIGH_ACCURACY but with different interval let's say 30s because he knows his workers are not able to move so quickly so 30s would be better for battery saving.

As to age, it seems that should be required because how else does a form designer ensure that a point is recent? Am I missing something?

I said that we should agree on default location-ages as well, for example as above:
the default interval for PRIORITY_HIGH_ACCURACY is 5s so the default location-age could be let's say 20s to collect ~4 points and select the best one.

If someone wants to collect more points he might pass a different value (bigger) and if someone wants to log the last point (always) he should pass a value <= default location-interval.

That's why we should allow users to add different values for both (location-interval and location-age)

@yanokwa
Copy link
Member Author

yanokwa commented Dec 13, 2018

I've updated the specification with your feedback @grzesiek2010. I think the meat of this is done, so I think you should start implementing it in Collect. If you can split things in separate PRs to make it easier to review, please do.

@adamvert @smoyth @zestyping @aurdipas Any final feedback?

@aurdipas
Copy link
Member

looks very fine to me. I agree on start implementation.

@admbtlr
Copy link

admbtlr commented Dec 13, 2018

LGTM!

@grzesiek2010
Copy link
Member

grzesiek2010 commented Dec 13, 2018

I was thinking about default values we should agree on. Google recomends values for some of modes but I think they are not perfect for us.

Proposed default values:

location-mode location-interval location-age description
high_accuracy 10s 40s This will return the finest location available
balanced 30s 2m Block level accuracy is considered to be about 100 meter accuracy. Using a coarse accuracy such as this often consumes less power
low_power 1m 4m City level accuracy is considered to be about 10km accuracy. Using a coarse accuracy such as this often consumes less power
no_power 1m 4m No locations will be returned unless a different client has requested location updates in which case this request will act as a passive listener to those locations

What do you think about those values.

A user would have to select the mode (depending on his needs and devices he use) and additionaly would have a possibility to set different location-interval and location-age if default values are not good for him.

@danbjoseph
Copy link
Member

what's the difference between low_power and no_power? (values are the same?)

@grzesiek2010
Copy link
Member

grzesiek2010 commented Dec 14, 2018

As in the description:
low_power uses only a coarse accuracy but no_power doesn't record any location on itself at all, it just uses a location if recorded for example by other apps.
I don't think that last one would be really used but if it exists let's implement it as well.

@danbjoseph
Copy link
Member

i was confused by a user being able to set values to something other than the default, but then one of the default settings seeming to have details set by something other than the adjustable values.

@zestyping
Copy link

It sounds like in @grzesiek2010's proposal there is a third hidden parameter that controls how the location is sensed (GPS vs. wifi vs. passive).

@yanokwa
Copy link
Member Author

yanokwa commented Dec 17, 2018

@zestyping To be precise, the FusedProvider lets us set priority (what we call mode). The priority is...

a strong hint to the LocationClient for which location sources to use. For example, PRIORITY_HIGH_ACCURACY is more likely to use GPS, and PRIORITY_BALANCED_POWER_ACCURACY is more likely to use WIFI & Cell tower positioning, but it also depends on many other factors (such as which sources are available) and is implementation dependent.
https://developers.google.com/android/reference/com/google/android/gms/location/LocationRequest.html#setPriority(int)

The FusedProvider also lets us set an interval.

The location client will actively try to obtain location updates for your application at this interval, so it has a direct influence on the amount of power used by your application. Choose your interval wisely. This interval is inexact. You may not receive updates at all (if no location sources are available), or you may receive them slower than requested. You may also receive them faster than requested (if other applications are requesting location at a faster interval).
https://developers.google.com/android/reference/com/google/android/gms/location/LocationRequest.html#setInterval(long)

So, @danbjoseph, if a user sets a mode alone, then we are proposing to set what we think is a good interval and age for our use-case. The age isn't used by Android at all, it's used by Collect to put a limit on how old of a location we should consider writing to the log.

@grzesiek2010 maybe we should call it priority since we're are adding a level of abstraction that isn't really needed and now that I'm trying to explain it, it doesn't sound that user-friendly.

Also, I'm having a really hard time deciding what would be good defaults. How did you decide 40 secs for high_accuracy? Why not 30 seconds? Maybe we should recommend defaults (e.g., in the docs), but not encode them. Encoding them locks a particular version of pyxform/Collect to some magic value and that seems dangerous.

@grzesiek2010
Copy link
Member

maybe we should call it priority since we're are adding a level of abstraction that isn't really needed

you mean location-mode -> loction-priority?

How did you decide 40 secs for high_accuracy? Why not 30 seconds?

location-age in all cases is 4x location-interval in order to collect ~4 points, I thought it's a good value as I said above intervals are not equal so sometimes it will be 4 points sometimes 3 and maybe 5. If we decide to use let's say 3x location-interval it will be ~3 points so sometimes only 2.

@grzesiek2010
Copy link
Member

Ok, when it comes to location-mode -> loction-priority I agree since that priority word is used in the doc.

@yanokwa
We were talking about default values and you asked me to star the implementation without them and then we can add it in the future. Does that mean if one parameter is not set it shouldn't work at all?

@yanokwa
Copy link
Member Author

yanokwa commented Dec 19, 2018

@grzesiek2010 Correct. All three params are required.

@grzesiek2010
Copy link
Member

I have a problem with the dialog we display on the first load.
The main problem is that we are nor able to revoke permissions/disable location provider programmatically we could just open settings as we do in GeoWidgets and give a user an opportunity to change settings (the same with permissions should be possible).

But still I don't like it... let's imagine a case:

We have a device with Android 6 (permissions not granted and location provider disabled) and our user wants to collect location coordinates.
Then on the first load, we display a dialog:

Paragraph 1: This form continuously collects your location in the background and includes it with the form data.
Paragraph 2: Revoke Collect's location permissions to fill the form without any location data.
Buttons: OK | Revoke location permissions

What doesn't look well itself because the message is just about revoking/disabling and it's already revoked/disabled.

As I said our user wants to collect location coordinates. He can click on Revoke location permissions what is super not intuitive and enable permissions in settings but location provider is still disabled so it's not enough...

I think that the first dialog should just contain a message that we collect locations but after closing it a user should be asked for enabling location provider and granting permissions anyway.

What do you think?

@grzesiek2010
Copy link
Member

grzesiek2010 commented Jan 2, 2019

There was no answer to my question so I decided to go ahead. Here is the pr getodk/collect#2772

As I said I had a problem with Data privacy issues so I implemented it in a bit different way:
When the form is started we check if Google Play Services are available https://github.com/opendatakit/collect/pull/2772/files#diff-03ff45091550285dd88b56927010c457R2459
if not, a Snackbar is displayed (and an event is logged).
If everything is ok we check if location permission is granted: https://github.com/opendatakit/collect/pull/2772/files#diff-03ff45091550285dd88b56927010c457R2460
if not we display a request and log an event if denied.
After that, we check if GPS is enabled: https://github.com/opendatakit/collect/pull/2772/files#diff-03ff45091550285dd88b56927010c457R2464
if it's enabled a Snackbar which says:

This form continuously collects your location in the background and includes it with the form data. Disable GPS or revoke location permissions to fill the form without any location data.

is displayed (There is an option to go to settings).

if it's disabled a dialog is displayed and there is also an option to go to settings (an event is logged as well here).

Testing would be appreciated 😄

@yanokwa
Copy link
Member Author

yanokwa commented Jan 2, 2019

@grzesiek2010 Thanks for your work so far. These features always seem to get more complicated as we dig into them!

I think the high-level goal here should be to inform users what is happening and give them a chance to bypass. What if we add a '(Enable/Disable) background location' in the form entry activity menu?

Then on first launch, we show this dialog:

This form continuously collects your location in the background and includes it with the form data.
OK | Disable background location

If you say OK, we pop two dialogs and ask for permission/providers. We also set a form-specific location allowed toggle.

If you say Disable, we don't ask for location permissions or enabling the location provider. We show this snackbar.

This form needs to continuously collect your location. You can enable this feature in the menu.

On second launch, we need the toggle, permissions, and the provider. If you have all three, we show this snackbar:

This form collects your location continuously. You may disable this feature in the menu.

If you don't have all three, then we show this snackbar.

This form needs to continuously collect your location, but you have disabled it. You may enable this feature in the menu.

And when you go to the menu, and select 'Enable background location' we pop the two dialogs if necessary and ask for the permissions.

Does that sound reasonable?

@grzesiek2010
Copy link
Member

Generally, it sounds good but I'm not sure whether adding a new option to the form entry activity menu is a good solution since there might be not much space... don't you like the solution with a new option in General Settings -> Form management (in Form filling section)?

@yanokwa
Copy link
Member Author

yanokwa commented Jan 3, 2019

Adding it in the General Settings makes it impossible to toggle on a form by form basis.

@grzesiek2010
Copy link
Member

grzesiek2010 commented Jan 3, 2019

Ok, I'm going to follow your approach but have some concerns:

1. ..............................................................................................................................

Then on first launch, we show this dialog:

This form continuously collects your location in the background and includes it with the form data.
OK | Disable background location

Are you sure we need that Disable background location button? Maybe it should be the same message like we have in a Snackbar:

This form collects your location continuously. You may disable this feature in the menu.

and that's all.

2. ..............................................................................................................................

if on first launch the option is disabled displaying the same dialog doesn't make sense. Should we then display a dialog which says eg:

This form wants to collect your location in the background and includes it with the form data but the background location option is disabled. You may enable this feature in the menu.

3. ..............................................................................................................................

If you don't have all three, then we show this snackbar.

This form needs to continuously collect your location, but you have disabled it. You may enable this feature in the menu.

This doesn't make sense to me. I think we should ask for permissions and enabling providers everytime the background location option is enabled and display appropriate snackbars if something is not granted/enabled. Your proposal message indicates that it's possible to enable all three options (the main option/permissions/location providers) just clicking once in the menu what's not possible of course.

@yanokwa
Copy link
Member Author

yanokwa commented Jan 4, 2019

I agree with you suggestions. And to make sure I've got it correct...

On first launch or if location has been allowed, we check for permissions and provider enabled. If we don't have permissions/provider, we ask in dialogs. We display snack bars if the user says no in the dialogs.

Then we show snackbars to inform users about what is happening.

If you have have enabled background location.

This form collects your location continuously in the background. You may disable this feature in the menu.

If you don't have background location enabled

This form needs to collect your location in the background, but the background location option is disabled. You may enable this feature in the menu.

Then in the menu we have the form specific toggle.

What happens if you switch from Collect to the Android settings, revoke permissions, and come back to Collect? Do we show the dialogs again? Show the snackbars again?

@grzesiek2010
Copy link
Member

grzesiek2010 commented Jan 4, 2019

Sound much better now, Thanks!

On first launch or if location has been allowed, we check for permissions and provider enabled.

I wonder if we need that first launch check at all. I think we should ask for permissions/providers everytime it's needed when a form is started (no matter if it's the first launch) or the background location option is changed (enabled).

My scenario would be:
Case 1: A user opens a form which needs to collect location in the background.

  1. Check if Google Play Services are available, if not display a Snackbar:

This form needs to collect your location in the background, but it's not possible because Google
Play Services are not available.

and log an event: GOOGLE_PLAY_SERVICES_NOT_AVAILABLE

otherwise, go to the next step...

  1. Check if the background location option is enabled, if not display a Snackbar:

This form needs to collect your location in the background, but the background location option is disabled. You may enable this feature in the menu.

and log an event: BACKGROUND_LOCATION_DISABLED

otherwise, go to the next step...

  1. Check if location permissions are granted, if not display a request, if not granted log an event:
    LOCATION_PERMISSIONS_NOT_GRANTED

otherwise, go to the next step...

  1. Check if location providers are enabled, if not display a Dialog:

This form needs to collect your location in the background, but location providers are disabled. You may enable this feature in the settings.

with an option Go to settings.

otherwise, show a Snackbar:

This form collects your location continuously in the background. You may disable this feature in the menu.

and log location coordinates!

Case 2: A form is started and a user disables background location
just stop the location client and log an event BACKGROUND_LOCATION_DISABLED

Case 3: A form is started and a user enables background location
every step like in the Case 1

so as I mentioned above I don't see any reason for using that firstLaunch check.

Do you agree?

......................................................................................................................................................

What happens if you switch from Collect to the Android settings, revoke permissions, and come back to Collect? Do we show the dialogs again? Show the snackbars again?

I don't think we need that.

@yanokwa
Copy link
Member Author

yanokwa commented Jan 4, 2019

Case 1: I like the sequencing! Note that you'll need to also show a snackbar if you have all the permissions/providers to inform user that their location is being collected.

Case 2: Works for me.

Case 3: Works for me.

The case where you switch from Collect to Android and revoke permissions, you'll need to add that in the log, right? Seems like we should at least throw up a snackbar in addition when the location permissions/providers changes during data collection. What do you think?

@grzesiek2010
Copy link
Member

Note that you'll need to also show a snackbar if you have all the permissions/providers to inform user that their location is being collected.

sure! updated.

The case where you switch from Collect to Android and revoke permissions, you'll need to add that in the log, right?

yes, would be good to have such a log.

Seems like we should at least throw up a snackbar in addition when the location permissions/providers changes during data collection. What do you think?

I'm not sure. I'll be thinking about it finalizing the implementation. Logging an event is more important that's for sure because it's a note for someone who analyzes saved forms but maybe...

Great, we finally agreed on it!

@yanokwa yanokwa moved this from Under consideration to Spec Approved in Roadmap Jan 14, 2019
@yanokwa yanokwa moved this from Spec Approved to In progress in Roadmap Jan 14, 2019
@yanokwa yanokwa moved this from In progress to Done in Roadmap Feb 20, 2019
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
No open projects
Development

No branches or pull requests

7 participants