-
Notifications
You must be signed in to change notification settings - Fork 1.4k
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
Getting location updates using FusedLocationProvider #508
Comments
Yeah, this change can be done without updating play services. |
Ok, how about we start making incremental progress on #507 for the next release (~2 weeks) and then start the API change at the beginning of the next release cycle? That will give more time to identify any subtle behavior changes. As you refactor, one thing you may want to think about is providing an abstraction for all API calls as described by Martin Fowler here. |
This sounds very great, one of the features I'd like to see is the location being continuously captured while the FormEntryActivity is open. The rationale would be:
The key concern is presumably battery life. The timer log will probably only be enabled for a small percentage of forms, although that may change if it proves useful. Also for a form that has no location widgets and is not recording a time log then having a location capture running while data is being collected would seem to be wasteful. However maybe the FusedLocationProvider API is sufficiently efficient that this is not an issue. |
I can have a look at this one issue and I think there's a callback that's getting called when the location service detect location changes. |
@lognaturel Looking into this issue. Would be interested in taking it up! |
Very interesting article on the Android Developers Blog today - https://android-developers.googleblog.com/2017/06/reduce-friction-with-new-location-apis.html |
Hey everyone! I'm a new contributor working with the Nafundi folks—migrating the FusedLocationProvider is the first improvement we've decided to tackle. While looking at the existing Location code, I came up with six classes (all Activities) that will need to be updated from LocationManager to the Fused Location Provider. To accomplish this, we'll abstract the new code away into what we’ll call the “LocationClient”. This new class will keep the details of retrieving and tracking location updates out of the Activities and make it easier for us to update or migrate them again in the future. With the LocationClient in place, we’ll then move to migrating the Activities to use our new class instead of the system’s LocationManager. As a side benefit, this should simplify the Activities quite a bit. While we’re doing this we’ll also correct a number of minor issues we came across (mostly repeated/unnecessary statements, a couple of formatting errors, some inefficient string concatenation, etc.). Although testing the LocationClient itself is probably not worth it, we should consider adding some instrumentation tests to the Activities before performing the refactor/abstraction work so we can make sure they retain their intended behaviour—especially considering I’m not as personally familiar with them. At the very least I would recommend testing the Activities that return values via Intents. Here’s a quick recap of the specific changes we intend to make: LocationClient
GeoShapeGoogleMapActivity, GeoShapeOsmMapActivity, GeoTraceGoogleMapActivity, GeoTraceOsmMapActivity
GeoPointMapActivity, GeoPointOsmMapActivity
Instrumentation Tests
If anyone has any questions or suggestions just let me know. |
This sounds like a great plan. I love getting code out of Depending on what the |
Something just occurred to me. The geopoint widget without map is the most widely used of these location-based widgets. It currently does not require having Google Play Services installed and it would be nice to maintain that since we know that some users on older devices do not have Play Services installed. @jknightco I see that you did not explicitly mention any projected changes to |
@lognaturel ah not sure how GeoPointActivity got left out, it should definitely be included in the list. What do you think about hiding the LocationManager/Play Services decision inside of of LocationClient? LC can check if Play Services are available and start up the FusedLocationProvider if they are, or LocationManager if they're not. |
@lognaturel draft of the dual LocationClients here: https://github.com/PembrokeStudio/collect/tree/issue-508-locationupdate/collect_app/src/main/java/org/odk/collect/android/location LocationClient is the shared interface, AndroidLocationClient uses LocationManager, and GoogleLocationClient uses Play Services. LocationClients provides a static method for retrieving the correct implementation. None of these have been tested yet but they shouldn't need significant changes from how they are now. I believe the provided methods should answer all of our existing use cases but I'll have to double check. Let me know what you think! |
I only took a quick look but it is looking fantastic! The fallback behavior seems great and in fact is relevant for the OpenStreetMap versions of the map-based widgets as well as for the map-less geopoint so it's really good to have in there (the Google ones need Play Services for the map). |
Awesome, thanks Hélène. First thing next week I'll add tests and then move
on to modifying the Activities.
…On Fri, Jul 21, 2017 at 10:58 PM Hélène Martin ***@***.***> wrote:
I only took a quick look but it is looking fantastic! The fallback
behavior seems great and in fact is relevant for the OpenStreetMap versions
of the map-based widgets as well as for the map-less geopoint so it's
really good to have in there (the Google ones need Play Services for the
map).
—
You are receiving this because you were mentioned.
Reply to this email directly, view it on GitHub
<#508 (comment)>,
or mute the thread
<https://github.com/notifications/unsubscribe-auth/ACWrAWWuCfz8WWx6n90nk4np6kEcWoj3ks5sQRDjgaJpZM4MTL4h>
.
|
Closed in #1336 |
Problem description
For getting the location, FusedLocationProvider API would be a better idea, given that Google recommends using it.
FusedLocationProvider API uses all available location data to give as accurate of a location as possible, as quickly as possible, with as little power consumption as possible. It has a different API, though one that is similar in some respects to the LocationManager API. It was released as a part of Play Services some 4 years ago.
I propose that we move to this. The transition won't be very steep.
The text was updated successfully, but these errors were encountered: