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

Trajectory processing, port to ROS 2 #63

Merged
merged 9 commits into from
Jun 12, 2019

Conversation

vmayoral
Copy link
Contributor

No description provided.

@vmayoral
Copy link
Contributor Author

vmayoral commented Apr 8, 2019

See #62 (comment)

@ahcorde ahcorde mentioned this pull request May 24, 2019
14 tasks
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")
Copy link
Contributor

@mlautman mlautman May 29, 2019

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

Copy link
Contributor

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.

Copy link
Contributor

Choose a reason for hiding this comment

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

#91

@ahcorde
Copy link
Contributor

ahcorde commented May 31, 2019

@davetcoleman @mlautman @henningkayser ready to merge

@ahcorde
Copy link
Contributor

ahcorde commented Jun 3, 2019

@henningkayser or @davetcoleman can we merge this PR?

@anasarrak
Copy link
Contributor

@mlautman mlautman merged commit 938d622 into moveit:master Jun 12, 2019
@anasarrak anasarrak deleted the trajectory-processing branch June 13, 2019 07:08
MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this pull request Aug 15, 2022
MikeWrock pushed a commit to MikeWrock/moveit2 that referenced this pull request Aug 15, 2022
* 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
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.

None yet

4 participants