-
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
[gazebo-migration] Make gazebo_ros_pkgs optional in nav2_system_tests #4095
[gazebo-migration] Make gazebo_ros_pkgs optional in nav2_system_tests #4095
Conversation
* This allows users to build all of nav2 with new gazebo installed without any special flags * Leverage common environment variable GZ_VERSION * Add author warning when some tests are disabled * Making gazebo classic optional is one of the first things to help migration Signed-off-by: Ryan Friedman <[email protected]>
Codecov ReportAll modified and coverable lines are covered by tests ✅
Additional details and impacted files@@ Coverage Diff @@
## main #4095 +/- ##
==========================================
+ Coverage 90.25% 90.27% +0.01%
==========================================
Files 417 417
Lines 18842 18842
==========================================
+ Hits 17006 17009 +3
+ Misses 1836 1833 -3 ☔ View full report in Codecov by Sentry. |
I disagree with this charge writ large. The system tests are ... system tests. Not making the simulation tests as part of it defeats much of the purpose of the package. There's no reason AFAIK to do this. For the GZ migration, the nav2 bringup is largely decoupled from the system tests which will continue to use gazebo classic until we upgrade those separately (but one step at a time). My understandign is that 24.04 will have gazebo classic binaries distributed by someone else, so there's no a step function where they instantly stop working. If there was, then this would make sense as a temporary work around, but I might just prefer to put a |
I think it would be helpful to understand what the migration plan for ROS packages is when we have a dependency conflict. The oversight of whoever allowed gazebo classic into Ubuntu 22 and then let the new gazebo in and depend on a conflicting debian package is a disaster. The PR here proposes a path for a smooth migration plan, but since you aren't a fan... it would be helpful to know how you plan to migrate to new gazebo on |
We have pending PRs for gazebo2 that are not yet complete. That's a stepping stone to get our bringup to use gazebo2 and use those changes as a model to update our tests. The gazebo and gazebo2 bringup files will exist in parallel in Jazzy and probably for the immediately foreseeable future, but the gazebo2 tests will be fully transitioned over once they are stable |
@SteveMacenski do you know who would be doing that? I'm not aware of any source for Gazebo classic binaries on 24.04 atm. |
Canonical / Ubuntu is doing it I was told my OSRF folks a few times. I don't have first hand documentation of that, more general conversations with your team and mentioning frustration with that. Maybe I'm mistaken / misrecalling? |
Yeah, it seems like there's some confusion. We've posted https://discourse.ros.org/t/gazebo-classic-end-of-life-ros-2-jazzy/36239 to hopefully clear things up. As stated there, Gazebo Classic binaries will not be available on Ubuntu 24.04. |
Basic Info
Description of contribution in a few bullet points
GZ_VERSION
to control which gazebo is linked to in system_testshumble
and should not result in any change in behavior for existing CIDescription of documentation updates required from your changes
Future work that may be required in bullet points
Screenshots of warnings when you are missing gazebo classic
These will show up on
colcon build
if you have theGZ_VERSION
environment variable set to something other than an empty string.For Maintainers: