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

Initial support to the new Gazebo #3634

Merged

Conversation

ahcorde
Copy link
Contributor

@ahcorde ahcorde commented Jun 16, 2023

Basic Info

Info Please fill out this column
Ticket(s) this addresses #2997

Description of contribution in a few bullet points

Modified tb3_simulation_launch.py launch file to use the new Gazebo. It includes some new dependencies: gz_ros_bridge, gz_ros_sim and gz_ros2_control with diff_drive_controller and joint_state_broadcaster controllers

Test it

ros2 launch nav2_bringup tb3_simulation_launch.py use_gz:=True robot_sdf:=`ros2 pkg prefix nav2_bringup`/share/nav2_bringup/urdf/gz_turtlebot3_waffle.urdf world:=`ros2 pkg prefix nav2_bringup`/share/nav2_bringup/worlds/gz_world_only.model
ros2 launch nav2_bringup tb3_simulation_launch.py use_gz:=False headless:=False

nav2_demo

@mergify
Copy link
Contributor

mergify bot commented Jun 16, 2023

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

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@ahcorde ahcorde force-pushed the ahcorde/initial_support/gazebo branch from ad975c0 to c52b2b0 Compare June 16, 2023 15:03
nav2_bringup/launch/tb3_simulation_launch.py Outdated Show resolved Hide resolved
nav2_bringup/urdf/turtlebot3_waffle.urdf Outdated Show resolved Hide resolved
nav2_bringup/urdf/gz_turtlebot3_waffle.urdf Outdated Show resolved Hide resolved
nav2_bringup/config/control.yaml Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

Thanks! The screen cap looks great ❤️

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@mergify
Copy link
Contributor

mergify bot commented Jun 16, 2023

@ahcorde, 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_bringup/CMakeLists.txt Outdated Show resolved Hide resolved
nav2_bringup/urdf/gz_turtlebot3_waffle.urdf Outdated Show resolved Hide resolved
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
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.

Otherwise, beyond the other open item LGTM

nav2_bringup/launch/tb3_gz_simulation_launch.py Outdated Show resolved Hide resolved
nav2_bringup/launch/tb3_gz_simulation_launch.py Outdated Show resolved Hide resolved
nav2_bringup/launch/tb3_gz_simulation_launch.py Outdated Show resolved Hide resolved
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@SteveMacenski
Copy link
Member

Check CI for linting errors, I think when you adjusted the copyright you misformatted it a little

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.

Otherwise LGTM.

nav2_bringup/urdf/gz_turtlebot3_waffle.urdf Outdated Show resolved Hide resolved
@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 20, 2023

I want to have someone else take a look for me as well on this one since there's alot of small subtle things that could cause problems + I know someone that's been working with new-gazebo alot recently. @bperseghetti could you do the honors?

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@ahcorde
Copy link
Contributor Author

ahcorde commented Jun 20, 2023

This one is fixed, odom frame is in the right place

2023-06-20_23-15

@SteveMacenski
Copy link
Member

This one is fixed, odom frame is in the right place

Huh?

@ahcorde
Copy link
Contributor Author

ahcorde commented Jun 20, 2023

This one is fixed, odom frame is in the right place

Huh?

it's weird, but this one is fixed, I'm trying to fix the other turtlebot3 model in the nav2_simple_commander package

@SteveMacenski
Copy link
Member

Sorry, I read this one before the simple commander one 😄

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 21, 2023

The min range cannot be zero? Why is that?

@ahcorde
Copy link
Contributor Author

ahcorde commented Jun 21, 2023

The min range cannot be zero? Why is that?

it's an OGRE(2) restriction:

what():  OGRE EXCEPTION(2:InvalidParametersException): Near clip distance must be greater than zero. in Frustum::setNearClipDistance at ./OgreMain/src/OgreFrustum.cpp (line 127)

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
@SteveMacenski
Copy link
Member

SteveMacenski commented May 28, 2024

@azeey any thoughts on the get_package_share_directory('turtlebot3_gazebo')s? There's no turtlebot3-anything in 24.04 (except msgs, which I'm not ensurely sure what those are for) so we can't get `turtlebot3_common, gazebo, the sandbox world, probably)

Edit Looking at the job, its looking for gazebo ros pkgs which it'll never find: https://build.ros2.org/job/Rbin_uN64__turtlebot3_gazebo__ubuntu_noble_amd64__binary/38/console

@azeey
Copy link
Contributor

azeey commented May 28, 2024

@azeey any thoughts on the get_package_share_directory('turtlebot3_gazebo')s? There's no turtlebot3-anything in 24.04 (except msgs, which I'm not ensurely sure what those are for)

I reached out to the maintainers two weeks ago about migrating to the new Gazebo, but no word so far. We've already created issues (ROBOTIS-GIT/turtlebot3_simulations#196, ROBOTIS-GIT/turtlebot3_simulations#179) and PRs (ROBOTIS-GIT/turtlebot3_simulations#180) to help make the transition, but there has been no updates.

@SteveMacenski
Copy link
Member

SteveMacenski commented May 28, 2024

Any thoughts about moving to the TB4? I know there's no Iron/Jazzy/Rolling versions yet, but I believe their humble is based on Ignition-Gazebo so it should be relatively less of a lift. I actually emailed Ryan at CPR today asking him about providing some engineering support to getting the TB4 for Jazzy/Rolling for this purpose. That could cut this problem out and upgrade to the more modern robot option which I prefer greatly and is still a 'community standard' / OSRF affiliated.

or do you know of another robot with a new-gazebo simulation that could be lifted to Jazzy/Rolling for our use here? Another option is that I can create a "simplified" TB4 that we control in the ros-navigation org that removes dependencies on icreate and other software packages that would need to be updated which we don't need. That's what we ended up doing with the TB3 which I was hoping to avoid here, but may be necessary.

I suppose we also need a new simulation environment since the sandbox is tied to TB3. We discussed some warehouses, I think that's a good move and I just need to look into the 'how's and refamiliarize myself with the threads on that.

Edit: good sign, CI is happy on this PR minus a test that is unrelated to this I need to look into. That's on 24.04/Rolling and was able to resolve all the dependencies then fine.

@SteveMacenski
Copy link
Member

I was curious if it were possible to use a Fuel world as part of a description file and remove some element from it.

The AWS warehouse world model is great, but the manual forklift in the aisle isn't what I'd want for a default bringup to do more traditional demos. I actually prefer the Depot model, but the trusses and fan in the ceiling make it difficult for a user to see what's going on if they're setting the gazebo client on to watch the robot in simulation from above - so I'd want to remove those.

I don't recall any subtractive-operations in the SDF format, but maybe there are (or can override existing object names with new ones that I set its color properties to transparent?)

@azeey
Copy link
Contributor

azeey commented May 29, 2024

I was curious if it were possible to use a Fuel world as part of a description file and remove some element from it.

It is possible to include a model (only works with models not worlds) and remove some elements from it in SDFormat. The feature to do this is under an experimental XML namespace even though it's been around for a while. It should become a permanent feature, we just haven't gotten around to it yet. If you are okay with using that, then I can switch to the Depot model and remove the trusses and fan.

Any thoughts about moving to the TB4? I know there's no Iron/Jazzy/Rolling versions yet, but I believe their humble is based on Ignition-Gazebo so it should be relatively less of a lift.

I'm willing to provide support to make that happen. I looked to see if it would be quick but I realized it requires changes in https://github.com/iRobotEducation/create3_sim as well.

In the meantime, I have a fork of turtlebot3 that works on Rolling. We can build that from source if you'd like.

@SteveMacenski
Copy link
Member

SteveMacenski commented May 29, 2024

I looked to see if it would be quick but I realized it requires changes in https://github.com/iRobotEducation/create3_sim as well.

I realized the same thing which is what led me to: 'maybe we can just make a minimum nav2_tb4_description package that's self contained.' Too many cooks in the kitchen and too few have interest in keeping up with the latest in ROS 2. Doesn't need to be nav2 specific even, but figured I'd just open a new repo in ros-navigation with the minimal TB4 stuff and release that. I'm open to other more generic options though.

In the meantime, I have a fork of turtlebot3 that works on Rolling. We can build that from source if you'd like.

That's a good interim solution, I will test that after I get this comment out. I wonder if its better not to just replace it all in the first salvo, but this PR would be done if we could have that built with CI and workable. Then someone in parallel can work on using that for the system tests while I work on updating the robot model / world (or I'll get to it too). If you add your fork to https://github.com/ros-navigation/navigation2/blob/main/tools/underlay.repos (uncommented) then it'll be grabbed and built for CI.

The major question with that is: is that fork going to be able to make it upstream + released for Jazzy/Rolling? If so, good solution, lets do it. We can use that for our system tests so it has real value not to have rewrite those. If this won't get out in binary format for TB3, then its probably just worth stripping out TB3 up front since adding it here now won't really buy us anything in terms of unblocking Nav2's release in binary format. This PR gets merged (which isn't nothing for sure), but it seems more organizational since we'd immediately need to strip it out again for something that can be released.

@SteveMacenski
Copy link
Member

@azeey I'm unable to get it to build with the following error:

--- stderr: turtlebot3_gazebo                                 
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacle2.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacles.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
gmake[2]: *** [CMakeFiles/obstacle2.dir/build.make:76: CMakeFiles/obstacle2.dir/models/turtlebot3_dqn_world/obstacle_plugin/obstacle2.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:195: CMakeFiles/obstacle2.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
gmake[2]: *** [CMakeFiles/obstacles.dir/build.make:76: CMakeFiles/obstacles.dir/models/turtlebot3_dqn_world/obstacle_plugin/obstacles.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:221: CMakeFiles/obstacles.dir/all] Error 2
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacle1.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

@azeey
Copy link
Contributor

azeey commented May 30, 2024

@azeey I'm unable to get it to build with the following error:

--- stderr: turtlebot3_gazebo                                 
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacle2.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacles.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.
gmake[2]: *** [CMakeFiles/obstacle2.dir/build.make:76: CMakeFiles/obstacle2.dir/models/turtlebot3_dqn_world/obstacle_plugin/obstacle2.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:195: CMakeFiles/obstacle2.dir/all] Error 2
gmake[1]: *** Waiting for unfinished jobs....
gmake[2]: *** [CMakeFiles/obstacles.dir/build.make:76: CMakeFiles/obstacles.dir/models/turtlebot3_dqn_world/obstacle_plugin/obstacles.cc.o] Error 1
gmake[1]: *** [CMakeFiles/Makefile2:221: CMakeFiles/obstacles.dir/all] Error 2
In file included from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/ObstacleAnimator.hh:19,
                 from /jazzy_ws/src/turtlebot3_simulations/turtlebot3_gazebo/models/turtlebot3_dqn_world/obstacle_plugin/obstacle1.cc:17:
/opt/ros/rolling/opt/gz_sim_vendor/include/gz/sim8/gz/sim/System.hh:28:10: fatal error: gz/transport/parameters/Registry.hh: No such file or directory
   28 | #include <gz/transport/parameters/Registry.hh>
      |          ^~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~~
compilation terminated.

That's really strange. I tried building it multiple times both on rolling and jazzy, but could not reproduce what you're getting.

@SteveMacenski
Copy link
Member

SteveMacenski commented May 30, 2024

I’m in a docker container based on ros-rolling-desktop-full & obtained gazebo from the ROS 2 integration instructions via apt install ros-rolling-ros-gz and then all the other rolling items related to Nav2. I pulled that base image in on Friday from dockerhub. What’s your setup?

@azeey
Copy link
Contributor

azeey commented May 30, 2024

Okay, I tracked it down to a missing dependency, cppzmq-dev. If you install that, the build should succeed.

What's strange is that if you start with a rolling-ros-core image and install ros-rolling-ros-gz, you'll get cppzmq-dev automatically. I assume it will be the same if you start with an ubuntu:noble image and install ros-rolling-desktop-full. I think the issue is that cppzmq-dev is a recommended package of libzmq3-dev, which gz-transport depends on. So, I wonder if the desktop-full docker image is created with apt --no-install-recommends.

Either way, gz-transport should declare a direct dependency on cppqmq. There is no rosdep key for cppzmq, so I'll have to create one and add that to gz-transport's package.xml. I'll plan on doing that tomorrow.

@azeey
Copy link
Contributor

azeey commented May 30, 2024

It turns out cppzmq-dev is a new package in Ubuntu Noble. Potential rosdep fix in ros/rosdistro#41413

@SteveMacenski
Copy link
Member

SteveMacenski commented May 30, 2024

So, I wonder if the desktop-full docker image is created with apt --no-install-recommends.

My docker image does the following as I'm dog fooding the Docker development tutorial on docs.nav2.org during this exercise

RUN apt update \
    && DEBIAN_FRONTEND=noninteractive apt install -y --no-install-recommends --no-install-suggests \
  ros-dev-tools \
  wget

A quick google search shows that in use all over the place in ROS-Docker-World and I never had reason before to question it. Should I remove that from my tutorial?

OK - I assume that new key will be added to some package(s) that will hit jazzy/rolling for the next sync?


That did make things work! I'm taking a look now and seeing the following:

  • Gazebo Classic: GUI 30% CPU, Server 105% CPU
  • Gazebo Harmonic: GUI 160% CPU, Server 120% CPU

I think the 105-120% on the server is fine enough not worth nitpicking, but I was surprised to see the GUI be 5x as consuming with these configs. Is there a way to reduce that to even just 2x?

I'll be keeping this up for a bit, sending goals, relaunching, etc to check stability but I don't expect to find anything to report. Mostly just for my own education.

^C[WARNING] [launch]: user interrupted with ctrl-c (SIGINT)
[parameter_bridge-2] [INFO] [1717089793.776311576] [rclcpp]: signal_handler(signum=2)
[ERROR] [ruby $(which gz) sim-4]: process has died [pid 176564, exit code -2, cmd 'ruby $(which gz) sim -r -s /tmp/nav2__ca33khs.sdf --force-version 8'].
[INFO] [robot_state_publisher-6]: process has finished cleanly [pid 176566]
[INFO] [parameter_bridge-2]: process has finished cleanly [pid 176562]
[ERROR] [ruby $(which gz) sim-5]: process has died [pid 176565, exit code -2, cmd 'ruby $(which gz) sim -v4 -g  --force-version 8'].

I don't love the crashing on shutdown, but I'm sure this is something the GZ team is aware of so I don't need to belabor it.


Thoughts on the steps forward for your TB3 fork? I think the path here boils down to if you think there's a reasonable chance that binaries will be possible. If so, then we can add this to our repos file and unlock the work for updating our system tests to use your work to get that part of our CI up and running. I'll probably still want to replace the default bringup before releasing to Jazzy with a new robot platform / environment, but that's non-blocking for this PR and squarely in the "Steve's problem" camp (though would love your reviews). The TB4 is the direction I think is most sensible unless anyone has a better idea.

@azeey
Copy link
Contributor

azeey commented May 30, 2024

My docker image does the following as I'm dog fooding the Docker development tutorial on docs.nav2.org during this exercise

The --no-install-recommends thing is why we didn't catch it in our testing. I'm not saying you should stop using it.

But the root cause of the issue is that the C++ headers were removed from libzmq3-dev in Noble and placed in their own package. My proposed fix is to update the libzmq3-dev rosdep key to include cppzmq-dev. If that gets merged, the fix would be available immediately (or when the docker image is rebuilt). The maintainers might decide it's better to add a new key for cppzmq. In that case it will be available in the next sync.

I think the 105-120% on the server is fine enough not worth nitpicking, but I was surprised to see the GUI be 5x as consuming with these configs. Is there a way to reduce that to even just 2x?

I think we'll need to profile the GUI to figure out what's happening. I don't see a quick fix for this.

I don't love the crashing on shutdown, but I'm sure this is something the GZ team is aware of so I don't need to belabor it.

This is caused by a bug in launch: ros2/launch#757. We're working on better launch integration (see gazebosim/ros_gz#552 and gazebosim/ros_gz#544) that will hopefully resolve this.

Thoughts on the steps forward for your TB3 fork?

Maybe @clalancette can chime in on this. I'm not sure if we are at the point of replacing the upstream package, but if we decide to go that route, we should probably move that repo into an OSRF org.

Honestly, the easiest thing to do would be to put the bits we need from turtlebot3_gazebo into nav2. I see y'all already have the files in https://github.com/ros-navigation/navigation2/tree/main/nav2_system_tests/models/turtlebot3_waffle and https://github.com/ros-navigation/navigation2/tree/main/nav2_system_tests/models/turtlebot3_burger, so I don't see why we couldn't do that more broadly instead of just for tests.

@SteveMacenski
Copy link
Member

SteveMacenski commented May 30, 2024

Honestly, the easiest thing to do would be to put the bits we need from turtlebot3_gazebo into nav2. I see y'all already have the files in https://github.com/ros-navigation/navigation2/tree/main/nav2_system_tests/models/turtlebot3_waffle and https://github.com/ros-navigation/navigation2/tree/main/nav2_system_tests/models/turtlebot3_burger, so I don't see why we couldn't do that more broadly instead of just for tests.

Good point. I'd be fine with that. Want to do that? That would then unblock re-enabling the system tests using it + GZ Harmonic. I don't love owning all of TB3 stack for Nav2 Bringup, but the systems test package is a little ... messy to begin with and as you point out we already have a bunch of files anyway. I'm not commiting you to updating all our system tests with Harmonic in this PR or ever, but certainly if you were willing to, I wouldn't object.

Our bringup still needs some work for getting a robot that is distributed in binary format, but I'm actively working on that now. I've created a set of minimal TB4 description + simulation files for Nav2 based on the CPR + iRobot files and your work here. Their repos have a bunch of other stuff we don't need which are blocking updates + releases for Jazzy/Rolling so I figure stripping out to just the bare minimum will help make it easy to forward port things in a distribution-agnostic way. Its in https://github.com/ros-navigation/nav2_min_tb4_sim and my plan is to release these in binary format then we can dep on that for nav2_bringup so that package doesn't have robot specific files in it but is able to bring up with the TB4's. Porting the existing TB4 packages to Jazzy involves porting a bunch of custom ignition plugins and panels which is more than I'm able to commit to, so this seems like a good way forward and when TB4 catches up, we can switch to that.

This way in the future, we're not dependent on companies supporting the bleeding edge, we can just have these packages in rolling and should always work (unless launch / URDF format changes).

@SteveMacenski
Copy link
Member

@azeey what do you think? Should we replace all the existing TB3 files with your fork in the system tests and possibly port ~1 over to it as an example? Then I think we can merge this in and I or other community members can come in to work on porting the rest of the system tests + update the bringup package with a robot in binary format.

@SteveMacenski SteveMacenski merged commit bc6e3b2 into ros-navigation:main Jun 5, 2024
9 of 10 checks passed
@SteveMacenski
Copy link
Member

@ahcorde @azeey merging. I'm pretty far into a TB4 analog with the TB3 for both options in modern GZ

One thing I've noticed is that the simulation takes a long time to initialize with the depot world and the TB4. I haven't looked into it, but after the migration is done, that is worth me/someone looking into.

Path forward:

@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 5, 2024

I refactored the TB4 into an independent simulation package in the minimal repository linked above. I also took the TB3 work done here and refactored that + the turtlebot3_gazebo assets into its own nav2_minimal_tb3_sim package so that it contains only the minimum things required without external dependencies so it can be released into rolling and available in all future distributions without work. I then refactored your work @ahcorde and @azeey in the PR linked below to move to that simulation package and link it back to the bringup in Nav2 to vendorize out the spawning of the specific robot platform in line with the TB4 example.

#4401

Now we can ship both, in binary form, without issues waiting for either of the turtlebot vendors! Phew, this was a lot of refactoring for a day...

@stevedanomodolor
Copy link
Contributor

@ahcorde @azeey merging. I'm pretty far into a TB4 analog with the TB3 for both options in modern GZ

One thing I've noticed is that the simulation takes a long time to initialize with the depot world and the TB4. I haven't looked into it, but after the migration is done, that is worth me/someone looking into.

Path forward:

Is there an example on how to run this yet?, so I can start porting the system tests too.

@SteveMacenski
Copy link
Member

If you clone the nav2 minimal turtlebot repo into your workspace and build them, the TB3/4 demos should work!

@stevedanomodolor
Copy link
Contributor

stevedanomodolor commented Jun 11, 2024

If you clone the nav2 minimal turtlebot repo into your workspace and build them, the TB3/4 demos should work!

I am able to run as is, I will start porting the system test and open the pr when its ready

@SteveMacenski
Copy link
Member

SteveMacenski commented Jun 11, 2024

Great! I just finished up the simple commander migration today, if you could even open a PR once you have a couple of them ported over, I could use those as a template for jumping in to help! The simple commander may help you as well: #4417

The things to look out for:

  • If can't find the meshes or something errors, appending the potential missing location with the AppendEnvironmentalVariable for that package in the examples I show in the PR resolved those for me quickly
  • If using the TB4, we need to update the locations since the world is different. If using the TB3, then as long as the test wasn't using gazebo to add new obstacles, the robot should spawn in the same place in the same world, so that's nice!
  • Check the logs and make sure its doing what we think its doing and there's no unexpected errors
  • Definitely worth going one at a time, not trying to do bulk operations, there's alot of unique details for each
  • You should be able to use the nav2_minimal_turtlebot_sim packages, so we should be able to delete all those TB3 URDF/files in the system tests and rely fully on this new package now, which should actually be an accelerator for you not to try to port any of that stuff over.

Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
* Initial support to the new Gazebo

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed build

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feeback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix copyright and urdf

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* min range cannot be zero

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Simplify files

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Try to reduce load

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed cast shadows

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use Gazebo plugins instead of gz_ros2_control

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* update dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed ros2_control and use ogre

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use param file to configure bridge

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use sdf model and change to ogre instead of ogre2 for sensors

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Support Garden and later, performance improvements

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Use xacro for world file (ros-navigation#2)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Reviewer feedback

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Add comment explaining temp file

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Fix linter

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Update nav2_bringup/worlds/gz_world_only.sdf.xacro

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>
Marc-Morcos pushed a commit to Marc-Morcos/navigation2 that referenced this pull request Jul 4, 2024
* Initial support to the new Gazebo

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed build

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feeback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix copyright and urdf

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* min range cannot be zero

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Simplify files

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Try to reduce load

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed cast shadows

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use Gazebo plugins instead of gz_ros2_control

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* update dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed ros2_control and use ogre

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use param file to configure bridge

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use sdf model and change to ogre instead of ogre2 for sensors

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Support Garden and later, performance improvements

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Use xacro for world file (ros-navigation#2)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Reviewer feedback

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Add comment explaining temp file

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Fix linter

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Update nav2_bringup/worlds/gz_world_only.sdf.xacro

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>
savalena pushed a commit to savalena/navigation2 that referenced this pull request Jul 5, 2024
* Initial support to the new Gazebo

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed build

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feeback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix copyright and urdf

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* min range cannot be zero

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Simplify files

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Try to reduce load

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed cast shadows

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use Gazebo plugins instead of gz_ros2_control

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* update dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed ros2_control and use ogre

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use param file to configure bridge

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use sdf model and change to ogre instead of ogre2 for sensors

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Support Garden and later, performance improvements

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Use xacro for world file (ros-navigation#2)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Reviewer feedback

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Add comment explaining temp file

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Fix linter

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Update nav2_bringup/worlds/gz_world_only.sdf.xacro

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>
Manos-G pushed a commit to Manos-G/navigation2 that referenced this pull request Aug 1, 2024
* Initial support to the new Gazebo

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed build

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feeback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fixed physics tag

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix copyright and urdf

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* min range cannot be zero

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Simplify files

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Added feedback

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Fix

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Try to reduce load

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed cast shadows

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use Gazebo plugins instead of gz_ros2_control

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* update dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed dependency

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Removed ros2_control and use ogre

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use param file to configure bridge

Signed-off-by: Alejandro Hernández Cordero <[email protected]>

* Use sdf model and change to ogre instead of ogre2 for sensors

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Support Garden and later, performance improvements

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Use xacro for world file (ros-navigation#2)

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Reviewer feedback

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Add comment explaining temp file

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Fix linter

Signed-off-by: Addisu Z. Taddese <[email protected]>

* Update nav2_bringup/worlds/gz_world_only.sdf.xacro

Co-authored-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>

---------

Signed-off-by: Alejandro Hernández Cordero <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Addisu Z. Taddese <[email protected]>
Signed-off-by: Steve Macenski <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Addisu Z. Taddese <[email protected]>
Co-authored-by: Steve Macenski <[email protected]>
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.

9 participants