navigation2 icon indicating copy to clipboard operation
navigation2 copied to clipboard

Add nav2_fake_localization

Open jihoonl opened this issue 3 years ago • 8 comments


Basic Info

Info Please fill out this column
Ticket(s) this addresses None
Primary OS tested on Ubuntu 22.04
Robotic platform tested on motor-only simulated robot based on kobuki_soft

Description of contribution in a few bullet points

  • I have ported a subset of fake_localization to nav2 format with a few changes.
  • Instead of base_pose_ground_truth, it listens to odom.
  • It doesn't provide amcl_pose and particlecloud

Description of documentation updates required from your changes


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

jihoonl avatar Sep 18 '22 02:09 jihoonl

@jihoonl, 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

mergify[bot] avatar Sep 18 '22 03:09 mergify[bot]

@jihoonl, your PR has failed to build. Please check CI outputs and resolve issues. You may need to rebase or pull in main due to API changes (or your contribution genuinely fails).

mergify[bot] avatar Sep 18 '22 03:09 mergify[bot]

It seems to fail due to the package I never touched. Would I need to address the issue?

jihoonl avatar Sep 19 '22 07:09 jihoonl

Yeah that's fine - understood, that won't block this PR. We've been having some odd stuff with our CI recently and just now working on smoothing out the kinks.

A question: What's the motivation to add this package? This was more helpful in the earlier days of Navigation development when we lacked a great deal of compute power, but these days a full robot stack with simulation only takes about 50% of a consumer laptop. I think its best to have simulation test the full systems together to model the error that is actually present in localization algorithms being used. But maybe there's a use-case here I'm not thinking of.

Besides that, for packages to come into Nav2, we require test coverage > 90% with documentation on navigation.ros.org (configuration guide page for parameter descriptions, perhaps a tutorial for something that isn't default-used so that users know how to use it - but can be relatively simple). Unlike in ROS 1 Navigation, I've set some pretty high quality standards to make sure maintaining this long term is possible and provides enough tools for users to know how to use the things that exist here so there aren't any "hidden" features. Something like this though could be tested via a nav2_system_test and probably hit that 90% number in some trivial A->B navigation task using this for localization and just checking that its working within reasonable bounds.

SteveMacenski avatar Sep 19 '22 17:09 SteveMacenski

Any followup?

SteveMacenski avatar Sep 28 '22 05:09 SteveMacenski

A question: What's the motivation to add this package? This was more helpful in the earlier days of Navigation development when we lacked a great deal of compute power, but these days a full robot stack with simulation only takes about 50% of a consumer laptop. I think its best to have simulation test the full systems together to model the error that is actually present in localization algorithms being used. But maybe there's a use-case here I'm not thinking of.

The motivation is to test navigation planner and controller parameters in various environments with minimal effort. Although the great improvements in simulation, it still requires high computation and effort to design a testing environment. With this node, I can purely test planner and controller parameters in various environments and situations with only map images.

I agree that localization matters for navigation but also I believe that many things can be tested without localization concerns. For example, I wanted to set up the infrastructure to test application layer(system above navigation stack) and multi robot coordination with various environments and with lesser computation concern. 😃

Besides that, for packages to come into Nav2, we require test coverage > 90% with documentation on navigation.ros.org (configuration guide page for parameter descriptions, perhaps a tutorial for something that isn't default-used so that users know how to use it - but can be relatively simple). Unlike in ROS 1 Navigation, I've set some pretty high quality standards to make sure maintaining this long term is possible and provides enough tools for users to know how to use the things that exist here so there aren't any "hidden" features. Something like this though could be tested via a nav2_system_test and probably hit that 90% number in some trivial A->B navigation task using this for localization and just checking that its working within reasonable bounds.

I agree with having high-quality standards for long term maintenance. I will try to meet the standards as well as address comments soon.

jihoonl avatar Oct 03 '22 02:10 jihoonl

I will try my best to follow up before roscon.

jihoonl avatar Oct 03 '22 02:10 jihoonl

With this node, I can purely test planner and controller parameters in various environments and situations with only map images.

Why do you need localization for either of those? Planner doesn't rely on localization - this is an example benchmarking framework showing that https://github.com/ros-planning/navigation2/tree/main/tools/planner_benchmarking. The controller uses odometry and doesn't actually rely on a map->odom transform.

SteveMacenski avatar Oct 03 '22 17:10 SteveMacenski

I couldn't find time to follow up on the PR. I will reopen it when I get sufficient time to work on it.

jihoonl avatar Oct 26 '22 12:10 jihoonl