-
Notifications
You must be signed in to change notification settings - Fork 1.3k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Port system tests in nav2_system_tests #4440
Port system tests in nav2_system_tests #4440
Conversation
Signed-off-by: stevedan <[email protected]>
@SteveMacenski I have a question about something I am experiencing. I ported just the test_bt_navigator for now. When I run the test for the first time, everything works fine,
When I run for the second time, the estimation of the robot pose goes awall and return a wrong value hence in some scenario the robot is not able to find a path to the goal.
It seem it is caching something somehow. The behavior is consistent. See the video below with rviz. The test runs fine but since the location estimate is wrong, it fails during the planning/navigation path. I suspect that the Odom tf is not cleaned properly. issue.mp4 |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
This pull request is in conflict. Could you fix it @stevedanomodolor? |
@stevedanomodolor can you check if gazebo is still running? That seems to be the issue that its not shutting down so the instance is still up keeping the robot alive at its position. Between tests, check Is this reasonably deterministic or how often does this happen? It was maybe 1/100 in CI for Gazebo Classic which caused a level of flakiness, but wasn't frequent enough that caused us much concern. If this is deterministic or like 1+/20 that is a problem. |
@SteveMacenski it deterministic and it happens consistently. The first time it works, on the second time it fails. When i run multiple tests at once, like the test_bt_navigator tests, the first one passes but the seconds fails with the same behavior. Just to add info, I am running everything using dockers, and I always almost(not to say always) have to shut down the docker and restart ot again for it to work. |
After killing the process from ps aux, I was able to run the tests. I do have a workaround to ensure all gz sim are clean after every test, but that does not tackle the root cause of why the gazebo does not shut down correctly. |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: Stevedan Ogochukwu Omodolor <[email protected]>
Signed-off-by: stevedan <[email protected]>
@azeey FYI - a gazebo CI deterministic bottleneck with an ... unappealing workaround to |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
ros_gz_sim_dir = get_package_share_directory('ros_gz_sim') | ||
|
||
world_sdf_xacro = os.path.join(sim_dir, 'worlds', 'tb3_sandbox.sdf.xacro') | ||
robot_sdf = os.path.join(sim_dir, 'urdf', 'gz_waffle.sdf') |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think this will include a .xacro
shortly from the changes in ros-navigation/nav2_minimal_turtlebot_simulation#11 FYI (probably other launch files too). Just be aware that this change is coming :-) (for all instances)
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
this will affect the localization test though. What if you add both there after I correct the test I have ported with the new xacro, then I will delete it from there.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I think you can leave it as is, I'm just giving you an FYI that this will change once I get a chance on Friday to test the multirobot fixes to merge those in. So at that point, these would need to be updated once merged.
If this is merged before I make the changes, then I'll fix these tests. If its not merged, then you'll need to fix them. Its a race! 😆
nav2_system_tests/src/system/test_system_with_obstacle_launch.py
Outdated
Show resolved
Hide resolved
This is all really great, thanks so much for your time and effort! Lets leave this PR in the current state and not add more to it - add to new PRs instead to keep them bite sized for review / merging. Often 4-5 PRs can be merged and integrated faster than 1 mega-PR |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
as soon as this is merged, then I can merge other tests faster because I use the kill gz sim utils |
Minor linting issue and we can merge! |
How did you check this?. I am not able to see this locally |
Look at the failed CI job - it shows there |
Signed-off-by: stevedan <[email protected]>
Signed-off-by: stevedan <[email protected]>
@SteveMacenski I am not sure exactly why its failing. Am I missing some information somewhere? |
Merged! |
* wip, ported only test_bt_navigator Signed-off-by: stevedan <[email protected]> * include test_bt_navigator_with dijlstra and test_bt_navigator_2 Signed-off-by: stevedan <[email protected]> * uncomment some lines Signed-off-by: stevedan <[email protected]> * More tests Signed-off-by: stevedan <[email protected]> * Include end of line Signed-off-by: stevedan <[email protected]> * move gz_sim cleanup process to utils nav2_simple_commander Signed-off-by: stevedan <[email protected]> * fix linter Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * removed unused path Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * more cleanup Signed-off-by: stevedan <[email protected]> * reduce set initial pose time Signed-off-by: stevedan <[email protected]> * remove repeated variable Signed-off-by: stevedan <[email protected]> * Remove log Signed-off-by: stevedan <[email protected]> * Remove todo Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * include copyright Signed-off-by: stevedan <[email protected]> * correct year Signed-off-by: stevedan <[email protected]> --------- Signed-off-by: stevedan <[email protected]> Signed-off-by: Stevedan Ogochukwu Omodolor <[email protected]>
* wip, ported only test_bt_navigator Signed-off-by: stevedan <[email protected]> * include test_bt_navigator_with dijlstra and test_bt_navigator_2 Signed-off-by: stevedan <[email protected]> * uncomment some lines Signed-off-by: stevedan <[email protected]> * More tests Signed-off-by: stevedan <[email protected]> * Include end of line Signed-off-by: stevedan <[email protected]> * move gz_sim cleanup process to utils nav2_simple_commander Signed-off-by: stevedan <[email protected]> * fix linter Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * removed unused path Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * more cleanup Signed-off-by: stevedan <[email protected]> * reduce set initial pose time Signed-off-by: stevedan <[email protected]> * remove repeated variable Signed-off-by: stevedan <[email protected]> * Remove log Signed-off-by: stevedan <[email protected]> * Remove todo Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * include copyright Signed-off-by: stevedan <[email protected]> * correct year Signed-off-by: stevedan <[email protected]> --------- Signed-off-by: stevedan <[email protected]> Signed-off-by: Stevedan Ogochukwu Omodolor <[email protected]>
* wip, ported only test_bt_navigator Signed-off-by: stevedan <[email protected]> * include test_bt_navigator_with dijlstra and test_bt_navigator_2 Signed-off-by: stevedan <[email protected]> * uncomment some lines Signed-off-by: stevedan <[email protected]> * More tests Signed-off-by: stevedan <[email protected]> * Include end of line Signed-off-by: stevedan <[email protected]> * move gz_sim cleanup process to utils nav2_simple_commander Signed-off-by: stevedan <[email protected]> * fix linter Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * removed unused path Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * more cleanup Signed-off-by: stevedan <[email protected]> * reduce set initial pose time Signed-off-by: stevedan <[email protected]> * remove repeated variable Signed-off-by: stevedan <[email protected]> * Remove log Signed-off-by: stevedan <[email protected]> * Remove todo Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * include copyright Signed-off-by: stevedan <[email protected]> * correct year Signed-off-by: stevedan <[email protected]> --------- Signed-off-by: stevedan <[email protected]> Signed-off-by: Stevedan Ogochukwu Omodolor <[email protected]>
* wip, ported only test_bt_navigator Signed-off-by: stevedan <[email protected]> * include test_bt_navigator_with dijlstra and test_bt_navigator_2 Signed-off-by: stevedan <[email protected]> * uncomment some lines Signed-off-by: stevedan <[email protected]> * More tests Signed-off-by: stevedan <[email protected]> * Include end of line Signed-off-by: stevedan <[email protected]> * move gz_sim cleanup process to utils nav2_simple_commander Signed-off-by: stevedan <[email protected]> * fix linter Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * removed unused path Signed-off-by: stevedan <[email protected]> * cleanup Signed-off-by: stevedan <[email protected]> * more cleanup Signed-off-by: stevedan <[email protected]> * reduce set initial pose time Signed-off-by: stevedan <[email protected]> * remove repeated variable Signed-off-by: stevedan <[email protected]> * Remove log Signed-off-by: stevedan <[email protected]> * Remove todo Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * use robot publisher Signed-off-by: stevedan <[email protected]> * include copyright Signed-off-by: stevedan <[email protected]> * correct year Signed-off-by: stevedan <[email protected]> --------- Signed-off-by: stevedan <[email protected]> Signed-off-by: Stevedan Ogochukwu Omodolor <[email protected]>
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: