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

Add ROS2 documentation #379

Merged
merged 10 commits into from
Dec 17, 2024
Merged

Conversation

martinaxgloria
Copy link
Member

This PR aims to:

  • Remove the documentation about the ROS1 support (deprecated);
  • Add the documentation about the ROS2 support based on what was documented previously (I added an admonition at the top of the page, saying that we are not supporting ROS1 anymore) and a brief paragraph with the xcub-moveit2 link to the documentation.

As the last supported **ROS 1** distribution (ROS Noetic Ninjemys) is reaching its End of Life (EOL) on May 2025, with the distro release [`v2024.11.1`](../sw_versioning_table/2024.11.1.md) ROS1 support was deprecated.

## Overwiew
Make sure that you read [YARP's documentation about ROS](https://yarp.it/latest/group__nws__and__nwc__architecture.html#autotoc_md58).
Copy link
Member

Choose a reason for hiding this comment

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

I am not sure this link is particularly useful, what do you think?

Copy link
Member

Choose a reason for hiding this comment

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

I would get rid of it.


All the YARP [Network Wrapper Server (NWS) and Network Wrapper Client (NWC)](https://www.yarp.it/latest//group__nws__and__nwc__architecture.html) devices that use ROS 2 can be found under [`yarp-devices-ros2`](https://github.com/robotology/yarp-devices-ros2) repository.

`yarp-devices-ros2` contains the devices and utilities for YARP-ROS2 compatibility. The devices are in the form of NWS/NWC that read and/or write information from ROS2 topics and make them available via the YARP API. Starting from the distro [`v2024.11.1`](../sw_versioning_table/2024.11.1.md), it could be compiled within the robotology-superbuild by enabling the `ROBOTOLOGY_USES_ROS2` CMake option .
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`yarp-devices-ros2` contains the devices and utilities for YARP-ROS2 compatibility. The devices are in the form of NWS/NWC that read and/or write information from ROS2 topics and make them available via the YARP API. Starting from the distro [`v2024.11.1`](../sw_versioning_table/2024.11.1.md), it could be compiled within the robotology-superbuild by enabling the `ROBOTOLOGY_USES_ROS2` CMake option .
`yarp-devices-ros2` contains the devices and utilities for YARP-ROS2 compatibility. The devices are in the form of NWS/NWC that read and/or write information from ROS2 topics and make them available via the YARP API. Starting from the distro [`v2024.11.1`](../sw_versioning_table/2024.11.1.md), it can be compiled within the robotology-superbuild by enabling the `ROBOTOLOGY_USES_ROS2` CMake option .

@traversaro
Copy link
Member

Style comment: unless used in code, the suggested spelling is ROS 2, not ROS2 (see https://shouldiputaspacebetweenrosand2.com/).


### Modules description

`controlBoard_nws_ros2` is the controlBoard network wrapper server for ROS2. As per the controlBoard_nws_yarp, this device can be used to publish the joint position information on ROS2 topics instead of YARP ports. The device uses the [yarpDeviceParamParserGenerator](https://yarp.it/latest/group__yarpDeviceParamParserGenerator.html) and [here](https://yarp.it/latest/classControlBoard__nws__ros2__ParamsParser.html) there is the list of the required parameters.
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
`controlBoard_nws_ros2` is the controlBoard network wrapper server for ROS2. As per the controlBoard_nws_yarp, this device can be used to publish the joint position information on ROS2 topics instead of YARP ports. The device uses the [yarpDeviceParamParserGenerator](https://yarp.it/latest/group__yarpDeviceParamParserGenerator.html) and [here](https://yarp.it/latest/classControlBoard__nws__ros2__ParamsParser.html) there is the list of the required parameters.
`controlBoard_nws_ros2` is the controlBoard network wrapper server for ROS2. As per the `controlBoard_nws_yarp`, this device can be used to publish the joint position information on ROS2 topics instead of YARP ports. The device uses the [yarpDeviceParamParserGenerator](https://yarp.it/latest/group__yarpDeviceParamParserGenerator.html) and [here](https://yarp.it/latest/classControlBoard__nws__ros2__ParamsParser.html) there is the list of the required parameters.

Copy link
Member

Choose a reason for hiding this comment

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

If we keep YARP devices names with fixed font, we can do it consistently.

Comment on lines 40 to 46
More in particular, this file is meant to be used as part of the same `yarprobotinterface` with which you launch the robot. To include it among the devices that you launch with it, you should modify the `ergocub_all.xml`/`icub_all.xml` (or any file that is launched with your `yarprobotinterface` when you launch the robot) to include the line:

```xml
<xi:include href="wrappers/motorControl/alljoints-mc_nws_ros2.xml" />
```

Moreover, the NWS/NWC for YARP and ROS2 are organized into separate wrappers. In this way, it is possible to attach multiple wrappers to the same device, so a `controlBoard_nws_yarp` for YARP and a `controlBoard_nws_ros2` for ROS2, and include them to the same configuration file used to launch the yarprobotinterface.
Copy link
Member

@traversaro traversaro Dec 13, 2024

Choose a reason for hiding this comment

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

At least for ergocub, I think this section may be confusing. All ergoCub robots already have this files defined in their configuration files, so what users needs to do is to run the yarprobotinterface of the robot with the option yarprobotinterface --enable_tags "(enable_ros2)", see robotology/robots-configuration#641 and robotology/robots-configuration#663 .

The situation is a bit different for iCub, where not all iCubs configuration files have been modified to support ROS 2 . Perhaps we can have two different sections, one for ergoCub and one for iCub, and the instructions on the in-detailed modifications of the conf files are just there for iCub? Perhaps we could also suggest users to open an issue in https://github.com/robotology/robots-configuration if they want to use their iCub robots with ROS 2, so that we can direct them in contributing their modified files to the main repo instead of keeping their configuration files forked?

docs/icub_sw.md Outdated
@@ -15,7 +15,7 @@
- [`Standard Calibration Types`](./robot_calibration_types/standard_calibration_types.md)
- [`iCub 3 Calibration Types`](./robot_calibration_types/icub3_calibration_types.md)
- [`Configure IP on a setup for ETH boards`](./configure_static_ip/configure-static-ip.md)
- [`Using iCub with ROS`](./icub_ros/index.md)
- [`Using iCub/ergoCub with ROS2`](./icub-ergocub_ros2/index.md)
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- [`Using iCub/ergoCub with ROS2`](./icub-ergocub_ros2/index.md)
- [`Using iCub or ergoCub with ROS2`](./icub-ergocub_ros2/index.md)

Nitpick, I think explicitly writing "or" is more readable.

mkdocs.yml Outdated
@@ -301,7 +301,7 @@ nav:
- Encoders and Joint limits (manual): icub_testing/encoders_manual.md
- Configure IP on a setup for ETH boards:
- Configure IP on a setup for ETH boards: configure_static_ip/configure-static-ip.md
- Using iCub with ROS: icub_ros/index.md
- Using iCub/ergoCub with ROS2: icub-ergocub_ros2/index.md
Copy link
Member

Choose a reason for hiding this comment

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

Suggested change
- Using iCub/ergoCub with ROS2: icub-ergocub_ros2/index.md
- Using iCub or ergoCub with ROS2: icub-ergocub_ros2/index.md

Nitpick, I think explicitly writing "or" is more readable.


`yarp-devices-ros2` contains the devices and utilities for YARP-ROS2 compatibility. The devices are in the form of NWS/NWC that read and/or write information from ROS2 topics and make them available via the YARP API. Starting from the distro [`v2024.11.1`](../sw_versioning_table/2024.11.1.md), it could be compiled within the robotology-superbuild by enabling the `ROBOTOLOGY_USES_ROS2` CMake option .

## Publishing `iCub/ergoCub`'s joints state: the `controlBoard_nws_ros2` module
Copy link
Member

Choose a reason for hiding this comment

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

Perhaps we should explain that this just applies to real robots, while we could have a different section for simulated robots?

Copy link
Member Author

Choose a reason for hiding this comment

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

Do you mean the entire documentation or a specific paragraph?

Copy link
Member

Choose a reason for hiding this comment

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

At least the paragraph it seems to me that it only applies to real robot, right? I am not sure about the rest of the documentation, but if also the rest it does only apply to real robots and not simulated ones, I think it may be worth mentioning it.

Copy link
Member Author

Choose a reason for hiding this comment

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

Maybe I'm wrong, but after reading the entire page I added it seems that it could be applied to both real and simulated robots. The only difference between the two it's the usage of controlBoard_nws_ros2 since in simulation it is required to modify the urdf for passing the proper XML to the yri plugin (cc @Nicogene)

Copy link
Member

Choose a reason for hiding this comment

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

The only difference between the two it's the usage of controlBoard_nws_ros2 since in simulation it is required to modify the urdf for passing the proper XML to the yri plugin (cc @Nicogene)

Exactly. I may be wrong, but I am not sure this is step that is obvious for the target reader of this documentation (for example, I am afraid it would not be obvious that how "yarprobotinterface --enable_tags "(enable_ros2)" needs to be translated in the case of simulated robots, see also robotology/gz-sim-yarp-plugins#163 .

So either we document this, or we just clearly state that the documentation is just targeting real robots.

Copy link
Member Author

Choose a reason for hiding this comment

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

As far as I understand, the enable/disable_tags are not available for simulation models yet (correct me if I'm wrong). If it's so, I prefer to specify that the documentation refers to the real robots.

Copy link
Member

Choose a reason for hiding this comment

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

Yes they are not supported in gz-sim at the moment, any solution is fine for me.

@martinaxgloria
Copy link
Member Author

I think I addressed the requested changes via 31775e4

@pattacini
Copy link
Member

Thanks @martinaxgloria
I'll wait for @traversaro's final pass before proceeding.

Copy link
Member

@pattacini pattacini left a comment

Choose a reason for hiding this comment

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

Fine by me!
I've pushed some inline mods to make it read a bit more smoothly.

A general comment when working with links: try to avoid attaching links to words like here or the like.

docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
docs/icub-ergocub_ros2/index.md Outdated Show resolved Hide resolved
@pattacini
Copy link
Member

Hi @traversaro, if the PR looks good to you I'll be happy to merge it!

@traversaro
Copy link
Member

Sure, I had missed the sentence on real robots.

@pattacini pattacini merged commit 52ba64e into icub-tech-iit:master Dec 17, 2024
1 check passed
@martinaxgloria martinaxgloria deleted the feat/ros2_doc branch December 17, 2024 08:07
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.

3 participants