Report bt action request error#6289
Conversation
BtActionNode returned failure directly when a goal was rejected or sending a goal was interrupted. Derived nodes could not record those request failures. This commit adds callbacks for rejected goals and interrupted goal requests before returning failure. A unit test verifies the rejected goal callback. Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
BT action nodes returned failure when a goal was rejected or could not be sent without populating the error-code output port. This commit adds action result codes for both conditions and writes them from every BtActionNode implementation, including docking and following nodes. It also reports FollowObject timeouts and checks the goal-rejection code in the action-node test. Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
Signed-off-by: Dylan De Coeyer <dylan.decoeyer@quimesis.be>
| /** | ||
| * @brief Function to perform work in a BT Node when the action server rejects a goal | ||
| * Such as setting the error code ID status for action clients. | ||
| */ | ||
| virtual void on_goal_rejected() | ||
| { | ||
| return; | ||
| } | ||
|
|
||
| /** | ||
| * @brief Function to perform work when sending a goal to the action server fails | ||
| * Such as setting the error code ID status for action clients. | ||
| */ | ||
| virtual void on_send_goal_failure() | ||
| { | ||
| return; | ||
| } |
There was a problem hiding this comment.
Maybe do
setOutput("error_code_id", ActionResult::GOAL_REJECTED);
Here so that the other BT nodes don't actually need to be updated unless they want to do more? Maybe type traits can be used to see if the goal fields in the response exist before doing so and/or pre-processor conditions? This may also expose any actions that also should be updated that were not
| on_goal_rejected(); | ||
| return BT::NodeStatus::FAILURE; | ||
| } else if (e.what() == std::string("send_goal failed")) { | ||
| on_send_goal_failure(); | ||
| // Action related failure that should not fail the tree, but the node |
There was a problem hiding this comment.
Should we have it return the BT NodeStatus from these new on_XYZ() callbacks in case they want to handle the situation differently? Many of our callbacks do. I don't 100% think that's needed here, but a motivational question for you if you can imagine a situation that it would be necessary.
Changing this API/ABI later would be painful, so I'd like to do it here and now if we can imagine such a situation
|
Neat. This seems related to #6279 so linking that up. |
BtActionNodereturnedFAILUREwhen an action goal was rejected or couldnot be sent, without giving derived nodes a way to report the reason.
This PR:
error_code_idfor both failures.Basic Info
Description of contribution in a few bullet points
Description of documentation updates required from your changes
Description of how this change was tested
Future work that may be required in bullet points
For Maintainers:
backport-*.