-
Notifications
You must be signed in to change notification settings - Fork 519
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
Comments
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. |
As an immediately available workaround, I'm also curious if the |
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 But so far I'm experiencing a drawback : I can't anymore plan multiple goals. 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. |
Nice! I think with the proposed changes we could/should add tests that permit both multiple goals and immediate cancellation. |
So... I tried to make a fix but I failed. What would have been so easy would have been to replace
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 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. |
Hi folks, I was looking into the state of actions support and came up with some ideas. Unfortunately, I'm not using rosbridge at the moment so I have no immediate interest in doing too much more development myself. However, I wanted to share my idea in case anyone does want to try it. rosbridge_suite PR: #953 Otherwise, for any further improvements, happy to review PRs! |
Motivating Issue
We have a web app that uses
roslibjs
androsbridge
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:
self.goal_handle
instead ofself.result
and perhaps a change in the result status type).Tagging the authors/reviewers of the merged ROS2 action PRs in rosbridge and roslibjs: @sea-bass @EzraBrooks @MatthijsBurgh @sathak93 Thoughts?
The text was updated successfully, but these errors were encountered: