-
Notifications
You must be signed in to change notification settings - Fork 0
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
GPS: Parsers and Unit Tests #176
base: main
Are you sure you want to change the base?
GPS: Parsers and Unit Tests #176
Conversation
906141c
to
ec23247
Compare
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.
This is looking really awesome!
A lot of convention changes.
One important, easy-to-implement change with the way we enable interrupts.
@DeflateAwning I've addressed all the PR comments except the one asking for some clarification. Let me know what you think and I can make some updates so you can continue with the PR review |
Changes required:
|
Just merged the EPS firmware into main; you'll have to merge main into this branch, or rebase this branch to main. Sorry about that. |
…ate the rtos thread
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.
As requested, it looks good overall! There's a few things that should be fixed, and this branch should be rebased to main (or main merged into this branch), as it's starting to diverge a sizeable amount.
If there's a specific section you'd like me to check over, let me know! Obviously can't thoroughly review 2000 LOC several times.
Thanks for the quick look. I will address the comments you have mentioned. I was hoping you would look into the |
GPS Testing Successful. Modifying the PR to address the testing results. I will break up this PR and my code into segments for future integration. |
Related Issues
Closes #16
Summary
This PR is the first implementation step for integrating the GNSS Receiver with the system. This includes receiving the ASCII response from the receiver, parsing the response, and passing the parsed responses to their appropriate structs for BESTXYZA and TIMEA log function response.
Changes Made
The changes made to realize this were as follows:
uart_handler
to receive the GNSS response in interrupt mode.gps.c
andgps_types.h
containing the functions, data structs, and enum definitions.unit_test_gps.c
file, which contains all the unit tests related to the GNSS receiver.LOG_message
function to handle a larger buffer for printing to UARTGEN_int64_to_str
that handles converting int64_t values into stringsTesting Instructions
I verified the accuracy of the GPS functions by running the
run_all_unit_tests
telecommand and ensured that all my unit tests passed. I have left myLOG_message
function calls in to show the error that is being invoked. This will be removed during the review to declutter the unit test call function with all the prints to UART.Special Notes for Your Reviewer(s)
There are some
TODO
tags within the code based on my current thoughts on the implementation I have done. Let me know if they are justified and we can discuss how to implement them and if there are other considerations I am missing.During the Hexagon Office Tour, some of the engineers questioned my implementation method of using ASCII instead of binary. They were in favor of using binary mode. @robotoshi had a more in-depth conversation with the engineers than I did, he could inform us what he was talking about. Furthermore, they mentioned that they had an open-source library called EDIE for ASCII and binary decoding of the GNSS receiver.
The link is here: https://novatel.com/support/bulletins-alerts-release-notices/hexagon-novatel-announces-version-3-00-of-its-encode-decode-interface-engine