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

Router config: set peers_failover_brokering to false #341

Merged
merged 1 commit into from
Dec 17, 2024

Conversation

JEnoch
Copy link
Contributor

@JEnoch JEnoch commented Dec 16, 2024

In Zenoh, the routing.router.peers_failover_brokering configuration is by default set to true.

This setting makes a router to route the messages between 2 peers, until it detects (via gossip scouting) that they are directly inter-connected with each other. For other use cases than ROS, it makes sense to have this configuration enabled by default in case some peers cannot establish direct connections.

But in ROS default configuration, all Nodes are running on the same host and will establish direct connections with each other. If some Nodes are running on a distinct hosts, they could:

  • either rely on a co-localized router to communicate with the robot's nodes (peer-router-router-peer connection)
  • either be configured in "client" mode connected to the robot's router (client-router-peer connection, e.g. for RViz)

Thus, for ROS having peers_failover_brokering: true serves no purpose and has some drawbacks related to the additional management overhead by the router and extra messages during the Nodes startup.

Copy link
Collaborator

@clalancette clalancette left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Makes sense to me, thanks for the detailed explanation.

@clalancette clalancette merged commit df6ec26 into ros2:rolling Dec 17, 2024
6 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants