fix: resolve bugs in MoveGroupSequenceAction class (main branch)

[MarcoMagriDev]

Description

Bugs fixed in MoveGroupSequenceAction class.

Modifications

1. Invalid goal_handle transition
   [move_group-1] what(): goal_handle attempted invalid transition from state EXECUTING with event EXECUTE, at ./src/rcl_action/goal_handle.c:95

2. move_feedback_ not initialized
   [move_group-1] Segmentation fault (Address not mapped to object [(nil)])

3. plan_callback_ not assigned
   [move_group-1] what(): bad_function_call

Original pull request on humble branch #1783

[codecov]

Codecov Report

Base: 50.29% // Head: 50.26% // Decreases project coverage by -0.04% ⚠️

Coverage data is based on head (bb16cc9) compared to base (25d086c).
Patch has no changes to coverable lines.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #1797      +/-   ##
==========================================
- Coverage   50.29%   50.26%   -0.03%     
==========================================
  Files         374      374              
  Lines       31286    31286              
==========================================
- Hits        15733    15723      -10     
- Misses      15553    15563      +10     

Impacted Files	Coverage Δ
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%)	⬇️

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.

[henningkayser]

Thank you for these fixes! I have not tested them yet, but at least 2 and 3 seem obviously correct to me (@v4hn might be interested in a cherry-pick of 3). I'm a bit puzzled about proper GoalHandle usage though, since I don't really find consistent information about who is calling execute(). ACCEPT_AND_EXECUTE seems to imply this call but I couldn't find it in rclcpp besides in the unit tests. The examples also don't use it, so I'll go with this change.

Side-note: you can close #1783, we're backporting this to humble right after merging this.

[MarcoMagriDev]

Thank you for these fixes! I have not tested them yet, but at least 2 and 3 seem obviously correct to me (@v4hn might be interested in a cherry-pick of 3). I'm a bit puzzled about proper GoalHandle usage though, since I don't really find consistent information about who is calling execute(). ACCEPT_AND_EXECUTE seems to imply this call but I couldn't find it in rclcpp besides in the unit tests. The examples also don't use it, so I'll go with this change.

Side-note: you can close #1783, we're backporting this to humble right after merging this.

You are welcome!

Regarding point 1) I think that the answer that you are looking for is here.
The goal state is updated to EXECUTING if the ACCEPT_AND_EXECUTE is used.
Using ACCEPT_AND_DEFER instead of ACCEPT_AND_EXECUTE here the execute has to be manually called as it was here.