rclcpp icon indicating copy to clipboard operation
rclcpp copied to clipboard

rclcpp_action: take and execute service entities in priority.

Open fujitatomoya opened this issue 3 months ago • 5 comments

address https://github.com/ros2/rclcpp/issues/2451 and https://github.com/ros2/rclcpp/issues/1679

Note: more tests need to be done before CI.

fujitatomoya avatar Apr 01 '24 07:04 fujitatomoya

  • IMO, we do not guarantee all feedback messages must be delivered to the client. (if the goal accept response is not yet delivered, that would be dropped. but eventually, feedback messages will be received on the client once goal handle is being tracked.)
  • Service responses should be handled with priority than feedback and status topics because it changes internal action state in the client.

fujitatomoya avatar Apr 01 '24 07:04 fujitatomoya

@alsora thanks for the review.

this will change the action client behavior internally so i need to do more verification locally, and i would like to have more feedback for this before further. please keep it open for a while.

fujitatomoya avatar Apr 01 '24 21:04 fujitatomoya

Uhm, this implementation is ontop of a racy state...

We should merge https://github.com/ros2/rclcpp/pull/2250 first.

jmachowinski avatar Apr 03 '24 16:04 jmachowinski

Regarding the issue itself. From my point of view, the design of the action is broken, as it uses multiple communication channels to communicate, thus creating this kind of problems.

Btw you can create the same bug by spamming feedback, and make an action never terminate.

jmachowinski avatar Apr 03 '24 16:04 jmachowinski

@jmachowinski thanks for the comment. this one is not in the merge queue yet, i need to take some more time to verify this, since this changes the action behavior, i am expecting that userspace (tests) would break.

fujitatomoya avatar Apr 03 '24 16:04 fujitatomoya