-
Notifications
You must be signed in to change notification settings - Fork 818
Closed
Description
Retry node with while loop implementation can cause infinite loop in specific cases;
repeat_countis -1 (which means infinite)- The child of the retry node is condition node (never returns running)
- The child returns FAILURE in first tick.
- The child can return SUCCESS only when the state of the module is changed by information gathered by other module with ros2 spin.
Possible fix can be modifying the retry node logic;
- Rather than trying another loop on child failure, return RUNNING and wait for next tick.
- while (try_index_ < max_attempts_ || max_attempts_ == -1)
+ NodeStatus child_state = child_node_->executeTick();
+ switch (child_state)
{
- NodeStatus child_state = child_node_->executeTick();
- switch (child_state)
- {
- case NodeStatus::SUCCESS:
- {
- try_index_ = 0;
- haltChild();
- return (NodeStatus::SUCCESS);
- }
+ case NodeStatus::SUCCESS: {
+ try_index_ = 0;
+ haltChild();
+ return (NodeStatus::SUCCESS);
+ }
- case NodeStatus::FAILURE:
+ case NodeStatus::FAILURE: {
+ try_index_++;
+ haltChild();
+ if (try_index_ >= max_attempts_ && max_attempts_ != -1)
{
- try_index_++;
- haltChild();
+ try_index_ = 0;
+ return (NodeStatus::FAILURE);
}
- break;
+ }
- case NodeStatus::RUNNING:
- {
- return NodeStatus::RUNNING;
- }
+ case NodeStatus::RUNNING: {
+ return (NodeStatus::RUNNING);
+ }
- default:
- {
- throw LogicError("A child node must never return IDLE");
- }
+ default: {
+ throw LogicError("A child node must never return IDLE");
}
}
-
- try_index_ = 0;
- return NodeStatus::FAILURE;
}But I'm worried it might affect some nodes that runs asynchronously so I need confirmation before updating the code.
Thanks :)
Reactions are currently unavailable
Metadata
Metadata
Assignees
Labels
No labels