-
Notifications
You must be signed in to change notification settings - Fork 123
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
fuse -> ROS 2 fuse_constraints: Port fuse_constraints #290
fuse -> ROS 2 fuse_constraints: Port fuse_constraints #290
Conversation
a99bd21
to
d47d1de
Compare
5b66f22
to
89b1315
Compare
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
Signed-off-by: methylDragon <methylDragon@gmail.com>
89b1315
to
2ae878a
Compare
@ros-pull-request-builder retest this please! |
find_package(benchmark QUIET) | ||
if(benchmark_FOUND) | ||
# Normal Delta Pose 2D benchmark | ||
add_executable(benchmark_normal_delta_pose_2d benchmark_normal_delta_pose_2d.cpp) |
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.
There's a utility ament_add_google_benchmark
that could be used here. It's a little different from this pattern. Instead of finding Google benchmark quietly, it requires it but disables the test unless -DAMENT_RUN_PERFORMANCE_TESTS=ON
is set.
Ah it looked good to me, but CI is failing with
|
fuse_constraints/CMakeLists.txt
Outdated
${geometry_msgs_TARGETS} | ||
pluginlib::pluginlib | ||
rclcpp::rclcpp | ||
SuiteSparse::CCOLAMD |
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.
Two comments. First:
This target doesn't exist, but we can create it in FindSUITESPARCE.cmake
.
At the bottom of that find module, create an imported target
add_library(SuiteSparce::CC0LAMD IMPORTED)
target_link_libraries(SuiteSparce::CC0LAMD IMPORTED ${SUITESPARCE_LIBRARIES}`)
target_include_directories(SuiteSparce::CC0LAMD IMPORTED ${SUITESPARCE_INCLUDE_DIRS}`)
The advantage of doing it that way is the target can still be transitively depended upon downstream.
Second:
I didn't see any use of SuiteSparce
in the header files. I think that means we can make this a PRIVATE
dependency here:
target_link_libraries(${PROJECT_NAME} PRIVATE
SuiteSparce::CC0LAMD)
@ros-pull-request-builder retest this please! |
d1f82eb
to
40fd349
Compare
@ros-pull-request-builder retest this please! |
40fd349
to
e6640bc
Compare
@ros-pull-request-builder retest this please! |
Signed-off-by: methylDragon <methylDragon@gmail.com>
e6640bc
to
d643a79
Compare
@ros-pull-request-builder retest this please! |
Signed-off-by: Shane Loretz <sloretz@osrfoundation.org>
@ros-pull-request-builder retest this please! |
See: #276
Description
This PR ports the entirety of fuse_constraints, including:
Other stuff like docs are in other PRs.
Caught a bug from the time port in the tests due to signature changes between ros::Time and rclcpp::Time, fixed (rclcpp time needed a conversion to nanoseconds from seconds)
Notes
Pinging @svwilliams for visibility.