Allow RobotState::setFromIK to work with subframes by TSNoble · Pull Request #3077 · moveit/moveit2

TSNoble · 2024-11-10T12:48:47Z

Description

Adds passing regression / sanity tests which verify that a RobotState can setFromIK with a collision object
Adds failing tests demonstrating that RobotState cannot setFromIK with a subframe
Fixes failing tests by modifying RobotState::setFromIK to consider subframes
Adds some helper functions to the pilz tests for attaching objects / subframes and checking joint values

I tried adding the tests directly to the robot state tests, however, I was getting issues with no kinematic solver being associated with the planning group of the OneRobot fixture.

Pilz already had a test which was checking setFromIK, and appears to be setup with a kinematic solver, so I've added the tests there.

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 (no GUI changes)
While waiting for someone to review your request, please help review another open pull request to support the maintainers

…demonstrating failure with subframes

TSNoble · 2024-11-10T18:57:59Z

@sjahr I believe this is ready for review (just the usual tutorial image failures)

sea-bass

This is great! Especially appreciate the tests.

Mostly small comments from my side.

...lanners/pilz_industrial_motion_planner/test/unit_tests/src/unittest_trajectory_functions.cpp

riv-tnoble · 2024-11-11T09:33:17Z

@sea-bass Should be ready for review again.

Additionally, I'm always a little unsure of what types of changes require extending the tutorials & api notes. In theory the interface hasn't changed (the tip parameter still accepts a string and is therefore backwards-compatible, but the semantics are a little different in that it now accepts subframe names)

riv-tnoble · 2024-11-11T09:36:40Z

Ah, also if this could be marked for backport that'd be much appreciated. I'll get on fixing those PRs ASAP

sea-bass · 2024-11-11T09:37:36Z

@sea-bass Should be ready for review again.

Additionally, I'm always a little unsure of what types of changes require extending the tutorials & api notes. In theory the interface hasn't changed (the tip parameter still accepts a string and is therefore backwards-compatible, but the semantics are a little different in that it now accepts subframe names)

Thank you!

Your assessment is right; API wise, you're fine.

That said, i think subframes as a whole are not generally super well documented in the tutorials. So if you're a big user of this feature, you should in futire consider contributing a tutorial! That would be great.

sea-bass

LGTM, will await a 2nd review from someone else as well.

riv-tnoble · 2024-11-11T13:50:52Z

@sjahr Would you mind providing a second review when you have time?

sjahr

Looks good thanks!

* Adds regression tests for setFromIK with objects. Adds failing tests demonstrating failure with subframes * Modifies RobotState::setFromIK to account for subframes * Fixes formatting * Fixes formatting * Fixes formatting * Applies PR suggestions * Applies PR comments --------- Co-authored-by: Tom Noble <tom@rivelinrobotics.com> Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit ab34495) # Conflicts: # moveit_core/robot_state/src/robot_state.cpp

…3085) * Allow RobotState::setFromIK to work with subframes (#3077) * Adds regression tests for setFromIK with objects. Adds failing tests demonstrating failure with subframes * Modifies RobotState::setFromIK to account for subframes * Fixes formatting * Fixes formatting * Fixes formatting * Applies PR suggestions * Applies PR comments --------- Co-authored-by: Tom Noble <tom@rivelinrobotics.com> Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai> (cherry picked from commit ab34495) # Conflicts: # moveit_core/robot_state/src/robot_state.cpp * Fixes merge conflicts (humble backport 3077) (#3087) Co-authored-by: Tom Noble <tom@rivelinrobotics.com> --------- Co-authored-by: Tom Noble <53005340+TSNoble@users.noreply.github.com> Co-authored-by: Tom Noble <tom@rivelinrobotics.com> Co-authored-by: Sebastian Castro <4603398+sea-bass@users.noreply.github.com>

* Adds regression tests for setFromIK with objects. Adds failing tests demonstrating failure with subframes * Modifies RobotState::setFromIK to account for subframes * Fixes formatting * Fixes formatting * Fixes formatting * Applies PR suggestions * Applies PR comments --------- Co-authored-by: Tom Noble <tom@rivelinrobotics.com> Co-authored-by: Sebastian Jahr <sebastian.jahr@picknik.ai>

TSNoble added 2 commits November 10, 2024 12:45

Adds regression tests for setFromIK with objects. Adds failing tests …

f703a70

…demonstrating failure with subframes

Modifies RobotState::setFromIK to account for subframes

d7ae68e

TSNoble changed the title ~~(WIP) Allow RobotState::setFromIK to work with subframes~~ Allow RobotState::setFromIK to work with subframes Nov 10, 2024

Fixes formatting

a0f1295

TSNoble marked this pull request as ready for review November 10, 2024 15:21

TSNoble and others added 2 commits November 10, 2024 15:53

Fixes formatting

e31a136

Fixes formatting

29bc40e

This was referenced Nov 10, 2024

PILZ Industrial motion planner is ignoring the goal frame_id passed #2857

Closed

Request for a new release of MoveIt into Humble #3012

Closed

sea-bass reviewed Nov 10, 2024

View reviewed changes

riv-tnoble added 2 commits November 11, 2024 08:17

Applies PR suggestions

df2b734

Applies PR comments

1e8cf7a

sea-bass approved these changes Nov 11, 2024

View reviewed changes

sea-bass added backport-humble Mergify label that triggers a PR backport to Humble backport-iron Mergify label that triggers a PR backport to Iron labels Nov 11, 2024

riv-tnoble mentioned this pull request Nov 12, 2024

FCL collision checking crashes during motion #3083

Closed

Merge branch 'main' into bugfix/set-state-from-ik-with-subframes

744395a

sjahr approved these changes Nov 13, 2024

View reviewed changes

sjahr enabled auto-merge November 13, 2024 10:53

sjahr added this pull request to the merge queue Nov 13, 2024

Merged via the queue into moveit:main with commit ab34495 Nov 13, 2024

mergify bot mentioned this pull request Nov 13, 2024

Allow RobotState::setFromIK to work with subframes (backport #3077) #3085

Merged

6 tasks

mergify bot mentioned this pull request Nov 13, 2024

Allow RobotState::setFromIK to work with subframes (backport #3077) #3086

Closed

6 tasks

TSNoble deleted the bugfix/set-state-from-ik-with-subframes branch November 13, 2024 17:15

This was referenced Nov 14, 2024

RobotState::getRigidlyConnectedParentLinkModel() does not recognise subframes #3089

Closed

Fixes merge conflicts (humble backport 3077) #3087

Merged

Fixes pilz tests #3095

Merged

TSNoble mentioned this pull request Dec 3, 2024

Allow attached objects in move_group's /compute_ik pose frames #3144

Open

6 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Allow RobotState::setFromIK to work with subframes#3077

Allow RobotState::setFromIK to work with subframes#3077
sjahr merged 8 commits intomoveit:mainfrom
TSNoble:bugfix/set-state-from-ik-with-subframes

TSNoble commented Nov 10, 2024 •

edited

Loading

Uh oh!

TSNoble commented Nov 10, 2024

Uh oh!

sea-bass left a comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

sea-bass commented Nov 11, 2024 •

edited

Loading

Uh oh!

sea-bass left a comment

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

sjahr left a comment •

edited

Loading

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

Conversation

TSNoble commented Nov 10, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Checklist

Uh oh!

TSNoble commented Nov 10, 2024

Uh oh!

sea-bass left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

sea-bass commented Nov 11, 2024 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

sea-bass left a comment

Choose a reason for hiding this comment

Uh oh!

riv-tnoble commented Nov 11, 2024

Uh oh!

sjahr left a comment • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

TSNoble commented Nov 10, 2024 •

edited

Loading

sea-bass commented Nov 11, 2024 •

edited

Loading

sjahr left a comment •

edited

Loading