jamesmontemagno / GeolocatorPlugin

Geolocation plugin for Xamarin and Windows
MIT License
293 stars 158 forks source link

Include HasAltitude, HasBearing, HasSpeed, and HasAccuracy in Position #311

Closed jixer closed 3 years ago

jixer commented 5 years ago

Fixes # 310.

Changes Proposed in this pull request:

 

Implementation approach:

jixer commented 5 years ago

One additional note is that the previous value assignments for Speed, Heading, etc are unchanged with this PR. Therefore, there should be no breaking changes here and developers can migrate to use the new "Has" property booleans if and when they need them.

EDIT The value assignments for speed & bearing are different for iOS with the change to utilize >=0 rather than > -1. It is assumed that this won't result in a change in functionality, but the change better aligns our code with Apple's documentation

jixer commented 4 years ago

@jamesmontemagno This PR was closed without merge. Is there something I should do differently in my next PR? Hoping to knock out issue #311.

jamesmontemagno commented 4 years ago

May i also recommend you PR to github.com/xamarin/essentials :)

jamesmontemagno commented 4 years ago

oh wait, why did that happen....

jamesmontemagno commented 4 years ago

So, i am wondering... should we just make these nullable? instead of "HasX"

jixer commented 4 years ago

@jamesmontemagno Sorry for the delayed response on this. I didn't go the Nullable route in case we wanted to avoid a potentially breaking change (type mismatch) in people's code when they update (e.g., double myLat = position.Latitude). That being said, I was probably just being unnecessarily cautious and conceptually nullable is much more intuitive. I'll work on porting over.

gmarbury commented 3 years ago

@jamesmontemagno and @jixer as a suggestion you can use double.NaN if you are concerned with avoiding potential breaking changes with double? = null. We still have to check NaN too anyway. But what you have now looks very good. Please resolve the conflicts and get this checked in.