Skip to content

Allow zenoh tests to run with multicast#992

Merged
ahcorde merged 3 commits intorollingfrom
mjcarroll/zenoh_multicast
Mar 26, 2025
Merged

Allow zenoh tests to run with multicast#992
ahcorde merged 3 commits intorollingfrom
mjcarroll/zenoh_multicast

Conversation

@mjcarroll
Copy link
Copy Markdown
Member

No description provided.

Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
@mjcarroll mjcarroll self-assigned this Mar 21, 2025
@mjcarroll
Copy link
Copy Markdown
Member Author

Build Status

Signed-off-by: Yadunund <yadunund@gmail.com>
Signed-off-by: Yadunund <yadunund@gmail.com>
return lines


def get_rmw_additional_env(rmw_implementation: str) -> Dict[str, str]:
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

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

I really think that rmw_implementation should handle those configuration but ros2cli.
again, i do not want to block this zenoh development, but i am not convinced to take this approach with following software design...

  • Separation of Concerns: By moving the logic to the RMW API, we can maintain a clear separation between the middleware-specific logic and the ros2cli. This makes the codebase cleaner and easier to maintain.
  • Single Source of Truth: Having the environment variable logic centralized in the RMW API ensures that any changes or updates to the middleware's environment variables are made in one place. This reduces the risk of inconsistencies.
  • Reusability: Other tools or components that need the same environment variable information can reuse the RMW API.

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

@fujitatomoya these are excellent points. Our goal is not not bias the various ROS 2 repos towards rmw_zenoh by having this be a permanent change. We will definitely revert this change before Kilted is released but changes like these are needed to help test rmw_zenoh with the existing infra while we upgrade infra in parallel to support starting the router. In fact, @cottsay is working on additions to the RMW API to avoid the changes needed here.
I'll add an agenda item for tomorrow's PMC meeting to discuss this in greater length.

@ros-discourse
Copy link
Copy Markdown

This pull request has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-pmc-minutes-for-march-25-2025/42807/1

@Yadunund
Copy link
Copy Markdown
Member

Yadunund commented Mar 26, 2025

CI (default ros2.repos with changes from this branch): https://gist.githubusercontent.com/Yadunund/c4074b12ea5bdedcddae7b60f60c34bc/raw/d5b9bab270790d8d4d3b8ac0bb737af21da1fa5f/ros2_ci.repos
BUID_ARGS: --packages-up-to ros2action ros2cli ros2doctor ros2lifecycle ros2node ros2param ros2service ros2topic
TEST_ARGS: --packages-select ros2action ros2cli ros2doctor ros2lifecycle ros2node ros2param ros2service ros2topic

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

@ahcorde ahcorde marked this pull request as ready for review March 26, 2025 09:56
@ahcorde
Copy link
Copy Markdown
Contributor

ahcorde commented Mar 26, 2025

Including rmw_zenoh

  • Linux Build Status
  • Linux-aarch64 Build Status
  • Linux-rhel Build Status
  • Windows Build Status

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.

5 participants