Skip to content
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

Clarification for GPSFix message "dip" field #62

Open
rgov opened this issue Jun 25, 2022 · 4 comments
Open

Clarification for GPSFix message "dip" field #62

rgov opened this issue Jun 25, 2022 · 4 comments

Comments

@rgov
Copy link

rgov commented Jun 25, 2022

The GPSFix message has the fields pitch, roll, dip and corresponding uncertainty fields (err_pitch, etc.).

These fields do not appear populated anywhere by the code in this repository.

These are non-standard names for what I think are supposed to be Euler angles (pitch, roll, yaw). This is especially confusing since "dip" is almost a synonym for "pitch" or maybe "roll".

This is called out in at least one other place.

Can this be documented and these fields be renamed or removed in a future version of ROS?

@danthony06
Copy link
Collaborator

The GPSFix is a holdover from the original developers of this repo, and I'm not sure what their rationale for some of these fields was. I think they may have gotten dip mixed up with heading from the GPSD documentation: https://gpsd.gitlab.io/gpsd/gpsd_json.html. In reality, I think we'd want to call it course over ground, since this message is meant to be specific to GPS/GNSS devices.

I'm guessing most people are using the NavSatFix message. I'm pretty hesitant to modify the GPSFIx message at this point, because the people who are using it probably have long since worked around its quirks, and I'd hate to break compatibility between systems. Instead, I think it makes sense to create a GPSFix2.msg, or maybe GNSSFIx.msg, and publish that in parallel, or have the user choose which one they want. We could then fix these field names and make sure everything is getting populated.

What do you think about that plan?

@daniel-dsouza
Copy link

Instead, I think it makes sense to create a GPSFix2.msg

I'd support an updated GPSFix2.msg

@rgov
Copy link
Author

rgov commented Jun 10, 2024

I don't think it would be good for the ROS ecosystem if messages are versioned like this. It adds a lot of complexity, like having to indefinitely publish the old style of message for backwards compatibility.

Probably the better way to do it is make it a breaking change for whatever the next ROS distro is (Jazzy+1) as soon as possible and then let developers of other packages that use GPSFix discover the incompatibility through breakage. You could leave a comment in the file about what changed so it's easy to find.

The basic point of doing a major distro release is to create an opportunity to make breaking changes.

@danthony06
Copy link
Collaborator

I agree. I've been working on an alternative implementation for ROS 2 that defines a GNSS message that exposes a lot of the information that can come from GPSd, while publishing the old messages as an option. I'll try to get it out in the near future.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

3 participants