Skip to content

Conversation

@sloretz
Copy link

@sloretz sloretz commented Jul 4, 2025

Quaternion.h was deprecated in ros2/geometry2#720 and removed in ros2/geometry2#789 .

This PR updates base_realsense_node.h to use Quaternion.hpp. This fixes the build on Rolling that I saw in the CI of #3374

@sysrsbuild
Copy link

Can one of the admins verify this patch?

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jul 6, 2025

@sloretz thanks!
Will this work on Humble/Jazzy as well?
We might need some @ifdef ro make sure we do it for rolling(/Kilted?) only?

@Nir-Az Nir-Az requested a review from remibettan July 6, 2025 08:18
@Nir-Az
Copy link
Collaborator

Nir-Az commented Jul 7, 2025

Can you please change the PR to target ros2-development branch?

@sloretz sloretz changed the base branch from ros2-master to ros2-development July 7, 2025 15:54
@sloretz
Copy link
Author

sloretz commented Jul 7, 2025

Can you please change the PR to target ros2-development branch?

Done!

Quaternion.h was deprecated in ros2/geometry2#720 and removed in ros2/geometry2#789 .

This PR updates base_realsense_node.h to use Quaternion.hpp

Signed-off-by: Shane Loretz <[email protected]>
@sloretz sloretz force-pushed the sloretz_tf2_deprecated_header branch from 7d3e3dd to b9f7d2c Compare July 7, 2025 15:56
Copy link
Collaborator

@remibettan remibettan left a comment

Choose a reason for hiding this comment

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

Change works, but I had to perform:
sudo apt install --reinstall ros-jazzy-tf2
for the Quaternion.hpp file to be found.

Since a reinstall must be done, we will implement it with some ifdef statement checking the ROS2 distro (only in case of rolling or kilted, we will use the hpp file).
@sloretz do you want to perform this improvement?
Anyway, thanks a lot for your contribution!

Copy link
Collaborator

@remibettan remibettan left a comment

Choose a reason for hiding this comment

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

commented - please see "ifdef" request above

@remibettan
Copy link
Collaborator

@sloretz please inform if you want to correct acc to latest comment, or should we take it from here?

@sloretz
Copy link
Author

sloretz commented Jul 10, 2025

Change works, but I had to perform:
sudo apt install --reinstall ros-jazzy-tf2
for the Quaternion.hpp file to be found.

Hi @remibettan, it looks like you had out-of-date ROS Jazzy packages installed on your system. Instead of adding an #ifdef, I've updated the package.xml to require the minimum version of tf2 that has Quaternion.hpp for all active ROS distros.

If you're building from source with an out-of-date version of tf2, rosdep install should automatically update tf2 to the latest version.

@remibettan
Copy link
Collaborator

Change works, but I had to perform:
sudo apt install --reinstall ros-jazzy-tf2
for the Quaternion.hpp file to be found.

Hi @remibettan, it looks like you had out-of-date ROS Jazzy packages installed on your system. Instead of adding an #ifdef, I've updated the package.xml to require the minimum version of tf2 that has Quaternion.hpp for all active ROS distros.

If you're building from source with an out-of-date version of tf2, rosdep install should automatically update tf2 to the latest version.

Hi @sloretz , thanks for your advice, but rosdep is designed to install missing dependencies, not to upgrade existing packages even if a newer version is available that matches version_gte. So users that already have their system working with previous distros will encounter issues with your solution, even though it is much more elegant than the ifdef pragma.

@Nir-Az
Copy link
Collaborator

Nir-Az commented Jul 28, 2025

Thanks for the PR :)
Fix already merged as part of #3374

@Nir-Az Nir-Az closed this Jul 28, 2025
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

Successfully merging this pull request may close these issues.

4 participants