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

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

Merged
merged 3 commits into from
Nov 21, 2024

Conversation

mergify[bot]
Copy link

@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).

* 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
Copy link
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

@codecov-commenter
Copy link

codecov-commenter commented Nov 21, 2024

⚠️ 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
Contributor

@rr-mark thanks for resolving these!

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.

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

@TSNoble
Copy link
Contributor

TSNoble commented Nov 21, 2024

@sea-bass Should be fixed with #3121

@TSNoble
Copy link
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
Contributor

@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
7 checks passed
@sea-bass sea-bass deleted the mergify/bp/humble/pr-3041 branch November 21, 2024 23:08
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

4 participants