Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Asynchronous ROS2 Action Cancellation #909

Open
amalnanavati opened this issue Feb 19, 2024 · 5 comments
Open

Asynchronous ROS2 Action Cancellation #909

amalnanavati opened this issue Feb 19, 2024 · 5 comments
Assignees

Comments

@amalnanavati
Copy link

amalnanavati commented Feb 19, 2024

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
Copy link
Contributor

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
Copy link
Contributor

sea-bass commented Feb 21, 2024

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
Copy link

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
Copy link
Contributor

sea-bass commented Feb 25, 2024

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

@JohannKa
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants