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

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

Open
JEnoch opened this issue Feb 2, 2024 · 10 comments
Labels
enhancement New feature or request

Comments

@JEnoch
Copy link
Member

JEnoch commented Feb 2, 2024

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
Copy link

Timple commented Mar 4, 2024

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
Copy link

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
Copy link

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
Copy link
Member Author

JEnoch commented Mar 22, 2024

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
Copy link

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
Copy link

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

@JEnoch
Copy link
Member Author

JEnoch commented Mar 25, 2024

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
Copy link

thanks - that all makes sense, am looking into it

@JEnoch JEnoch added the enhancement New feature or request label Apr 15, 2024
@mdxtinkernick
Copy link

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
Copy link

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants