Skip to content

Enhancement/moveit ros1 ports (backport #3041)#3118

Merged
sea-bass merged 3 commits intohumblefrom
mergify/bp/humble/pr-3041
Nov 21, 2024
Merged

Enhancement/moveit ros1 ports (backport #3041)#3118
sea-bass merged 3 commits intohumblefrom
mergify/bp/humble/pr-3041

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify bot commented Nov 21, 2024

Description

This PR ports the following ROS1 PRs to moveit2:

More in-depth descriptions can be found on those pull requests, however, for summary:

  • Modifies MoveGroupInterace to use a moveit_msgs::msgs::RobotSate as the considered_start_state_

    • This prevents unnecessary conversions to and from message types when planning
    • This allows start states to be set using RobotState diffs to avoid passing around large messages
    • Public methods are modified to continue returning robot_state::RobotStatePtr to avoid breaking changes

  • Disables updating of attached collision objects when updating states during a pilz sequence motion

    • This prevents unnecessary copying of attached collision objects, which do not change throughout a sequence plan
    • This fixes a bug where the poses of attached collision objects accumulate error throughout a sequence plan

I've more-or-less copied the changes from those PRs in verbatim, so if there are differences in moveit2 that need to be taken into account, let me know and I'll make them.

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference (No changes to user-facing functionality)
  • Document API changes relevant to the user in the MIGRATION.md notes (preserves existing API)
  • Create tests, which fail without this PR reference (Modifies implementation details. Should preserve existing test behaviour)
  • Include a screenshot if changing a GUI (No GUI changes)
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers
This is an automatic backport of pull request #3041 done by [Mergify](https://mergify.com).

@TSNoble @mergify
Enhancement/moveit ros1 ports (#3041)
f0a5ba9 
* Ports moveit/moveit#3592

* Ports moveit/moveit#3590

* Fixes compile errors

---------

Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai>
(cherry picked from commit 02ebcba)

# Conflicts:
#	moveit_ros/planning_interface/move_group_interface/src/move_group_interface.cpp
@mergify mergify bot added the conflicts label Nov 21, 2024
@mergify
Copy link
Copy Markdown
Author

mergify bot commented Nov 21, 2024

Cherry-pick of 02ebcba has failed:

On branch mergify/bp/humble/pr-3041
Your branch is up to date with 'origin/humble'.

You are currently cherry-picking commit 02ebcba72.
  (fix conflicts and run "git cherry-pick --continue")
  (use "git cherry-pick --skip" to skip this patch)
  (use "git cherry-pick --abort" to cancel the cherry-pick operation)

Changes to be committed:
	modified:   moveit_planners/pilz_industrial_motion_planner/src/command_list_manager.cpp

Unmerged paths:
  (use "git add <file>..." to mark resolution)
	both modified:   moveit_ros/planning_interface/move_group_interface/src/move_group_interface.cpp

To fix up this pull request, you can check it out locally. See documentation: https://docs.github.com/en/pull-requests/collaborating-with-pull-requests/reviewing-changes-in-pull-requests/checking-out-pull-requests-locally

@sea-bass sea-bass mentioned this pull request Nov 21, 2024
Closed
@codecov-commenter
Copy link
Copy Markdown

codecov-commenter commented Nov 21, 2024 •
edited
Loading

⚠️ Please install the 'codecov app svg image' to ensure uploads and comments are reliably processed by Codecov.

Codecov Report

Attention: Patch coverage is 0% with 16 lines in your changes missing coverage. Please review.

Project coverage is 42.27%. Comparing base (ccf7e5c) to head (5f755d7).
Report is 16 commits behind head on humble.

Files with missing lines Patch % Lines
.../move_group_interface/src/move_group_interface.cpp 0.00% 14 Missing ⚠️
...strial_motion_planner/src/command_list_manager.cpp 0.00% 2 Missing ⚠️

❗ Your organization needs to install the Codecov GitHub app to enable full functionality.

Additional details and impacted files 
@@            Coverage Diff             @@
##           humble    #3118      +/-   ##
==========================================
- Coverage   51.15%   42.27%   -8.88%     
==========================================
  Files         382      668     +286     
  Lines       31902    57751   +25849     
  Branches        0     7322    +7322     
==========================================
+ Hits        16317    24406    +8089     
- Misses      15585    33159   +17574     
- Partials        0      186     +186

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

🚨 Try these New Features:

@sea-bass
Copy link
Copy Markdown
Contributor

sea-bass commented Nov 21, 2024

@rr-mark thanks for resolving these!

sea-bass
sea-bass suggested changes Nov 21, 2024
View reviewed changes
Copy link
Copy Markdown
Contributor

@sea-bass sea-bass left a comment •
edited
Loading

Choose a reason for hiding this comment

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

Build failures. There are some renamed member variables with underscores, it seems. Hence the conflicts.

@TSNoble
Copy link
Copy Markdown
Contributor

TSNoble commented Nov 21, 2024

@sea-bass Should be fixed with #3121

@TSNoble
Copy link
Copy Markdown
Contributor

TSNoble commented Nov 21, 2024

@sea-bass

I believe these are sporadic test failures. I've seen it on a few of the humble backports, e.g.:

https://github.com/moveit/moveit2/actions/runs/11856401984/job/33042608098
https://github.com/moveit/moveit2/actions/runs/11723728898/job/32656021443
https://github.com/moveit/moveit2/actions/runs/11579145965/job/32234657733

@sea-bass
Copy link
Copy Markdown
Contributor

sea-bass commented Nov 21, 2024

@sea-bass

I believe these are sporadic test failures. I've seen it on a few of the humble backports, e.g.:

I recently re-learned while going through issues that this test is also flaky on main, but it just happens to be disabled there.

#2821

@sea-bass sea-bass merged commit 172c128 into humble Nov 21, 2024
@sea-bass sea-bass deleted the mergify/bp/humble/pr-3041 branch November 21, 2024 23:08
@riv-tnoble riv-tnoble mentioned this pull request Nov 24, 2024
Closed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

1 more reviewer

@sea-bass sea-bass sea-bass approved these changes

Reviewers whose approvals may not affect merge requirements

Assignees

No one assigned

Labels

conflicts

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

4 participants

@codecov-commenter @sea-bass @TSNoble @riv-mjohnson