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.

Sign up for GitHub

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

Removed the BULLET_ENABLE flag #489

Merged
merged 4 commits into from
Jun 28, 2021
Merged

Removed the BULLET_ENABLE flag #489

Show file tree
Hide file tree
Changes from 2 commits
Commits
Show all changes
4 commits
Select commit Hold shift + click to select a range
4950815
set values for bullet CmakeList variables
jrgnicho Jun 8, 2021
23a18d0
removed the BULLET_ENABLED flag
jrgnicho Jun 8, 2021
0d47e11
Merge branch 'main' into fix-collision-bullet-build
henningkayser Jun 16, 2021
cd2fd4e
Fix Bullet cmake configuration
henningkayser Jun 16, 2021
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
15 changes: 6 additions & 9 deletions moveit_core/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,7 +15,6 @@ include(ConfigExtras.cmake)

list(APPEND CMAKE_MODULE_PATH "${CMAKE_CURRENT_SOURCE_DIR}/CMakeModules")

# TODO: Move collision detection into separate packages
find_package(Bullet 2.87 REQUIRED)

find_package(PkgConfig REQUIRED)
Expand Down Expand Up @@ -59,7 +58,7 @@ set(THIS_PACKAGE_INCLUDE_DIRS
backtrace/include
collision_detection/include
collision_detection_fcl/include
${BULLET_INC}
collision_detection_bullet/include
constraint_samplers/include
controller_manager/include
distance_field/include
Expand Down Expand Up @@ -93,7 +92,7 @@ set(THIS_PACKAGE_LIBRARIES
moveit_planning_interface
moveit_collision_detection
moveit_collision_detection_fcl
${BULLET_LIB}
moveit_collision_detection_bullet
moveit_kinematic_constraints
moveit_planning_scene
moveit_constraint_samplers
Expand Down Expand Up @@ -128,9 +127,10 @@ set(THIS_PACKAGE_INCLUDE_DEPENDS
Eigen3
eigen3_cmake_module
OCTOMAP
${BULLET_ENABLE}
Copy link
Member

Choose a reason for hiding this comment

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

Bullet needs to be listed as include depend, no?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I think I omitted it because I was mirroring the FCL setup. In fact the FCL include dependency is not in this list but I can add the Bullet one if that's what's preferred.

)

pluginlib_export_plugin_description_file(moveit_core collision_detector_fcl_description.xml)
pluginlib_export_plugin_description_file(moveit_core collision_detector_bullet_description.xml)

include_directories(SYSTEM ${EIGEN3_INCLUDE_DIRS}
${LIBFCL_INCLUDE_DIRS}
Expand Down Expand Up @@ -176,12 +176,9 @@ add_subdirectory(distance_field)
add_subdirectory(collision_distance_field)
add_subdirectory(kinematics_metrics)
add_subdirectory(dynamics_solver)
# TODO(henningkayser): enable bullet once the library is migrated to ROS2
if(BULLET_ENABLE)
add_subdirectory(collision_detection_bullet)
else()
install(FILES collision_detection_bullet/empty_description.xml DESTINATION share/${PROJECT_NAME} RENAME collision_detector_bullet_description.xml)
endif()

pluginlib_export_plugin_description_file(moveit_core collision_detector_bullet_description.xml)

install(
TARGETS ${THIS_PACKAGE_LIBRARIES}
Expand Down
18 changes: 8 additions & 10 deletions moveit_core/collision_detection_bullet/CMakeLists.txt
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
set(MOVEIT_LIB_NAME moveit_collision_detection_bullet)

add_library(${MOVEIT_LIB_NAME}
add_library(${MOVEIT_LIB_NAME} SHARED
src/bullet_integration/bullet_utils.cpp
src/bullet_integration/bullet_discrete_bvh_manager.cpp
src/bullet_integration/bullet_cast_bvh_manager.cpp
Expand All @@ -25,29 +25,27 @@ target_link_libraries(${MOVEIT_LIB_NAME}
${BULLET_LIBRARIES}
Copy link
Member

Choose a reason for hiding this comment

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

Can you add Bullet to the ament_target_dependencies block above? The variables seem to match the right pattern, so this should work.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Isn't that only necessary for ros2 dependencies?

Copy link
Member

Choose a reason for hiding this comment

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

Isn't that only necessary for ros2 dependencies?

no, it works for all kinds of packages as long as they provide the right variables (_LIBRARIES, _INCLUDE_DIRS, ...).

)

add_library(collision_detector_bullet_plugin src/collision_detector_bullet_plugin_loader.cpp)
add_library(collision_detector_bullet_plugin SHARED src/collision_detector_bullet_plugin_loader.cpp)
set_target_properties(collision_detector_bullet_plugin PROPERTIES VERSION "${${PROJECT_NAME}_VERSION}")
ament_target_dependencies(collision_detector_bullet_plugin
target_link_libraries(collision_detector_bullet_plugin PUBLIC
${MOVEIT_LIB_NAME}
moveit_planning_scene
)
ament_target_dependencies(collision_detector_bullet_plugin PUBLIC
rclcpp
urdf
visualization_msgs
pluginlib
rmw_implementation
)
target_link_libraries(collision_detector_bullet_plugin
${MOVEIT_LIB_NAME}
moveit_planning_scene
)

install(DIRECTORY include/ DESTINATION include)
install(TARGETS ${MOVEIT_LIB_NAME} EXPORT ${MOVEIT_LIB_NAME}
TARGETS collision_detector_bullet_plugin EXPORT collision_detector_bullet_plugin
LIBRARY DESTINATION lib
ARCHIVE DESTINATION lib
RUNTIME DESTINATION bin
Copy link
Member

Choose a reason for hiding this comment

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

I think the runtime destination should be lib/${PROJECT_NAME}, otherwise all executables will be accessible in the global bin path. That's a change that we should probably apply for the whole codebase.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

OK, I can change this

)
install(DIRECTORY include/ DESTINATION include)

install(DIRECTORY include/ DESTINATION ${CATKIN_GLOBAL_INCLUDE_DESTINATION})

if(BUILD_TESTING)
if(WIN32)
Expand Down
2 changes: 1 addition & 1 deletion moveit_core/collision_detector_bullet_description.xml
View file
Open in desktop
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
<library path="lib/libcollision_detector_bt_plugin">
<library path="collision_detector_bullet_plugin">
<class name="Bullet" type="collision_detection::CollisionDetectorBtPluginLoader"
base_class_type="collision_detection::CollisionPlugin">
<description>
Expand Down