-
Notifications
You must be signed in to change notification settings - Fork 496
GH-400: Location improvements #414
GH-400: Location improvements #414
Conversation
merge master
xamarinGH-388 If activity is required and null throw null exception with inf…
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
|
Please note that this pr is not tested under iOS/UWP, as I did not have access to such devices at the point of creating this pr. It is on request of @Redth |
jamesmontemagno
left a comment
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Besides this can we just figure out what they name should be for heading... I think heading is confusing maybe cause it is different then how the geolocator stuff works for getting heading.
| Accuracy = location.HasAccuracy ? location.Accuracy : (float?)null | ||
| Accuracy = location.HasAccuracy ? location.Accuracy : default(float?), | ||
| Bearing = location.HasBearing && location.Bearing != 0.0d ? location.Bearing : default(double?), | ||
| Speed = location.HasSpeed && location.Speed == 0.0d ? location.Speed : default(double?) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Ternary operations like this scare me, Can you add additional () in there... Why do we need to check against 0.0d?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
so 0.0 means it's unavailable on android. We should probably check if it's nearly equal to 0 though instead because precision.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Like Math.Abs(location.Bearing) < 0.000001 or something.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
The exact null comparison works for me. It returns positive zero if no data is available. If you insist I will change this to use delta equals
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Other question on speed check to see if it is ==, should it be !=
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@jamesmontemagno whoops, good catch! changed equality and added braces to clarify operator precedence
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
|
| File | Status | Preview URL | Details |
|---|---|---|---|
| Details |
- Line 80: [Warning] Failed to run script W:\1bst-s_dependentPackages\ECMA2Yaml\tools\Run.ps1 with exit code -1
For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.
|
I also changed this to heading as suggested in the issue. Would you please also mark this as wip until someone gets to test this on uwp&iOS? |
|
| File | Status | Preview URL | Details |
|---|---|---|---|
| Details |
- Line 80: [Warning] Failed to run script W:\j0ye-s_dependentPackages\ECMA2Yaml\tools\Run.ps1 with exit code -1
For more details, please refer to the build report.
Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report.
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
✅ Validation status: passed
For more details, please refer to the build report. Note: If you changed an existing file name or deleted a file, broken links in other files to the deleted or renamed file are listed only in the full build report. |
* GH-400: Location improvements (#414) * Implemented maps as mentioned in branch name issue. also fixed a spelling mistake * Finished reset of fork * LocationBearing * removed requirements * Speed and bearing docs * braces & equality * Renamed Bearing to Heading * Heading in location * Update for Course and updated logic for UWP for NaN
Description of Change
Added location improvements as specified in #400
No unit tests as there are none up until this point
Bugs Fixed
None
API Changes
List all API changes here (or just put None), example:
Added:
IntegrationPoint: Location
public double? Speed { get; set; }
public double? Bearing { get; set; }
Changed:
Behavioral Changes
None
PR Checklist