Skip to content

cherry-picking mjeronimo:fix_bt_crash_after_reset#1515

Merged
SteveMacenski merged 1 commit into
ros-navigation:masterfrom
Jconn:fix_bt_crash_after_thread_swap
Feb 11, 2020
Merged

cherry-picking mjeronimo:fix_bt_crash_after_reset#1515
SteveMacenski merged 1 commit into
ros-navigation:masterfrom
Jconn:fix_bt_crash_after_thread_swap

Conversation

@Jconn
Copy link
Copy Markdown
Contributor

@Jconn Jconn commented Feb 11, 2020

This is #1322 targeting head of master.

Like that ticket says,

Rather than trying to re-use the behavior tree, create the behavior tree each time NavigateToPose is called. This avoids some problems in the BT library when using CoroActionNodes (coroutines).

should fix
#1285
and
#1439

Recreation of the tree is taking 200-300ms in my test environment. This delay directly adds to the time it takes the nav stack to receive to a NavigateToPose request.

Future work can be done to reduce this time.

Tested with rviz start/stop commands in simulation and a script with some start/stop commands.

@codecov
Copy link
Copy Markdown

codecov Bot commented Feb 11, 2020

Codecov Report

Merging #1515 into master will increase coverage by 0.01%.
The diff coverage is 0%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1515      +/-   ##
==========================================
+ Coverage   37.53%   37.54%   +0.01%     
==========================================
  Files         226      226              
  Lines       11774    11762      -12     
  Branches     5084     5080       -4     
==========================================
- Hits         4419     4416       -3     
+ Misses       3981     3977       -4     
+ Partials     3374     3369       -5
Flag Coverage Δ
#project 37.54% <0%> (+0.01%) ⬆️
Impacted Files Coverage Δ
...nclude/nav2_behavior_tree/behavior_tree_engine.hpp 100% <ø> (+83.33%) ⬆️
nav2_behavior_tree/src/behavior_tree_engine.cpp 26.31% <ø> (ø) ⬆️
nav2_bt_navigator/src/bt_navigator.cpp 32.5% <0%> (-1.36%) ⬇️
nav2_util/src/robot_utils.cpp 10% <0%> (-10%) ⬇️
...ontroller/dwb_plugins/src/kinematic_parameters.cpp 14.43% <0%> (-4.13%) ⬇️
nav2_costmap_2d/src/collision_checker.cpp 50% <0%> (-4.06%) ⬇️
nav2_navfn_planner/src/navfn.cpp 45.68% <0%> (-0.22%) ⬇️
nav2_controller/src/nav2_controller.cpp 27.86% <0%> (ø) ⬆️
nav2_map_server/src/map_server.cpp 42.85% <0%> (ø) ⬆️
nav2_amcl/src/amcl_node.cpp 38.84% <0%> (+0.78%) ⬆️
... and 2 more

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 7a4c272...d1e3512. Read the comment docs.

@SteveMacenski
Copy link
Copy Markdown
Member

Can you file a ticket before we merge this on the navigation time to process?

@Jconn
Copy link
Copy Markdown
Contributor Author

Jconn commented Feb 11, 2020

added a ticket for reducing time it takes to process request

@SteveMacenski SteveMacenski merged commit 2a37b51 into ros-navigation:master Feb 11, 2020
@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Feb 11, 2020

Awesome, I really appreciate the help. I wouldn't have been able to get to this for awhile. Plus removes 2 critical tagged issues.

SteveMacenski pushed a commit to SteveMacenski/navigation2 that referenced this pull request Mar 2, 2020
SteveMacenski added a commit that referenced this pull request Mar 3, 2020
* added a red circle for target pose location. changed the way marker id numbers are generated. resized markers

* edited a comment line

* cherry-picking mjeronimo:fix_bt_crash_after_reset (#1515)

* syncing eloquent with 1940266

* bump to 0.3.4

Co-authored-by: Melih Erdogan <h.meliherdogan@gmail.com>
Co-authored-by: John Connolly <johnconn@umich.edu>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants