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

Using setup-gazebo GitHub Action to install Gazebo in CI #127

Merged
merged 1 commit into from
Nov 4, 2024

Conversation

sauk2
Copy link
Contributor

@sauk2 sauk2 commented Nov 4, 2024

Closes #126

Summary

This PR changes the CI to use the setup-gazebo GitHub Action to configure a Gazebo environment. It replaces Harmonic's installation steps and passes input to the action to take care of the installation.

@srmainwaring
Copy link
Collaborator

@sauk2 thanks for the PR. Can you please squash and rebase your changes on main. The workflow favoured by ArduPilot does not use merge commits (https://ardupilot.org/dev/docs/git-rebase.html).

@srmainwaring srmainwaring self-assigned this Nov 4, 2024
@srmainwaring srmainwaring self-requested a review November 4, 2024 15:32
@srmainwaring srmainwaring added the enhancement New feature or request label Nov 4, 2024
@sauk2 sauk2 force-pushed the feat/add-setup-gazebo branch 2 times, most recently from bfb41c8 to 0719776 Compare November 4, 2024 15:49
Signed-off-by: Saurabh Kamat <[email protected]>
@sauk2
Copy link
Contributor Author

sauk2 commented Nov 4, 2024

@sauk2 thanks for the PR. Can you please squash and rebase your changes on main. The workflow favoured by ArduPilot does not use merge commits (https://ardupilot.org/dev/docs/git-rebase.html).

Sure, thanks for pointing it out! I have squashed all commits into one

Copy link
Collaborator

@srmainwaring srmainwaring left a comment

Choose a reason for hiding this comment

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

Thanks, this is definitely an improvement and we should be able to apply the same change to some of out other repos that depend on Gazebo and ros_gz.

Copy link
Contributor

@Ryanf55 Ryanf55 left a comment

Choose a reason for hiding this comment

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

This is an excellent improvement. I checked through the action.yml file to see if there were any other options we could to set to speed up the CI, and didn't see any.

@sauk2
Copy link
Contributor Author

sauk2 commented Nov 4, 2024

This is an excellent improvement. I checked through the action.yml file to see if there were any other options we could to set to speed up the CI, and didn't see any.

A little off-topic but you could look at increasing the scope of your workflow by spawning multiple jobs using a matrix in case you decide to support multiple Gazebo distributions on different ubuntu versions.

@srmainwaring srmainwaring merged commit 4b30a3d into ArduPilot:main Nov 4, 2024
3 checks passed
@Ryanf55
Copy link
Contributor

Ryanf55 commented Nov 4, 2024

Yep, we use matrix on some other repos to maintain multiple ROS distros.

Are you interested in doing any more gazebo+CI+ROS related tasks for ArduPilot's Gazebo usage? We have a few you might be able to knock out that would be of great benefit to this project.

  • Fixing rosdep errors by using Gazebo's rosdep keys list
  • Adding some basic smoke tests of our examples that run in CI (arm, takeoff, send waypoint command, check the vehicle reaches a location)
  • Adding support in CI for ROS 2 Jazzy

@sauk2 sauk2 deleted the feat/add-setup-gazebo branch November 7, 2024 13:10
@sauk2
Copy link
Contributor Author

sauk2 commented Nov 7, 2024

Are you interested in doing any more gazebo+CI+ROS related tasks for ArduPilot's Gazebo usage? We have a few you might be able to knock out that would be of great benefit to this project.

  • Fixing rosdep errors by using Gazebo's rosdep keys list
  • Adding some basic smoke tests of our examples that run in CI (arm, takeoff, send waypoint command, check the vehicle reaches a location)
  • Adding support in CI for ROS 2 Jazzy

Sorry, I missed this comment. I would be happy to contribute more! Could I ask you to raise issues so we can discuss this more?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Proposal to use setup-gazebo GitHub Action in CI
3 participants