Unique network flows optional checks by thomasmoore-torc · Pull Request #872 · ros2/rmw_fastrtps

thomasmoore-torc · 2026-03-31T17:20:56Z

Description

As part of #711, the ros_discovery_info subscription was configured with RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_OPTIONALLY_REQUIRED. However, there are no criteria under which an optional unique network flow is actually optional and it ultimately behaves exactly the same as setting RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_STRICTLY_REQUIRED. There are several conditions under which unique network flows are not supported:

When locators are explicitly specified (relevant Fast DDS docs) - this results in FastDDS generating an error if unique network flow is requested when locators are explicitly specified
When static EDP is utilized (Unique Network Flows and Static Endpoint Discovery are incompatible eProsima/Fast-DDS#6348) - this results in user data not being received by the subscriber

The intent of this PR is to incorporate checks for these conditions such that a unique network flow will not be requested under these conditions when a subscription is created with RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_OPTIONALLY_REQUIRED. It also adds the RMW_FASTRTPS_USE_UNIQUE_NETWORK_FLOWS_FOR_ROS_DISCOVERY_INFO environment variable to enable the ability to disable requesting uinique network flows for the ros_discovery_info topic.

Is this user-facing behavior change?

This will enable the user to utilize features such as explicit locator specification or static EDP via the Fast DDS XML configuration files. It will also give the user the option to disable unique network flows for the ros_discovery_info topic.

Did you use Generative AI?

Claude Opus 4.6 was used via GitHub Copilot to assist with the analysis of the issue and to provide an initial implementation. The implementation was then manually refined.

Additional Information

We would like to backport this to Jazzy.

thomasmoore-torc · 2026-03-31T18:16:43Z

It appears that the build is failing because e5c8a84 requires ros2/rmw@403ee71, but the build is utilizing rmw version 7.9.1-1noble.20260317.155120, which doesn't include ros2/rmw@403ee71.

I don't currently have permissions to the ci_launcher, so I'm unable to trigger a build with the needed version of rmw for the test to pass.

fujitatomoya

test would be ideal to add.

MiguelCompany

Apart from my comment below, this needs a rebase

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

…o RMW_FASTRTPS_ROS_DISCOVERY_INFO_UNIQUE_NETWORK_FLOWS Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc · 2026-04-21T03:04:37Z

@MiguelCompany - I have rebased and addressed your comment. However, I'm still unable to get a successful build because of the issue I shared above: #872 (comment)

MiguelCompany · 2026-04-21T06:00:40Z

@MiguelCompany - I have rebased and addressed your comment. However, I'm still unable to get a successful build because of the issue I shared above: #872 (comment)

Don't worry about the build issue, the CI (which is manually launched) builds all the packages (i.e. no binary packages are used)

MiguelCompany

LGTM with green CI

fujitatomoya · 2026-04-21T23:10:17Z

Pulls: #872
Gist: https://gist.githubusercontent.com/fujitatomoya/0b33e88646ff7ef2537377ce25f362c1/raw/30e78828e520f669597e1694145cfd1a38ce1593/ros2.repos
BUILD args: --packages-above-and-dependencies rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp
TEST args: --packages-above rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19040

Linux
Linux-aarch64
Linux-rhel
Windows

MiguelCompany · 2026-04-22T05:54:52Z

Manually rebuild with rmw_fastrtps_dynamic_cpp enabled

Linux
Linux-aarch64
Linux-rhel
Windows

Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

MiguelCompany · 2026-04-22T13:53:48Z

Linux
Linux-aarch64
Linux-rhel
Windows

MiguelCompany

We can always use rcutils_set_env, since its behavior is to always override.

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

MiguelCompany

LGTM with green CI

MiguelCompany · 2026-04-24T11:04:07Z

Linux
Linux-aarch64
Linux-rhel
Windows

…WORK_FLOWS is an empty string Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc · 2026-04-25T16:06:41Z

@MiguelCompany - hopefully the latest commit fixes the issues with the CI unit tests. I didn't realize that rcutils_get_env returns an empty string if the environment variable was set, so a warning was being emitted for all of the unit test, breaking those that expected certain output.

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc · 2026-04-25T16:26:48Z

I also decided to use SYSTEM_DEFAULT instead of DEFAULT for the environment variable to avoid any confusion between the default for the ros_discovery_info topic being RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_OPTIONALLY_REQUIRED and whatever the system default is as provided by RMW_UNIQUE_NETWORK_FLOW_ENDPOINTS_SYSTEM_DEFAULT.

MiguelCompany · 2026-04-27T05:52:31Z

New (hopefully last) CI:

Linux
Linux-aarch64
Linux-rhel
Windows

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc · 2026-04-27T16:24:21Z

@MiguelCompany - while looking into the build issues, I decided to downgrade the warning to an info that was emitted when UNF was skipped while being set to optional because that is the expected behavior. I also added a test to verify that no unexpected logs were generated.

Regarding the build issues, it looks like all of the builds failed in the same way as the recent nightly builds. How should we handle this?

MiguelCompany · 2026-04-28T05:45:19Z

+    const char * error_str;
+    error_str = rcutils_get_env("RMW_FASTRTPS_ROS_DISCOVERY_INFO_UNIQUE_NETWORK_FLOWS", &env_value);
+    ASSERT_EQ(error_str, nullptr) << "Error getting env var: " << error_str;
+    value = (*env_value != '\0') ? std::optional<std::string>(env_value) : std::nullopt;


Suggested change

value = (*env_value != '\0') ? std::optional<std::string>(env_value) : std::nullopt;

value = ((nullptr != env_value) && (*env_value != '\0')) ? std::optional<std::string>(env_value) : std::nullopt;

MiguelCompany · 2026-04-28T06:03:52Z

Regarding the build issues, it looks like all of the builds failed in the same way as the recent nightly builds. How should we handle this?

After approval, if we consider that test failures are unrelated to this PR, we could merge.

Note, however, that we are currently inside the freeze for Lyrical, and we would need to wait for the freeze to be released before merging.

christophebedard · 2026-05-08T17:24:52Z

Pulls: #872
Gist: https://gist.githubusercontent.com/christophebedard/d85cf1e23c791636db5282c086230f5a/raw/30e78828e520f669597e1694145cfd1a38ce1593/ros2.repos
BUILD args: --packages-above-and-dependencies rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp
TEST args: --packages-above rmw_fastrtps_cpp rmw_fastrtps_dynamic_cpp rmw_fastrtps_shared_cpp
ROS Distro: rolling
Job: ci_launcher
ci_launcher ran: https://ci.ros2.org/job/ci_launcher/19202

Linux
Linux-aarch64
Linux-rhel
Windows

fujitatomoya reviewed Apr 1, 2026

View reviewed changes

Comment thread rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/init_rmw_context_impl.hpp Outdated

Comment thread rmw_fastrtps_shared_cpp/src/init_rmw_context_impl.cpp Outdated

asymingt requested a review from MiguelCompany April 16, 2026 16:47

MiguelCompany requested changes Apr 17, 2026

View reviewed changes

Comment thread rmw_fastrtps_shared_cpp/include/rmw_fastrtps_shared_cpp/init_rmw_context_impl.hpp Outdated

thomasmoore-torc added 3 commits April 21, 2026 00:38

Add checks for when unique network flows is optional

4354b70

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Add RMW_FASTRTPS_USE_UNIQUE_NETWORK_FLOWS_FOR_ROS_DISCOVERY_INFO

b021c69

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Cleanup and add tests

970c249

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 2da5a66 to 41fb2ad Compare April 21, 2026 02:48

Change RMW_FASTRTPS_USE_UNIQUE_NETWORK_FLOWS_FOR_ROS_DISCOVERY_INFO t…

452ec4f

…o RMW_FASTRTPS_ROS_DISCOVERY_INFO_UNIQUE_NETWORK_FLOWS Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 41fb2ad to 452ec4f Compare April 21, 2026 02:51

MiguelCompany requested changes Apr 21, 2026

View reviewed changes

Comment thread rmw_fastrtps_shared_cpp/test/test_unique_network_flows.cpp Outdated

Comment thread rmw_fastrtps_shared_cpp/test/test_unique_network_flows.cpp

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch 2 times, most recently from 181e5e2 to 1aeb4f7 Compare April 21, 2026 15:19

MiguelCompany approved these changes Apr 21, 2026

View reviewed changes

MiguelCompany requested changes Apr 22, 2026

View reviewed changes

Comment thread rmw_fastrtps_shared_cpp/test/test_unique_network_flows.cpp Outdated

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 1aeb4f7 to 041f3c9 Compare April 22, 2026 13:15

thomasmoore-torc and others added 3 commits April 22, 2026 13:40

Update copyright

edbcb1f

Co-authored-by: Miguel Company <miguelcompany@eprosima.com> Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Add test that checks default when env var unset

c93d12e

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Use rcutils env functions to fix Windows build

02d35e5

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 041f3c9 to 02d35e5 Compare April 22, 2026 13:40

MiguelCompany requested changes Apr 22, 2026

View reviewed changes

Comment thread rmw_fastrtps_shared_cpp/test/test_unique_network_flows.cpp

Comment thread rmw_fastrtps_shared_cpp/test/test_unique_network_flows.cpp Outdated

MiguelCompany mentioned this pull request Apr 22, 2026

fix: typo in parameter documentation for overwrite ros2/rcutils#557

Merged

Use rcutils_set_env

7fbc3f8

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 7308ab6 to 7fbc3f8 Compare April 23, 2026 11:08

MiguelCompany approved these changes Apr 23, 2026

View reviewed changes

Don't issue a warning when RMW_FASTRTPS_ROS_DISCOVERY_INFO_UNIQUE_NET…

84ac6ac

…WORK_FLOWS is an empty string Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

thomasmoore-torc force-pushed the unique_network_flows_optional_checks branch from 2b78aec to 84ac6ac Compare April 25, 2026 16:04

Use SYSTEM_DEFAULT instead of DEFAULT

76416ed

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

MiguelCompany approved these changes Apr 27, 2026

View reviewed changes

thomasmoore-torc added 2 commits April 27, 2026 16:19

Change warning to info when UNF is optional and it is ignored

0d3d9ae

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

Add test to ensure no logs are generated when not expected

2d38a87

Signed-off-by: Thomas Moore <thomas.moore@torc.ai>

MiguelCompany requested changes Apr 28, 2026

View reviewed changes

christophebedard assigned thomasmoore-torc May 8, 2026

	value = (*env_value != '\0') ? std::optional<std::string>(env_value) : std::nullopt;
	value = ((nullptr != env_value) && (*env_value != '\0')) ? std::optional<std::string>(env_value) : std::nullopt;

Conversation

thomasmoore-torc commented Mar 31, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Description

Is this user-facing behavior change?

Did you use Generative AI?

Additional Information

Uh oh!

thomasmoore-torc commented Mar 31, 2026

Uh oh!

fujitatomoya left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

thomasmoore-torc commented Apr 21, 2026

Uh oh!

Uh oh!

Uh oh!

MiguelCompany commented Apr 21, 2026

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

fujitatomoya commented Apr 21, 2026

Uh oh!

MiguelCompany commented Apr 22, 2026

Uh oh!

Uh oh!

MiguelCompany commented Apr 22, 2026

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

Uh oh!

Uh oh!

MiguelCompany left a comment

Choose a reason for hiding this comment

Uh oh!

MiguelCompany commented Apr 24, 2026

Uh oh!

thomasmoore-torc commented Apr 25, 2026

Uh oh!

thomasmoore-torc commented Apr 25, 2026

Uh oh!

MiguelCompany commented Apr 27, 2026

Uh oh!

thomasmoore-torc commented Apr 27, 2026

Uh oh!

MiguelCompany Apr 28, 2026

Choose a reason for hiding this comment

Uh oh!

MiguelCompany commented Apr 28, 2026

Uh oh!

christophebedard commented May 8, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

4 participants

thomasmoore-torc commented Mar 31, 2026 •

edited

Loading