Skip to content

Change tf2_ros C to C++ headers #805

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.

Sign up for GitHub

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

Jump to bottom
Merged
ahcorde merged 3 commits into from
Jul 18, 2025
Merged

Change tf2_ros C to C++ headers #805

ahcorde merged 3 commits into from
Jul 18, 2025

Conversation

@garyservin
Copy link
Member

@garyservin garyservin commented Jul 11, 2025

Description

Similar to #720, this PR moves the .h files to .hpp, while keeping a message to users about the deprecated headers.

Fixes #746

@ahcorde
Copy link
Contributor

ahcorde commented Jul 14, 2025

Pulls: #805
Gist: https://gist.githubusercontent.com/ahcorde/17e1ee53652f4cd2c0fd37cd45352c5c/raw/1a757d665d704f138a588a37feffd4915b53edac/ros2.repos
BUILD args: --packages-above-and-dependencies test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
TEST args: --packages-above test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16486

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@garyservin
Copy link
Member Author

garyservin commented Jul 14, 2025

Should I also fix the order of the includes in this PR?

The other failing tests is because this line is too long. Not sure what the process is in these cases, I could rename the define to make it shorted or ignore the linting for that line?

Thanks!

ahcorde
ahcorde requested changes Jul 14, 2025
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Should I also fix the order of the includes in this PR?

The other failing tests is because this line is too long. Not sure what the process is in these cases, I could rename the define to make it shorted or ignore the linting for that line?

Thanks!

Yes, reorder the includes according with cpplint

@garyservin garyservin requested a review from ahcorde July 14, 2025 15:11
ahcorde
ahcorde requested changes Jul 15, 2025
View reviewed changes
Copy link
Contributor

@ahcorde ahcorde left a comment •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I can here some more deprecation warnings too https://ci.ros2.org/job/ci_linux/24290/clang-tidy/

@garyservin garyservin requested a review from ahcorde July 15, 2025 13:44
@ahcorde
Copy link
Contributor

ahcorde commented Jul 16, 2025

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

This was referenced Jul 17, 2025
Merged
Merged
Merged
@ahcorde
Copy link
Contributor

ahcorde commented Jul 17, 2025

Related PRs

@ahcorde
Copy link
Contributor

ahcorde commented Jul 17, 2025 •
edited
Loading

Pulls: #805, ros/robot_state_publisher#235, ros2/rviz#1529
Gist: https://gist.githubusercontent.com/ahcorde/fa4b25236bdc5813555fdcc87cf64953/raw/07ef33e1747e6de473d68d43300776fd32de3305/ros2.repos
BUILD args: --packages-above-and-dependencies test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
TEST args: --packages-above test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16514

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@garyservin garyservin mentioned this pull request Jul 17, 2025
Merged
@ahcorde
Copy link
Contributor

ahcorde commented Jul 17, 2025 •
edited
Loading

Pulls: #805, ros/robot_state_publisher#235, ros2/rviz#1529
Gist: https://gist.githubusercontent.com/ahcorde/0d6bdc46ee2dbe5ca020f149e2cfac5f/raw/07ef33e1747e6de473d68d43300776fd32de3305/ros2.repos
BUILD args: --packages-above-and-dependencies test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
TEST args: --packages-above test_tf2 tf2_bullet tf2_eigen tf2_geometry_msgs tf2_kdl tf2_ros tf2_sensor_msgs
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/16518

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde merged commit a99d88b into ros2:rolling Jul 18, 2025
2 checks passed
@Timple
Copy link
Contributor

Timple commented Jul 18, 2025

Will this also be backported without the deprecation message?

Such that packages don't have to do the whole " if ROS_DISTRO" cmake logic it they support multiple distros on a single branch?

@ahcorde
Copy link
Contributor

ahcorde commented Jul 18, 2025

@Timple we have done this in the past for other packages, happy to review the PR

@garyservin
Copy link
Member Author

garyservin commented Jul 18, 2025

Thanks for merging @ahcorde ! One question, would it make sense to backport these changes to jazzy and kilted as well?

@garyservin garyservin mentioned this pull request Jul 18, 2025
Merged
@ahcorde
Copy link
Contributor

ahcorde commented Jul 18, 2025 •
edited
Loading

Thanks for merging @ahcorde ! One question, would it make sense to backport these changes to jazzy and kilted as well?

As @Timple mentioned, we can backport these changes without the deprecation

@mini-1235 mini-1235 mentioned this pull request Jul 22, 2025
Merged
8 tasks
@garyservin garyservin mentioned this pull request Jul 23, 2025
Merged
@mergify mergify bot mentioned this pull request Jul 24, 2025
Merged
@Sushant-Chavan Sushant-Chavan mentioned this pull request Aug 5, 2025
Closed
8 tasks
@Timple
Copy link
Contributor

Timple commented Aug 18, 2025

Seems like @garyservin already did the honors:
#807

@yukkysaito yukkysaito mentioned this pull request Aug 18, 2025
Merged
@nbbrooks nbbrooks mentioned this pull request Sep 8, 2025
Merged
6 tasks
nbbrooks added a commit to moveit/moveit2 that referenced this pull request Oct 12, 2025
@nbbrooks @mosfet80
Fix deprecations in image_common and tf2_ros (#3567)
65aa94c 
* Fix image_common NodeT deprecation warnings from ros-perception/image_common#352 - migrate to NodeInterfaces

* Fix image_common rmw_qos_profile_t deprecation warnings from ros-perception/image_common#364 - migrate to rclcpp::QoS

* Fix rviz update float deprecation warnings from ros2/rviz#1533 - migrate to std::chrono::duration

* Fix geometry2 tf2_ros .h deprecation warnings from ros2/geometry2#805 - migrate Kilted and Rolling to .hpp

* Fix geometry2 tf2_ros NodeT deprecation warnings from ros2/geometry2#714 - migrate to NodeInterfaces

* Fix rclcpp spin_some deprecation warnings from ros2/rclcpp#2848 - migrate to SingleThreadedExecutor

---------

Co-authored-by: Andrea <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@clalancette clalancette Awaiting requested review from clalancette

@ahcorde ahcorde Awaiting requested review from ahcorde

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

Change tf2_ros C to C++ headers

3 participants

@garyservin @ahcorde @Timple