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

Create GNSSFix and GNSSStatus messages #63

Open
danthony06 opened this issue Jun 27, 2022 · 4 comments
Open

Create GNSSFix and GNSSStatus messages #63

danthony06 opened this issue Jun 27, 2022 · 4 comments

Comments

@danthony06
Copy link
Collaborator

As pointed out in #62, many of the fields in the GPSFix and GPSStatus are not populated, poorly named, or have other problems. We don't want to break compatibility, so instead we should create new messages that better represent the data stream we're getting from GPSD.

@danthony06
Copy link
Collaborator Author

@rgov What do you think of these two messages as replacements for the GPSFix and GPSStatus message?

https://github.com/danthony06/gps_umd/blob/new_messages/gps_msgs/msg/GNSSFix.msg
https://github.com/danthony06/gps_umd/blob/new_messages/gps_msgs/msg/GNSSStatus.msg

@rgov
Copy link

rgov commented Jun 29, 2022

It looks reasonable though I don't have a strong background or opinion in this area. I think if you're following gpsd's lead, which is what populates these fields in most cases, you can't do much better.

There's at least one typo I spotted, I think it was 'Consants'.

I don't know what ROS's general policy is about modifying message structures after they're published. I suppose you could do so aligned with a particular ROS release if you made your CMake choose a file conditional on the ROS version (or just had a separate branch with different definitions).

So your solution of creating a GNSSFix sidesteps this issue but complicates things further -- you'd have to publish three messages now, and make clear that users shouldn't be using GPSFix for new software, etc.

For the health of the ecosystem I would prefer a more opinionated stance — try to figure out which packages in the ROS package index rely on GPSFix messages, file issues with them that the message definition is changing but the most common fields stay the same (or which common ones are changing), and then drop support for the weird fields.

@rgov
Copy link

rgov commented Jun 29, 2022

(Not saying that you should bear the burden of that communication. Perhaps we could petition for OSRF for some tooling to make it easier to find packages based on their declared dependencies, and outputs a list of maintainer e-mails.)

@danthony06
Copy link
Collaborator Author

I opened an issue the other day when I was thinking about this. Currently, there's not a great way of notifying users' of pure message packages that something is deprecated or will be removed:

ros-infrastructure/bloom#679

Breaking message definitions is really tough in ROS, and is rarely done for established projects from what I've seen. I think PointCloud and PointCloud2 messages occurred because of a similar problem, and making a new message was PCL's workaround to the problem.

I'm thinking about just removing the old GPS messages in the ROS2 branch, since relatively few people are using ROS2 at this point, and it would be an easy point to make a change. Hopefully the ROS1 branch would then gracefully get deprecated, and the old GPS messages with it.

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

2 participants