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.

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

Port tf2_2d to ROS 2 #5

Merged
merged 12 commits into from
Nov 30, 2022
Merged

Port tf2_2d to ROS 2 #5

merged 12 commits into from
Nov 30, 2022

Conversation

sloretz
Copy link
Collaborator

@sloretz sloretz commented Nov 5, 2022

This ports tf2_2d to ROS Rolling. I both updated the code to ROS 2 and made it comply with ROS 2's linters. I made separate commits for required changes versus linter changes for easier review.

I noticed the code includes a workaround for a missing include in tf2, so I opened ros2/geometry2#559 to fix that upstream and left a TODO to remove the include when the fix is released.

Part of locusrobotics/fuse#276

Pinging @svwilliams for visibility.

@sloretz
Build sytem changes for ROS 2
7fb3e4a 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Code changes for ROS 2
8bf9f34 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Don't need int main(); because linking to GTEST_MAIN_LIBRARIES
c1b4a4b 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Skip ament_cmake_copyright
fdc7131 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Move .h headers to .hpp
808d23b 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Add .h headers with warning for backwards compatability
73af03d 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Linter fixes: Satisfy Uncrustify
186fadc 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Linter fixes: Satisfy cpplint
28369fa 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz sloretz self-assigned this Nov 5, 2022
methylDragon
methylDragon requested changes Nov 7, 2022
View reviewed changes
Copy link
Collaborator

@methylDragon methylDragon left a comment

Choose a reason for hiding this comment

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

Looking good! Just a couple of minor nits.

Tests pass, and the package builds as expected. Downstream usage is still unknown though, but we'll find out if there are kinks to iron out.

CMakeLists.txt Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
include/tf2_2d/rotation_impl.hpp Outdated Show resolved Hide resolved
include/tf2_2d/rotation.hpp Show resolved Hide resolved
include/tf2_2d/tf2_2d.hpp Show resolved Hide resolved
include/tf2_2d/transform.hpp Show resolved Hide resolved
include/tf2_2d/vector2.hpp Show resolved Hide resolved
include/tf2_2d/transform_impl.hpp Outdated Show resolved Hide resolved
include/tf2_2d/vector2_impl.hpp Outdated Show resolved Hide resolved
@methylDragon methylDragon mentioned this pull request Nov 7, 2022
Closed
54 tasks
@sloretz
Minimum CMake 3.14.4
07e683f 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
${tf2_geometry_msgs_TARGETS} -> tf2_geometry_msgs::tf2_geometry_msgs
c1f4ffe 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@sloretz
Bump copywrite date on redirection headers
63edaa5 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@methylDragon
Copy link
Collaborator

methylDragon commented Nov 12, 2022 •
edited
Loading

I managed to remove Scalar.h in rotation.hpp and transform.hpp as well (everywhere except vector2.hpp), though I think that's more because it gets transitively included via vector2.hpp 🤔

@sloretz
Remove Scalar.h include
6e611a6 
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
methylDragon
methylDragon approved these changes Nov 30, 2022
View reviewed changes
Copy link
Collaborator

@methylDragon methylDragon left a comment

Choose a reason for hiding this comment

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

It'd be nice to move the tests to a test CMakeLists.txt, but that's a bonus, all good to go!

@sloretz sloretz merged commit 369dc84 into locusrobotics:rolling Nov 30, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@methylDragon methylDragon methylDragon approved these changes

Assignees

@sloretz sloretz

Labels
None yet
Milestone
No milestone
Development

Successfully merging this pull request may close these issues.

None yet
2 participants
@sloretz @methylDragon