Skip to content
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

Added Loopback Simulator Package #4006

Open
wants to merge 13 commits into
base: main
Choose a base branch
from

Conversation

smitdumore
Copy link

@smitdumore smitdumore commented Dec 11, 2023


Basic Info

Info Please fill out this column
Ticket(s) this addresses #3924
Primary OS tested on Ubuntu 20.04.4
Robotic platform tested on Rviz
Does this PR contain AI generated software? No

Description of contribution in a few bullet points

  • This new package serves as a Gazebo replacement.
  • Designed for testing robot behaviors without the need for a complicated simulator.
  • Uses incoming velocity commands to forward simulate the robot pose.
  • A map is needed for navigation. The Robot can be spawned anywhere for repetative tests.

Description of documentation updates required from your changes

  • A detailed readme is required for the nav2_loopback_sim package.

Future work that may be required in bullet points

  • Needs to be tested for various robot platforms and scenarios.
  • Needs some nice to have features.

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

Copy link
Contributor

mergify bot commented Dec 11, 2023

@smitdumore, 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).

nav2_loopback_sim/package.xml Outdated Show resolved Hide resolved
nav2_loopback_sim/package.xml Outdated Show resolved Hide resolved
nav2_loopback_sim/.amentcpplint Outdated Show resolved Hide resolved
nav2_loopback_sim/.uncrustify.cfg Outdated Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Outdated Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Outdated Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Outdated Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Outdated Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

SteveMacenski commented Dec 11, 2023

I think there's something funky happening with the frames here. The timerCallback should be publishing map->odom, but its actually publishing the transform.translation as the full initial pose's map->base_link. Also twistCallback should be publishing odom->base_link (or init_pose_ header id if fine) but there appears to be something funky going on there -- or perhaps that is correct but I'm just looking at it oddly since that timerCallback issue is definitely doing something odd

Copy link
Contributor

mergify bot commented Mar 22, 2024

@smitdumore, 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).

Copy link
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.

Still quite a few problems, but getting closer!

nav2_loopback_sim/CMakeLists.txt Show resolved Hide resolved
nav2_loopback_sim/CMakeLists.txt Show resolved Hide resolved
rclcpp::Publisher<sensor_msgs::msg::LaserScan>::SharedPtr scan_publisher_;

// tf
std::unique_ptr<tf2_ros::StaticTransformBroadcaster> tf_broadcaster_;
Copy link
Member

Choose a reason for hiding this comment

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

These aren't static transforms, they should be using the TransformBroadcaster instead

nav2_loopback_sim/src/loopback_simulator.cpp Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Show resolved Hide resolved
nav2_loopback_sim/src/loopback_simulator.cpp Show resolved Hide resolved

// Publishing updated position of odom and base_link wrt map
publishTransform(relocalization_pose_, "map", "odom");
publishTransform(init_pose_.pose.pose, "map", "base_link");
Copy link
Member

Choose a reason for hiding this comment

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

The transform it should be publishing is odom->base_link, the map->base_link should never be published. That breaks the TF tree. Its map->odom and odom->base_link. I think that largely invalidates the code above.

  • Given the initial pose (map->base_link)
  • Find the current odom->base_link transform (or just have it stored from the previous iteration)
  • Back out the map->odom that keeps odom->base_link constant
  • Publish that map->odom

nav2_loopback_sim/src/loopback_simulator.cpp Show resolved Hide resolved
@SteveMacenski
Copy link
Member

Hey! Checking back in here - anything I can help answer?

@smitdumore
Copy link
Author

Hey Steve,
I apologize for being inactive. I am wrapping up my last semester and should be able to jump back within a week. Thanks

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.

None yet

3 participants