Skip to content

Refactor System Tests#1243

Merged
orduno merged 7 commits intoros-navigation:masterfrom
orduno:system_tests_refactor
Oct 21, 2019
Merged

Refactor System Tests#1243
orduno merged 7 commits intoros-navigation:masterfrom
orduno:system_tests_refactor

Conversation

@orduno
Copy link
Copy Markdown
Contributor

@orduno orduno commented Oct 15, 2019

Basic Info

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

Description of contribution in a few bullet points

Adding multi-robot system tests and addressing #1135 and #813.

  • Created a NavTester node which can be used for testing with single or multiple robots.
  • Added launch files to test multiple robots.
  • Added TB3 model files needed for running tests on ROS build farm.
  • Updated spawn_robot to accept any robot sdf description.

@orduno orduno mentioned this pull request Oct 15, 2019
Closed
@orduno orduno self-assigned this Oct 15, 2019
@orduno orduno added this to the E Turtle Release milestone Oct 15, 2019
SteveMacenski
SteveMacenski approved these changes Oct 15, 2019
View reviewed changes
Copy link
Copy Markdown
Member

@SteveMacenski SteveMacenski left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont have a huge opinion here, but would it not make sense to use a git submodule to link up the turtlebot parts vs keeping copies here? Catkin also included a catkin_download CMake macro to get assets for testing (bags, URDF, etc), maybe be good to use that here too?

@orduno orduno force-pushed the system_tests_refactor branch from 5de2a1b to 32e17b0 Compare October 16, 2019 21:45
@codecov
Copy link
Copy Markdown

codecov bot commented Oct 16, 2019 •
edited
Loading

Codecov Report

Merging #1243 into master will decrease coverage by 8.77%.
The diff coverage is n/a.

Impacted file tree graph

@@            Coverage Diff             @@
##           master    #1243      +/-   ##
==========================================
- Coverage   35.98%   27.21%   -8.78%     
==========================================
  Files         243      244       +1     
  Lines       11012    11005       -7     
  Branches     4294     3470     -824     
==========================================
- Hits         3963     2995     -968     
- Misses       4436     6024    +1588     
+ Partials     2613     1986     -627
Flag Coverage Δ
#project 27.21% <ø> (-8.78%) ⬇️
Impacted Files Coverage Δ
...nerator_cpp/dwb_msgs/msg/trajectory2_d__struct.hpp 0% <0%> (-100%) ⬇️
...ns/include/dwb_plugins/standard_traj_generator.hpp 0% <0%> (-100%) ⬇️
...ller/dwb_critics/include/dwb_critics/goal_dist.hpp 0% <0%> (-100%) ⬇️
...ller/dwb_critics/include/dwb_critics/path_dist.hpp 0% <0%> (-100%) ⬇️
...ugins/include/dwb_plugins/kinematic_parameters.hpp 0% <0%> (-100%) ⬇️
...dwb_critics/include/dwb_critics/rotate_to_goal.hpp 0% <0%> (-100%) ⬇️
.../dwb_critics/include/dwb_critics/base_obstacle.hpp 0% <0%> (-100%) ⬇️
...gation2/nav2_amcl/include/nav2_amcl/angleutils.hpp 0% <0%> (-100%) ⬇️
...or_cpp/nav_2d_msgs/msg/pose2_d_stamped__struct.hpp 0% <0%> (-100%) ⬇️
..._2d/include/nav2_costmap_2d/observation_buffer.hpp 0% <0%> (-100%) ⬇️
... and 82 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 caea1d2...73b10a7. Read the comment docs.

@orduno orduno mentioned this pull request Oct 16, 2019
Merged
@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Oct 17, 2019

I'm deploying an ament_download that's equal to catkin_download for you

samsung-ros/ament_download#1

@orduno
Copy link
Copy Markdown
Contributor Author

orduno commented Oct 17, 2019

Yes, I'd prefer that option. However, I know we recently modified some TB files to work with the addition of --ros-args. I'll give it a try.

@orduno
Copy link
Copy Markdown
Contributor Author

orduno commented Oct 17, 2019

Actually, can we leave that change to a follow-up PR? I'm already doing a bit of changes.

@SteveMacenski
Copy link
Copy Markdown
Member

SteveMacenski commented Oct 17, 2019

that's fine

@orduno orduno mentioned this pull request Oct 18, 2019
Closed
ghost
ghost approved these changes Oct 21, 2019
View reviewed changes
nav2_bt_navigator/behavior_trees/navigate_w_replanning_and_recovery.xml Outdated
<root main_tree_to_execute="MainTree">
<BehaviorTree ID="MainTree">
<RecoveryNode number_of_retries="6" name="NavigateRecovery">
<RecoveryNode number_of_retries="6">
Copy link
Copy Markdown

@ghost ghost Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why are we removing the names here?

Copy link
Copy Markdown
Contributor Author

@orduno orduno Oct 21, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

hmm, I'll add the names back. Not sure why these got deleted.

orduno added 6 commits October 21, 2019 16:20
@orduno
Creating a system test with multiple robots
a64ef05 
Removing dependency on TB package
@orduno
Generalized tester node to handle arbitrary number of robots
2481fc9
@orduno
Refactoring system tests
1d2fee6 
* Re-using launch files in nav2_bringup
* Consolidating multi and single robot tests
* Removing turtlebot pkgs dependencies
@orduno
Rebasing & addressing linter fails
ba97693
@orduno
Fix typo
20753ec
@orduno
Rebasing
9cb8957
@orduno orduno force-pushed the system_tests_refactor branch from 40ed6bf to 9cb8957 Compare October 21, 2019 23:29
@orduno orduno merged commit 5d1319e into ros-navigation:master Oct 21, 2019
This was referenced Oct 21, 2019
Closed
Closed
@orduno orduno deleted the system_tests_refactor branch October 21, 2019 23:46
@orduno orduno mentioned this pull request Oct 22, 2019
Closed
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
@orduno
Refactor system tests and add multi-robot testing (ros-navigation#1243)
fa92095 
* Generalized tester node (NavTester) to handle an arbitrary number of robots.
* Created a test that runs two robots in parallel to test multi-robot capability.
* Added TB3 model files needed for running tests on ROS build farm.
* Updated spawn_robot to accept any robot sdf description.
* Re-using some launch files used in `nav2_bringup`.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

@SteveMacenski SteveMacenski SteveMacenski approved these changes

+1 more reviewer
Reviewers whose approvals may not affect merge requirements

Assignees

@orduno orduno

Labels

None yet

Projects

None yet

Milestone

Eloquent (E Turtle) Release

Development

Successfully merging this pull request may close these issues.

2 participants

@orduno @SteveMacenski