uupks commented 5 months ago

rmw_zenoh now add DOMAIN_ID and replace / with % when creating zenoh key, take demo_nodes_cpp/talker for example:

NameSpace	NodeName	TopicName	ZenohKey
	talker	/chatter	0/%chatter
	talker	/talker/get_parameters	0/%talker%get_parameters
demo	talker	/demo/chatter	0/%demo%chatter
demo	talker	/demo/talker/get_parameters	0/%demo%talker%get_parameters

While zenoh-plugin-ros2dds map ros topic name to zenoh key in this way:

NameSpace	NodeName	TopicName	ZenohKey
	talker	/chatter	chatter
	talker	/talker/get_parameters	talker/get_parameters
demo	talker	/demo/chatter	demo/chatter
demo	talker	/demo/talker/get_parameters	demo/talker/get_parameters

We need use zenoh key when communicate with zenoh native applications.

When using 0/%demo%chatter, zenoh-pico listener can receive messages correctly, but it is difficult to get the correct zenoh key. So, was there any specific considerations mapping zenoh key initially? Can we just map ROS topic names similar to how zenoh-plugin-ros2dds does?

Yadunund commented 5 months ago

We need use zenoh key when communicate with zenoh native applications.

The intended use case of this rmw is not to communicate with other native Zenoh applications such as zenoh-pico listener.

but it is difficult to get the correct zenoh key.

Can you explain what the difficulty is? We simple map / to % so you should be able to map it backwards. The only difference between the mapping in rmw_zenoh and the zenoh-plugin-ros2dds is that the latter uses § as the slash replacement instead.

uupks commented 5 months ago

The intended use case of this rmw is not to communicate with other native Zenoh applications

Yes, but that is a capability of zenohd.

Can you explain what the difficulty is? We simple map / to % so you should be able to map it backwards.

The Zenoh key is prefixed with the ros_domain_id. We cann't get the DOMAIN_ID via ros2 topic list on ohter host...

As for zenoh-plugin-ros2dds, with DEBUG log:

2024-06-13T06:18:44.680452Z DEBUG async-std/runtime ThreadId(35) zenoh_plugin_ros2dds::route_service_srv: Route Service Server (ROS:/talker/get_parameters <-> Zenoh:talker/get_parameters) announce via token @ros2_lv/37cdaaef9a265d7a04ac82e51051f07c/SS/talker§get_parameters/rcl_interfaces§srv§GetParameters

2024-06-13T06:18:44.679489Z  INFO async-std/runtime ThreadId(35) zenoh_plugin_ros2dds::routes_mgr: Route Publisher (ROS:/chatter -> Zenoh:chatter) created
2024-06-13T06:18:44.679525Z DEBUG async-std/runtime ThreadId(35) zenoh_plugin_ros2dds::route_publisher: Route Publisher (ROS:/chatter -> Zenoh:chatter) now serving local nodes {"/talker"}

@ros2_lv/37cdaaef9a265d7a04ac82e51051f07c/SS/talker§get_parameters/rcl_interfaces§srv§GetParameters seems like a liveness token and talker/get_parameters is the zenoh keyexpr.

@JEnoch Do you have any good suggestions?

JEnoch commented 5 months ago

I see 3 points to consider when defining the mapping of ROS topics/services/actions names to Zenoh key expressions:

1. ROS_DOMAIN_ID

The ROS_DOMAIN_ID have been introduced as a mapping to the DDS Domain ID. See the reasons in documentation:

ROS 2 nodes on the same domain can freely discover and send messages to each other, while ROS 2 nodes on different domains cannot. All ROS 2 nodes use domain ID 0 by default. To avoid interference between different groups of computers running ROS 2 on the same network, a different domain ID should be set for each group.

Using Namespaces within the same Domain is not enough with DDS because even if using difference namespaces avoid topics conflicts, the DDS discovery still occurs between all Nodes despite the namespaces and it's already quite a burden.

Thus in my opinion the ROS_DOMAIN_ID exists anyway in ROS 2 and is useful for some users. Even with Zenoh it shall prevent communication between Publishers and Suvbscribers on the same topic, but in distinct Domains. If we want to keep this feature, the only solution with current Zenoh is to include the ROS_DOMAIN_ID in the key expression.

That said, most of rmw_zenoh users won't set the ROS_DOMAIN_ID because there is by default no possible interference between different ROS 2 groups. For inter-hosts communications, the connectivity has to be explicitly configured between routers Maybe we could consider the default Domain ID (0) as a specific case which doesn't appear as a prefix in key expressions. The drawback is a complexification of rmw_zenoh, and the surprise of users with pure-Zenoh applications in case they switch from Domain 0 to Domain N.

2. Topic name

The only difference between the mapping in rmw_zenoh and the zenoh-plugin-ros2dds is that the latter uses § as the slash replacement instead.

Actually the zenoh-plugin-ros2dds does this replacement only for Liveleness Tokens keyexprs which also contain type name and QoS. This is an easy trick to make sure the namespaced topic name is always at the same index within the keyexpr. But for pub/sub/queries keyexp the zenoh-plugin-ros2dds keeps the namespaced ROS name. The benefit is a straight forward mapping for developers of pure-Zenoh applications.

The replacement by rmw_zenoh of / with % has another drawback: this forces the key espressions used in configuration of Downsampling and Access Control to use $* instead of * or ** which is less efficient (regex vs. tree traversal).

3. Type name

171 would add to the keyexpr the type name as a suffix, to solve the case where a user use the same topic name, but with different types, leading to a error in subscriber.

I would argue that the same error will still occur even with DDS if the user uses the same type name, but with different type definition (IDL). In my view using different types (by names or definitions) for a same topic is a misusage or a wrong design of the system by the user.

Adding the type name in key expression brings little benefit and complexifies the configuration for Downsampling and Access Control as well as for development of pure-Zenoh applications.

TL;DR

My suggestion is such a mapping:

ROS_DOMAIN_ID	NameSpace	NodeName	TopicName	ZenohKey
0		talker	/chatter	chatter
0		talker	/talker/get_parameters	talker/get_parameters
0	demo	talker	/demo/chatter	demo/chatter
0	demo	talker	/demo/talker/get_parameters	demo/talker/get_parameters
1		talker	/chatter	§1/chatter
1		talker	/talker/get_parameters	§1/talker/get_parameters
1	demo	talker	/demo/chatter	§1/demo/chatter
1	demo	talker	/demo/talker/get_parameters	§1/demo/talker/get_parameters

(Note the § character used in front of Domain ID to distinguish this segment from a namespace)

Yadunund commented 5 months ago

We discussed this further today and here's what we agreed on

Keep the domain_id in the keyexpr (something to revisit later)
Do not replace / with % in keyexprs. Liveliness token keyexpr will still perform the replacement since we need to unpack the string into certain data types by splitting the string based on %.
Add both topic type and type hash to the keyexpr

I will adapt #171 accordingly.

uupks commented 5 months ago

Thus in my opinion the ROS_DOMAIN_ID exists anyway in ROS 2 and is useful for some users. Even with Zenoh it shall prevent communication between Publishers and Suvbscribers on the same topic, but in distinct Domains. If we want to keep this feature, the only solution with current Zenoh is to include the ROS_DOMAIN_ID in the key expression.

This documentation shows that DDS will determine the UDP port according DOMAIN_ID, and according to the rmw_zenoh design,

It is assumed that a Zenoh router is running on the local system. This router will be used for discovery and host-to-host communication. However it is not used for intra-host comms (i.e., as a message broker); that is done via direct peer-to-peer connections.

Here are my suggestions:

Using one zenohd instance for all ROS_DOMAIN_ID will increase the burden on discovery
For each ROS_DOMAIN_ID, use a separate zenohd router, determining the router listen port and session connect port based on the ROS_DOMAIN_ID.

For example: AcutalPort = BasePort + ROS_DOMAIN_ID

ROS_DOMAIN_ID	Router Listen Port	Session Connect Port
0	7447	7447
1	7448	7448

fujitatomoya commented 5 months ago

/assign @Yadunund

ros2 / rmw_zenoh

Specifying different `ros_domain_ids` should connect to different routers and not simply prefix topic keyexpressions #201

1. ROS_DOMAIN_ID

2. Topic name

3. Type name

171 would add to the keyexpr the type name as a suffix, to solve the case where a user use the same topic name, but with different types, leading to a error in subscriber.

TL;DR