RobotWebTools / rosbridge_suite

Server Implementations of the rosbridge v2 Protocol
https://robotwebtools.github.io
BSD 3-Clause "New" or "Revised" License
866 stars 506 forks source link

Asynchronous ROS2 Action Cancellation #909

Open amalnanavati opened 4 months ago

amalnanavati commented 4 months ago

Motivating Issue

We have a web app that uses roslibjs and rosbridge to be a ROS2 action client. The web app needs to send goals to a ROS2 action server and cancel an executing goal.

In practice, the web app is able to send goals to the action server but not cancel goals. Specifically, when we try to cancel a goal that was sent by the web app, in practice the cancellation is only processed after goal execution is completed. Needless to say, an inability to asynchronous cancel actions can be dangerous.

What Causes the Issue?

In the current ROS2 Action implementation (PR #886), the "send_action_goal" operation waits until the action is complete before returning. As a result, the incoming message queue is blocked here; it doesn't process any new messages until after the "send_action_goal" operation is completed, i.e., the action is completed. As a result, asynchronous action cancellation is not possible, because any cancel requests that arrive while the goal is executing will only be processed after the goal is finished executing.

Note that this issue didn't arise in an earlier un-merged implementation of ROS2 actions (PR #813 ). That's because in that earlier implementation, the "send_goal" operation returns once the goal is accepted/rejected, and does not wait for the goal to finish executing.

Requested Feature

Support asynchronous action cancellation. Options to do this:

  1. IMHO, the most straightforward way to do so would be to change the "send_action_goal" operation to return once the goal is accepted/rejected, not wait for execution to complete (this would involve a change here to wait for self.goal_handle instead of self.result and perhaps a change in the result status type).
  2. If for some reason we want to keep the current synchronous implementation of "send_action_goal," I think we should add another "send_action_goal_asynch" operation in rosbridge that returns once the goal is accepted/rejected, and extend roslibjs to have the option of calling that.

Tagging the authors/reviewers of the merged ROS2 action PRs in rosbridge and roslibjs: @sea-bass @EzraBrooks @MatthijsBurgh @sathak93 Thoughts?

sea-bass commented 4 months ago

Your suggestion 1 seems very reasonable!

Of course, we still need the rosbridge protocol to send a goal result message when the action does eventually complete... so a change like this would maybe require yet another message that gives you some information about the goal being accepted/rejected?

If you have time to put together a PR for this, I'm happy to take a look.

sea-bass commented 4 months ago

As an immediately available workaround, I'm also curious if the send_action_goals_in_new_thread parameter I've added in can allow canceling a goal while another is working?

JohannKa commented 4 months ago

Thanks @amalnanavati for this well documented issue : I was fighting with that for days thinking I wrongly set my roslibjs request...

@sea-bass I just tested the send_action_goals_in_new_thread parameter set to true and yes it allows an immediate cancelling of the actual goal(s?).

But so far I'm experiencing a drawback : I can't anymore plan multiple goals. My use-case is to use a SLAM map on which I can click to set waypoints (goal poses) and it is working fine with send_action_goals_in_new_thread parameter set to false (the robot goes to each waypoints one by one) but that doesn't work anymore with send_action_goals_in_new_thread parameter set to true (each of my click redirect the robot to this latest click).

I continue my testing but I don't want to spam this ticket then don't hesitate if you need more information and I'll come back anyway if I find something interesting.

sea-bass commented 4 months ago

Nice! I think with the proposed changes we could/should add tests that permit both multiple goals and immediate cancellation.

JohannKa commented 4 months ago

So... I tried to make a fix but I failed. My idea was to keep the normal logic for any op but to use a separate thread for the cancel_action_goal capability.

What would have been so easy would have been to replace protocol.register_operation("cancel_action_goal", self.cancel_action_goal) by

        protocol.register_operation(
           "cancel_action_goal",
           lambda msg: Thread(target=self.cancel_action_goal, args=(msg,)).start(),
        )

here

but of course it didn't do the trick as the cancel goal command is still queued behind the previous goals...

I then tried to find a spot between receiving the op and sending it to the queue but I'm too noob in Python for that and didn't had a clear view on the threads logic that is used, the queuing opacifying it.

So, as mainly a web developer, I used the usual JS good practice : working-around the workaround :-p and queued the sending of goals in my JS app with the send_action_goals_in_new_thread parameter set to true and bypassing my queue for cancel ops.

Though so far I'm good if you need me to help fixing the bridge on that, let me know but I'll need clear directions on how to do it.