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

Fix include paths for ROS2 headers #37

Merged
merged 2 commits into from
Aug 16, 2023

Conversation

kgreenek
Copy link
Contributor

This is a minor fix. I noticed a couple of include paths for ROS2 headers are using .h instead of .hpp, so it was failing to compile for me.

@fixposition-support
Copy link
Collaborator

Hi @kgreenek can you tell us which version of ROS2 are you using. Because your change seems to not work on ROS2 Foxy at least. (Works on Humble though)

@kgreenek
Copy link
Contributor Author

kgreenek commented Aug 15, 2023

Oh interesting, good thing for CI! I am using Humble.

I see that the tf2_eigen.h header is deprecated as of Humble, but it is still present -- it triggers a deprecation warning when compiling.

For more context, I'm building with bazel using these rules: https://github.com/mvukov/rules_ros2

I found out the bazel target for tf2_eigen does not include the deprecated .h file, which is why it didn't work for me: (https://github.com/mvukov/rules_ros2/blob/main/repositories/geometry2.BUILD.bazel#L139).

I see two ways I can fix this:

  1. Update rules_ros2 to also include the deprecated .h header.
  2. Update fixposition_driver to check for the existence of the .hpp header.

The benefit of 2 is you won't cause folks to hit the deprecation warning when building.

I went ahead and updated my PR to do approach 2. But if you prefer I do 1 instead, I am happy to do that.

@fixposition-support
Copy link
Collaborator

@kgreenek Thanks a lot for the contribution!

@fixposition-support fixposition-support merged commit 8079811 into fixposition:main Aug 16, 2023
fixposition-support pushed a commit that referenced this pull request Sep 5, 2023
* Fix include paths for ROS2 headers

* Add conditional include on deprecated header
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.

2 participants