Add timeout to BT action node

[naiveHobo]

Signed-off-by: Sarthak Mittal [email protected]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#985
Primary OS tested on	Ubuntu 18.04
Robotic platform tested on	Gazebo simulation of TurtleBot3

---

Description of contribution in a few bullet points

• Added timeout parameter to BT Action Node to timeout when action takes too much time

Description of documentation updates required from your changes

• Added new parameter, so need to add that to default configs and documentation page

[SteveMacenski]

I think this is an invalid PR - because on any termination a proper action server should return some result. I think this would make sense in terms of a heartbeat to make sure that this server is still up. But setting some timeout for the action isn't going to be stable. You can't know the valid length in general of a following task, for instance. It can follow a 100km path in a straight line for 5 hours and that could be OK. I think the context for this would have to be in looking for feedback and not receiving it for a certain timeout, if the feedback is supposed to be available. Or seeing if there's something like bond for ROS2 to get a literal server heartbeat that its not crashed.

That brings up a good question about how we "know" that a server hasn't crashed - we should reject goals on submitting them if they're not up, but if it crashes in the middle of processing one, we wouldn't know and we'd wait forever. I think in that situation we want to exit and make BT navigator as unsafe to use. At the moment, I don't know of any heartbeats on the action servers, but maybe check the API and see if there's something appropriate to use. Maybe the pub/sub/client/server objects themselves have some heartbeat mechanism.

[ghost]

To add to what @SteveMacenski said, the action nodes themselves don't really know how long an action should take, but it seems reasonable to add timeouts at the behavior tree level when customizing it for your particular use case.

This can be easily added using the standard Timeout decorator node found here.
https://github.com/BehaviorTree/BehaviorTree.CPP/blob/master/src/decorators/timeout_node.cpp.

[SteveMacenski]

@naiveHobo maybe you slightly misinterpreted the stable ticket - that's mostly talking about timeouts for the spinning calls, which could block infinitely.

Though a heartbeat on the major servers (BT, planner, controller) is a good idea. Bond did this in ROS1, but it looks like the common thought is that lifecycle replaces bond, so maybe we have the lifecycle manager take a more active role in pinging its neighbors to see if they're alive and in an active state?

[naiveHobo]

Ah, I understand. I was under the impression that having node level timeouts especially for recovery actions would make sense. You wouldn't want a 180° rotation to take more than 60s for example. But as Carl suggested, there's already a standard timeout node for that purpose.

I'll go ahead and close this one.

[SteveMacenski]

OK, I'll file a ticket for the heartbeat, that could be useful. I wouldn't have considered that without you bringing this up, so that's still a great outcome