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

Fix CheckLinkSelfCollision and CheckEndEffectorCollision #1443

Open
wants to merge 23 commits into
base: production
Choose a base branch
from

Conversation

snozawa
Copy link
Collaborator

@snozawa snozawa commented Oct 1, 2024

Summary

  • Fix bugs that several of KinBody::CheckLinkSelfCollision and Manipulator::CheckEndEffectorCollision, which are mostly used from IK related code path.

Bug fix 1

  • Issue
    • KinBody::CheckLinkSelfCollision functions have a code trying to handle grabbed bodies.
    • However, it uses the overload of CheckCollision(KinBody, KinBody), and it ignores the collision checking between two bodies attached each other.
      if( pchecker->CheckCollision(shared_kinbody_const(), KinBodyConstPtr(pgrabbedbody),report) ) {
    • Thus, it does not check the collision at all.
  • Resolution
    • Re-use the same logic in CheckSelfCollision.
    • Separate the self collision checking functionality about grabbed bodies. Use it both from CheckSelfCollision and CheckLinkSelfCollision.
    • In addition, add functionalities to fit with CheckLinkSelfCollision use cases:

Bug fix 2

  • Issue
    • does not check the self collision with grabbed bodies at all, if bIgnoreManipulatorLinks=true is specified :
      FOREACHC(itindependentlink,vindependentinks) {
      if( *itlink != *itindependentlink && (*itindependentlink)->IsEnabled() ) {
      if( pselfchecker->CheckCollision(*itlink, *itindependentlink,report) ) {
      if( !bAllLinkCollisions ) { // if checking all collisions, have to continue
      RAVELOG_VERBOSE_FORMAT("gripper link self collision with link %s", (*itlink)->GetName());
      return true;
      }
      bincollision = true;
      }
  • Resolution
    • Resolve this by calling CheckLinkSelfCollision as other code path.

Bug fix 3

  • Issue
    • Manipulator::CheckEndEffectorSelfCollision is implemented hand-made link-link collision checking if bIgnoreManipulatorLinks=true is specified :
      FOREACHC(itindependentlink,vindependentinks) {
      if( *itlink != *itindependentlink && (*itindependentlink)->IsEnabled() ) {
      if( pselfchecker->CheckCollision(*itlink, *itindependentlink,report) ) {
      if( !bAllLinkCollisions ) { // if checking all collisions, have to continue
      RAVELOG_VERBOSE_FORMAT("gripper link self collision with link %s", (*itlink)->GetName());
      return true;
      }
      bincollision = true;
      }
    • But, it does not consider the self-collision specific notion like isSelfCollisionIgnored and adjacent links.
    • Thus, it can result in false-positive results.
  • Resolution
    • Added the vIncludedLinks arguments to one of the CollisionCheckerBase::CheckStandaloneSelfCollision overload, which takes plink from the argument. Accordingly, add the same argument to KinBody::CheckLinkSelfCollision as well.
    • Notes for this change
      • We can add the code to handle self-collision-specific notion in CheckEndEffectorSelfCollision side. But, it's hard to maintain. So, better to support included links in KinBody::CheckLinkSelfCollision.
      • In terms of the argument, we can implement both excluded links and included links, since both are used in the existing code of CollisionCheckerBase. In these two, I used included links in this PR, since the included link in this situation can be easily computed from GetIndependentLinks of manipulator.

Misc improvements

  • CheckEndEffectorSelfCollision and CheckLinkSelfCollision have duplicated codes. Reduce it by introducing common functions.

Shunichi Nozawa added 13 commits January 5, 2025 23:45
…ed variables. 2) remove unused kinbodysaver from the first overload function. 3) remove linksaver, since it will not be necessary when _CheckGrabbedBodiesSelfCollision will be used.
…fCollsionCheck overload. In addition, keep the plink transform for grabbed bodies checking.
…ion checking context. So far, we supply vindependentlinks. Although we can do that by both inclusive links and exclusive links, introduce inclusive links here because we can just use vindependent links in CheckEndEffectorSelfCollisionCheck function
…ing ilinkindex.

- Issue
  - In the previous implementation, Manipulator::CheckEndEffectorCollision with bIgnoreManipulatorLinks does not properly consider the conditions which CheckStandaloneSelfCollision covers.
  - For example, isSelfCollisionIgnored or adjacent links is not considered at all.
  - In addition, the code itself looks duplicated.
- Resolution.
  - CheckLinkSelfCollision with specifying ilinkindex supports vector of included links. if empty, consider all possible pairs. if not empty, check links only in included links.
  - In the collisionchecker.h, there are both excluded links and included links. In this particular case, Manipulator::CheckEndEffectorCollision computes included links as vindependentinks and using it is convenient.
@snozawa snozawa force-pushed the fixGrabbedSelfCollisionCheckInOtherAPIs branch from cd89346 to 3850b85 Compare January 5, 2025 14:52
@snozawa snozawa changed the title WIP: Fix grabbed self collision check in other APIs Fix CheckLinkSelfCollision and CheckEndEffectorCollision Jan 6, 2025
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.

1 participant