Bug fix: RobotTrajectory append()#1813
Conversation
b2e0cf5 to
364ca53
Compare
Codecov ReportBase: 50.29% // Head: 50.27% // Decreases project coverage by
Additional details and impacted files@@ Coverage Diff @@
## main #1813 +/- ##
==========================================
- Coverage 50.29% 50.27% -0.01%
==========================================
Files 374 374
Lines 31286 31286
==========================================
- Hits 15733 15727 -6
- Misses 15553 15559 +6
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 at Codecov. |
364ca53 to
9ad28ef
Compare
|
|
||
| // After append() we should have 10 waypoints, all with 0.1s duration | ||
| const double EXPECTED_DURATION = 0.1; | ||
| initial_trajectory->append(*traj2, 0.1, 0, 5); |
There was a problem hiding this comment.
The 5 here doesn't make sense to me. Since the trajectory has 5 waypoints, I think the last index should be 4. But the test passes. I'm confused.
The doxygen says:
* \param end_index - index of last traj point of the part to append from the source traj, the default is to add until
* the end of the source traj
At a minimum, I think the comment should be updated. Any suggestions?
9ad28ef to
3385acc
Compare
henningkayser
left a comment
There was a problem hiding this comment.
Good find! I wonder if anyone would disagree with this, but the fixed behavior looks correct to me. I was still somewhat under the impression that duration_from_previous_ would start indexing at the second waypoint since the first waypoint doesn't have a "previous" 🤔.
3385acc to
0472c37
Compare
* Add a test for append() * Don't add to the timestep, rather overwrite it (cherry picked from commit 35bb662)
* Add a test for append() * Don't add to the timestep, rather overwrite it (cherry picked from commit 35bb662)
back-port of moveit/moveit2#1813
back-port of moveit/moveit2#1813
Description
The first commit seems to show something unexpected - an extra timestep of 0.1s is appended at index 5. The second commit fixes it.