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

[Bug] Cannot write ROS 2 Launch file as mandatory "--ros-args" not handled by Zenoh #38

Closed
ciandonovan opened this issue Dec 20, 2023 · 6 comments
Labels
bug Something isn't working

Comments

@ciandonovan
Copy link

Describe the bug

I want to run Zenoh as a ROS 2 package in a container. However, ros2 run does not install an explicit signal handler for SIGTERM, and when run as PID1 in a container, a special case where the default disposition of those signals being changed from Terminate to Ignore applies. Further, if the container is initialized with a lightweight init manager such as tini, ros2 run will explicitly set the disposition of SIGINT to Ignore because it doesn't detect a controlling terminal.

I therefore want to create a simple launchfile that correctly handles these signals (notably SIGINT) and forwards them to its children. The launchfile will be configured explicitly with "emulate_tty=True" to trick Zenoh installing the SIGINT handler. The problem is that as far as I can tell, ROS 2 Launch will always append --ros-args to argv of the node its launching, and Zenoh doesn't like that.

Could Zenoh be modified to detect --ros-args and harmlessly ignore it? At least until there's possibly more ROS-specific functionality integrated in which care maybe reading the args might be useful.

To reproduce

zenoh_launch.py

from launch import LaunchDescription
from launch_ros.actions import Node

def generate_launch_description():
    return LaunchDescription([
        Node(
            package='zenoh_bridge_ros2dds',
            executable='zenoh_bridge_ros2dds',
            name='zenoh_bridge_ros2dds',
            emulate_tty=True,
        )
    ])

Error log:

[INFO] [launch]: Default logging verbosity is set to INFO
[INFO] [zenoh_bridge_ros2dds-1]: process started with pid [21470]
[zenoh_bridge_ros2dds-1] [2023-12-20T19:34:39Z INFO  zenoh_bridge_ros2dds] zenoh-bridge-ros2dds v0.1.0-dev built with rustc 1.70.0 (90c541806 2023-05-31) (built from a source tarball)
[zenoh_bridge_ros2dds-1] error: Found argument '--ros-args' which wasn't expected, or isn't valid in this context
[zenoh_bridge_ros2dds-1] 
[zenoh_bridge_ros2dds-1] 	If you tried to supply `--ros-args` as a value rather than a flag, use `-- --ros-args`
[zenoh_bridge_ros2dds-1] 
[zenoh_bridge_ros2dds-1] USAGE:
[zenoh_bridge_ros2dds-1]     zenoh_bridge_ros2dds [OPTIONS]
[zenoh_bridge_ros2dds-1] 
[zenoh_bridge_ros2dds-1] For more information try --help
[ERROR] [zenoh_bridge_ros2dds-1]: process has died [pid 21470, exit code 2, cmd '/opt/ros/overlay_ws/install/zenoh_bridge_ros2dds/lib/zenoh_bridge_ros2dds/zenoh_bridge_ros2dds -c /root/.zenoh/lab.json5 -- --ros-args'].
[zenoh_bridge_ros2dds-1] 

System info

  • Platform: Official Open Source Robotics Foundation Docker image (Humble) running on Debian 12 host.
  • CPU: AMD Ryzen Embedded V2718 with Radeon Graphics
  • Zenoh verison/commit: 2c52d0b
@ciandonovan ciandonovan added the bug Something isn't working label Dec 20, 2023
@JEnoch
Copy link
Member

JEnoch commented Jan 5, 2024

It would indeed be nice is zenoh-bridge-ros2dds acts as a ROS 2 Node and supports all or at least a subset of ROS Command Line Args.

A first step would be to just log a "not supported ROS argument ... - ignored" warning for each one.
No warning will be logged in your case, according to your cmd line.

But strangely, when I try your zenoh_launch.py in my Humble env (not in container), it run this cmd line which is different than yours:

[ERROR] [zenoh_bridge_ros2dds-1]: process has died [pid 78745, exit code 2, cmd '/home/guest/eclipse-zenoh/zenoh-plugin-ros2dds/install/zenoh_bridge_ros2dds/lib/zenoh_bridge_ros2dds/zenoh_bridge_ros2dds --ros-args -r __node:=zenoh_bridge_ros2dds'].

So I think at least -r __node:=node_name should be supported and mapped to the ros2dds.nodename configuration.
I created #48 to address this.

@ciandonovan
Copy link
Author

But strangely, when I try your zenoh_launch.py in my Humble env (not in container), it run this cmd line which is different than yours:

Looks like you're only getting the std output from ros_launch and not the nodes running underneath, is your logging verbosity set differently on your host install? (Mine is INFO) Were you running with emulate_tty=True so ros_launch would pass through the std output from the node?

@JEnoch
Copy link
Member

JEnoch commented Jan 8, 2024

I get roughly the same logs than you with "error" and "usage" messages.
But I was pointing to the command lines and especially to the ordering of arguments that differ between you:
cmd '/opt/ros/overlay_ws/install/zenoh_bridge_ros2dds/lib/zenoh_bridge_ros2dds/zenoh_bridge_ros2dds -c /root/.zenoh/lab.json5 -- --ros-args'
and me:
cmd '/home/guest/eclipse-zenoh/zenoh-plugin-ros2dds/install/zenoh_bridge_ros2dds/lib/zenoh_bridge_ros2dds/zenoh_bridge_ros2dds --ros-args -r __node:=zenoh_bridge_ros2dds'

I find yours quite strange because:

  1. the -- appears before the --ros-args, while the spec says it shall be the other way around
  2. you don't have a -r __node:=zenoh_bridge_ros2dds option while I have, despite I was using the same zenoh_launch.py with a Node description specifying the name= zenoh_bridge_ros2dds
  3. I don't see the -c /root/.zenoh/lab.json5 arguments in your zenoh_launch.py. How do you set those ?

For a correct support of --ros-args I would need to understand what's the cause of those differences, and moreover what to expect as arguments.

@ciandonovan
Copy link
Author

https://github.com/ciandonovan/ros2_zenoh_bridge

Here's a full reproducible example of my setup, running the latest zenoh-plugin-ros2dds main branch installed as a ROS package in a submodule.

I'm not sure how I added -c /root/.zenoh/lab.json5 before, I may have accidentally copied the wrong output before.
As of now, I think my output matches yours.

@JEnoch
Copy link
Member

JEnoch commented Jan 17, 2024

#48 is now complete. zenoh-bridge-ros2dds now supports --ros-args but only with -r __ns:=... or -r __node:=....
I tested with the podman example you provided.

I created #58 for an eventual support of remapping also for topics/services/actions names.

@JEnoch
Copy link
Member

JEnoch commented Jan 17, 2024

Fixed via #58

@JEnoch JEnoch closed this as completed Jan 17, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

No branches or pull requests

2 participants