eclipse-zenoh / zenoh-plugin-ros2dds

A Zenoh plug-in for ROS2 with a DDS RMW. See https://discourse.ros.org/t/ros-2-alternative-middleware-report/ for the advantages of using this plugin over other DDS RMW implementations.
https://zenoh.io
Other
126 stars 29 forks source link

Expose all ROS Nodes of a remote system instead of just a `zenoh_bridge_ros2dds` Node #79

Open JEnoch opened 9 months ago

JEnoch commented 9 months ago

Describe the feature

It was a deliberate choice for the first implementation to expose only 1 zenoh_bridge_ros2dds Node in place of all the remote Nodes this bridge is serving. My fear is that in a system with tens of robots, each with thousands of Nodes, the ROS graph within a fleet manager host would be overwhelmed with >10000 Nodes.

However, this causes troubles for Parameters support (#72) and prevent some other use cases where a granular remote view of the ROS Graph for a robot is required.

The plugin should provide an option allowing to choose if a bridge exposes all the Nodes of a remote system, or only 1 zenoh_bridge_ros2dds for all it's topics/services/actions, with the drawback that parameters are not supported (later we might consider to have the bridge Node itself re-exposing all the parameters of remote Nodes).

The question of default behaviour remain open. Any opinion from ROS community ?

Timple commented 8 months ago

My expected behavior was that the bridge would be 'transparent'. So all our robots would have a bridge, our laptop can connect it's bridge to one of the robots. Then doing a ros2 node/topic/service/param list would give exactly the same information on my laptop as it would on the robot.

So the least alteration of information would be preferred i.m.o.

mdxtinkernick commented 8 months ago

I am currently setting this up in the context of #85 - the current behaviour fixes one problem we had and creates a new one - students can no longer introspect the robot with rqt_graph.

Being able to set the behaviour will be great - it would nice as well in rqt_graph to have the bridge be considered as a debug node so you have the option of displaying it or not, or maybe even have a new category of bridge, but that is probably a wider conversation.

My preference would be for the default setting to be that it exposes all the nodes as that fits with my use case :)

For larger deployments I would expect you are going to do test with a couple of devices first, and then consider how to adjust parameters for your system, which is what I am doing at the moment. So if/when that functionality gets added I will be happy and it doesn't matter what the default is.

As this bridge is a good solution for some of the current issues with turtlebot4 there maybe people looking to use it as a simple solution with a single robot with just two bridges on the system. For them it would be better if the behaviour is as close to the behaviour without the bridges, without needing to set or understand any parameters.

mdxtinkernick commented 8 months ago

Is this expected to be in the 1.0.0 release in April, or is it further down the line than that ? I couldn't see it mentioned in the roadmap - but don't know whether this is something that ultimately needs work in the main Zenoh repo to set it up, or is something that just needs to work in this repo

JEnoch commented 8 months ago

No need to change anything in main Zenoh repo. Only this repo need some changes for this.

Unfortunately I didn't found time to work on this (and any other issue from this repo) during the pas weeks. Hopefully I'll get some time in the coming weeks, since I still would like to add this for 1.0.0.

mdxtinkernick commented 8 months ago

Thanks for the quick reply. What needs changing/adding - I've looked through the code for the plugin and bridge to see if there was something obvious that set that option, but could not work it out. If you can point me in the right direction I would be interested in trying to get it working - no promises as I haven't done any rust before, but this feels like a way to start to learn some - with the motivation that I need the feature

mdxtinkernick commented 8 months ago

I think I have found it in the zenoh dds bridge - it looks like it is the forward discovery setting

JEnoch commented 8 months ago

With this forward discovery setting the zenoh-bridge-dds forwards to remote bridges the ParticipantInfo message received on ros_discovery_into topic as such. Then the remote bridges are updating this message with their own Readers/Writers GIDs before republishing it on ros_discovery_into topic.

The zenoh-bridge-ros2dds has a different and more efficient way to propagate the discovery information than forwarding the ParticipantInfo messages. It relies on Zenoh Liveliness Tokens which are more compact and reactive to connection losses. Moreover, it optimizes the propagation of Services and Actions with only 1 token for each, where ParticipantInfo message requires 2 GIDs per Service (because of Request and Reply topics) and 8 per Action (3 Services + 2 topics).

Thus, copying the forward discovery setting from zenoh-bridge-dds is not the way to go. It rather requires a change in the key expressions used for Liveliness Tokens to also include the Node's name and namespace. And then, to reflect those in the ParticipantInfo data maintained by the bridge in ros_discovery.rs file. Currently it contains only 1 NodeEntitiesInfo with the bridge's name and namespace. This has to change for 1 NodeEntitiesInfo per-Node, following what's announced by remote bridges via the Liveliness Tokens.

Moreover, it will certainly happen that 2 remote bridges discover different Nodes with a Subscriber on the same topic for instance. In such case, the bridge creates only 1 Subscriber per topic. Otherwise it would receive all publication twice and have to filter out duplicates before routing via Zenoh. Thus, only 1 DDS Reader is created. Still, for the ROS graph to reflect different Nodes with distinct Readers. I hope that reusing the same Reader's GID in distinct NodeEntitiesInfo won't cause troubles to ROS when reconstructing the ROS graph.

mdxtinkernick commented 8 months ago

thanks - that all makes sense, am looking into it

mdxtinkernick commented 5 months ago

Hi

Not been able to spend the time I had hoped to, but have been looking at this for a few days now. At the moment my basic knowledge in rust and zenoh doesn't feel close to enough to work this out. I will continue to try and make progress, but I think that will be closer to personal development than functional contributions for now.

mdxtinkernick commented 4 months ago

Have made some progress - added a node field to the publisher liveness key, and altered all the functions that use that key so the code compiles ok. Have put in a hard coded value for now where the liveness token gets built as I am struggling with separating out the ros2-dds-zenoh-rust terms to see where the name of the node name in the originating ros2 system can be found in ros_discovery.rs.