-
Notifications
You must be signed in to change notification settings - Fork 2
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
Add option to convert position into Area ID #197
Conversation
One last change added to error check the latitude/longitude values, but ended up pulling in anyhow |
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 the more i think about this with the offset & the call to trunc, I am worried about if this will exactly match the C code. At 54_000_000
you've actually ran out of bits to describe integer intervals in single precision, and the choice to do the math in floating point then cast to u32 may not be the same as casting to u32 then doing the math.
I'd have to see the ground truth of the C code, to know if this matches but I added a suggestion.
For what it's worth, if you did this all in double precision the numbers would be able to describe exact integers at those sizes of numbers but you made the change to f32 explicitly to match the other implementation.
I don't have context to the importance of getting this exactly right for all inputs, so if this is just navel gazing, feel free to just ignore my comment and merge away.
Co-authored-by: Patrick Crumley <[email protected]>
This adds another command line flag to automatically convert the given position into an Area ID (
--pos-to-area-id
). If either this new flag or the--area-id
argument are given then the CRA message with Area ID is sent. I also changed several of the uses off64
to usef32
instead to make sure there weren't rounding issues in the Area ID calculations.Is this an acceptable way to squeeze in this functionality? I was also thinking about adding these functions into https://github.com/swift-nav/swiftnav-rs, but that's not a dependency for ntripping and would potentially complicate the portability since it pulls in C code as well.