Bug fix: RobotTrajectory append()

[AndyZe]

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.

/home/andy/ws_ros2/src/moveit2/moveit_core/robot_trajectory/test/test_robot_trajectory.cpp:217: Failure
Expected equality of these values:
  initial_trajectory->getWayPointCount()
    Which is: 9
  10
/home/andy/ws_ros2/src/moveit2/moveit_core/robot_trajectory/test/test_robot_trajectory.cpp:219: Failure
Expected equality of these values:
  initial_trajectory->getWayPointDurationFromPrevious(5)
    Which is: 0.2
  EXPECTED_DURATION
    Which is: 0.1

[codecov]

Codecov Report

Base: 50.29% // Head: 50.27% // Decreases project coverage by -0.02% ⚠️

Coverage data is based on head (0472c37) compared to base (fca8e9b).
Patch coverage: 100.00% of modified lines in pull request are covered.

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     

Impacted Files	Coverage Δ
...eit_core/robot_trajectory/src/robot_trajectory.cpp	56.65% <100.00%> (ø)
moveit_ros/moveit_servo/src/servo_calcs.cpp	65.97% <0.00%> (-1.45%)	⬇️
moveit_ros/moveit_servo/src/pose_tracking.cpp	77.11% <0.00%> (-0.46%)	⬇️
moveit_core/robot_state/src/robot_state.cpp	47.95% <0.00%> (+0.08%)	⬆️
...dl_kinematics_plugin/src/kdl_kinematics_plugin.cpp	79.93% <0.00%> (+1.14%)	⬆️

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.
📢 Do you have feedback about the report comment? Let us know in this issue.

[AndyZe]

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?

[henningkayser]

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" 🤔.