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

Migrate CHOMP and collision tutorials from ROS1 #492

Merged
merged 7 commits into from
Aug 10, 2023

Conversation

MikeWrock
Copy link
Contributor

Description

Please explain the changes you made, including a reference to the related issue if applicable

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • While waiting for someone to review your request, please consider reviewing another open pull request to support the maintainers

Copy link
Contributor

@stephanie-eng stephanie-eng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thanks for doing this!

I put in quite a few nitpicks on the original tutorial - I realize some of these aren't related to your changes, but I think it's a good opportunity to bit of cleanup.

doc/how_to_guides/chomp_planner/chomp_planner_tutorial.rst Outdated Show resolved Hide resolved
doc/how_to_guides/chomp_planner/chomp_planner_tutorial.rst Outdated Show resolved Hide resolved
doc/how_to_guides/chomp_planner/chomp_planner_tutorial.rst Outdated Show resolved Hide resolved
doc/how_to_guides/chomp_planner/chomp_planner_tutorial.rst Outdated Show resolved Hide resolved
doc/how_to_guides/chomp_planner/chomp_planner_tutorial.rst Outdated Show resolved Hide resolved
CMakeLists.txt Outdated Show resolved Hide resolved
@AndyZe AndyZe changed the title Migrated tutorial from ROS1 Migrated CHOMP tutorial from ROS1 Sep 27, 2022
@AndyZe AndyZe changed the title Migrated CHOMP tutorial from ROS1 Migrate CHOMP and collision tutorials from ROS1 Sep 27, 2022
Copy link
Member

@AndyZe AndyZe left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've tested out the functionality now, it looks good 👍

I like the way you mixed the collision object tutorial in with the CHOMP tutorial.

Will approve after you address the remaining minor comments.

Comment on lines +45 to +46
rviz_full_config = os.path.join(rviz_base, "moveit_chomp.rviz")
rviz_empty_config = os.path.join(rviz_base, "moveit_chomp.rviz")
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If these lines are meant to be the same, do we need to have two configuration variables and the IfCondition for rviz_node_tutorial and rviz_node?

@henningkayser henningkayser added the backport-humble This label signals Mergify to backport this PR to Humble label Feb 20, 2023
@mergify
Copy link

mergify bot commented Feb 25, 2023

This pull request is in conflict. Could you fix it @MikeWrock?

@sjahr sjahr requested a review from stephanie-eng March 7, 2023 19:57
Copy link
Contributor

@stephanie-eng stephanie-eng left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good pending the question about rviz_full_config and rviz_empty_config looking like the same thing.

@mergify
Copy link

mergify bot commented Mar 14, 2023

This pull request is in conflict. Could you fix it @MikeWrock?

1 similar comment
@mergify
Copy link

mergify bot commented Jul 18, 2023

This pull request is in conflict. Could you fix it @MikeWrock?

CMakeLists.txt Outdated Show resolved Hide resolved
@sjahr sjahr merged commit 725c7d8 into moveit:main Aug 10, 2023
8 of 9 checks passed
mergify bot pushed a commit that referenced this pull request Aug 10, 2023
* Migrated tutorial from ROS1

* Fix merging error in CMakeLists.txt

---------

Co-authored-by: Henning Kayser <[email protected]>
Co-authored-by: Sebastian Jahr <[email protected]>
(cherry picked from commit 725c7d8)

# Conflicts:
#	CMakeLists.txt
#	doc/examples/chomp_planner/chomp_planner_tutorial.rst
#	doc/examples/planning_adapters/planning_adapters_tutorial.rst
#	doc/how_to_guides/how_to_guides.rst
sjahr added a commit that referenced this pull request Aug 10, 2023
… humble (#742)

* Migrate CHOMP and collision tutorials from ROS1 (#492)

* Migrated tutorial from ROS1

* Fix merging error in CMakeLists.txt

---------

Co-authored-by: Henning Kayser <[email protected]>
Co-authored-by: Sebastian Jahr <[email protected]>
(cherry picked from commit 725c7d8)

# Conflicts:
#	CMakeLists.txt
#	doc/examples/chomp_planner/chomp_planner_tutorial.rst
#	doc/examples/planning_adapters/planning_adapters_tutorial.rst
#	doc/how_to_guides/how_to_guides.rst

* Address rebasing conflicts

* Remove references to non-existing files and cleanup

* Cleanups

* Update doc/examples/examples.rst

---------

Co-authored-by: Michael Wrock <[email protected]>
Co-authored-by: Sebastian Jahr <[email protected]>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-humble This label signals Mergify to backport this PR to Humble
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants