Documentation Error - Efficient Intra Process Communication

[chedanix]

Documentation Location: Website: https://docs.ros.org/en/foxy/Tutorials/Intra-Process-Communication.html?highlight=intra Github File: source/Tutorials/Intra-Process-Communication.rst

Under the section Pipeline with two image viewers, the image viewers do not show the correct information. When I perform the same command ros2 run intra_process_demo image_pipeline_with_two_image_view, all six addresses are exactly the same. However, in the current documentation, the addresses are different, as shown below.

The documentation points out the address being different. But the explanation then says the two image viewers should share the same image resource. This is contradicting because if the same image resource is shared, all six addresses should be the same, as shown below. I ran it on Ubuntu 20.04 with ROS Galactic.

Note that it does take several tries to get all six addresses to be the same because of timing issue. However, even if it's not timed correctly, the three addresses in each image viewer should still be the same, unlike the current documentation.

I would like to become a contributor. Please let me fix it if this is indeed an error.

[ivanpauno]

I think the wording is correct but the image was never updated, right? The same seems to happen in this section.

I would like to become a contributor. Please let me fix it if this is indeed an error.

Sounds good! Feel free to tag me in the PR to ask for a review.

[roncapat]

I think this is partially correct... but:

• first two addresses of both images shall be always the same.
• one of the two images shall also show the third address equal to the first two
• the other one should be different

If I'm not wrong, only 5 addresses shall be equal then.

Here is my screenshot:

I can make a PR, but please confirm me first my reasoning is ok.

[ivanpauno]

I can make a PR, but please confirm me first my reasoning is ok.

Yes, that's fine.

The same seems to happen in this section.

I think this one could also be updated. Again, 5 pointers should be the same and the image view node in a separate process should've a different pointer.

[roncapat]

Correct. However, the tutorial tells the user to stop the viewers to check for addresses, while actual ROS2 intra-process and demo nodes implementation will always show a stable address for every one of the six shown, so no more need to stop the viewers and tell the user to do so neiter. Shall I rewrite the affected paragraphs also?

[roncapat]

Another question: shall I push to rolling? I tested the above assumptions on ROS2 Foxy, so from Foxy onwards it should be ok.

[roncapat]

Here are the two screenshots, ready for inclusion

https://imgur.com/a/N8dz122

I can do the push when someone tells me the right branch :)

[fujitatomoya]

shall I push to rolling?

yes, we can fix the rolling and then we can do backport. that is usual procedure.

[ijnek]

The Efficient Intra Process Communication guide seems heavily outdated, it mentions that unique_ptr is the only way to achieve zero-copy intra process comms, but there's been major improvements and shared_ptrs can be used too.

I believe https://design.ros2.org/articles/intraprocess_communications.html is packed with the most recent intra process design and we should rewrite this guide based off that.

---

@chedanix:

I would like to become a contributor. Please let me fix it if this is indeed an error.

If there is quite a bit of work, are you still willing to do the work or are you happy for someone else to make the changes? If I don't hear back, I might go ahead and start making changes.

[clalancette]

The Efficient Intra Process Communication guide seems heavily outdated, it mentions that unique_ptr is the only way to achieve zero-copy intra process comms, but there's been major improvements and shared_ptrs can be used too.

From an end-user point-of-view, I don't think it is heavily outdated (just mildly outdated). That is, while the implementation details of how we deliver data between publishers and subscribers has changed significantly, I don't think that it makes a large change on that API. That said, I could be wrong so looking over the page and fixing it up with the latest information is definitely appreciated.

I believe https://design.ros2.org/articles/intraprocess_communications.html is packed with the most recent intra process design and we should rewrite this guide based off that.

Just to be doubly clear, I don't think we should pull all of those details in here. This is a tutorial, so is meant to show the user how to use this stuff, not exactly how it is implemented. We should definitely have a link to the design article in there, and should make sure the content is up-to-date, but I wouldn't put details of the ring buffers (for instance) in that tutorial.

[ijnek]

@clalancette I believe the code itself is correct (works), but the demo itself is missing some important points that have changed due to PRs that were merged since the article was first written. (Although it may be just be personal preference), the way the code is written also doesn't seem to follow the current best practices of writing a node.

Just to be doubly clear, I don't think we should pull all of those details in here. This is a tutorial, so is meant to show the user how to use this stuff, not exactly how it is implemented. We should definitely have a link to the design article in there, and should make sure the content is up-to-date, but I wouldn't put details of the ring buffers (for instance) in that tutorial.

Yes, I don't mean to put the details in here. We can add a link to it, but more importantly come up with demos that cover the capabilities of the intra process comms that have changed over time.

I've raised a new issue to discuss improvements: #2955