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

Add ConfigurationToTSRwithTrajectoryConstraint #566

Open
wants to merge 9 commits into
base: gilwoo/planner_dart
Choose a base branch
from

Conversation

gilwoolee
Copy link
Contributor

In preparation for #467, this PR contains the following update:

  • Add planner::dart::ConfigurationToTSRwithTrajectoryConstraint
  • Add planner::dart::CRRTConfigurationToTSRwithTrajectoryConstraintPlanner
  • Add planner_dart component and make relevant changes in CMakeLists files.

The last change is necessary because CRRTConfigurationToTSRwithTrajectoryConstraintPlanner needs to call planner_ompl, which needs to be built before this planner is compiled.

Testing
This PR passes existing unit tests. I'll add unit tests for this planner after the initial review.


Before creating a pull request

  • Document new methods and classes
  • Format code with make format

Before merging a pull request

  • Set version target by selecting a milestone on the right side
  • Summarize this change in CHANGELOG.md
  • Add unit test(s) for this change

@brianhou
Copy link
Contributor

I haven't thought about this very much, but I initially thought it might make sense to add trajectory constraints as a part of all Problems rather than as a specific planner that can deal with them. What do you think? (I guess part of the challenge is that we don't have an reusable component that can plan with those constraints, so I could see an argument that we should just be pragmatic and call this a separate planner type until we do.)

Also, I think it would be good to separate the new planner_dart component into its own PR.

@gilwoolee gilwoolee changed the base branch from master to gilwoo/planner_dart March 3, 2020 23:03
…ido into gilwoo/ConfigurationToTSRwTrajConstraint
…:personalrobotics/aikido into gilwoo/ConfigurationToTSRwTrajConstraint
@gilwoolee
Copy link
Contributor Author

Thanks for the comments @brianhou !
I created another PR #567 as you suggested.

it might make sense to add trajectory constraints as a part of all Problems rather than as a specific planner that can deal with them. What do you think? (I guess part of the challenge is that we don't have an reusable component that can plan with those constraints, so I could see an argument that we should just be pragmatic and call this a separate planner type until we do.)

Yep I agree on both of these. I mainly chose this way for the latter challenge you mentioned and also because this is the only constrained planning problem type we use now. I think it's worth revisiting for a more general solution once we have more of these constrained planning problem types.

@sniyaz sniyaz removed their request for review September 25, 2020 20:07
@sniyaz sniyaz added this to the Aikido 0.5.0 milestone Sep 25, 2020
@sniyaz sniyaz self-assigned this Sep 25, 2020
@sniyaz sniyaz requested a review from egordon September 25, 2020 20:07
@aditya-vk aditya-vk removed their request for review September 17, 2021 03:53
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.

3 participants