Significant location changes / deferred location updates

MatthewGerber commented 8 years ago

Fixes #222.

Changes Proposed in this pull request:

Add ability to monitor significant location changes in iOS.
Add ability to defer location updates in iOS.

This fixes/implements:

[ ] Bug
[x] Feature Request

Which plugin does this impact:

[ ] Battery
[ ] Connectivity
[ ] Contacts
[ ] DeviceInfo
[ ] ExternalMaps
[x] Geolocator
[ ] Media
[ ] Permissions
[ ] Settings
[ ] Text To Speech
[ ] Vibrate
Other:

MatthewGerber commented 8 years ago

I haven't yet added significant location change monitoring, but I've added deferred location updates.

Am I on the right track given your coding approach?

MatthewGerber commented 8 years ago

Also, I'm on a Mac and can't load the UWP project, so the build checks are failing.

jamesmontemagno commented 8 years ago

So this is a tricky one and here are my thoughts:

DeferralSettings

I think that instead of expanding the parameters ( bool defersLocationUpdates = false, double deferralDistanceMeters = -1, double deferralTimeSeconds = -1)

Let's make this a class called Deferral Settings that you have to pass in and it defaults to null:

public class DeferLocationUpdateSettings
{
    public double DistanceInMeters {get;set;} = 100; // some default that is good
    public TimeSpan TimeBetweenUpdates {get; get;}  = TimeSpan.FromMintues(10); // or some default idk
}

I don't really think this is a setting that will be changed at all to be honest with you. I would remove the

public bool DefersLocationUpdates { get; set; }

and replace it with just a: DeferLocationUpdateSettings deferSettings;

We would set this when they pass it in.

And if they pass in the setting then we would enable it and then when they StopListening() we would turn it off and null it out.

Also reading the docs it says: This method requires GPS hardware to be available, CLLocationManager.DistanceFilter to be CLLocationDistance.None, and CLLocationManager.DesiredAccuracy be either CLLocation.AccuracyBest or CLLocation.AccuracyBestForNavigation.

We should just set that to be sure.

For UWP you will have to open up the source code file and modify the constructor after these changes to get it building in Appveyor

MatthewGerber commented 8 years ago

@jamesmontemagno I think I've successfully added both significant change listening and deferral of location updates. Can you take a look and publish a new Nuget if it looks good to you?

jamesmontemagno commented 8 years ago

@MatthewGerber I am getting ready to leave for a few days of holiday, but will take a peak and will release as a beta nuget first

jamesmontemagno commented 8 years ago

I left a few more comments in the code. I need to investigate it a bit more an do some refactoring and testing. If you can comment back.

My confusion is around: significant change listening and deferral of location updates. Is there a difference?

A few concerns I have is that there is a locationManager:didFinishDeferredUpdatesWithError that they say can get called if there is an error.

https://developer.apple.com/library/ios/documentation/UserExperience/Conceptual/LocationAwarenessPG/CoreLocation/CoreLocation.html is a good read through at the very bottom.

they also have this really odd clause: To prevent itself from calling the allowDeferredLocationUpdatesUntilTraveled:timeout: method multiple times, the delegate uses an internal property to track whether updates have already been deferred. It sets the property to YES in this method and sets it back to NO when deferred updates end.

MatthewGerber commented 8 years ago

@jamesmontemagno I added a note about the difference between significant change listening and update deferral. Also replied to the other comments.

As for locationManager:didFinishDeferredUpdatesWithError, the docs say that if you call allowDeferredLocationUpdatesUntilTraveled:timeout multiple times then the previous deferral will be canceled. I'm not sure why this would be bad though. I added a commit to do something similar to the example code.

MatthewGerber commented 8 years ago

@jamesmontemagno Have you had a chance to inspect the changes up through the following commit:

708f297ab45a22cb649c11758f7e6fcdd61db9ff

If you could release these changes as a beta Nuget I can help test them.

jamesmontemagno commented 8 years ago

Matthew,

I will be going through a lot of this tonight, it has been a busy time. Give me 24 hours to pull some stuff down and investigate.

Best, James

On Wed, Feb 24, 2016 at 1:52 PM, Matthew Gerber notifications@github.com wrote:

@jamesmontemagno https://github.com/jamesmontemagno Have you had a chance to inspect the changes up through the following commit:

708f297 https://github.com/jamesmontemagno/Xamarin.Plugins/commit/708f297ab45a22cb649c11758f7e6fcdd61db9ff

If you could release these changes as a beta Nuget I can help test them.

— Reply to this email directly or view it on GitHub https://github.com/jamesmontemagno/Xamarin.Plugins/pull/226#issuecomment-188468347 .

James Montemagno http://www.montemagno.com @jamesmontemagno http://www.twitter.com/jamesmontemagno

jamesmontemagno / Xamarin.Plugins

Significant location changes / deferred location updates #226

DeferralSettings