Skip to content

Jazzy migration #628

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

Open
wants to merge 69 commits into
base: main
Choose a base branch
from
Open

Jazzy migration #628

wants to merge 69 commits into from

Conversation

Flova
Copy link
Member

@Flova Flova commented Nov 23, 2024

No description provided.

@JanNiklasFeld
Copy link
Member

TODO: Fix cppformat in Jazzy

@Flova Flova changed the title Jazzy devcontainer Jazzy migration Nov 24, 2024
@Flova
Copy link
Member Author

Flova commented Nov 24, 2024

The CI seems to fail due to floating point errors in the team_communication.

@texhnolyze
Copy link
Contributor

I've adjusted the team_communication snapshots

texhnolyze and others added 9 commits December 5, 2024 17:44
starting from `~` instead of using the whole path
to prevent issues when interacting with the repository both from within
the container and outside the container, due to permissions not being
correct
in `Dockerfile`, because the `updateRemoteUserUID` setting of the
devcontainer does not change the `GID` of the `containerUser`
dynamically to the one of the host user if the group exists in the
container already microsoft/vscode-remote-release#2402.

In our case the `containerUser` is set to `bitbots`, because it
automatically uses the last `USER` instruction from the `Dockerfile` and
the `remoteUser` inherits from `containerUser`.

For reference see: microsoft/vscode-remote-release#1155
as with upgrade to jazzy or packages floating point handling seems to
have changed slightly
@texhnolyze texhnolyze force-pushed the feature/jazzy-ubuntu2404-devcontainer branch from 4b8b0e9 to 8ecfb93 Compare December 5, 2024 16:44
@Flova
Copy link
Member Author

Flova commented Dec 12, 2024

TODO after merge: Revert #634

Flova and others added 12 commits December 12, 2024 10:50
to ensure that the CI `colcon test` run works, because with a change to
python 3.12 the `unittest` standard library used by default with colcon
now exits with an error code of 5 for an empty test suite.

See: colcon/colcon-core#678
See: python/cpython#102051
When using the colcon `--symlink-install` flag (as we usually do),
one gets shown the following huge warning for every Python-package:

```
/usr/lib/python3/dist-packages/setuptools/command/develop.py:40: EasyInstallDeprecationWarning: easy_install command is deprecated.
!!

        ********************************************************************************
        Please avoid running ``setup.py`` and ``easy_install``.
        Instead, use pypa/build, pypa/installer or other
        standards-based tools.

        See pypa/setuptools#917 for details.
        ********************************************************************************

!!
  easy_install.initialize_options(self)
---
```

Has been solved by: ros2/ros2#1577 (comment)
This resolves the following cmake warning, that seems to appear when pybind is in use:
```
lto-wrapper: warning: using serial compilation of 2 LTRANS jobs
lto-wrapper: note: see the ‘-flto’ option documentation for more information
```
```
--- stderr: bitbots_webots_sim
CMake Warning at /opt/ros/jazzy/share/gazebo_msgs/cmake/gazebo_msgs-extras.cmake:6 (message):
  This gazebo_msgs package hosts messages designed initially for Gazebo
  Classic which is not available on ROS 2 Jazzy which is unavailable on ROS 2
  Jazzy.  The new Gazebo uses the ros_gz bridge message vailable
  at:https://github.com/gazebosim/ros_gz/tree/ros2/ros_gz_bridge#bridge-communication-between-ros-and-gazebo

  To avoid this warning and use this Gazebo Classic messages anyway you can
  use flag -DIGNORE_GAZEBO_MSGS_WARNING
Call Stack (most recent call first):
  /opt/ros/jazzy/share/gazebo_msgs/cmake/gazebo_msgsConfig.cmake:41 (include)
  CMakeLists.txt:18 (find_package)
```
@jaagut
Copy link
Member

jaagut commented Feb 12, 2025

Uff, why was this pyi stuff in cmakelists necessary?

@Flova
Copy link
Member Author

Flova commented Feb 13, 2025

Protobuf generates Python code. That Python code makes mypy unhappy. But we can not exclude it, because ros2 fucked up it's mypy configuration by adding another layer on top of it. So we need to generate stubs for the protobuf messages, which is better either way, as you also get better ide support with it. These are the .pyi files. And because ament_mypy is broken in another way it does not recognize stub files. This was fixed last week in rolling, but it is not available yet. We therefore need to manually discover all necessary files in our cmake and pass them to ament_mypy

@texhnolyze texhnolyze force-pushed the feature/jazzy-ubuntu2404-devcontainer branch from df63762 to 5d1cad1 Compare February 27, 2025 14:12
@texhnolyze texhnolyze force-pushed the feature/jazzy-ubuntu2404-devcontainer branch from 451ffc2 to 2860b1a Compare March 27, 2025 10:53
@texhnolyze texhnolyze force-pushed the feature/jazzy-ubuntu2404-devcontainer branch 2 times, most recently from 2860b1a to d9191c6 Compare March 27, 2025 11:14
@jaagut jaagut marked this pull request as ready for review April 8, 2025 11:37
@jaagut jaagut moved this from 🏗 In progress to 👀 In review in Software Apr 8, 2025
@jaagut jaagut force-pushed the feature/jazzy-ubuntu2404-devcontainer branch from 6ef7985 to 2594e44 Compare April 8, 2025 12:13
Copy link
Member Author

Choose a reason for hiding this comment

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

Could we separate this into a different PR? This is a new feature and needs to be tested differently performance wise. Also things like catching might need to be implemented.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
Status: 👀 In review
Development

Successfully merging this pull request may close these issues.

5 participants