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
Fixed executor conflict #126
Fixed executor conflict #126
Conversation
Signed-off-by: Bitterisland6 <bitterisland6@gmail.com>
Signed-off-by: Bitterisland6 <bitterisland6@gmail.com>
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Works as expected!
I'm not sure if overriding the rclpy method is the best way to go here, is there any other method that could serve this purpose, implementing |
event = Event() | ||
future = client.call_async(request) | ||
rclpy.spin_until_future_complete(self._node, future, None, timeout) | ||
future.add_done_callback(lambda _: event.set()) | ||
|
||
event.wait(timeout) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is what client.call()
does. I'd recommend this instead:
try:
return client.call(request)
except:
raise AsyncServiceCallFailed(hint='the target node may not be spinning')
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@Bitterisland6 Would you prefer rlcpy client class function that Shane recommended or keep using python events?
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
@quarkytale the problem with solution recommended by Shane is that in the call
method the waiting for event is started without timeout, so there is a risk that it'll never end. Therefore I'd prefer to keep on using python events.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Okay I see, using the rclpy function would be preferred but I don't see any reason to block this. Thanks for your patience, I'll run CI.
Any update on this? |
@quarkytale Could you please backport it to humble and iron? |
@Mergifyio backport iron humble |
✅ Backports have been created
|
* fixed executor conflict Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> * fixed formatting errors Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> --------- Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> (cherry picked from commit e199a0d)
* fixed executor conflict Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> * fixed formatting errors Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> --------- Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> (cherry picked from commit e199a0d)
Fixed executor conflict (#126) * fixed executor conflict Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> * fixed formatting errors Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> --------- Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> (cherry picked from commit e199a0d) Co-authored-by: Aleksander Szymański <bitterisland6@gmail.com>
Fixed executor conflict (#126) * fixed executor conflict Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> * fixed formatting errors Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> --------- Signed-off-by: Bitterisland6 <bitterisland6@gmail.com> (cherry picked from commit e199a0d) Co-authored-by: Aleksander Szymański <bitterisland6@gmail.com>
Rqt spawns a multi threaded executor, and provides all open plugins in a single node which is spun inside it. But in the
rqt_reconfigure
plugin when calling service, there was added waiting for the end of the call withspin_until_future_complete
. This method was giving the control of the rqt to the default rclpy executor.This issue caused problems like:
To fix this issue it's enough to wait for the service request without the
spin_until_future_complete
method. One solution is to use python events andadd_done_callback
method from the future object returned bycall_async
method. The waiting is done with eventwait
function and provided timeout, and can be interrupted by setting the event flag in the callback specified in theadd_done_callback
.