Port planning_scene to ROS2 #43
Conversation
Authored by @anasarrak
| @@ -1,7 +1,8 @@ | |||
| set(MOVEIT_LIB_NAME moveit_planning_scene) | |||
|
|
|||
| add_library(${MOVEIT_LIB_NAME} src/planning_scene.cpp) | |||
| set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}") | |||
| #TODO: Fix the versioning | |||
| # set_target_properties(${MOVEIT_LIB_NAME} PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}") | |||
There was a problem hiding this comment.
This was added in moveit/moveit#273. Maybe @v4hn can confirm this but perhaps we can take the same approach as the urdf_parser
| target_link_libraries(test_planning_scene ${MOVEIT_LIB_NAME}) | ||
| endif() | ||
| #TODO: Fix this so it can be built with colcon | ||
| # if(BUILD_TESTING) |
There was a problem hiding this comment.
Tests failing. Let's try to solve AcutronicRobotics#13 first please. We should be able to re-enable all of them afterwards.
There was a problem hiding this comment.
You all fixed this somewhere in AcutronicRobotics#71, is that correct?
There was a problem hiding this comment.
Yes however they should be submitted here (and we should rebase this code as well). @ahcorde started doing it iteratively while we review the changes.
There was a problem hiding this comment.
+1 to restoring tests before merge
moveit_core/planning_scene/include/moveit/planning_scene/planning_scene.h
Outdated
Show resolved
Hide resolved
|
I applied the clang formatting to this PR. Once we have the tests enabled, and we resolved why world is being made non-const +1 |
| target_link_libraries(test_planning_scene ${MOVEIT_LIB_NAME}) | ||
| endif() | ||
| #TODO: Fix this so it can be built with colcon | ||
| # if(BUILD_TESTING) |
There was a problem hiding this comment.
+1 to restoring tests before merge
|
@mlautman updated |
| @@ -1,30 +1,56 @@ | |||
| set(MOVEIT_LIB_NAME moveit_planning_scene) | |||
|
|
|||
| add_library(${MOVEIT_LIB_NAME} src/planning_scene.cpp) | |||
| if(WIN32) | |||
| # set(append_library_dirs "$<TARGET_FILE_DIR:${PROJECT_NAME}>;$<TARGET_FILE_DIR:${PROJECT_NAME}_TestPlugins1>") | |||
There was a problem hiding this comment.
This should be fixed, commented or removed.
| include_directories(${moveit_resources_INCLUDE_DIRS}) | ||
|
|
||
| if(UNIX OR APPLE) | ||
| set(append_library_dirs "${CMAKE_CURRENT_BINARY_DIR}:${CMAKE_CURRENT_BINARY_DIR}/../utils:${CMAKE_CURRENT_BINARY_DIR}/../collision_detection_fcl:${CMAKE_CURRENT_BINARY_DIR}/../collision_detection") |
There was a problem hiding this comment.
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 should work just as well with ament_add_gtest
There was a problem hiding this comment.
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.
|
Ready to merge @henningkayser |
|
@henningkayser or @davetcoleman can we merge this PR? |
Update CI to moveit_ci
* clang formatting * adding clang-format notes to README * fixing build warnings
* updated cmake to colcon/ament * file cleanup and commented out unconfigured build_depend in package.xml * more cleanup * removed subdirectory moveit_cpp * Switch to travis-ci.com in readme links * fix to travis status in readme * Update conf.py to generate CNAME * updated to meta.keys() (moveit#45) * fix travis for custom domain (moveit#47) * updated cmake with moveit_package, removed empty cmake, added vcs file * package.xml cleanup * updated readme with source build instructions * updated travis to foxy * updated repos to include moveit2.repos * fixed typo * ament format * more ament formatting * removed code coverage ci test added moveit.repos * updated cmake to colcon/ament * file cleanup and commented out unconfigured build_depend in package.xml * more cleanup * removed subdirectory moveit_cpp * updated cmake with moveit_package, removed empty cmake, added vcs file * package.xml cleanup * updated readme with source build instructions * updated travis to foxy * updated repos to include moveit2.repos * fixed typo * ament format * more ament formatting * removed code coverage ci test added moveit.repos * fixed travis and repose files, rebased on main * re-added fix travis for custom domain Co-authored-by: Alex Goldman <alex@picknik.ai>
Authored by @anasarrak