Handle case where execute is called with single SubTrajectory #6
Conversation
henningkayser
force-pushed
the
fix_single_sub_traj_execution
branch
from
5c6c452 to
004f303
Compare
Codecov Report
@@ Coverage Diff @@
## ros2 #6 +/- ##
=======================================
Coverage 42.51% 42.51%
=======================================
Files 84 84
Lines 8060 8060
=======================================
Hits 3426 3426
Misses 4634 4634 📣 We’re building smart automated test selection to slash your CI/CD build times. Learn more |
sea-bass
left a comment
sea-bass
left a comment
There was a problem hiding this comment.
Looks good! Just a minor comment.
Should we also handle the case where the solution has zero subtrajectories and simply return false early in this function?
| auto make_description = [size = solution.sub_trajectory.size()](const std::size_t index) { | ||
| return std::to_string(index + 1) + "/" + std::to_string(size); | ||
| }; | ||
| // TODO: Case for only one subtrajectory |
There was a problem hiding this comment.
I guess this TODO was entirely missing, haha. Nice.
henningkayser
force-pushed
the
fix_single_sub_traj_execution
branch
from
c4fe306 to
968869e
Compare
sea-bass
left a comment
sea-bass
left a comment
There was a problem hiding this comment.
Sorry, I didn't realize that scene_diffs vector was being pushed to but then cleared on each iteration -- so my suggestion to reserve was based on there not being that clear() call in the loop.
Anyways, LGTM
|
we are still testing against Humble, but MoveIt has become incompatible. Do we just ignore the CI result or should we fix the requested jobs? (The missing header issue is not from this PR) |
henningkayser
force-pushed
the
fix_single_sub_traj_execution
branch
from
f2f0cb1 to
0985f1b
Compare
|
CI now seems to fail for unrelated build issues... |
5 participants
The capability would basically access the non-existent successor of the first trajectory, causing a segfault. Fixes https://github.com/PickNikRobotics/moveit_studio/issues/3568