-
Notifications
You must be signed in to change notification settings - Fork 24
Adding NED coordinate system #71
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
base: master
Are you sure you want to change the base?
Conversation
This adds the type of NED coordinates (standard for navigation and aerospace applications), transformations to and from ENU, and other transformations by composition through ENU
Added also a few tests and some cosmetic changes
|
Just want to say thanks for providing this. It's a handy reference for define my own conversion. |
anowacki
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.
All looks good bar a couple of textual things (ENU{T} in and '…extrema care must be taken…' in README.md). Consider reverting the new whitespace inserted in src/transformations.jl.
| ##### `ENU{T}` - east-north-up | ||
|
|
||
| The `ENU` type is a local Cartesian coordinate that encodes a point's distance | ||
| The `ENU` type stores local Cartesian coordinates that encode a point's distance |
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.
I think the old text is fine. Whether a coordinate is a single encoding of position or only one of the numbers which encodes position is an age-old debate.
| extreme care must be taken to choose an appropriate zone for the `UTM` methods. | ||
| (In the future, we implement the exact UTM transformation as a fallback — | ||
| contributions welcome!) | ||
| extreme care must be taken to choose an appropriate zone for the `UTM` methods |
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 original text is correct—the proposed change is typographically not.
| """ | ||
| ECEFfromUTM(zone, isnorth, datum) = ECEFfromLLA(datum) ∘ LLAfromUTM(zone, isnorth, datum) | ||
|
|
||
|
|
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.
No need to introduce extra whitespace in an unrelated PR.
|
Thank you @jleny for your contribution! Your approach of composed transformations makes sense to me. Overall it looks very good. Similarly to Claire, I 'do not love' the proliferation of conversions of things trivially computable from ENU by the user, but I accept that NED is much more common in some fields so I think it makes sense. I think I would be less encouraging for future PRs that add one or more of NWU, WSU, SEU, WND, SWD and ESD! |
As discussed in issue #69: adds NEDfromENU / ENUfromNED transformations and the other transformations by composition.