Add node name to the Read(Write)SplitEvent message

[MichaelOrlov]

• Rationale:
  When running multiple instances of the rosbag2 recorder or player there are no way to know from what instance arrived notification about files split event. With node name in event message we will be able to identify instance of the recorder or player from which event arrived.

[MichaelOrlov]

@fujitatomoya @clalancette could you please review/approve this small PR.
I would like to land it to the Jazzy release.

[fujitatomoya]

having fully qualified node name to the event publisher makes sense to me.

[MichaelOrlov]

Gist: https://gist.githubusercontent.com/MichaelOrlov/dea13b6d6c78211829cf64ab14776a38/raw/5f18de8a350a609a04881a4b1c99815361bdb75b/ros2.repos
BUILD args: --packages-above-and-dependencies rosbag2_interfaces rosbag2_transport rosbag2_tests
TEST args: --packages-above rosbag2_interfaces rosbag2_transport rosbag2_tests
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/13700

• Linux
• Linux-aarch64
• Windows

[MichaelOrlov]

@clalancette @emersonknapp Could some of you please additionally formally approve this PR to be able to merge it?

[MichaelOrlov]

@marcoag Could you please approve and merge this PR? We need it for Jazzy.

[MichaelOrlov]

@clalancette @marcoag friendly ping here.

[clalancette]

I have to say that this seems like an odd change. A lot of what ROS is built around is the concept of anonymous pub/sub, and this is explicitly breaking that.

Can I have more of an explanation of the reason for this change? Also, are there other ways we can accomplish the same thing?

[MichaelOrlov]

@clalancette Ok here is more context:
When rosbag2 recorder recording with the option to split recorded files by size or by duration, when the split event happens we publish a message on the specific topic "events/write_split" where we specify closed and newly opened files.
We need this sort of notification for third-party nodes or scripts which will wait for such split messages and upon arrival can process freshly written rosbag2 file while the other one still writing.
One of the common use cases is uploading to the cloud freshly written files or doing custom compression or encryption.

Now imagine if we would have multiple instances of the rosbag2 recorder running in parallel. A valid case to split writing to the separate NVME drives to achieve higher throughput or not mess up high-frequency topics like IMU with the slower camera or point cloud images. The monitoring script/node which is running in a separate process needs to know what instance of the rosbag2 recorder sent a notification. It might be a different logic for processing files from different instances. One may need encryption another one may not for example.

As regards "Also, are there other ways we can accomplish the same thing?" - Hmm, I don't know on top of my head.

Honestly, I don't quite understand why the concept of the anonymous pub/sub could be relevant or somehow matter in this case.

[clalancette]

Honestly, I don't quite understand why the concept of the anonymous pub/sub could be relevant or somehow matter in this case.

The reason I'm hesitant here is because it breaks the idea of being anonymous, which is a core design philosophy of ROS 2. That is, the node name now matters, so doing a remap during playback, for instance, won't work anymore.

I need to think about this more, and we should probably discuss it in the ROS 2 meeting tomorrow. I very well may be wrong, but I don't think we should rush this in right now.

[MichaelOrlov]

Update: On today's ROS 2 sync meeting we decided to merge this PR as is for user convenience.

However, the alternative way to detect the sender node name on the receiver side could be as proposed in the ros2/geometry2#545

[MichaelOrlov]

@clalancette I need your formal approval to be able to merge this PR

[ros-discourse]

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

https://discourse.ros.org/t/freeze-of-ros-2-base-packages-upcoming-branch-and-tutorial-party-for-jazzy-jalisco/37191/2

[mwcondino]

Any chance this will be ported to Iron? I can use the mentioned workaround if not

[MichaelOrlov]

@mwcondino Sorry, but we can't backport this PR to Iron since it has breaking API changes.