Connect/Disconnect notification/callback support on publisher

[fujitatomoya]

Feature request

Feature description

• RMW can notify the event that any subscriptions connect / disconnect on the topic to each publisher.
• Based on this capability, in rclcpp and rclpy, user application can register user callback that will be called when subscription connects or disconnects on the topic for publisher.

ROS 1 Similar function: https://docs.ros.org/en/noetic/api/roscpp/html/classros_1_1NodeHandle.html#ae4711ef282892176ba145d02f8f45f8d

Use case

On visualization node, to suppress unnecessary CPU utilization, enable its visualization process only when RViz is connected.

Related Information / Reference

• image_pipeline comment out subscription match for temporary
• ROS2 publisher callback on subscription match
• pointcloud_to_laserscan does polling to check on subscription match

Implementation considerations

Since all tier-1 rmw implementation is DDS implementation, we can support this feature with on_publication_matched of DataWriterListener Interface. (we confirmed the code of Fast-DDS.)

• [ ] rmw https://github.com/ros2/rmw/pull/331
• [ ] rmw_implementation
• [ ] rmw_fastrtps
• [ ] rmw_connextdds
• [ ] rmw_cyclonedds
• [ ] rcl
• [ ] rclcpp
• [ ] rclpy
• [ ] demo
• [ ] ros2_documentation

[fujitatomoya]

@ivanpauno @wjwwood @asorbini @MiguelCompany @alsora @SteveMacenski @mikeferguson CC: @ros2/middleware_working_group @Barry-Xu-2018

This is to open discussion, could you share your thoughts?

[ivanpauno]

Adding this like a new rmw_event_t seems reasonable to me

[mikeferguson]

I think this is super useful - at the moment, you can't easily build something like rgbd_launch in ROS2 (where all of the various pipeline elements are lazy publishers/subscribers) - I didn't even realize this wasn't already ticketed somewhere...

I had some notes about use cases (and my current lazy subscriber workaround with a timer that checks get_subscription_count once a second) here: https://www.robotandchisel.com/2020/06/16/ubr1-on-ros2/

EDIT: there was also additional information in my post on 5 things ROS2 needed in 2020: https://www.robotandchisel.com/2020/07/29/five-things-ros2/

[alsora]

I'm not familiar with the on_publication_matched API. Would it be equivalent to checking get_subscription_count() on the publishers objects? Obviously having a callback would be much better than continuously checking that API, but I just want to make sure that I understand how it would work.

In general this looks a great feature to add.

[fujitatomoya]

Would it be equivalent to checking get_subscription_count() on the publishers objects?

the main difference is, application can be notified with event on connect/disconnect.

[MiguelCompany]

This would be a nice addition, probably as a new rmw_event_t as @ivanpauno suggests. We could interpret it as having an event notifying of changes on get_subscription_count()

We could do the same for subscriptions, though we should probably have that as a separate issue.

[Barry-Xu-2018]

We all agreed adding new event. I submitted a PR to add connect/disconnect event and define rmw_matched_status_t for publisher and subscription event callback.
Please review it.

[Barry-Xu-2018]

@fujitatomoya

We should remove rmw_implementation in change list since the rmw interface isn't changed in https://github.com/ros2/rmw/pull/331

[MiguelCompany]

We should remove rmw_implementation in change list since the rmw interface isn't changed in #331

@Barry-Xu-2018 I don't think so, since that repository is there the tests for this feature should be added.

[Barry-Xu-2018]

@MiguelCompany

I don't think so, since that repository is there the tests for this feature should be added.

Thank you for your kind reminder.
Yes. I will check and add tests.

[Barry-Xu-2018]

In rmw_implementation, there is no test related to event currently (Including existing events).
So I need to add events test file.
Besides, rmw_implementation test depend on rmw_xxxx (e.g. rmw_fastrtps).
So I will implement rmw_fastrtps, rmw_connextdds, rmw_cyclonedds at first and then add test to rmw_implementation.

[fujitatomoya]

@Barry-Xu-2018 thanks for checking, i was thinking that there would be some related code or test cases in there, so just make sure that everything we may change on that table at 1st. if we do not need to update, that is also fine, i will remove rmw_implementaion from the issue header.

[ros-discourse]

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-01-19/29423/1

[Barry-Xu-2018]

Related PRs have been submitted.

• [rmw] https://github.com/ros2/rmw/pull/331
• [rmw_implementation] https://github.com/ros2/rmw_implementation/pull/216
• [rmw_fastrtps] https://github.com/ros2/rmw_fastrtps/pull/645
• [rmw_cyclonedds] https://github.com/ros2/rmw_cyclonedds/pull/435
• [rmw_connextdds] https://github.com/ros2/rmw_connextdds/pull/101
• [rcl] https://github.com/ros2/rcl/pull/1033
• [rclcpp] https://github.com/ros2/rclcpp/pull/2105
• [rclpy] https://github.com/ros2/rclpy/pull/1083

TODO

• [Demo]
• [ros2_documentation]

[fujitatomoya]

@Barry-Xu-2018 thanks, i will be reviewing for sure.

CC: @iuhilnehc-ynos

[fujitatomoya]

@Barry-Xu-2018 Can we start the CI with all dependent PRs to see all related tests complete w/o error? i will be working on review at the same time.

[Barry-Xu-2018]

@Barry-Xu-2018 Can we start the CI with all dependent PRs to see all related tests complete w/o error? i will be working on review at the same time.

Yes. I will run CI in all dependent PRs.

[Barry-Xu-2018]

@fujitatomoya

CI on Linux passed. https://github.com/ros2/rmw/pull/331#issuecomment-1436568818
Next, run CI on Windows.

[ros-discourse]

This issue has been mentioned on ROS Discourse. There might be relevant details there:

https://discourse.ros.org/t/ros-2-tsc-meeting-minutes-2023-02-16/29927/1

[fujitatomoya]

@iuhilnehc-ynos can you do the another review on these PRs?

[fujitatomoya]

• ros2_documentation
  • Not exactly QoS feature, but events are extended aligned with QoS events.
  • Probably we could extend the docs for https://docs.ros.org/en/rolling/Concepts/About-Quality-of-Service-Settings.html with dedicated section.
• demo
  • after APIs are fixed, we can add the demo codes in the following folders.
  • https://github.com/ros2/demos/tree/rolling/demo_nodes_cpp
  • https://github.com/ros2/demos/tree/rolling/demo_nodes_py

[fujitatomoya]

@Barry-Xu-2018 can you start the CI with all related PRs?

[Barry-Xu-2018]

can you start the CI with all related PRs?

Okay. I will run CI for all related PRs again.

[fujitatomoya]

@iuhilnehc-ynos @Barry-Xu-2018 once everything is rebased, can you start the CI?

[Barry-Xu-2018]

@fujitatomoya

once everything is rebased, can you start the CI?

Yes. After all are rebased, I will ask @iuhilnehc-ynos help to run CI.

[Barry-Xu-2018]

@fujitatomoya

CI was done.
https://github.com/ros2/rmw/pull/331#issuecomment-1473429709

The failed test cases on Windows are unrelated to my change.

[mjcarroll]

Everything up to rclcpp/rclpy have been approved and merged. I'm going to leave this to you to close when demos/documentation are updated. Thanks for all the hard work and iteration!

[fujitatomoya]

@mjcarroll appreciate it for taking care of those PRs 👍 thanks a lot 🙇‍♂️ (ありがとう)

[fujitatomoya]

@Barry-Xu-2018

the rest will be https://github.com/ros2/rmw/issues/330#issuecomment-1447239439, can you also address these?

[Barry-Xu-2018]

the rest will be https://github.com/ros2/rmw/issues/330#issuecomment-1447239439, can you also address these?

Yeah, I am writing demo code and updating the document.