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

Ref 22742. Easy mode CLI tutorial #241

Open
wants to merge 3 commits into
base: main
Choose a base branch
from

Conversation

EugenioCollado
Copy link
Contributor

No description provided.

Signed-off-by: Eugenio Collado <[email protected]>
@EugenioCollado EugenioCollado added the documentation Improvements or additions to documentation label Mar 20, 2025
@EugenioCollado EugenioCollado force-pushed the 22742/easy_mode_cli_tutorial branch from 45473ce to 17b810c Compare March 21, 2025 07:11
Signed-off-by: Eugenio Collado <[email protected]>
@EugenioCollado EugenioCollado force-pushed the 22742/easy_mode_cli_tutorial branch from 17b810c to 0330a4e Compare March 21, 2025 07:16
Signed-off-by: Eugenio Collado <[email protected]>
Copy link
Contributor

@cferreiragonz cferreiragonz left a comment

Choose a reason for hiding this comment

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

Great tutorial explaining all the CLI features with GIFs and diagrams! Leaving some suggestion to clarify a few things


.. code-block:: bash

# Terminal 1 -> Host A
Copy link
Contributor

Choose a reason for hiding this comment

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

Again, if you follow the tutorial linearly, these servers will already be running. Should we stop them after each sub-section to have a "clean" setup?

Comment on lines +204 to +205
This command can be used on any server to connect it to any other server, removing the concept of master servers and enabling a fully meshed network where all
servers can communicate with each other. For example, if A connects to B and B connects to C, then A and C are also connected.
Copy link
Contributor

@cferreiragonz cferreiragonz Mar 31, 2025

Choose a reason for hiding this comment

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

From my point of view, this seems to indicate that the add command is responsible of the mesh topology, which is not accurate. If A is the master and B and C connect to it, B and C will also be connected, without the need of using the add command. I would rewrite this to explain that the add command enables joining new servers to the topology without the need of restarting them or if a new master_ip wants to be used. Also, we can add the explanation of the mesh topology to the note of the Start section that details the concept of master_ip

This can be observed as the publisher gets stuck in the "Waiting for at least one matching subscriber" state.
* Next, we will add Host B as a remote server to the Discovery Server on Host A. This establishes a connection between A and B, but Host C still does not receive any data.
* Finally, we will add Host C as a remote server to the Discovery Server on Host B. Due to the meshed network setup, Hosts A, B, and C are now fully connected.

Copy link
Contributor

Choose a reason for hiding this comment

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

We need to add the commands that the user must use in each terminal in order to replicate this example

# Terminal 1 -> Host A
ROS2_EASY_MODE=$B_IP fastdds discovery start -d 1 $B_IP:1
ROS_DOMAIN_ID=1 ROS2_EASY_MODE=$B_IP ros2 topic pub /chatter std_msgs/msg/String "{data: 'Hello world in domain 1 from HOST_A'}" --once --spin-time 2
ROS2_EASY_MODE=$C_IP fastdds discovery set -d 1 $C_IP:1
Copy link
Contributor

Choose a reason for hiding this comment

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

We need to handle this case in the CLI

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
documentation Improvements or additions to documentation
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants