-
Notifications
You must be signed in to change notification settings - Fork 554
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
Trajectory processing, port to ROS 2 #63
Trajectory processing, port to ROS 2 #63
Conversation
See #62 (comment) |
moveit_core/trajectory_processing/src/iterative_time_parameterization.cpp
Outdated
Show resolved
Hide resolved
if(WIN32) | ||
# set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>;$<TARGET_FILE_DIR:${PROJECT_NAME}_TestPlugins1>") | ||
else() | ||
set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR};${CMAKE_CURRENT_BINARY_DIR}/../robot_trajectory;${CMAKE_CURRENT_BINARY_DIR}/../utils") |
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.
Hardcoding relative paths is a bad idea.
I would recommend that for every library in this package that is used else ware you set a <lib_name>_lib_dir
environment variable that can then be used by the tests
A good example is the rcl_lib_dir
I have taken the time to show how it is used below:
/home/mike/ws_ros2/src/ros2/rcl/rcl/CMakeLists.txt:
78
79: # rcl_lib_dir is passed as APPEND_LIBRARY_DIRS for each ament_add_gtest call so
80 # the librcl that they link against is on the library path.
81 # This is especially important on Windows.
82 # This is overwritten each loop, but which one it points to doesn't really matter.
83: set(rcl_lib_dir "$<TARGET_FILE_DIR:${PROJECT_NAME}>")
84
/home/mike/ws_ros2/src/ros2/rcl/rcl/test/CMakeLists.txt:
16
17: set(extra_lib_dirs "${rcl_lib_dir}")
18
/home/mike/ws_ros2/src/ros2/rcl/rcl/test/CMakeLists.txt:
34 rcl_add_custom_gtest(test_client${target_suffix}
35 SRCS rcl/test_client.cpp
36: INCLUDE_DIRS ${osrf_testing_tools_cpp_INCLUDE_DIRS}
37 ENV ${rmw_implementation_env_var}
38 APPEND_LIBRARY_DIRS ${extra_lib_dirs}
The example uses rcl_add_custom_gtest but it works just as well with ament_add_gtest
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.
If you don't mind I prefer to merge this. And then I will fix them all together. Because we have other merged that use this style. I can open an issue to avoid forgetting it.
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.
@davetcoleman @mlautman @henningkayser ready to merge |
@henningkayser or @davetcoleman can we merge this PR? |
* Add ROS2 compatible launch file and RViz config files for RViz quickstart tutorial * Update Quickstart in RViz tutorial and related images * Increase width of new image
No description provided.