Skip to content

Fix content filtering on Windows with modern Connext DDS (backport #226)#230

Merged
sloretz merged 1 commit into
lyricalfrom
mergify/bp/lyrical/pr-226
May 12, 2026
Merged

Fix content filtering on Windows with modern Connext DDS (backport #226)#230
sloretz merged 1 commit into
lyricalfrom
mergify/bp/lyrical/pr-226

Conversation

@mergify
Copy link
Copy Markdown

@mergify mergify Bot commented May 12, 2026

Description

This PR fixes Content Filtering on Windows when using modern versions of RTI Connext DDS (6.x and 7.x).

The Problem

Previously, the rmw_connextdds implementation on Windows was forced into a "Compatibility Mode." This mode:

  1. Disabled Writer-side filtering: All samples were sent to all matching readers, regardless of whether they matched the filter.
  2. Forced Reader-side filtering: Filtering was performed after deserialization. However, the built-in RTI SQL filter expects samples in the Connext memory layout. Because ROS 2 uses its own internal format, the filter would fail "open" and accept all samples.

This led to failures in rcl, rclcpp, and rcl_action tests where samples were received by subscribers despite being explicitly filtered out by SQL expressions.

The Fix

  1. Enable Serialized Evaluation: Updated CMakeLists.txt to only enable RMW_CONNEXT_BUILTIN_CFT_COMPATIBILITY_MODE on Windows for Connext versions older than 6.0.0. For modern versions, we now use the internal register_contentfilterI API, which supports evaluation on CDR-serialized samples.
  2. Fix Redefinition Conflict: Adjusted custom_sql_filter.hpp to use the official RTI declaration of register_contentfilterI (available in modern headers) to avoid linkage errors on MSVC.
  3. Correct Cookie Handling: Fixed a bug in RTI_CustomSqlFilter_writer_evaluated_result where it assumed RTI's built-in filter always returned a discontiguous sequence of cookies. It now correctly handles both contiguous and discontiguous sequences.

Verification

Verified on Windows using a targeted CI run with Connext 7.7.0. All content filtering tests in rcl, rclcpp, and rcl_action passed.

CI Result (Windows 10, Connext 7.7.0): https://ci.ros2.org/job/ci_windows/27743/

Is this user-facing behavior change?

No, it fixes a regression in existing functionality.

Did you use Generative AI?

Yes. Gemini CLI:2.0-Flash [web_fetch, run_shell_command, replace, git, gh]


This is an automatic backport of pull request #226 done by [Mergify](https://mergify.com).

* Fix content filtering on Windows with Connext 7

1. Fix a bug in writer-side filtering where it assumed the built-in SQL filter returned a discontiguous sequence of cookies. Fallback to taking addresses of contiguous cookies if necessary.

2. Disable RMW_CONNEXT_BUILTIN_CFT_COMPATIBILITY_MODE on Windows for Connext 6.0+ to enable serialized evaluation and internal registration, which is required for correct filtering of ROS 2 samples.

Assisted-by: Gemini CLI:2.0-Flash

* Fix redefinition of DDS_DomainParticipant_register_contentfilterI on Windows

Modern RTI Connext headers (6.0.0+) already provide this declaration. Re-declaring it manually causes linkage errors on MSVC.

Assisted-by: Gemini CLI:2.0-Flash

* Fix incorrect preprocessor check for legacy Connext API

RMW_CONNEXT_DDS_API_PRO_LEGACY is always defined (defaults to 0), so #if defined() was always true. Switching to #if value ensures correct skipping on modern Connext.

Assisted-by: Gemini CLI:2.0-Flash

* Apply @fgallegosalido suggestion to assert discontiguous buffer

Signed-off-by: Shane Loretz <sloretz@intrinsic.ai>

---------

Signed-off-by: Shane Loretz <sloretz@intrinsic.ai>
Co-authored-by: Shane Loretz <sloretz@intrinsic.ai>
(cherry picked from commit 9e3beb8)
@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented May 12, 2026

CI (build: --packages-up-to rcl rcl_action rclpy ros2cli sros2 ros2multicast test_rmw_implementation rclcpp rosbag2_transport test_communication test_tf2 test_security rmw_connextdds_common test: --packages-select rcl rcl_action rclpy ros2cli sros2 ros2multicast test_rmw_implementation rclcpp rosbag2_transport test_communication test_tf2 test_security rmw_connextdds_common)

  • Linux Build Status
  • Linux-aarch64 Build Status (Expected failure; CI ignores ConnextDDS on aarch64)
  • Linux-rhel Build Status
  • Windows Build Status

@sloretz
Copy link
Copy Markdown
Contributor

sloretz commented May 12, 2026

I think Windows CI is ok. The only test failure in window CI not in the nightly is rosbag2_transport.test_record_services.gtest.missing_result. It's weird because I'm using the Lyrical repos instead of the rolling repos, but Clara did say today the test failures are new due to an infra issue.

@sloretz sloretz merged commit 721e7b3 into lyrical May 12, 2026
2 checks passed
@sloretz sloretz deleted the mergify/bp/lyrical/pr-226 branch May 12, 2026 18:54
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants