moveit / moveit2

:robot: MoveIt for ROS 2
https://moveit.ai/
BSD 3-Clause "New" or "Revised" License
1.03k stars 508 forks source link

RobotTrajectory append() end_index seems to be off-by-one #1822

Closed AndyZe closed 1 year ago

AndyZe commented 1 year ago

Description

Looking at the unit test here:

https://github.com/ros-planning/moveit2/blob/35bb662a6656f08101f0c1bd6aff5496d2172bf9/moveit_core/robot_trajectory/test/test_robot_trajectory.cpp#L216

We're appending 5 waypoints to another trajectory with 5 waypoints. Why is the end_index argument equal to 5? It should be 4.

initial_trajectory->append(*traj2, expected_duration, 0 /* start index */, 5 /* end_index */);

henningkayser commented 1 year ago

With iterators, end always refers to the element after the last one: https://en.cppreference.com/w/cpp/iterator/end. The code also makes sure that the index is reset to waypoints_.size() which would also be 5, not 4. The code is correct, no bug. We could however change end_index to count, similar to string append. https://en.cppreference.com/w/cpp/string/basic_string/append