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

[MSA] SRDF Setup #1057

Merged
merged 14 commits into from
Mar 2, 2022
Merged

Conversation

DLu
Copy link
Contributor

@DLu DLu commented Feb 2, 2022

Description

  • Ports the remaining SRDF configuration steps (PlanningGroups/RobotPoses/EndEffectors/PassiveJoints) to the new framework
  • Makes some changes to the existing SRDF configuration steps (DefaultCollisions/VirtualJoints) to take advantage of shared code.

Squashed Bug

Edit: I fixed this with 5f77106

As mentioned in MIGRATION.md there is currently a bug that I can't figure out. Steps to reproduce:

  • Start MSA with new configuration
  • Configure a PlanningGroup
  • Try to make a pose for your new Group. It should give you an error because it can't find the associated JointModelGroup

Checklist

  • Required by CI: Code is auto formatted using clang-format
  • Extend the tutorials / documentation reference
  • Document API changes relevant to the user in the MIGRATION.md notes
  • Create tests, which fail without this PR reference
  • Include a screenshot if changing a GUI
  • While waiting for someone to review your request, please help review another open pull request to support the maintainers

@mergify
Copy link

mergify bot commented Feb 2, 2022

Please target the main branch for development, we will backport the changes to feature/msa for you if approved and if they don't break API.

@vatanaksoytezer
Copy link
Contributor

@DLu thanks a ton for the PR. I'll review this tomorrow and get back to you. In the meantime do we want to merge immediately with the bug you mentioned or hold until it's resolved. It seemed critical enough to me.

@DLu
Copy link
Contributor Author

DLu commented Feb 2, 2022

Nah, let's review and investigate the bug here

@codecov
Copy link

codecov bot commented Feb 2, 2022

Codecov Report

Merging #1057 (5f77106) into feature/msa (dc80acc) will not change coverage.
The diff coverage is n/a.

❗ Current head 5f77106 differs from pull request most recent head 5528a59. Consider uploading reports for the commit 5528a59 to get more accurate results

Impacted file tree graph

@@             Coverage Diff              @@
##           feature/msa    #1057   +/-   ##
============================================
  Coverage        57.86%   57.86%           
============================================
  Files              307      307           
  Lines            25918    25918           
============================================
  Hits             14996    14996           
  Misses           10922    10922           

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update dc80acc...5528a59. Read the comment docs.

@vatanaksoytezer
Copy link
Contributor

clang-tidy has a few complaints @DLu

@DLu DLu marked this pull request as ready for review February 9, 2022 18:32
@DLu
Copy link
Contributor Author

DLu commented Feb 9, 2022

Fixed the major bug. Ready for review.

Copy link
Contributor

@vatanaksoytezer vatanaksoytezer left a comment

Choose a reason for hiding this comment

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

I just tested this and looks good

@vatanaksoytezer vatanaksoytezer merged commit 93be638 into moveit:feature/msa Mar 2, 2022
@DLu DLu deleted the feature/msa_srdf_plugins branch March 2, 2022 19:07
DLu added a commit to DLu/moveit2 that referenced this pull request May 19, 2022
vatanaksoytezer pushed a commit that referenced this pull request May 25, 2022
vatanaksoytezer pushed a commit that referenced this pull request May 31, 2022
vatanaksoytezer pushed a commit that referenced this pull request May 31, 2022
DLu added a commit to DLu/moveit2 that referenced this pull request Jun 13, 2022
DLu added a commit to DLu/moveit2 that referenced this pull request Jun 13, 2022
vatanaksoytezer pushed a commit that referenced this pull request Jun 14, 2022
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.

None yet

2 participants