Increase BT action node's action server timeout

[tonynajjar]

---

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	N/A
Primary OS tested on	Ubuntu Jammy
Robotic platform tested on	own simulation

---

Description of contribution in a few bullet points

Sometimes the BT nodes are launched before the action servers (as part of a launch file) causing the BT nodes initialization to fail, so I increased the timeout from 1 to 10 seconds

For Maintainers:

• Check that any new parameters added are updated in navigation.ros.org
• Check that any significant change is added to the migration guide
• Check that any new features OR changes to existing behaviors are reflected in the tuning guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[mergify]

@tonynajjar, please properly fill in PR template in the future. @SteveMacenski, use this instead.

• Check that any new parameters added are updated in navigation.ros.org
• Check that any significant change is added to the migration guide
• Check that any new features OR changes to existing behaviors are reflected in the tuning guide
• Check that any new functions have Doxygen added
• Check that any new features have test coverage
• Check that any new plugins is added to the plugins page
• If BT Node, Additionally: add to BT's XML index of nodes for groot, BT package's readme table, and BT library lists

[codecov]

Codecov Report

Base: 82.24% // Head: 82.10% // Decreases project coverage by -0.14% ⚠️

Coverage data is based on head (ec42e5a) compared to base (21eea4c).
Patch coverage: 100.00% of modified lines in pull request are covered.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3259      +/-   ##
==========================================
- Coverage   82.24%   82.10%   -0.15%     
==========================================
  Files         341      341              
  Lines       15537    15537              
==========================================
- Hits        12779    12756      -23     
- Misses       2758     2781      +23     

Impacted Files	Coverage Δ
...tree/include/nav2_behavior_tree/bt_action_node.hpp	80.70% <100.00%> (ø)
...av2_dwb_controller/dwb_critics/src/oscillation.cpp	72.00% <0.00%> (-21.34%)	⬇️
...stmap_2d/plugins/costmap_filters/binary_filter.cpp	94.80% <0.00%> (-2.60%)	⬇️
..._amcl/src/sensors/laser/likelihood_field_model.cpp	95.00% <0.00%> (-2.50%)	⬇️
...ostmap_2d/plugins/costmap_filters/speed_filter.cpp	95.18% <0.00%> (-2.41%)	⬇️
nav2_lifecycle_manager/src/lifecycle_manager.cpp	91.12% <0.00%> (-0.94%)	⬇️

Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here.

☔ View full report at Codecov.
📢 Do you have feedback about the report comment? Let us know in this issue.

[jwallace42]

I assume you are seeing this behavior in a custom launch file? I am not able to reproduce with the default launch files.

You would also have to extend this timeout for the other Bt base classes. (service, cancel ....).

I think the best approach is to pull this out into a parameter and allow the user to set the max wait time.

[SteveMacenski]

This shouldn’t be an issue when using lifecycle nodes. the lifecycle manager only brings up constituent nodes after the previous others are already initialized. Maybe you just need to select a more reasonable order for node initialization of the navigation system? The order of nodes in the launch file given to the lifecycle manager is not arbitrary. It is ordered so that the last things are dependent on the previous so it doesn’t come up until they’re already so.

Please explain 🙂

[SteveMacenski]

any update?

[tonynajjar]

not yet but not forgotten! I'm busy migrating the whole system to Humble and this was one small hickup we had. I'll update once I get back to it

[tonynajjar]

I assume you are seeing this behavior in a custom launch file? I am not able to reproduce with the default launch files.

yes custom launch files, I should have mentioned that

This shouldn’t be an issue when using lifecycle nodes. the lifecycle manager only brings up constituent nodes after the previous others are already initialized. Maybe you just need to select a more reasonable order for node initialization of the navigation system?

I see what you mean and I think I see the issue now. I have custom BT Action nodes as well as custom behaviors/action servers. These action servers are launched non-deterministically in the launch file; they are python nodes so unfortunately I can't use lifecycle nodes.

Do you have other recommendations for my case other than increasing the timeout?

[SteveMacenski]

They are python nodes so unfortunately I can't use lifecycle nodes

Can't you? https://github.com/ros2/rclpy/tree/rolling/rclpy/rclpy/lifecycle they appear to be in Humble and Rolling. Demo: https://github.com/ros2/demos/tree/humble/lifecycle_py

But otherwise, I believe that Launch is deterministic in execution order provided, so you can bring up all the servers in the launch file first before the BT navigator. Maybe that'll be enough?

Hackier: use the launch prefix field to basically ask for a sleep as this answer points out: https://answers.ros.org/question/233353/set-delay-between-starting-nodes-within-launch-file/ for the BT navigator node to give the other nodes times to initialize.

Better yet: https://www.reddit.com/r/ROS/comments/qxbi3z/ros2_is_it_possible_to_have_something_delayed_at/ there's a timed delay action launch node it appears.

Best: use the rclpy lifecycle nodes

[tonynajjar]

Can't you? https://github.com/ros2/rclpy/tree/rolling/rclpy/rclpy/lifecycle they appear to be in Humble and Rolling

Best: use the rclpy lifecycle nodes

Oh I thought these would never see the light, thanks for the info. But it's too much work for the moment

But otherwise, I believe that Launch is deterministic in execution order provided, so you can bring up all the servers in the launch file first before the BT navigator. Maybe that'll be enough?

really? But even then I don't think launching the action servers a couple of seconds earlier would fix the issue

there's a timed delay action launch node it appears.

Yes I know about this one, it could work

I still think increasing the timeout would be the easiest/quickest solution, do you see any drawbacks to it (apart from users having to wait a bit longer to see an error message)

[SteveMacenski]

But it's too much work for the moment

😢

do you see any drawbacks to it

That's a long delay to fail and would give users a bad user-experience if they launched BTs with task servers legit just not existing. Waiting 10 seconds to throw and then fail the navigation task is not very "reactive". As a user of this, I'd be pretty frustrated to have to wait 10 seconds to be told of a trivial typo error while prototyping.

[tonynajjar]

Alright then, if there is no solution to contribute here, feel free to close the PR. Thanks for the alternatives, will look into them more.

[SteveMacenski]

OK - sorry for not being able to get this one in. But I think there are some optimal (lifecycle py) options and sketchier but suitable solutions (timed action) at launch time.