-
Notifications
You must be signed in to change notification settings - Fork 93
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
Use Gazebo ROS vendor packages #277
Conversation
Changes: * Update all package.xml and CMakeLists files to use Gazebo vendor packages that are now available in rolling and the upcoming jazzy. * Update Github Actions to run on Ubuntu Noble * Clean up code where both ign and gz headers were supported. Signed-off-by: Addisu Z. Taddese <[email protected]>
ros-rolling-ros-gz-sim is still not released, or is there an other issue with the config? |
Yeah, I think that's the only issue. |
ros-rolling-ros-gz-sim is not available on ros2-testing. |
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.
There are some errors on CI, do you mind to take a look ?
Has there been a release of |
yes, today ;) ros/rosdistro#41012 |
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.
That change looks like it would also need a README update.
Apart from that I tested building this locally on ubuntu noble with the testing repo enabled and did a quick test using ur_simulation_gz against this.
Signed-off-by: Addisu Z. Taddese <[email protected]>
I've updated the README to add Jazzy. Let me know if there's a specific change you had in mind. |
Yes, these are the changes that I had in mind, thank you for updating this. Currently, I find it a bit troublesome to find out which "rolling" means "rolling an noble" and which means "rolling on jammy, we haven't updated our docs, yet". The proposed changes make that clear in my opinion. |
Signed-off-by: Addisu Z. Taddese <[email protected]>
I've fixed flake8 failures in affdb76. CI should hopefully be green now. Please take a look. |
@azeey will the vendor packages be released for distros < Jazzy as well? Up to this PR we were able to compile the rolling version of the ros2_control stack on iron and humble, but now this obviously fails https://github.com/ros-controls/ros2_control_ci/actions/runs/8994845511 |
We're currently not planning to release the vendor packages for earlier versions. If there is a lot of interest, we might consider releasing them, but they would require more work, so I don't think it will be anytime soon. |
I wasn't aware of this. I don't see a clean way to accomplish this with the vendor packages. In general, I was under the impression that best practice was to have a branch for each ROS version and backport changes if necessary. This is at least what the packages in the ROS core do. So I assumed |
You are right and in principle we use this scheme. But we wanted to have the opportunity to use new features in the LTS distros (which are the only ones used by the industry) without the extra work on backporting them with proper deprecations/without any ABI/API breaks. This is why we added these CI checks to see if we break our code to be compilable on the older LTS distros. This is now the case with this PR (which is fine for rolling/jazzy and makes totally sense). I tried to just import all the vendor packages but it fails on dependencies not available on jammy ros-controls/ros2_control_ci#77 What would be necessary to add the vendor package at least for the humble LTS? (we can move the discussion to the ros2_control_ci PR) |
Changes: