Conversation
|
@wyattrees @henningkayser Could you please review this .? |
3b75b5b to
6fd3334
Compare
| while (result_future.wait_for(std::chrono::milliseconds(10)) != std::future_status::ready) { | ||
| if (pimpl()->preempt_requested_) { | ||
| auto cancel_future = ac->async_cancel_goal(goal_handle); | ||
| if (rclcpp::spin_until_future_complete(node, cancel_future) != rclcpp::FutureReturnCode::SUCCESS) { |
There was a problem hiding this comment.
why do we need to spin here? I thought that there would be an executor spinning the node already, no?
There was a problem hiding this comment.
As you can see here it's created locally only for execution
There was a problem hiding this comment.
I guess that makes sense for now... But aren't there some stages that depend on ROS interfaces? Those will need an executor as well, no?
|
|
||
| void ExecuteTaskSolutionCapability::initialize() { | ||
| action_callback_group_ = | ||
| context_->moveit_cpp_->getNode()->create_callback_group(rclcpp::CallbackGroupType::Reentrant); |
There was a problem hiding this comment.
What's the reason to use Reentrant here? This seems inherently unsafe if not handled appropriately.
There was a problem hiding this comment.
To make it possible to call the cancel callback while executing the goal callback
There was a problem hiding this comment.
I see, I thought at first that "Reentrant" would only apply to the same callback and not to other's in the same group. To me it seems like there is a type missing. What I don't really like about this is that you have to take care about redundant calls to the same callback manually, even though that's definitely an edge case.
6fd3334 to
2d797ac
Compare
09593dc to
71a604c
Compare
2d797ac to
a760804
Compare
…ting the goal so it does not block the server from accepting a cancel request. While client is waiting for the result future, repeatedly check if preempting was requested and send a cancel request if it was
a760804 to
fcdce1e
Compare
Rebased #3 on
ros2and small clean-up to minimize diff for later sync between master & ros2 branches