Respsect LocationRequest Parameters #146
Conversation
…hen reporting location
sarahsnow1
force-pushed
the
142-respsect-request-params
branch
from
b5aca0c to
50a9548
Compare
| private Map<PendingIntent, LocationRequest> intentToLocationRequests; | ||
| private Map<LocationCallback, LocationRequest> callbackToLocationRequests; | ||
| private Map<LocationRequest, Long> requestToLastReportedTime; | ||
| private Map<LocationRequest, Location> requestToLastReportedLocation; |
There was a problem hiding this comment.
Can we replace requestToLastReportedTime and requestToLastReportedLocation with a single private instance of ReportedChanges since that seems to encapsulate the same thing?
| return new ReportedChanges(updatedRequestToReportedTime, updatedRequestToReportedLocation); | ||
| } | ||
|
|
||
| abstract class Notifier<T> { |
There was a problem hiding this comment.
Why not make Notifier an interface since there is no implementation here?
There was a problem hiding this comment.
Good call, changed to an interface
There was a problem hiding this comment.
Overall changes look good. A couple of small comments inline. Were you able to test these changes in a demo app?
I'm thinking we should test the following use cases to ensure correct behavior:
-
Multiple location clients that each make a single location request with different fastest interval and smallest displacement values (similar to onLocationChanged is called from a LocationRequest of another fragment #142).
-
We should also test a single client that makes multiple location requests with varying interval and displacement values.
Updates from a single client/request seem still to be working fine so no issues there.
| } | ||
|
|
||
| abstract class Notifier<T> { | ||
| abstract void notify(LostApiClient client, T obj); |
There was a problem hiding this comment.
👍
I really like the use of generics to allows us to treat LocationListener, PendingIntent, and LocationCallback objects in a similar fashion when sending location updates.
|
Yep, I did test those things in a dummy app. I'll update our samples to include tests for these now. |
| private static final int LOCATION_PERMISSION_REQUEST = 1; | ||
|
|
||
| LostApiClient lostApiClient; | ||
| LostApiClient otherLostApiClient; |
There was a problem hiding this comment.
This is the single client example but there are still two clients here?
Given the multiple client activity/fragment example I think it would be more useful to test a single instance of the location client that makes multiple location requests here. What do you think?
Overview
This PR adds support for respecting
LocationRequest#getFastestInterval()andLocationRequest#getSmallestDisplacement()when deciding whether to notify listeners/pending intents/callbacks.Proposed Changes
Now listeners/pending intents/callbacks are invoked if the elapsed time between location updates is greater than/equal to the fastest interval and the distance between the last location reported and the current location is greater than/equal to the smallest displacement in meters.
Closes #142