Use BT.CPP recommended sleep for event-driven trees?

[tonynajjar]

Feature request

Feature description

I would be surprised if I'm the first one to bring up this discussion but I didn't find it anywhere. Why aren't we using the recommended Tree::sleep() from BT.CPP instead of the ROS rclcpp::WallRate to leverage event-driven trees?

I guess an obvious reason would be the better integration of rclcpp::WallRate with simulation time. But shouldn't we thrive to have all the benefits that both offer?

In a couple of instances we use emitWakeUpSignal, but afaik these should be useless without Tree::sleep() right @facontidavide?

Implementation considerations

[SteveMacenski]

Sorry for the delay, ROSCon, ROS-I, and so forth kept me from a computer.

Why aren't we using the recommended Tree::sleep()

Because it didn't exist in BT.CPP when we created that bit of software :-) My immediate thought is that I'm perfectly happy moving to the Tree::sleep() API. We use the WallRate for this operation, so simulation time isn't even considered for the BT ticking.

After looking at the BT.CPP implementation, I don't see any downsides to that change. @tonynajjar want to submit a PR?

[facontidavide]

Exactly! Tree::sleep() was created to potentially lower the latency, compared to a regular "sleep".

Tree::sleep() is basically a a std::condition_variable::wait_for under the hood.

This allows, as @tonynajjar point out, to wake uyp immediately when ANY node invokes emitWakeUpSignal.

Note that in general I suggest people to tick at 1 KHz using Tree::sleep(). Overhead should be minimal.

Unfortunately, there is a problem here, that prevent us ticking at high framerate.: https://github.com/ros-navigation/navigation2/blob/main/nav2_behavior_tree/src/behavior_tree_engine.cpp#L57-L74

The tick frequency is also the frequency of onLoop(); and what happens in thta callback could be arbitrarily slow 😢

Summarizing, I agree that using Tree::sleep() is an iprovement, but the latency of onLoop will also play a role.

[tonynajjar]

Will give it a shot!

[SteveMacenski]

Open a PR if its working well! I think it should ;-)