-
Notifications
You must be signed in to change notification settings - Fork 3
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
Bugfixes and Improvement #2
base: master
Are you sure you want to change the base?
Conversation
Without having checked in-depth, I support changes 1. and 2. conceptually. I have my doubts concerning change 3., see my comment here: rock-drivers/drivers-gps_base#7 (comment) |
…e to gps_base::GPS_SOLUTION_TYPE according to discussion at rock-drivers/drivers-gps_base#7
Discussion at rock-drivers/drivers-gps_base#7 clarified the correct use of the GPS_SOLUTION_TYPE struct from gps_base. Applied the changes accordingly. |
I forgot about this since I don't work with an OmniStar anymore. I think that the dependency of this PR (rock-drivers/drivers-gps_base#7) got clarified and merged (I think it was a clarification in the documentation). This PR was adapted to the outcome of rock-drivers/drivers-gps_base#7. I think no one raised another blocking point... so yes, please re-review and consider merging. |
manifest.xml
Outdated
@@ -16,6 +16,6 @@ | |||
--> | |||
<depend package="base/types" /> | |||
<depend package="drivers/imu_an_spatial" /> | |||
<depend package="drivers/mb500" /> | |||
<depend package="geographic" /> | |||
<depend package="drivers/gps_base" /> |
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.
Should be drivers/orogen/gps_base
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.
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.
@Rauldg did the merge as you proposed
Fix orogen gps base dep
This actually has been lingering here ... I think for lack of a person feeling entitled to merge. I would suggest that one of you takes that role... |
This PR contains three changes:
gps_base
defines common types for GPS devicesmb500
shoud not be further used for that purpose.gnss_fix_omnistar
. In clarify the meaning of the solution types drivers-gps_base#7gps_base
is extended with the valueOMNISTAR
to reflect this. This component here is extended to set the value accordingly.Depends on rock-drivers/drivers-gps_base#7 DO NOT MERGE BEFORE