-
Notifications
You must be signed in to change notification settings - Fork 6
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
Port tf2_2d to ROS 2 #5
Port tf2_2d to ROS 2 #5
Conversation
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
There was a problem hiding this 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.
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
I managed to remove |
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
There was a problem hiding this 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!
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.