Unified Collision Environment Bullet #1572

j-petit · 2019-07-18T12:50:52Z

Description

This PR unifies the Bullet collision environment and gets the feature branch to build again.

I reduced / improved the Bullet glue code in many places through creating a common parent class for both discrete and cast collision checkers and removed a lot of unnecessary code in bullet_utils.h.

rhaschke · 2019-08-23T18:42:51Z

@j-petit, looks like you messed up the commit history by rebasing? There are many commits of the master branch listed in this PR.

BryceStevenWilley · 2019-08-23T18:48:22Z

@rhaschke That's intentional. This PR needs the changes from #1584, which just made it into master. I advised Jens to rebase this branch, and then I'd rebase the feature branch on master.

j-petit · 2019-08-23T19:12:41Z

@rhaschke That's intentional. This PR needs the changes from #1584, which just made it into master. I advised Jens to rebase this branch, and then I'd rebase the feature branch on master.

Not done with rebasing yet @BryceStevenWilley, I have one failing test case which I still need to work on...

rhaschke · 2019-08-23T19:30:53Z

That's intentional. This PR needs the changes from #1584, which just made it into master.

I see. But I think this PR not only requires #1584 but also the changes made in the branch feature-bullet-trajopt, which is the target branch of this PR. However, this feature branch is somewhat behind master. I think, we should either rebase the feature branch onto master or merge master into the feature branch. Only then, we can rebase Jens' new commits corresponding to this PR onto the result.

BryceStevenWilley · 2019-08-23T19:38:14Z

I think, we should either rebase the feature branch onto master or merge master into the feature branch. Only then, we can rebase Jens' new commits corresponding to this PR onto the result.

The plan is to rebase the feature branch onto master. I asked Jens to go ahead an resolve many of the rebase conflicts by rebasing himself, and then I'll force-push the conflict-free version of the feature branch once I get the go-ahead.

j-petit · 2019-08-26T11:27:04Z

Can anyone give me a hint to understand the Travis failure (clang-tidy):

fatal: ambiguous argument 'feature-bullet-trajopt..HEAD': unknown revision or path not in the working tree.

Use '--' to separate paths from revisions, like this:

'git <command> [<revision>...] -- [<file>...]'

No modified .cpp files

rhaschke · 2019-08-26T11:29:05Z

I need to analyze this failure. Looks like the feature branch is not checked out by Travis...
It's definitely not your fault.

rhaschke · 2019-08-26T12:28:27Z

I pushed a fix for the clang-tidy check. However, the root cause of your Travis failure is not the clang-tidy check, but the clang build:

In file included from /root/ros_ws/src/moveit/moveit_core/collision_detection_bullet/src/bullet_integration/bullet_discrete_bvh_manager.cpp:34:
In file included from /root/ros_ws/src/moveit/moveit_core/collision_detection_bullet/include/moveit/collision_detection_bullet/bullet_integration/bullet_discrete_bvh_manager.h:37:
In file included from /root/ros_ws/src/moveit/moveit_core/collision_detection_bullet/include/moveit/collision_detection_bullet/bullet_integration/bullet_utils.h:39:
In file included from /root/ros_ws/install/include/geometric_shapes/mesh_operations.h:40:
In file included from /root/ros_ws/install/include/geometric_shapes/shapes.h:47:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/7.4.0/../../../../include/c++/7.4.0/memory:81:
In file included from /usr/bin/../lib/gcc/x86_64-linux-gnu/7.4.0/../../../../include/c++/7.4.0/bits/shared_ptr.h:52:
/usr/bin/../lib/gcc/x86_64-linux-gnu/7.4.0/../../../../include/c++/7.4.0/bits/shared_ptr_base.h:588:8: warning: delete called on non-final 'collision_detection_bullet::BulletDiscreteBVHManager' that has virtual functions but non-virtual destructor [-Wdelete-non-virtual-dtor]
              delete __p;

clang complains that BulletDiscreteBVHManager doesn't have a virtual destructor.

j-petit · 2019-08-27T07:46:31Z

clang complains that BulletDiscreteBVHManager doesn't have a virtual destructor.

Addressed the issue, Travis passes now.

* shortening namespace * documentation improvements * virtual destructor of BVH manager * remove extra speed benchmark * bugfix for not initialized managers

Levi-Armstrong · 2019-08-29T20:18:42Z

I noticed due to the restructuring some the authors and copyright information was not transferred to the new files. It would be worth taking another look.

j-petit · 2019-08-30T17:17:52Z

Thanks for pointing that out, I will have a closer look at all the licenses and copyright info again. Levi Armstrong <notifications@github.com> schrieb am Do., 29. Aug. 2019, 22:18:

…

I noticed due to the restructuring some the authors and copyright information was not transferred to the new files. It would be worth taking another look. — You are receiving this because you were mentioned. Reply to this email directly, view it on GitHub <#1572>, or mute the thread <https://github.com/notifications/unsubscribe-auth/AETC5JWPYNAQGR3BAPEDBJ3QHAVKPANCNFSM4IE2M4UA> .

Levi-Armstrong · 2019-08-31T15:28:35Z

@j-petit Also it would be good to add your name to the author list where you feel it is relevant since you ported it to work with moveit.

j-petit · 2019-09-03T06:20:39Z

Addressed the license comments in last commit

mamoll

I left some small comments. This is a massive diff. Would it be possible to add some unit tests? Is it documented somewhere how you select the collision checker in MoveIt?

Do you have a benchmarking tool to compare Bullet vs. FCL performance for a given robot & environment model? I imagine Bullet is typically faster, but it'd be nice to know when perhaps FCL should be used (perhaps when the robot is represented is by a collection of spheres or the environment by a point cloud?).

...ion_bullet/include/moveit/collision_detection_bullet/bullet_integration/bullet_bvh_manager.h

...ullet/include/moveit/collision_detection_bullet/bullet_integration/bullet_cast_bvh_manager.h

* replaced include guards through pragma * used default instead of empty {} for ctor/dtor

j-petit · 2019-09-17T20:16:25Z

Thanks for taking the time to review @mamoll! Indeed this PR is big, but this mainly comes from the fact that I combined the two separate collision manager parent classes into a single one. It does not really add new functionality but involves a lot of copying code around. Tests are covered through the existing templated collision checking tests and an additional test suite for continuous collision detection.

There exists a tutorial on how to use Bullet in a pending PR. Furthermore, there is a benchmarking script available which is described in the README PR #1648 with a corresponding benchmarking environment. I also updated the top post in my GSoC github issue #1427 to reflect the most recent speed improvements.

My benchmarking setup is by far not exhaustive and it would be nice to have many more scenarios like you described (point clouds, etc.). See also my future work post here #1646. However, I am not sure when and if I will be able to work on those personally.

j-petit · 2019-11-10T15:31:55Z

I am not sure what is the current status of this PR, any thoughts @mamoll ? I'd like to see this merged :)

mamoll

I am ok with getting this merged in.

j-petit · 2019-11-10T17:17:15Z

awesome, then let's get this merged!

* 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

* 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 BryceStevenWilley, davetcoleman, henningkayser, rhaschke and v4hn as code owners July 18, 2019 12:50

j-petit mentioned this pull request Jul 18, 2019

GSoC: Improved Collision Checking #1427

Open

BryceStevenWilley force-pushed the feature-bullet-collision branch from c143e20 to 2129045 Compare July 25, 2019 04:44

j-petit changed the title ~~Unified Collision Environment [WIP]~~ Unified Collision Environment Bullet [WIP] Jul 25, 2019

mlautman closed this Aug 2, 2019

mlautman reopened this Aug 2, 2019

mlautman changed the base branch from feature-bullet-collision to feature-bullet-trajopt August 2, 2019 23:30

j-petit force-pushed the collision_env branch 3 times, most recently from 0975404 to f8d507f Compare August 7, 2019 11:20

v4hn added the wip label Aug 12, 2019

Templated tests adapted for unified collision env

a1a7d70

j-petit force-pushed the collision_env branch from ac2730e to 9ba947b Compare August 23, 2019 18:37

j-petit requested review from 130s, bmagyar, gavanderhoorn, IanTheEngineer, jonbinney, jrgnicho, mikeferguson and mlautman as code owners August 23, 2019 18:37

BryceStevenWilley added awaits 2nd review one maintainer approved this request and removed wip labels Aug 26, 2019

j-petit force-pushed the collision_env branch 3 times, most recently from 7cdb9e4 to b78b8b5 Compare August 26, 2019 19:18

PR review:

39b5ec0

* shortening namespace * documentation improvements * virtual destructor of BVH manager * remove extra speed benchmark * bugfix for not initialized managers

j-petit force-pushed the collision_env branch from b78b8b5 to 39b5ec0 Compare August 27, 2019 13:50

j-petit mentioned this pull request Aug 28, 2019

Readme for speed benchmark #1648

Merged

Licenses revised of old tesseract files

37a9164

mamoll reviewed Sep 16, 2019

View reviewed changes

PR review:

ac127e1

* replaced include guards through pragma * used default instead of empty {} for ctor/dtor

Comments in Bullet readme about thread safety and speed

f2d9a93

mamoll self-requested a review November 10, 2019 16:33

mamoll approved these changes Nov 10, 2019

View reviewed changes

mamoll merged commit 564829e into moveit:feature-bullet-trajopt Nov 11, 2019

sjahr pushed a commit to sjahr/moveit that referenced this pull request Jun 21, 2024

Remove unused header (moveit#1572)

2992967

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Unified Collision Environment Bullet #1572

Unified Collision Environment Bullet #1572

j-petit commented Jul 18, 2019 •

edited

Loading

rhaschke commented Aug 23, 2019

BryceStevenWilley commented Aug 23, 2019

j-petit commented Aug 23, 2019

rhaschke commented Aug 23, 2019

BryceStevenWilley commented Aug 23, 2019

j-petit commented Aug 26, 2019

rhaschke commented Aug 26, 2019

rhaschke commented Aug 26, 2019 •

edited

Loading

j-petit commented Aug 27, 2019 •

edited

Loading

Levi-Armstrong commented Aug 29, 2019

j-petit commented Aug 30, 2019 via email

Levi-Armstrong commented Aug 31, 2019

j-petit commented Sep 3, 2019

mamoll left a comment

j-petit commented Sep 17, 2019 •

edited

Loading

j-petit commented Nov 10, 2019

mamoll left a comment

j-petit commented Nov 10, 2019

Unified Collision Environment Bullet #1572

Unified Collision Environment Bullet #1572

Conversation

j-petit commented Jul 18, 2019 • edited Loading

Description

rhaschke commented Aug 23, 2019

BryceStevenWilley commented Aug 23, 2019

j-petit commented Aug 23, 2019

rhaschke commented Aug 23, 2019

BryceStevenWilley commented Aug 23, 2019

j-petit commented Aug 26, 2019

rhaschke commented Aug 26, 2019

rhaschke commented Aug 26, 2019 • edited Loading

j-petit commented Aug 27, 2019 • edited Loading

Levi-Armstrong commented Aug 29, 2019

j-petit commented Aug 30, 2019 via email

Levi-Armstrong commented Aug 31, 2019

j-petit commented Sep 3, 2019

mamoll left a comment

Choose a reason for hiding this comment

j-petit commented Sep 17, 2019 • edited Loading

j-petit commented Nov 10, 2019

mamoll left a comment

Choose a reason for hiding this comment

j-petit commented Nov 10, 2019

j-petit commented Jul 18, 2019 •

edited

Loading

rhaschke commented Aug 26, 2019 •

edited

Loading

j-petit commented Aug 27, 2019 •

edited

Loading

j-petit commented Sep 17, 2019 •

edited

Loading