[ros2]: Fixed preempting of execution #363
Conversation
|
@mechwiz Please review |
Codecov Report
@@ Coverage Diff @@
## ros2 #363 +/- ##
==========================================
- Coverage 38.85% 38.82% -0.03%
==========================================
Files 78 78
Lines 7520 7526 +6
==========================================
Hits 2921 2921
- Misses 4599 4605 +6
Continue to review full report at Codecov.
|
|
I tested these changes on a project I'm working on and can verify they work. LGTM! |
| @@ -95,7 +97,8 @@ void ExecuteTaskSolutionCapability::initialize() { | |||
| ActionServerType::CancelCallback( | |||
| std::bind(&ExecuteTaskSolutionCapability::preemptCallback, this, std::placeholders::_1)), | |||
| ActionServerType::AcceptedCallback( | |||
| std::bind(&ExecuteTaskSolutionCapability::goalCallback, this, std::placeholders::_1))); | |||
| std::bind(&ExecuteTaskSolutionCapability::goalCallback, this, std::placeholders::_1)), | |||
There was a problem hiding this comment.
I believe the std::bind isn't needed here since 1 argument is expected
…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
JafarAbdi
force-pushed
the
pr-fix_preempt_execution
branch
from
a760804
to
fcdce1e
Compare
There was a problem hiding this comment.
preempt was added to allow the user to stop planning.
As execution is just a thin wrapper around the action server it seems like a clumsy interface to repurpose preempt for execution instead.
Wouldn't it be better and more flexible to expose the action client to the user to cancel execution there?
| error_code.val = moveit_msgs::msg::MoveItErrorCodes::PREEMPTED; | ||
| return error_code; | ||
| } | ||
| } |
There was a problem hiding this comment.
Is that really the best we can do for this kind of feature? 10ms waits are quite bad style :(
Same as JafarAbdi#6