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

Planners ompl constraints overconstrain #2775

Draft
wants to merge 4 commits into
base: main
Choose a base branch
from

Conversation

BerendPijnenburg
Copy link

Description

When creating OMPL constraints the AmbientDimension often gets overconstraint as the coDimension of the constraint is set to 3 even when you constrict less than 3 dimensions.

Current implementation uses the bounds class to never give an error for a dimension if that dimension is set using -1 but still "constrains" that dimension by setting the coDimension of the ompl::base::constraint to 3.
this causes you to be unable to use both a position and orientation constraint with <7 DOF robots as an AmbientDimension of 6 - 3 - 3 = 0 . #2614

Changes

  • I removed the derivative function of the bounds class as the sign can already be obtained from the penalty function.

  • I Consolidated the BoxConstraint and EqualityConstraint.

  • I changed the coDimension of the box and orientation constraints. Now a dimension of a position constraint can be unconstrained if the respective dimension of the constraint region is less than 0 or equal to infinity.

  • A dimension of an orientation constraint can be unconstrained if the absolute axis tolerance of the orientation is less than 0 or equal to infinity.

  • I removed the init function, it doesn't make sense to have an uninitialized constraint

  • I removed i removed the functions: calcError and calcErrorJacobian as their implementation belongs in function and jacobian

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@jih189
Copy link

jih189 commented Apr 2, 2024

It works.

@sjahr sjahr self-requested a review April 10, 2024 16:42
Copy link

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale Inactive issues and PRs are marked as stale and may be closed automatically. label May 27, 2024
@sjahr
Copy link
Contributor

sjahr commented Jul 11, 2024

@BerendPijnenburg Is this still a draft or ready for review?

@github-actions github-actions bot removed the stale Inactive issues and PRs are marked as stale and may be closed automatically. label Jul 15, 2024
Copy link

github-actions bot commented Oct 9, 2024

This PR is stale because it has been open for 45 days with no activity. Please tag a maintainer for help on completing this PR, or close it if you think it has become obsolete.

@github-actions github-actions bot added the stale Inactive issues and PRs are marked as stale and may be closed automatically. label Oct 9, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
stale Inactive issues and PRs are marked as stale and may be closed automatically.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants