Sync with MoveIt1 #1900

Abishalini · 2023-01-26T18:11:41Z

Description

Syncing the following commits

* [2022-09-09] [1245f1513] | Merge PR #3197: Improve computeCartesianPath() {{Robert Haschke}} 
* [2022-09-08] [636376679] | Gracefully handle gtest 1.8 (Melodic) {{Robert Haschke}} 
* [2022-09-08] [0440f23d1] | Add unit tests for computeCartesianPath() {{Robert Haschke}} 
* [2022-09-08] [97c69fa5f] | Add utils to simplify (approximate) equality checks for Eigen entities {{Robert Haschke}} 
* [2022-09-08] [ccdde2cce] | robot_model_test_utils: Add loadIKPluginForGroup() {{Robert Haschke}} 
* [2022-09-08] [bd662121c] | Simplify test_cartesian_interpolator.cpp {{Robert Haschke}} 
* [2022-09-04] [a1b448e64] | Simplify MGI::computeCartesianPath() {{Robert Haschke}} 
* [2022-08-27] [37e4163f6] | Generalize computeCartesianPath() to consider a link_offset {{Robert Haschke}} 
* [2022-08-27] [415fdf10e] | Cleanup CartesianInterpolator {{Robert Haschke}}

TODO:

Figure out why kinematics plugins are not found

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

- Fixup doc comments - Add API providing the translation vector = direction * distance - Simplify implementation

This allows performing a circular motion about a non-link origin.

gtest 1.8 doesn't provide SetUpTestSuite(). Thus, we cannot share the RobotModel across tests.

henningkayser · 2023-01-27T17:26:32Z

moveit_core/robot_state/test/test_cartesian_interpolator.cpp

  EXPECT_EQ(full_traj_len, traj.size());  // traj should not be cut
  EXPECT_NEAR(1.0, fraction, 0.01);
 }

+// Gracefully handle gtest 1.8 (Melodic)


You should be able to remove _OLD_GTEST everywhere

henningkayser · 2023-01-27T17:46:47Z

moveit_core/utils/src/robot_model_test_utils.cpp

+
+  jmg->setSolverAllocators(
+      [=](const JointModelGroup* jmg) -> kinematics::KinematicsBasePtr {
+        kinematics::KinematicsBasePtr result = loader->createUniqueInstance(plugin);


@rhaschke apparently, we have some issues finding the plugin: C++ exception with description "According to the loaded plugin descriptions the class kdl_kinematics_plugin/KDLKinematicsPlugin with base class type kinematics::KinematicsBase does not exist. Declared types are " thrown in SetUpTestSuite(). (link). I also recall that there could be issues when loading plugins from the same package. I'm not sure if this was specific to ROS 2, though. Do you have any ideas?

Looks like no plugins are found at all. No idea why. I don't have time to look into this next week.

#1901

Could this PR be related?

unsure, it would be weird if it was related, but I would certainly be happy about that surprise. This code works just fine in MoveIt 1 and we have no issues loading IK plugins or other in a normal launch. To me it looks like a bug (or lack of proper support) to use plugins from the same package that exports them (plugin xml), not a header-not-found kind of thing.

.. I've thought about this a bit more and I think we should move the test instance next to the IK plugin package. The test behavior really depends on the plugin implementation and we might want to test different plugins against the same behavior. Ideally, we would also test computeCartesianPath() with a mock implementation of an IK solver so that we are not introducing solver-specific issues into our unit tests.

I have opened #2099 to tackle this. I have commented out the tests for now so this PR can be merged

codecov · 2023-04-09T11:16:01Z

Codecov Report

Patch coverage has no change and project coverage change: -0.07 ⚠️

Comparison is base (9d6ceb2) 50.91% compared to head (1c1be5d) 50.84%.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1900      +/-   ##
==========================================
- Coverage   50.91%   50.84%   -0.07%     
==========================================
  Files         391      391              
  Lines       32126    32147      +21     
==========================================
- Hits        16355    16343      -12     
- Misses      15771    15804      +33

Impacted Files	Coverage Δ
moveit_core/utils/src/robot_model_test_utils.cpp	`72.62% <0.00%> (-5.15%)`	⬇️

... and 5 files with indirect coverage changes

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report in Codecov by Sentry.
📢 Do you have feedback about the report comment? Let us know in this issue.

moveit_core/robot_state/include/moveit/robot_state/cartesian_interpolator.h

moveit_core/robot_state/test/test_cartesian_interpolator.cpp

moveit_ros/planning_interface/move_group_interface/src/move_group_interface.cpp

rhaschke and others added 9 commits September 2, 2022 11:05

Cleanup CartesianInterpolator

415fdf1

- Fixup doc comments - Add API providing the translation vector = direction * distance - Simplify implementation

Generalize computeCartesianPath() to consider a link_offset

37e4163

This allows performing a circular motion about a non-link origin.

Simplify MGI::computeCartesianPath()

a1b448e

Simplify test_cartesian_interpolator.cpp

bd66212

robot_model_test_utils: Add loadIKPluginForGroup()

ccdde2c

Add utils to simplify (approximate) equality checks for Eigen entities

97c69fa

Add unit tests for computeCartesianPath()

0440f23

Gracefully handle gtest 1.8 (Melodic)

6363766

gtest 1.8 doesn't provide SetUpTestSuite(). Thus, we cannot share the RobotModel across tests.

Merge PR #3197: Improve computeCartesianPath()

1245f15

henningkayser reviewed Jan 27, 2023

View reviewed changes

github-actions bot added the stale Inactive issues and PRs are marked as stale and may be closed automatically. label Apr 3, 2023

Abishalini force-pushed the pr-sync-1245f15 branch from 10030ef to 90f3db7 Compare April 9, 2023 10:52

Abishalini marked this pull request as ready for review April 10, 2023 09:59

Merge moveit/moveit@1245f15

6dbeda7

Abishalini force-pushed the pr-sync-1245f15 branch from bc41692 to 6dbeda7 Compare April 10, 2023 10:44

Abishalini mentioned this pull request Apr 10, 2023

Enable Cartesian Interpolations tests #2099

Open

github-actions bot removed the stale Inactive issues and PRs are marked as stale and may be closed automatically. label Apr 10, 2023

tylerjw approved these changes Apr 10, 2023

View reviewed changes

moveit deleted a comment from github-actions bot Apr 10, 2023

AndyZe reviewed Apr 10, 2023

View reviewed changes

tylerjw assigned sjahr Apr 11, 2023

Readd comment and assign error code

1c1be5d

AndyZe approved these changes Apr 11, 2023

View reviewed changes

AndyZe enabled auto-merge (rebase) April 11, 2023 16:37

auto-merge was automatically disabled April 11, 2023 17:59
Rebase failed

Abishalini merged commit 0cb508a into moveit:main Apr 11, 2023

Abishalini deleted the pr-sync-1245f15 branch April 11, 2023 18:26

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Sync with MoveIt1 #1900

Sync with MoveIt1 #1900

Abishalini commented Jan 26, 2023 •

edited

Loading

henningkayser Jan 27, 2023

henningkayser Jan 27, 2023

rhaschke Jan 27, 2023

ChrisThrasher Feb 6, 2023

henningkayser Feb 8, 2023

henningkayser Feb 16, 2023

Abishalini Apr 10, 2023 •

edited

Loading

codecov bot commented Apr 9, 2023 •

edited

Loading

Sync with MoveIt1 #1900

Sync with MoveIt1 #1900

Conversation

Abishalini commented Jan 26, 2023 • edited Loading

Description

Checklist

henningkayser Jan 27, 2023

Choose a reason for hiding this comment

henningkayser Jan 27, 2023

Choose a reason for hiding this comment

rhaschke Jan 27, 2023

Choose a reason for hiding this comment

ChrisThrasher Feb 6, 2023

Choose a reason for hiding this comment

henningkayser Feb 8, 2023

Choose a reason for hiding this comment

henningkayser Feb 16, 2023

Choose a reason for hiding this comment

Abishalini Apr 10, 2023 • edited Loading

Choose a reason for hiding this comment

codecov bot commented Apr 9, 2023 • edited Loading

Codecov Report

Abishalini commented Jan 26, 2023 •

edited

Loading

Abishalini Apr 10, 2023 •

edited

Loading

codecov bot commented Apr 9, 2023 •

edited

Loading