search

moveit / moveit_task_constructor

A hierarchical multi-stage manipulation planner
https://moveit.github.io/moveit_task_constructor
BSD 3-Clause "New" or "Revised" License
183 stars 152 forks source link

Fix `MoveRelative` potentially empty trajectory issue #595

Closed pac48 closed 4 months ago

pac48 commented 4 months ago

We have run into an issue a few times with MoveRelative relative that causes a crash. If the `PlannerInterface implementation returns a trajectory with no waypoints, but a non-null robot_trajectory, then the back of an empty vector is accessed via robot_trajectory->getLastWayPointPtr() leading to a crash.

A potential fix is to consider empty trajectories as a failed plan. We run into this issue frequently when the robot starts in or very close to a collision.

codecov-commenter commented 4 months ago

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

Codecov Report

All modified and coverable lines are covered by tests :white_check_mark:

Project coverage is 56.60%. Comparing base (6b0f2c8) to head (b7017dd). Report is 33 commits behind head on master.

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

Additional details and impacted files ```diff @@ Coverage Diff @@ ## master #595 +/- ## ========================================== - Coverage 58.82% 56.60% -2.22% ========================================== Files 91 131 +40 Lines 8623 10667 +2044 Branches 0 952 +952 ========================================== + Hits 5072 6037 +965 - Misses 3551 4583 +1032 - Partials 0 47 +47 ```

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

pac48 commented 4 months ago

Thanks. Don't you need the same check here as well:

https://github.com/moveit/moveit_task_constructor/blob/e3cc52cd0f018d91c119675cc11ee6d921e4109c/core/src/stages/move_relative.cpp#L330

Yes, we need to add the check there as well. Thanks for pointing that out.