ros2 / rcl_interfaces

A repository for messages and services used by the ROS client libraries
Apache License 2.0
38 stars 42 forks source link

Naming of fields for Time message is inconsistent with rclcpp::Time #166

Open YoshuaNava opened 4 months ago

YoshuaNava commented 4 months ago

I'm working with both builtin_interfaces/msg/Time and rclcpp::Time, and I found that the naming of the fields is inconsistent between them:

Would it be possible for both time representations to use the same field names? If possible I would suggest following the convention from rclcpp::Time, which uses full yet succint names.

Thank you in advance! :pray:

Yoshua

clalancette commented 4 months ago

I'm not a huge fan of making this change. While I agree it is annoying that they are inconsistent, neither is unclear. And to fix either of them, we'd have to break some code that uses it. So my view on this is that it is a "wontfix", but maybe @ros2/team has a different opinion.

YoshuaNava commented 4 months ago

@clalancette thank you for your prompt reply.

At the moment I'm writing utility code to perform parameter fetching and validation, and I had to write two different functions for rclcpp and builtin_interfaces Time. This immediately confused my project partners, who would be downstream users of the code I'm writing.

Can you give some background on what would you see as blocking for this change?

clalancette commented 4 months ago

Can you give some background on what would you see as blocking for this change?

Doing some simple, non-exhaustive looks through rolling right now, I see that there are 177 references to builtin_interfaces::msg::Time spread across 67 files. There are 3145 references to rclcpp::Time spread across 888 files.

If we make a change to either of them, then someone is going to have to go through every single one of those files and make sure that they work with the new change. And this only counts the packages that are released; there are thousands of packages that haven't been, which we'd be introducing breakage for.

While this is certainly possible to do, I really don't think it is worth the effort. As I said, in my opinion both are clear, even if they are unfortunately different.