Allow zenoh tests to run with multicast#992
Conversation
Signed-off-by: Michael Carroll <mjcarroll@intrinsic.ai>
Signed-off-by: Yadunund <yadunund@gmail.com>
6d630f0 to
ffc7d2d
Compare
| return lines | ||
|
|
||
|
|
||
| def get_rmw_additional_env(rmw_implementation: str) -> Dict[str, str]: |
There was a problem hiding this comment.
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.
There was a problem hiding this comment.
@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.
|
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 |
|
CI (default ros2.repos with changes from this branch): https://gist.githubusercontent.com/Yadunund/c4074b12ea5bdedcddae7b60f60c34bc/raw/d5b9bab270790d8d4d3b8ac0bb737af21da1fa5f/ros2_ci.repos |
No description provided.