Adapt cmake for Bullet #1744

j-petit · 2019-11-20T12:50:47Z

Description

This PR adapts the build process in order to add the Bullet dependent code only if the available version is greater than Bullet version 2.86 (2.87 is the one available through apt on Ubuntu 18.04).

moveit_core/CMakeLists.txt

gavanderhoorn · 2019-11-20T16:30:39Z

moveit_core/CMakeLists.txt

+  set(BULLET_INCLUDE_DIRS "${BULLET_ROOT_DIR}/${BULLET_INCLUDE_DIRS}")
+  set(BULLET_INCLUDE_DIR "${BULLET_ROOT_DIR}/${BULLET_INCLUDE_DIR}")
+  set(BULLET_LIBRARY_DIRS "${BULLET_ROOT_DIR}/${BULLET_LIBRARY_DIRS}")
+  set(BULLET_LIBRARIES "libBulletDynamics.so;libBulletCollision.so;libLinearMath.so;libBulletSoftBody.so")


Can you explain why you added this?

Especially the BULLET_LIBRARIES values seem strange, as CMake typically uses absolute paths to libraries, and BULLET_LIBRARIES should already contain the proper values.

Somehow, the BulletConfig.cmake file coming with libbullet-dev does not fit the library files coming with it. I was also wondering why this is the case. See the content of the config:

# -*- cmake -*- # # BulletConfig.cmake(.in) # # Use the following variables to compile and link against Bullet: # BULLET_FOUND - True if Bullet was found on your system # BULLET_USE_FILE - The file making Bullet usable # BULLET_DEFINITIONS - Definitions needed to build with Bullet # BULLET_INCLUDE_DIR - Directory where Bullet-C-Api.h can be found # BULLET_INCLUDE_DIRS - List of directories of Bullet and it's dependencies # BULLET_LIBRARIES - List of libraries to link against Bullet library # BULLET_LIBRARY_DIRS - List of directories containing Bullet' libraries # BULLET_ROOT_DIR - The base directory of Bullet # BULLET_VERSION_STRING - A human-readable string containing the version set ( BULLET_FOUND 1 ) set ( BULLET_USE_FILE "lib/x86_64-linux-gnu/cmake/bullet/UseBullet.cmake" ) set ( BULLET_DEFINITIONS "" ) set ( BULLET_INCLUDE_DIR "include/bullet" ) set ( BULLET_INCLUDE_DIRS "include/bullet" ) set ( BULLET_LIBRARIES "LinearMath;Bullet3Common;BulletInverseDynamics;BulletCollision;BulletDynamics;BulletSoftBody" ) set ( BULLET_LIBRARY_DIRS "lib/x86_64-linux-gnu" ) set ( BULLET_ROOT_DIR "/usr" ) set ( BULLET_VERSION_STRING "2.87" )

And see the files coming with it at https://packages.ubuntu.com/bionic/i386/libbullet-dev/filelist. There is no BulletInverseDynamics coming with this package. I am also not very happy about the fact that I have to define them manually and do this concatenation with the root directory.. any ideas?

When I simply use find_package(Bullet) this works fine, as it uses the FindBullet.cmake. However, this does not include the version number.

I don't like this approach as well. What about providing our own FindBullet.cmake?

Isn't this file provided by the cmake installation?

Any proposals where in the source tree such a file should go?

In moveit_core/CMakeModules? In moveit_core/CMakeLists.txt you'd add something like this:

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

I am struggling to get this running. My idea is to have the custom FindBullet.cmake be more or less a replicate of the official one plus checking somehow the BulletConfig.cmake which comes with libbullet-dev for the BULLET_VERSION_STRING.

However if I do include(.../BulletConfig.cmake) in the FindBullet.cmake I get all the variables set which clutters the FindBullet.cmake to the point that I cannot build succesfully anymore. I would like to only partially include the BulletConfig.cmake but I cannot find that option in cmake.

Any ideas?

You can also use the cmake PkgConfig module (since bullet also has a bullet.pc file). Your FindBullet.cmake might then look something like this:

include(FindPackageHandleStandardArgs) find_package(PkgConfig) if(PKGCONFIG_FOUND) pkg_check_modules(bullet>=2.88) endif() find_package_handle_standard_args(Bullet DEFAULT_MSG BULLET_LIBRARIES BULLET_INCLUDE_DIRS)

That worked :), thank you @mamoll for the help!

.travis.yml

davetcoleman · 2019-11-20T18:33:20Z

moveit_core/CMakeLists.txt

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


What will this entail?

This was added as suggested here by @rhaschke.

Later, we should factor out FCL and Bullet collision checking into their own packages. Maybe add a todo as a reminder here.

So I guess create stand-alone packages?

rhaschke · 2019-11-21T08:24:55Z

@j-petit, this needs to be rebased onto the (rebased) feature branch now...

moveit_core/CMakeLists.txt

j-petit · 2020-01-12T13:08:01Z

Travis fails because the feature branch is behind master (issues with moveit_tutorials, therefore I created a rebase PR in #1831.

j-petit · 2020-01-13T09:13:40Z

So it passes, can someone give this a final review?

codecov-io · 2020-01-13T09:13:55Z

Codecov Report

❗ No coverage uploaded for pull request base (feature-bullet-trajopt@753604c). Click here to learn what that means.
The diff coverage is n/a.

@@                    Coverage Diff                    @@
##             feature-bullet-trajopt    #1744   +/-   ##
=========================================================
  Coverage                          ?   50.29%           
=========================================================
  Files                             ?      312           
  Lines                             ?    24536           
  Branches                          ?        0           
=========================================================
  Hits                              ?    12341           
  Misses                            ?    12195           
  Partials                          ?        0

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 753604c...6bab622. Read the comment docs.

j-petit · 2020-01-17T08:06:04Z

Ping @mamoll

mamoll · 2020-01-17T14:10:11Z

moveit_core/CMakeModules/FindBullet.cmake

+find_package(PkgConfig)
+
+if(PKGCONFIG_FOUND)
+  pkg_check_modules(BULLET bullet)


Shouldn't that be

pkg_check_modules(BULLET Bullet>=${Bullet_FIND_VERSION})

the version is handled in the last line through VERSION_VAR BULLET_VERSION which makes it possible to use find_package(Bullet 2.87) in the moveit_core/CMakeLists.txt. I found that clearer to have the requirement for the Bullet version in the moveit_core/CmakeLists.txt instead of hidden inside the FindBullet.cmake.

I missed that. That's even better.

* Empty collision checker template for usage with tesseract and bullet (#1499) * Adding documentation to collision detection (#1488) * CMake adaptions for Tesseract integration * Empty collision detector template for tesseract and bullet * Making bullet as a collision plugin available * Adding missing error messages * Adding new folders and libraries to cmake * Fixing the BSD license * clang-format * Generic collision detection test suite (#1543) Generalize collision detection tests by using a templated test fixture. * Bullet Collision Detection (#1504) * Added discrete BVH Bullet manager * Added continuous collision detection * Cleanup of tesseract code * removed simple collision managers * changed enum to enum classes * fixed typos * removing debugging statements * removing tesseract allowed collision matrix * removed tesseract macros * replaced typedefs of stl containers * removed tesseract attached object code * ACM members of collision robot and world removed and ACM check into callback out of class * remove ContactTestType and replace through MoveIt CollisionRequest * BodyType int changed to enum * removed tesseract_msgs dependency and unnecessary tesseract code * changed dependency from bullet3_ros to debian Bullet package * Adding bullet and tesseract simple collision manager to the template: * added benchmark case for checking collision speed * tests for bullet collision checking * Adding missing features: * attached objects * contact reporting max number of contacts fixed * Bullet plugin xml name fixed * padding and scaling for robot added * updated tests * Adding continuous collision detection to Bullet (#1551) * Adding continous collision detection: * check only active links added * benchmark FCL vs Bullet * renaming files from bt to bullet * renaming variables from bt to bullet * clang-tidy and clang-format * renaming of variables to be in moveit format * continuous tests added * CCD active links changed * distance testing added to panda test suite * New collision features: * broadphase early culling * minimal distance reporting * renaming collision filter and group * removed extra margin on AABB for bullet * ACM in test from SRDF * PR review * Unified Collision Environment Bullet (#1572) * Templated tests adapted for unified collision env * Unified Bullet collision environment: * broadphase filtering adapted for early culling * ACM from SRDF in test * cleanup for bullet single collision env * removed link2castcow for CCD and only use constructor of COW directly * parent class for collision managers * removed function pointer for ACM check * removed extra self-collision manager and use only single manager * speed benchmark for unified environment * PR review: * more descriptive variable names * added user to TODO * PR review: * shortening namespace * documentation improvements * virtual destructor of BVH manager * remove extra speed benchmark * bugfix for not initialized managers * Licenses revised of old tesseract files * PR review: * replaced include guards through pragma * used default instead of empty {} for ctor/dtor * Comments in Bullet readme about thread safety and speed * Readme for speed benchmark (#1648) * FCL Bullet benchmark readme * Benchmark script and launch file updated * PR review fixups * Adapt cmake for Bullet (#1744) * cmake for using Bullet only if correct version available * Fixup * PR review fixup * Rewrite with own FindBullet.cmake * Use pkg-config * Fix travis failure for melodic * Fix moveit_ros_planning dependency on Bullet * Rename PR2-related collision test files (#1856)

j-petit requested review from rhaschke and v4hn as code owners November 20, 2019 12:50

j-petit mentioned this pull request Nov 20, 2019

Using Bullet as a Collision Checker moveit/moveit_tutorials#337

Merged

gavanderhoorn reviewed Nov 20, 2019

View reviewed changes

moveit_core/CMakeLists.txt Show resolved Hide resolved

j-petit changed the title ~~[WIP] Adapt cmake for Bullet~~ Adapt cmake for Bullet Nov 20, 2019

gavanderhoorn reviewed Nov 20, 2019

View reviewed changes

j-petit mentioned this pull request Nov 20, 2019

Rebase master #1740

Merged

davetcoleman approved these changes Nov 20, 2019

View reviewed changes

davetcoleman mentioned this pull request Nov 20, 2019

Dropping MoveIt 'master' branch support of Ubuntu 16.04 #1570

Closed

rhaschke force-pushed the feature-bullet-trajopt branch from 564829e to c27556a Compare November 21, 2019 08:18

j-petit force-pushed the cmake-bullet-version branch from a7db055 to 1fd192d Compare November 21, 2019 09:03

davetcoleman reviewed Nov 21, 2019

View reviewed changes

moveit_core/CMakeLists.txt Outdated Show resolved Hide resolved

moveit_core/CMakeLists.txt Outdated Show resolved Hide resolved

j-petit force-pushed the cmake-bullet-version branch from 241a293 to 44ad915 Compare January 12, 2020 12:49

j-petit requested a review from henningkayser as a code owner January 12, 2020 12:49

BryceStevenWilley force-pushed the feature-bullet-trajopt branch from 4f77ab3 to 753604c Compare January 13, 2020 04:38

j-petit added 7 commits January 13, 2020 09:17

cmake for using Bullet only if correct version available

a92adff

Fixup

f13e30f

PR review fixup

c620e27

Rewrite with own FindBullet.cmake

6fa57e8

Use pkg-config

16a898f

Fix travis failure for melodic

e412224

Fix moveit_ros_planning dependency on Bullet

6bab622

j-petit force-pushed the cmake-bullet-version branch from 44ad915 to 6bab622 Compare January 13, 2020 08:19

j-petit closed this Jan 13, 2020

j-petit reopened this Jan 13, 2020

mamoll reviewed Jan 17, 2020

View reviewed changes

mamoll merged commit 8f75792 into moveit:feature-bullet-trajopt Jan 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Adapt cmake for Bullet #1744

Adapt cmake for Bullet #1744

j-petit commented Nov 20, 2019

gavanderhoorn Nov 20, 2019

j-petit Nov 20, 2019

j-petit Nov 20, 2019 •

edited

rhaschke Nov 20, 2019

j-petit Nov 25, 2019

j-petit Jan 7, 2020

mamoll Jan 7, 2020

j-petit Jan 7, 2020

mamoll Jan 7, 2020

j-petit Jan 8, 2020

davetcoleman Nov 20, 2019

j-petit Nov 25, 2019

rhaschke commented Nov 21, 2019

j-petit commented Jan 12, 2020

j-petit commented Jan 13, 2020

codecov-io commented Jan 13, 2020

j-petit commented Jan 17, 2020

mamoll Jan 17, 2020 •

edited

j-petit Jan 17, 2020

mamoll Jan 17, 2020

Adapt cmake for Bullet #1744

Adapt cmake for Bullet #1744

Conversation

j-petit commented Nov 20, 2019

Description

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-petit Nov 20, 2019 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

rhaschke commented Nov 21, 2019

j-petit commented Jan 12, 2020

j-petit commented Jan 13, 2020

codecov-io commented Jan 13, 2020

Codecov Report

j-petit commented Jan 17, 2020

mamoll Jan 17, 2020 • edited

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

j-petit Nov 20, 2019 •

edited

mamoll Jan 17, 2020 •

edited