Deprecate C Headers

[CursedRock17]

This pull request is an attempt to help #259, specifically the first task

Rename all .h -> .hpp files. This will have to also have a deprecation warning, and should be done post-Foxy.

Since it's now post-Foxy I did something similar to this message_filters pull request, which was to simply deprecate a .h file with a warning, include the .hpp file, and just copy paste the logic from the .h file into the .hpp file. There were some slight formatting changes that had to be made to make cpplint and uncrustify happy. There is currently a copyrighting problem within the LinearMath files since they seem to be from the bullet library. I would like to know where we should move from this point when it comes to those files.

[ahcorde]

copyright is failing

https://build.ros2.org/job/Rpr__geometry2__ubuntu_noble_amd64/123/testReport/junit/(root)/projectroot/copyright/

[ahcorde]

you should keep here both the h and the hpp

[clalancette]

you should keep here both the h and the hpp

Agreed.

[clalancette]

Besides some of @ahcorde 's inline comments, we will also need to have PRs to the rest of the ROS 2 core to use the .hpp files instead of the .h ones. Also, this is a large enough change that once we are ready to merge it, we should probably send a message to https://discourse.ros.org letting people know that we've done this.

[CursedRock17]

Yeah I figured we would have to extend PRs to various libraries, I have one linked from rviz, but I haven't found anymore, I'll have to do some digging around to find out if I've missed any more of the core ones. As far as the copyrighting goes, I believe those inline comments were the missing piece, I've submitted a fix.

[CursedRock17]

Updated downstream packages:

• Fix Deprecated tf2 headers rviz#1289
• Deprecating tf2 C Headers ros2_documentation#4828
• Deprecating tf2 C Headers cartographer_ros#75
• Deprecating tf2 C Headers ros/geometry#244
• Deprecating tf2 C Headers ros/geometry2#573
• Deprecate tf2 C Headers ros/geometry_tutorials#87
• Deprecate c headers ros-perception/imu_pipeline#25
• Deprecating tf2 C Headers ros-perception/laser_geometry#98
• Deprecate c headers ros-perception/laser_filters#206
• Deprecating tf2 C Headers ros-perception/image_pipeline#1039
• Deprecating tf2 C Headers ros-perception/perception_pcl#469
• Deprecating tf2 C Headers ros-controls/ros2_controllers#1325
• Deprecating tf2 C Headers ros-navigation/navigation2#4732
• Deprecate tf2 C Headers ros-industrial/industrial_training#420

These were all the repositories that I could find, within the core ROS organizations, in which tf2 was utilized. There are likely thousands of repositories which are going to have warnings, which are warnings as to not break API, but I certainly wouldn't mind making a general PSA.

[Timple]

Would it make sense to provide .hpp files for all active distros? With as only content the include of the .h file.

This would mean all projects that do not have a branch per release can use this syntax already. Preventing ugly statements like this everywhere: https://github.com/autowarefoundation/autoware.universe/blob/9bd0f77c255edcff129ff08d190a5fdd8c45b6dc/localization/yabloc/yabloc_common/src/pub_sub.cpp#L18

For rolling we can keep the deprecation flag, not for older distros.

[CursedRock17]

Yeah I don't see much of a compile downside, just a weirder file structure for older distributions. I assume we would just never phase out the .h files unlike the rolling branch in which we can remove them in L-Turtle. I would be able to add this "backport" later today if that's something we agree on.

Also there already seems to be some consensus in the other core repositories:

• Deprecating tf2 C Headers ros-perception/perception_pcl#469

we should create backwards compatibilty headers that emit compile warnings so users have time to migrate to the new headers.

• Deprecating tf2 C Headers ros-controls/ros2_controllers#1325

Can you add conditioning in the code so that from rolling on it uses the .hpp, and in the prior versions it uses the .h version?

[CursedRock17]

The following pull requests are the "backports" that will prevent gross include macros with the formulating of .hpp files for ROS' non-EOL distributions. Do I need to go back any further, or will these be alright for now?

• Humble "Backport" of #720 #722
• Jazzy "Backport" of #720 #723
• Iron Backport of #720 #724

[Timple]

Iron I guess?

[clalancette]

There is one major change I'd like to see in this PR, and that is to not make the linting updates to the tf2/include/tf2/LinearMath headers. While I think we may eventually want to do this, for now I think it will be easier to track provenance without doing that.

Once that is fixed, and we run CI on this and the downstream PRs, I think we can get this in.

[CursedRock17]

Changes have been implemented, that being said was it not just easier to do this now opposed to later, especially when naming conventions are going to be mixed?
Also I'll begin making necessary changes downstream.

[clalancette]

Changes have been implemented, that being said was it not just easier to do this now opposed to later, especially when naming conventions are going to be mixed?
Also I'll begin making necessary changes downstream.

No, sorry, that's not what I meant. I meant leave the .hpp files, but do not do things like remove the trailing whitespace.

[CursedRock17]

Ohhh, I was quite confused lol, I will be reverting the changes in addition to the whitespace changes.

[Ryanf55]

Iron I guess?

Can you do humble if it's not too much work?

[clalancette]

All right, one more relatively small thing to fix, then I think this will be ready for CI.

[clalancette]

This particular file is actually C code, and mostly internal, so I think we do not need to rename this. That is, keep just this file as visibility_control.h.

[CursedRock17]

When I get back to my workstation later today, I can roll these improvements (whitespacing & visibility_control) onto the other branches I adjusted for the backport.

[clalancette]

Pulls: #720, ros2/rviz#1289, ros-perception/laser_geometry#98
Gist: https://gist.githubusercontent.com/clalancette/b83127091186f2489c09c4c787585861/raw/3c16fd56b4d645d09b9cadf800da79cbfd1acbaf/ros2.repos
BUILD args:
TEST args:
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14792

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[clalancette]

So there are at least two problems here:

1. We need a PR to interactive_markers to deal with the change.
2. Windows doesn't seem to like the #warning construct, so we may have to do something special for Windows there. Maybe something like https://github.com/ros2/common_interfaces/blob/473511f5807610bb8ed7744dfd2986dba4601fa6/sensor_msgs/include/sensor_msgs/point_cloud_conversion.hpp#L41-L49

[CursedRock17]

Hopefully windows likes the current implemention. Also backport PRs are ready to go as well.

[ahcorde]

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[ahcorde]

There are still some warnings

[CursedRock17]

I think it's just the repos being built, I changed up interactive_markers which seems to be the repository throwing out some warnings as it's still on the rolling branch. My updated branch stops using the .h files that are the only ones triggering the warnings, which should be ok.