add rmw_get_default_xxx to return the structure with default values.

[fujitatomoya]

follow up for #378

• rmw_get_zero_initialized_xxx should return zero initialized data structure without any default non-zero values. zero initialized data is mostly used for copying the data object (allocating destination data structure) and finalization.
• rmw_get_default_xxx should return the data structure with default values, so that application can use those default values to process the initialization.

[fujitatomoya]

Pulls: #379
Gist: https://gist.githubusercontent.com/fujitatomoya/65e549d1cfbf47cfcfd10df7e86bd521/raw/5a23d57d187e0028fde4536c468462cbe0f56501/ros2.repos
BUILD args: --packages-above-and-dependencies rmw rcl rmw_implementation rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_connextdds_common rmw_cyclonedds_cpp
TEST args: --packages-above rmw rcl rmw_implementation rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_connextdds_common rmw_cyclonedds_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14581

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[clalancette]

Just as an FYI, I'm not entirely sure we need these get_default APIs. I haven't looked at the RMW APIs, but we often have init-style APIs which tend to init them already; maybe we can lean more heavily on those? I'd need to look closer at the RMW APIs to see if that makes sense here.

[methylDragon]

I'm not opposed in principle to these since this is an avenue for performance boosting, and I see in the associated PRs that they're already being used.

Pending green CI, of course.

[fujitatomoya]

but we often have init-style APIs which tend to init them already; maybe we can lean more heavily on those?

i thought that too. but i think zero-init is also useful for data copy and finalization. so i do not think those are exclusive APIs.

[fujitatomoya]

CI is failure to build, i think i misunderstood the usage of https://github.com/ros-tooling/ros-github-scripts?tab=readme-ov-file#ros-ci-for-pr

[fujitatomoya]

Pulls: #379, ros2/rcl#1186, ros2/rmw_implementation#241, ros2/rmw_fastrtps#778, ros2/rmw_connextdds#160, ros2/rmw_cyclonedds#513
Gist: https://gist.githubusercontent.com/fujitatomoya/18370bf1e0640d41a76698d03ad74dee/raw/7c320f893c82a71b0be07d942e42f59cb7e1df08/ros2.repos
BUILD args: --packages-above-and-dependencies rmw rcl rmw_implementation rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_connextdds_common rmw_cyclonedds_cpp
TEST args: --packages-above rmw rcl rmw_implementation rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_connextdds_common rmw_cyclonedds_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/14583

• Linux
• Linux-aarch64
• Linux-rhel
• Windows

[clalancette]

I've left some comments inline on why I don't think we actually need to add these functions to the RMW interface. We can probably just get by with changing the zero_initialized functions to actually zero initialize, and maybe fix some tests here and there.

[clalancette]

For instance, I don't believe we need this. In all "real" situations (not tests) where rmw_get_zero_initialized_event is called, it is immediately followed by either rmw_subscription_event_init or rmw_publisher_event_init. Because of that, I don't believe we need to add this API at all (though if we change rmw_get_zero_initialized_event to truly return all zeros, we may have to fix a couple of tests).

[clalancette]

Same here; all places we call rmw_get_zero_initialized_context we are either cleaning up after a failure, doing fini on the context, or this is immediately followed by an rmw_init which initializes the context. Again, we may have to fix some tests if we make rmw_get_zero_initialized_context actually return all zeros.

[clalancette]

And the same here.

[fujitatomoya]

@clalancette i take this case as an example. RMW_DEFAULT_DOMAIN_ID is not zero, so i was wondering that it should be returned by rmw_get_default_init_options originally. if we truly return the zero initialized option, this will affect the user application. i think we can have some options,

• get_zero_initialized_xxx returns with default values. (almost current implementation) IMO this does not sound right, because what it does is not really what it sounds like... initialized_xxx would sound better.
• get_zero_initialized_xxx truly returns with zero initialized fields, and provides APIs to get default values too. (these PRs)
• get_zero_initialized_xxx truly returns with zero initialized fields, and caller is responsible to fill in default values. (this does not really make sense, this is exactly whey get_default_xxx suggested.)

[clalancette]

I definitely think that get_zero_initialized_xxx should return a zero-initialized structure. So we are in agreement there.

But my main point is that all callers of get_zero_initialized_xxx (with the exception of maybe a few tests) immediately follow that up with a corresponding init_xxx call, which basically sets the defaults. For instance, considering the rmw_init_options_t structure, we have rmw_get_zero_initialized_init_options, which I believe should return a zero'ed structure. But all callers immediately follow that with a call to rmw_init_options_init. That function is implemented by the various RMWs:

• rmw_fastrtps - initializes the structure very much like your proposed change here
• rmw_cyclonedds - initializes the structure very much like your proposed change here
• rmw_connextdds - initializes the structure very much like your proposed change here
• rmw_zenoh - initializes the structure very much like your proposed change here

Thus, it doesn't seem like the new APIs are necessary, as we already have all of the functionality we need here.

To maybe ask this in a different way: in what situations are you expecting that rmw_get_default_init_options will be used?

[fujitatomoya]

I definitely think that get_zero_initialized_xxx should return a zero-initialized structure. So we are in agreement there.

👍

To maybe ask this in a different way: in what situations are you expecting that rmw_get_default_init_options will be used?

ah, i see now. i was thinking rmw_get_default_init_options returns implementation agnostic default option from ROS 2 RMW (e.g ROS 2 default domain ID), and rmw_init_options_init initializes the field with implementation specific values. but your point is that is not how it is implemented, so calling implementation rmw_init_options_init also initializes with default values.

i can check the related APIs to make sure, and adjust the PR accordingly. thanks!