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

supplement how to use Loaned Messages. #5068

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

Conversation

fujitatomoya
Copy link
Collaborator

ROS 2 documentation does not really explains how to use LoanedMessages or demonstration but only how to disable it.
It would be better to add more general description about LoanedMessages including RMW implementation support status.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya fujitatomoya self-assigned this Mar 4, 2025
@fujitatomoya fujitatomoya added the backport-all backport at reviewers discretion; from rolling to all versions label Mar 4, 2025
@fujitatomoya
Copy link
Collaborator Author

@christophebedard @MiguelCompany can you review this?
CC: @kscottz

Note

We can backport this as it is to jazzy and humble.

Copy link

github-actions bot commented Mar 4, 2025

HTML artifacts: https://github.com/ros2/ros2_documentation/actions/runs/13682385638/artifacts/2698157881.

To view the resulting site:

  1. Click on the above link to download the artifacts archive
  2. Extract it
  3. Open html-artifacts-5068/index.html in your favorite browser

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Collaborator Author

@MiguelCompany i addressed your comment, could you take a look again? thanks in advance.

@alsora
Copy link
Contributor

alsora commented Mar 5, 2025

We should at least point out this critical bug: ros2/rclcpp#2401
Until it's fixed, using loaned messages is not really possible outside of trivial examples or very specific and controlled use-cases

@fujitatomoya
Copy link
Collaborator Author

@alsora i think the problem is already linked in https://docs.ros.org/en/humble/How-To-Guides/Configure-ZeroCopy-loaned-messages.html#subscriptions to disable loaned message on subscription side. but i can also add ros2/rmw_cyclonedds#469 to the doc too.

Signed-off-by: Tomoya Fujita <Tomoya.Fujita@sony.com>
@fujitatomoya
Copy link
Collaborator Author

fujitatomoya commented Mar 5, 2025

We should at least point out this critical bug: ros2/rclcpp#2401 Until it's fixed, using loaned messages is not really possible outside of trivial examples or very specific and controlled use-cases

@alsora i just added ed3e8bf, what do you think?

@alsora
Copy link
Contributor

alsora commented Mar 6, 2025

Thanks, I didn't notice that it was already there at the end of the file.
However I still think that it should be very prominent.

My understanding is that as of today, loaned messages do not allow to achieve zero copy because as that ticket points out, they are disabled in subscriptions (and are unsafe to enable).

Advertising a broken feature can be dangerous.

Are there advantages in using this feature until that bug is resolved?

@fujitatomoya
Copy link
Collaborator Author

yeah, right there could be consequences on the subscription if user changes the setting from default, that is why in default it copies the message data from middleware. but how about publisher aspect? i thought that is still benefic for user application as a feature and it is safe as long as being with default setting?

what i am trying to add here is to add general explanation about LoanedMessage feature with sample. i am not trying to push unsafe behavior to enable LoanedMessage on the subscription, that is well-described the following section.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
backport-all backport at reviewers discretion; from rolling to all versions
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants