-
Notifications
You must be signed in to change notification settings - Fork 200
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
Parallel planning example #551
Conversation
2eb9bc2
to
ea2457b
Compare
0fae6e4
to
db1fe1d
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Looks good. I'd add another ompl planner to the example config. and show how you can use multiple planners from the same planner pipeline.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This looks great and worked for me no problem.
It would be nice if the tutorial printed not just how long a solution tool, but which planner ID was being used. I know the info is there if you read all the node names, but it would be a nice convenience to the user to just show things in one line.
Also, this doesn't have to be for this PR, but having a MoveGroup Interface example would be good as well.
doc/examples/parallel_planning/config/parallel_planning_moveit_cpp.yaml
Outdated
Show resolved
Hide resolved
doc/examples/parallel_planning/config/parallel_planning_moveit_cpp.yaml
Outdated
Show resolved
Hide resolved
c67dbc8
to
edaf149
Compare
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Did another pass, but it looks good for me after these small tweaks.
doc/how_to_guides/parallel_planning/config/parallel_planning_moveit_cpp.yaml
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/launch/parallel_planning_example.launch.py
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/launch/parallel_planning_example.launch.py
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/launch/parallel_planning_example.launch.py
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/launch/parallel_planning_example.launch.py
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No more comments
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm good with this PR, but have more comments to add if you want!
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
node_, { "ompl_rrtc", "pilz_lin", "chomp" } | ||
}; | ||
|
||
The constructor of this function will initialize :code:`PlanningRequestParameter` configurations based on the config that is provided in the node's |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Should this also be MultiPipelinePlanRequestParameters
, or am I wrong here?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
No, I rephrased the sentence to make it clearer what I am talking about:
The constructor of this class will initialize multiple :code:`PlanningRequestParameter` class members
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/src/parallel_planning_main.cpp
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/src/parallel_planning_main.cpp
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/src/parallel_planning_main.cpp
Outdated
Show resolved
Hide resolved
doc/how_to_guides/parallel_planning/src/parallel_planning_main.cpp
Outdated
Show resolved
Hide resolved
void setJointGoal(double const panda_joint1, double const panda_joint2, double const panda_joint3, | ||
double const panda_joint4, double const panda_joint5, double const panda_joint6, | ||
double const panda_joint7) | ||
{ | ||
auto robot_goal_state = planning_component_->getStartState(); | ||
robot_goal_state->setJointPositions("panda_joint1", &panda_joint1); | ||
robot_goal_state->setJointPositions("panda_joint2", &panda_joint2); | ||
robot_goal_state->setJointPositions("panda_joint3", &panda_joint3); | ||
robot_goal_state->setJointPositions("panda_joint4", &panda_joint4); | ||
robot_goal_state->setJointPositions("panda_joint5", &panda_joint5); | ||
robot_goal_state->setJointPositions("panda_joint6", &panda_joint6); | ||
robot_goal_state->setJointPositions("panda_joint7", &panda_joint7); | ||
|
||
// Set goal state | ||
planning_component_->setGoal(*robot_goal_state); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
... part of me wants to recommend that this accept a std::vector
or something, but it's fine if you don't want to do that extra work.
If you went that way you'd have to loop through like this (UNTESTED)
void setJointGoal(std::vector<double> const& panda_joint_angles)
{
size_t idx = 0;
auto robot_goal_state = planning_component_->getStartState();
for (const auto& joint_angle : panda_joint_angles) {
robot_goal_state->setJointPositions("panda_joint" + std::to_string(idx++), joint_angle);
}
planning_component_->setGoal(*robot_goal_state);
}
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I see you point. Maybe this would be worth adding to the PlanningComponent class
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Few small things, I'm good after this
"request_adapters": """default_planner_request_adapters/AddTimeOptimalParameterization \ | ||
default_planner_request_adapters/FixWorkspaceBounds \ | ||
default_planner_request_adapters/FixStartStateBounds \ | ||
default_planner_request_adapters/FixStartStateCollision \ | ||
default_planner_request_adapters/FixStartStatePathConstraints""", |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Indentation is weird here
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
provided by the :code:`parameters`, multiple threads are launched and each tries to solve the planning problem with a different planning pipeline. Once | ||
all pipelines have been terminated. Please keep in mind, that no solution is also a possible result, the :code:`solution_selection_callback` is called to determine which |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I commented on this earlier but it's gone away.
This "Once all pipelines have been terminated." sentence looks like an incomplete sentence that was left behind. I'm not sure what this means as it is.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Sorry, I put Please keep in mind, that no solution is also a possible result.
in the middle of a sentence
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
# PlanRequestParameters for the first parallel pipeline | ||
ompl_rrtc: | ||
plan_request_params: | ||
planning_attempts: 1 | ||
planning_pipeline: ompl | ||
planner_id: "RRTConnectkConfigDefault" | ||
max_velocity_scaling_factor: 1.0 | ||
max_acceleration_scaling_factor: 1.0 | ||
planning_time: 0.5 | ||
|
||
# PlanRequestParameters for the second parallel pipeline | ||
pilz_lin: | ||
plan_request_params: | ||
planning_attempts: 1 | ||
planning_pipeline: pilz_industrial_motion_planner | ||
planner_id: "LIN" | ||
max_velocity_scaling_factor: 1.0 | ||
max_acceleration_scaling_factor: 1.0 | ||
planning_time: 0.8 | ||
|
||
# PlanRequestParameters for the third parallel pipeline | ||
chomp: | ||
plan_request_params: | ||
planning_attempts: 1 | ||
planning_pipeline: chomp | ||
max_velocity_scaling_factor: 1.0 | ||
max_acceleration_scaling_factor: 1.0 | ||
planning_time: 1.5 |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For these comments, maybe add more than just "first, second, and third" and describe them as you did in the fourth one?
doc/how_to_guides/parallel_planning/parallel_planning_tutorial.rst
Outdated
Show resolved
Hide resolved
e39578b
to
d7b0438
Compare
Co-authored-by: Chris Thrasher <[email protected]> Co-authored-by: Sebastian Castro <[email protected]> Co-authored-by: Stephanie Eng <[email protected]>
Co-authored-by: Sebastian Castro <[email protected]>
Co-authored-by: Sebastian Castro <[email protected]>
Co-authored-by: Sebastian Castro <[email protected]>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
SHIP IT
Description
This PR adds an example of how to use the recently merged parallel planning interface for moveit2. To test it, run
Checklist