Add a timeout to the wait for transforms step of the costmap activation. #3866

kfabian · 2023-10-07T21:34:19Z

Basic Info

Info	Please fill out this column
Ticket(s) this addresses	#3749
Primary OS tested on	Ubuntu 22.04
Robotic platform tested on	only tested without robot by optionally manually providing the transform

Description of contribution in a few bullet points

Previously the on_activate method of the costmap was blocking indefinitely if the transform from the robot base frame to the global frame is not available
This adds a timeout so that the transition from configured to active fails if the transform is not available
The timeout can be configured by a costmap parameter
The controller server transition from configured to active fails if the corresponding costmap transitions fails

Description of documentation updates required from your changes

There is a new costmap parameter: wait_for_transforms_timeout

Future work that may be required in bullet points

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

Signed-off-by: Fabian König <fabiankoenig@gmail.com>

SteveMacenski

Generally speaking, LGTM!

nav2_controller/src/controller_server.cpp

nav2_costmap_2d/src/costmap_2d_ros.cpp

SteveMacenski · 2023-10-09T17:24:05Z

Alot of the system tests failed. I'm going to rerun them again in case that was a fluke, but that might be pointing to some activation errors this PR introduces

kfabian · 2023-10-09T19:32:29Z

Alot of the system tests failed. I'm going to rerun them again in case that was a fluke, but that might be pointing to some activation errors this PR introduces

Maybe the new timeout is just too low for tests with the gazebo simulation? Unfortunately I could not get the turtlebot simulation running on my Ubuntu arm64 virtual machine.

SteveMacenski

Still needs docs PR I described but otherwise LGTM!

codecov · 2023-10-09T21:01:20Z

Codecov Report

All modified lines are covered by tests ✅

Comparison is base (57dd8d4) 90.22% compared to head (47249d8) 90.34%.
Report is 2 commits behind head on main.

Additional details and impacted files

@@            Coverage Diff             @@
##             main    #3866      +/-   ##
==========================================
+ Coverage   90.22%   90.34%   +0.12%     
==========================================
  Files         413      413              
  Lines       18310    18316       +6     
==========================================
+ Hits        16520    16548      +28     
+ Misses       1790     1768      -22

Files	Coverage Δ
nav2_controller/src/controller_server.cpp	`89.22% <100.00%> (ø)`
...tmap_2d/include/nav2_costmap_2d/costmap_2d_ros.hpp	`100.00% <ø> (ø)`
nav2_costmap_2d/src/costmap_2d_ros.cpp	`88.16% <100.00%> (+0.21%)`	⬆️
nav2_planner/src/planner_server.cpp	`93.31% <100.00%> (ø)`

... and 5 files with indirect coverage changes

☔ View full report in Codecov by Sentry.
📢 Have feedback on the report? Share it here.

kfabian · 2023-10-09T21:32:50Z

@SteveMacenski the CI is complaining about reduced test coverage. Looking at the costmap test folder I'm wondering where a suitable test should be located.

SteveMacenski · 2023-10-09T21:47:48Z

A new unit test file would be fine!

kfabian · 2023-10-12T19:48:59Z

Still needs docs PR I described but otherwise LGTM!

I created the docs PR: ros-navigation/docs.nav2.org#477

SteveMacenski · 2023-10-12T20:22:23Z

Waiting on CI

Signed-off-by: Fabian König <fabiankoenig@gmail.com>

SteveMacenski · 2023-10-12T21:48:41Z

Thanks!!

…on. (#3866) * Add a timeout to the wait for transforms step of the costmap activation. Signed-off-by: Fabian König <fabiankoenig@gmail.com> * Rename wait_for_transforms_timeout to initial_transform_timeout * Activate costmap publishers only after transforms are checked * Check if controller server activation was succesful in planner_server * Add unittest for costmap activation Signed-off-by: Fabian König <fabiankoenig@gmail.com> --------- Signed-off-by: Fabian König <fabiankoenig@gmail.com>

* collision_monitor: dynamic polygon and source enable/disable (#3825) * Rename PushRosNamespace to PushROSNamespace * Fix min_points checking * initial * fix * fix * remove unrelated change * reset * fix format * PR fixes * Add test * fix comments * add to params * publish only if enabled * Add source dynamic enable/disable * add enabled param to sources * fix * add same to collision detector * Update README.md: fix typo (#3842) * Update package.xml * fix typo (#3850) * adjust link to point to v3.8 of behavior tree docs (#3851) BT.CPP_v3 is used, thereby the correct docs should be linked * Fix bug in nav2_behavior_tree/bt_action_node (#3849) * Fix bug in nav2_behavior_tree/bt_action_node * Fixed the bug in halt function inside nav2_behavior_tree/plugin/action/bt_action_node.hpp * Added new case to nav2_behavior_tree/plugin/action/bt_action_node.hpp for testing the scenario to cancel * Refactored existing cases in nav2_behavior_tree/plugin/action/bt_action_node.hpp Signed-off-by: CihatAltiparmak <cihataltiparmak1@gmail.com> * Fix bug in nav2_behavior_tree/bt_action_node * Fixed the bug in halt function inside nav2_behavior_tree/plugin/action/bt_action_node.hpp * Added new case to nav2_behavior_tree/plugin/action/bt_action_node.hpp for testing the scenario to cancel * Refactored existing cases in nav2_behavior_tree/plugin/action/bt_action_node.hpp Signed-off-by: CihatAltiparmak <cihataltiparmak1@gmail.com> --------- Signed-off-by: CihatAltiparmak <cihataltiparmak1@gmail.com> * Fix action test failure due to rate after Rolling sync API change (#3852) * Update CMakeLists.txt (#3843) * add option for sse4 and avs512 (#3853) * Remove all exit(-1) crash conditions (#3846) * Update transform_available_condition.cpp * wrapping all examples of get_plugin_type_param in exceptions and making that throw instead of crash * some linting * fix typo * Update controller.cpp * Update nav2_params.yaml * Update nav2_params.yaml * simplication of lattice_generator.py, fixes #3858 (#3859) * simplification of equation to compute the max_value/outer edge of the lattice based on number of headings. * Stop error diagnostics when pausing nav (#3830) * Added nodestate enum and a variable to keep track of current state of managed nodes. * Updating state_of_managed_nodes_ when switching states and using it to determine an accurate diagnostics message. * Fixing bugs. * Updated/added docstrings. * Publishing OK status when nodes are unconfigured. Changed if-else chain to switch case. * Renamed NodeState PAUSED to INACTIVE, state_of_managed_nodes_ to managed_nodes_state_ and replaced system_active_ with an inline method. * Bugfix. --------- Co-authored-by: Pekka Myller <pekka.myller@karelics.fi> * Add a timeout to the wait for transforms step of the costmap activation. (#3866) * Add a timeout to the wait for transforms step of the costmap activation. Signed-off-by: Fabian König <fabiankoenig@gmail.com> * Rename wait_for_transforms_timeout to initial_transform_timeout * Activate costmap publishers only after transforms are checked * Check if controller server activation was succesful in planner_server * Add unittest for costmap activation Signed-off-by: Fabian König <fabiankoenig@gmail.com> --------- Signed-off-by: Fabian König <fabiankoenig@gmail.com> * Fix class doxygen * fix minor typos (#3892) Signed-off-by: Anton Kesy <antonkesy@gmail.com> * Publish collision points for debug purposes (#3879) * Rename PushRosNamespace to PushROSNamespace * Fix min_points checking * . * fixes * add to collision detector * fix * fix * . * fixes * add namespace to topic * fixes * fix use after free (#3910) * fix build mppi (#3927) Signed-off-by: kevin <kevin@floatic.io> Co-authored-by: kevin <kevin@floatic.io> * Removing old TODOs * protect invalid max_velocity min_velocity (#3953) Co-authored-by: Guillaume Doisy <guillaume@dexory.com> * protect properly max_accel and max_decel (#3952) Co-authored-by: Guillaume Doisy <guillaume@dexory.com> * Fixed links for install and build in README (#3963) Currently the readme is linking to an invalida page in the docs (404 error). * adding support for rotate in place cusps (#3934) * Fix linting error (#3969) * Fix linting error * Update regulated_pure_pursuit_controller.cpp * fix a few outdated comments in smac planners (#3978) * adding soft realtime prioritization for collision monitor and velocity smoother (#3979) * adding soft realtime prioritization for collision monitor and velocity smoother * refactor simple action server to use new utils API * Update README.md * Synchronize map size information during map initialization (#4015) * Update costmap size configuration This commit updates the costmap_2d.cpp file to fix a bug where the costmap size wasn't appropriately updated. Two new lines of code have been added to ensure the size of the costmap is correctly configured each time it's instantiated. * Refactor costmap size assignment in Costmap2D class The code refactor eliminates the direct mutation of the size_x_ and size_y_ attributes in the Costmap2D class. Instead, the class uses the size of cells provided during initialization and calculation from map coordinates for better encapsulation and clarity. * check width&height params (#4017) Co-authored-by: GoesM <GoesM@buaa.edu.cn> * Fix SimpleActionServer nullprt callback (#4025) * add check before calling completion_callback_ * Fix lint * footprint checks (#4030) * footprint checks Signed-off-by: gg <josho.wallace@gmail.com> * lint fix Signed-off-by: gg <josho.wallace@gmail.com> --------- Signed-off-by: gg <josho.wallace@gmail.com> * Is path valid doc (#4032) * docs Signed-off-by: gg <josho.wallace@gmail.com> * update Signed-off-by: gg <josho.wallace@gmail.com> --------- Signed-off-by: gg <josho.wallace@gmail.com> * Update vision_opencv's branch for rolling Signed-off-by: Steve Macenski <stevenmacenski@gmail.com> * handle dynamically changes in parameters. (#4046) Signed-off-by: Sebastian Solarte <johan.solarte@kiwibot.com> * Add inflation_layer_name param (#4047) Signed-off-by: Renan Salles <renan028@gmail.com> * missing urdf dep (#4050) Signed-off-by: Guillaume Doisy <guillaume@dexory.com> Co-authored-by: Guillaume Doisy <guillaume@dexory.com> * bump to 1.2.6 for release --------- Signed-off-by: CihatAltiparmak <cihataltiparmak1@gmail.com> Signed-off-by: Fabian König <fabiankoenig@gmail.com> Signed-off-by: Anton Kesy <antonkesy@gmail.com> Signed-off-by: kevin <kevin@floatic.io> Signed-off-by: gg <josho.wallace@gmail.com> Signed-off-by: Steve Macenski <stevenmacenski@gmail.com> Signed-off-by: Sebastian Solarte <johan.solarte@kiwibot.com> Signed-off-by: Renan Salles <renan028@gmail.com> Signed-off-by: Guillaume Doisy <guillaume@dexory.com> Co-authored-by: Tony Najjar <tony.najjar@logivations.com> Co-authored-by: thandal <than@timbrel.org> Co-authored-by: Anton Kesy <antonkesy@gmail.com> Co-authored-by: CihatAltiparmak <cihataltiparmak1@gmail.com> Co-authored-by: Anil Kumar Chavali <44644339+akchobby@users.noreply.github.com> Co-authored-by: Plaqueoff <44152820+Plaqueoff@users.noreply.github.com> Co-authored-by: Pekka Myller <pekka.myller@karelics.fi> Co-authored-by: Fabian König <fabiankoenig@gmail.com> Co-authored-by: 정찬희 <60467877+ladianchad@users.noreply.github.com> Co-authored-by: kevin <kevin@floatic.io> Co-authored-by: Guillaume Doisy <doisyg@users.noreply.github.com> Co-authored-by: Guillaume Doisy <guillaume@dexory.com> Co-authored-by: Abiel Fernandez <empoleom@gmail.com> Co-authored-by: Michael Ferguson <mfergs7@gmail.com> Co-authored-by: Hao-Li-Bachelorarbeit <141755843+Hao-Li-Bachelorarbeit@users.noreply.github.com> Co-authored-by: GoesM <130988564+GoesM@users.noreply.github.com> Co-authored-by: GoesM <GoesM@buaa.edu.cn> Co-authored-by: BriceRenaudeau <48433002+BriceRenaudeau@users.noreply.github.com> Co-authored-by: Joshua Wallace <josho.wallace@gmail.com> Co-authored-by: Sebastian Solarte <89881453+Sunart24@users.noreply.github.com> Co-authored-by: Renan Salles <renan028@gmail.com>

…on. (ros-navigation#3866) * Add a timeout to the wait for transforms step of the costmap activation. Signed-off-by: Fabian König <fabiankoenig@gmail.com> * Rename wait_for_transforms_timeout to initial_transform_timeout * Activate costmap publishers only after transforms are checked * Check if controller server activation was succesful in planner_server * Add unittest for costmap activation Signed-off-by: Fabian König <fabiankoenig@gmail.com> --------- Signed-off-by: Fabian König <fabiankoenig@gmail.com> Signed-off-by: enricosutera <enricosutera@outlook.com>

Add a timeout to the wait for transforms step of the costmap activation.

677e1bc

Signed-off-by: Fabian König <fabiankoenig@gmail.com>

SteveMacenski requested changes Oct 9, 2023

View reviewed changes

nav2_controller/src/controller_server.cpp Show resolved Hide resolved

nav2_costmap_2d/src/costmap_2d_ros.cpp Outdated Show resolved Hide resolved

nav2_costmap_2d/src/costmap_2d_ros.cpp Outdated Show resolved Hide resolved

kfabian added 3 commits October 9, 2023 19:45

Rename wait_for_transforms_timeout to initial_transform_timeout

b63ac5e

Activate costmap publishers only after transforms are checked

2b87f87

Check if controller server activation was succesful in planner_server

1951faa

SteveMacenski approved these changes Oct 9, 2023

View reviewed changes

Add unittest for costmap activation

47249d8

Signed-off-by: Fabian König <fabiankoenig@gmail.com>

kfabian force-pushed the costmap_activate_timeout branch from f50d6c1 to 47249d8 Compare October 12, 2023 20:50

SteveMacenski merged commit 461a7ba into ros-navigation:main Oct 12, 2023
4 checks passed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add a timeout to the wait for transforms step of the costmap activation. #3866

Add a timeout to the wait for transforms step of the costmap activation. #3866

kfabian commented Oct 7, 2023

SteveMacenski left a comment

SteveMacenski commented Oct 9, 2023

kfabian commented Oct 9, 2023

SteveMacenski left a comment

codecov bot commented Oct 9, 2023 •

edited

Loading

kfabian commented Oct 9, 2023

SteveMacenski commented Oct 9, 2023 •

edited

Loading

kfabian commented Oct 12, 2023

SteveMacenski commented Oct 12, 2023

SteveMacenski commented Oct 12, 2023

Add a timeout to the wait for transforms step of the costmap activation. #3866

Add a timeout to the wait for transforms step of the costmap activation. #3866

Conversation

kfabian commented Oct 7, 2023

Basic Info

Description of contribution in a few bullet points

Description of documentation updates required from your changes

Future work that may be required in bullet points

For Maintainers:

SteveMacenski left a comment

Choose a reason for hiding this comment

SteveMacenski commented Oct 9, 2023

kfabian commented Oct 9, 2023

SteveMacenski left a comment

Choose a reason for hiding this comment

codecov bot commented Oct 9, 2023 • edited Loading

Codecov Report

kfabian commented Oct 9, 2023

SteveMacenski commented Oct 9, 2023 • edited Loading

kfabian commented Oct 12, 2023

SteveMacenski commented Oct 12, 2023

SteveMacenski commented Oct 12, 2023

codecov bot commented Oct 9, 2023 •

edited

Loading

SteveMacenski commented Oct 9, 2023 •

edited

Loading