Skip to content

Migrate to Kilted #94

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

Merged
merged 7 commits into from
Aug 7, 2025
Merged

Migrate to Kilted #94

merged 7 commits into from
Aug 7, 2025

Conversation

adivardi
Copy link
Contributor

Migrate the code to Kilted

@adivardi adivardi force-pushed the kilted branch 3 times, most recently from d0edf59 to da313f2 Compare July 28, 2025 10:07
@adivardi
Copy link
Contributor Author

I don't get why it is failing, and there seems to be no kilted rolling container.
I will hopefully have time to migrate to rolling in the next few weeks. Happy to to get some help on why this is failing on rolling in the meantime.

@SteveMacenski
Copy link
Member

It looks like nav2_util wasn't installed on the system, so it couldn't compile.

    By not providing "Findnav2_util.cmake" in CMAKE_MODULE_PATH this project
    has asked CMake to find a package configuration file provided by
    "nav2_util", but CMake did not find one.
  
    Could not find a package configuration file provided by "nav2_util" with
    any of the following names:
  
      nav2_utilConfig.cmake
      nav2_util-config.cmake

You're using a Rolling distro which doesn't have Nav2 binaries. If you want to update for Kilted, you should use a Kilted container which will be able to use rosdep to install Kilted Nav2 packages.

@SteveMacenski
Copy link
Member

I also just pushed a Kilted branch so we can also backport this into after merge in.

@adivardi
Copy link
Contributor Author

adivardi commented Jul 29, 2025

I tried kilted and got this error:

 Input kilted was not a valid ROS 2 distribution for 'target-ros2-distro'. Valid values:  dashing,eloquent,foxy,galactic,humble,iron,jazzy,rolling

I will try it again now

Edit: https://github.com/open-navigation/opennav_coverage/actions/runs/16591772590/job/46928948426?pr=94

@@ -16,7 +16,7 @@ jobs:
strategy:
fail-fast: false
matrix:
ros_distro: [jazzy]
ros_distro: [rolling]
Copy link
Member

Choose a reason for hiding this comment

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

Could do jazzy, kilted to run both

Copy link
Contributor Author

Choose a reason for hiding this comment

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

done

Copy link
Contributor Author

Choose a reason for hiding this comment

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

The test on Jazzy seems to be failing. Maybe just kilted is enough?

@@ -12,11 +12,11 @@ jobs:
ROS_DISTRO: ${{ matrix.ros_distro }}
RMW_IMPLEMENTATION: rmw_cyclonedds_cpp
container:
image: rostooling/setup-ros-docker:ubuntu-noble-latest
image: rostooling/setup-ros-docker:ubuntu-noble-ros-kilted-desktop-latest
Copy link
Member

Choose a reason for hiding this comment

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

This image doesn't appear to exist https://github.com/orgs/ros-tooling/packages?tab=packages&q=kited

Can you file a ticket on rostooling/setup-ros-docker with that error?

Copy link
Contributor Author

Choose a reason for hiding this comment

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

You have a typo in the search.
I found these 2 packages:
setup-ros-docker-ubuntu-noble-ros-kilted-desktop
setup-ros-docker-ubuntu-noble-ros-kilted-ros-base
https://github.com/orgs/ros-tooling/packages?packages&q=kilted

but it doesn't seem to help.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

I opened a ticket: ros-tooling/setup-ros-docker#90

Copy link
Member

Choose a reason for hiding this comment

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

🤦

Lets see what they say!

Choose a reason for hiding this comment

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

It's here: https://github.com/ros-tooling/setup-ros-docker/pkgs/container/setup-ros-docker%2Fsetup-ros-docker-ubuntu-noble-ros-kilted-desktop. Try:

Suggested change
image: rostooling/setup-ros-docker:ubuntu-noble-ros-kilted-desktop-latest
image: ghcr.io/ros-tooling/setup-ros-docker/setup-ros-docker-ubuntu-noble-ros-kilted-desktop

Choose a reason for hiding this comment

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

Note that images still get uploaded to Docker Hub, but that might eventually change, so I'd recommend switching to ghcr.io.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Thanks, I needed to add :master at the end, but seems to be working now

@adivardi adivardi force-pushed the kilted branch 4 times, most recently from e5e58f1 to 1b536b8 Compare August 4, 2025 09:29
@adivardi adivardi changed the base branch from main to kilted August 4, 2025 09:50
@SteveMacenski
Copy link
Member

@adivardi remove Jazzy now if this is only the kilted branch. That should let this pass and we can merge!

@@ -7,7 +7,7 @@ jobs:
name: ament_${{ matrix.linter }}
runs-on: ubuntu-latest
container:
image: rostooling/setup-ros-docker:ubuntu-noble-ros-jazzy-ros-base-latest
image: ghcr.io/ros-tooling/setup-ros-docker/setup-ros-docker-ubuntu-noble-ros-kilted-desktop:master
Copy link
Member

Choose a reason for hiding this comment

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

Is the full desktop really necessary?

@@ -12,18 +12,18 @@ jobs:
ROS_DISTRO: ${{ matrix.ros_distro }}
RMW_IMPLEMENTATION: rmw_cyclonedds_cpp
container:
image: rostooling/setup-ros-docker:ubuntu-noble-latest
image: ghcr.io/ros-tooling/setup-ros-docker/setup-ros-docker-ubuntu-noble-ros-kilted-desktop:master
Copy link
Member

Choose a reason for hiding this comment

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

Ditto

@adivardi
Copy link
Contributor Author

adivardi commented Aug 7, 2025

all done now

@SteveMacenski SteveMacenski merged commit d6e41a2 into open-navigation:kilted Aug 7, 2025
7 checks passed
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.

3 participants