-
Notifications
You must be signed in to change notification settings - Fork 957
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 new msa planners #2955
Add new msa planners #2955
Conversation
3f34344
to
b28da2d
Compare
Codecov Report
@@ Coverage Diff @@
## master #2955 +/- ##
==========================================
+ Coverage 61.55% 61.67% +0.12%
==========================================
Files 370 374 +4
Lines 33145 33261 +116
==========================================
+ Hits 20399 20509 +110
- Misses 12746 12752 +6
Continue to review full report at Codecov.
|
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.
Thanks for working on this. Please use templates instead of source code to generate files (see below).
moveit_setup_assistant/include/moveit/setup_assistant/tools/moveit_config_data.h
Show resolved
Hide resolved
...ssistant/templates/moveit_config_pkg_template/launch/ompl-chomp_planning_pipeline.launch.xml
Outdated
Show resolved
Hide resolved
...tup_assistant/templates/moveit_config_pkg_template/launch/stomp_planning_pipeline.launch.xml
Outdated
Show resolved
Hide resolved
014627c
to
2a6be2e
Compare
b09e2b0
to
cc84eab
Compare
@rhaschke I will update the |
This commit adds the launch and configuration files for the [STOMP algorithm](https://ros-planning.github.io/moveit_tutorials/doc/stomp_planner/stomp_planner_tutorial.html).
This commit adds the launch and configuration files for the [OMPL-CHOMP algorithm](https://ros-planning.github.io/moveit_tutorials/doc/chomp_planner/chomp_planner_tutorial.html#using-chomp-as-a-post-processor-for-ompl).
This commit removes several redundant parameters that were left after 6114fe5.
This makes sure the `ridge_factor` and `enable_failure_recovery` parameters in the CHOMP configuration file (see https://github.com/ros-planning/moveit/blob/4db626d9c3b5d6296b012188a4a0bfe4bddf6bce/moveit_setup_assistant/src/tools/moveit_config_data.cpp#L290-L309) are at their defaults (see https://github.com/ros-planning/moveit/blob/4db626d9c3b5d6296b012188a4a0bfe4bddf6bce/moveit_planners/chomp/chomp_interface/src/chomp_interface.cpp#L48-L83)
…unch/stomp_planning_pipeline.launch.xml Co-authored-by: Robert Haschke <[email protected]>
…unch/ompl-chomp_planning_pipeline.launch.xml Co-authored-by: Robert Haschke <[email protected]>
This commit replaces the `chomp_config.yaml` injection method by a simple template. This was done since the `chomp_config.yaml` file does not contain any variables that need to be dynamically generated.
cc84eab
to
70a24a0
Compare
This commit makes sure that the STOMP planning config that is generated using the MSA is general. The old version created the Panda Emika Franka stomp config found in the [panda_moveit_config](https://github.com/ros-planning/panda_moveit_config/blob/noetic-devel/config/stomp_planning.yaml) package.
@rhaschke This pull request is finished. I now made sure the STOMP config works with all robots instead of creating the Panda Stomp config (i.e. 87e0b87). I further cleaned up the CHOMP configuration file creation (70a24a0). I, however, found some problems when trying out the STOMP algorithm. Would you mind checking my comments made on this pull request before merging (i.e. comments regarding default values)? Stomp problems
|
@@ -52,14 +52,9 @@ void CHOMPInterface::loadParams() | |||
nh_.param("obstacle_cost_weight", params_.obstacle_cost_weight_, 1.0); | |||
nh_.param("learning_rate", params_.learning_rate_, 0.01); | |||
|
|||
// nh_.param("add_randomness", params_.add_randomness_, false); |
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.
Please double-check if these commented parameters can really be removed or that they will be enabled again in the future. The code that uses them was removed in 6114fe5 by
@raghavendersahdev. If they are needed we can drop 504d9fe.
moveit_setup_assistant/include/moveit/setup_assistant/tools/moveit_config_data.h
Show resolved
Hide resolved
moveit_setup_assistant/templates/moveit_config_pkg_template/config/chomp_planning.yaml
Show resolved
Hide resolved
moveit_setup_assistant/templates/moveit_config_pkg_template/config/chomp_planning.yaml
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.
I generally approve. Just a small nitpick:
moveit_setup_assistant/templates/moveit_config_pkg_template/config/chomp_planning.yaml
Show resolved
Hide resolved
...tup_assistant/templates/moveit_config_pkg_template/launch/stomp_planning_pipeline.launch.xml
Outdated
Show resolved
Hide resolved
…unch/stomp_planning_pipeline.launch.xml Co-authored-by: Robert Haschke <[email protected]>
@rhaschke Thanks for reviewing this pull request. Did you or @raghavendersahdev verify that the code removal I did in 6c930d7 is justified? |
As you said, the corresponding code was commented out a long time ago. I think it's good to have this cleanup. |
This pull request updates the MSA such that it:
It further also removes deprecated code from the CHOMP planner.
The output of the MSA can be found in this repository (Please NOTE that this config uses all the pull requests that are listed in moveit/panda_moveit_config#96).