Skip to content

Rolling: Add OMPL config to moveit_cpp.yaml #1039

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

Merged
merged 4 commits into from
Apr 20, 2025

Conversation

AndyZe
Copy link
Member

@AndyZe AndyZe commented Apr 16, 2025

It took me way too long to figure out why moveit_cpp was using the CHOMP planning pipeline. It's because ompl pipeline wasn't configured here, so MoveIt defaults to the first planning pipeline in the list.

(I've only tested this on Humble, hopefully it hasn't changed since then.)

@AndyZe AndyZe force-pushed the moveitcpp_config branch from 0e527b7 to d2e7fa7 Compare April 16, 2025 14:43
@sea-bass sea-bass requested a review from sjahr April 18, 2025 01:30
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Thanks, Andy! Unfortunately, it is different from Humble.

See for example the latest moveit_resources Panda config.

Specifically, notice the split of request_adapters and response_adapters, and that some of the adapters were renamed/removed.

Tagged @sjahr as his planning pipeline refactor from a little while back is what introduced this. moveit/moveit2#2429 and moveit/moveit2#2457

It may also be good to introduce something into this repo that actually tests the MoveItCpp pipeline modified here, because as far as CI is concerned, all is green.

@AndyZe AndyZe changed the base branch from main to humble April 18, 2025 13:14
@AndyZe AndyZe changed the base branch from humble to main April 18, 2025 13:14
@AndyZe
Copy link
Member Author

AndyZe commented Apr 18, 2025

I'll open a new PR to target Humble, and update this one per the new planning pipelines. Prob won't have time for a new test though.

Sad to think how many thousands of people probably tried moveit_cpp and just failed silently.

@sea-bass sea-bass changed the base branch from main to humble April 18, 2025 22:39
@sea-bass sea-bass changed the base branch from humble to main April 18, 2025 22:40
@moveit moveit deleted a comment from mergify bot Apr 19, 2025
@AndyZe AndyZe changed the title Add OMPL config to moveit_cpp.yaml Rolling: Add OMPL config to moveit_cpp.yaml Apr 19, 2025
Copy link
Contributor

@sea-bass sea-bass left a comment

Choose a reason for hiding this comment

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

Sweet, thanks!

@AndyZe AndyZe merged commit 4694a54 into moveit:main Apr 20, 2025
9 checks passed
@AndyZe AndyZe deleted the moveitcpp_config branch April 20, 2025 00:33
@github-project-automation github-project-automation bot moved this to ✅ Done in MoveIt Apr 20, 2025
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.

2 participants