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

Updating Intra Process Communication Demo #2955

Open
7 tasks
ijnek opened this issue Aug 16, 2022 · 1 comment
Open
7 tasks

Updating Intra Process Communication Demo #2955

ijnek opened this issue Aug 16, 2022 · 1 comment
Labels
help wanted Extra attention is needed

Comments

@ijnek
Copy link
Contributor

ijnek commented Aug 16, 2022

Intra Process Communication itself has had two major improvements (ros2/rclcpp#690, ros2/rclcpp#778) since Demos / Setting up efficient intra-process communication was written. Although the tutorial has been updated with new information, the whole guide focuses heavily on using unique pointers as (I believe) that was what was available when it was first written.

Although briefly mentioned in the existing docs, the main point to expand upon would be when to use a unique pointer vs when to use a shared pointer to achieve the least number of copies. (Using a unique pointer for subscription when there is more than one subscriber is bad, but a SharedConstPtr actually achieves zero-copy).

Here are some things that I think should be included:

  • Brief summary of what happens during intraprocess publishing
    • Does communication happen through DDS?
  • Publish / Subscribe using UniquePtr vs SharedConstPtr vs msg object
    • Unique_ptr is useful if:
      • You want to modify the incoming message, and (optionally) sent out again without any copying at all
      • Zero-copy only happens if pub/sub it 1-to-1.
    • Shared_ptr is useful if there is:
      • more than one subscriber
      • you don't want to "lose access" to the msg you sent
  • Demos:
    • Demo for when using unique_ptr is useful (cyclic pipeline, and simple camera pipeline)
    • Demo for when using shared_ptr is useful (image pipeline with two image viewers)

ROS2 Design: Intra-process Communications in ROS 2 has the latest information on intra-process communication, so we should update the docs and the demos based off the knowledge from that.

I'd like to hear whether or not this is something that we want, and like to be on the same page on the when to use UniquePtr vs SharedConstPtr topic.

@wjwwood
Copy link
Member

wjwwood commented Sep 1, 2022

So, the purpose of these demos was to demonstrate to ourselves (the original developers) and others that this feature was working. Not necessarily to enumerate all the ways that you might subscribe to something and in which situations each way is best. That's really valuable information, but I don't think that's the purpose of this demo.

So the "Brief summary of what happens during intraprocess publishing" and "Publish / Subscribe using UniquePtr vs SharedConstPtr vs msg object" are really good to have, but probably belong somewhere else, like https://github.com/ros2/ros2_documentation/tree/rolling/source/Concepts for both?

For "Demo for when using unique_ptr is useful", that's the current state of the "cyclic pipeline, and simple camera pipeline" demos I think.

For the "Demo for when using shared_ptr is useful"/"image pipeline with two image viewers", it is intentionally using two unique pointers so that when you get to the end you have different values in the images, thus demonstrating the zero-copy is sensitive to having multiple subscriptions, and in that case only delivers to one of them.

We could have a demo that uses shared pointer in the two image viewers demo, but it would need to be a separate demo with a different purpose. So I would at least advise against that last todo bullet you have.

ROS2 Design: Intra-process Communications in ROS 2 has the latest information on intra-process communication, so we should update the docs and the demos based off the knowledge from that.

Well... I'm not sure we want to try and keep design documents like that as living things. Instead, I think we should probably put a disclaimer on it that it was the original design, but that the implementation has moved on. The reason being is that we'd generally like to move away from the design website and instead place things like this in either a REP or as documentation in the repository where it lives. For example, moving and updating some of the content of that design document to rclcpp's documentation would be best I think. But again, I don't necessarily think the demo needs updating, and instead maybe we should have a new documentation page about it.

Updating the demo to clarify the purpose of it is fine too, maybe cross-referencing to the documentation that covers best practices, etc.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
help wanted Extra attention is needed
Projects
None yet
Development

No branches or pull requests

3 participants