Add support check for content filter feature in subscription by minggangw · Pull Request #1451 · RobotWebTools/rclnodejs

minggangw · 2026-03-23T06:04:36Z

Corresponding to rclpy d892204 and rcl's rcl_subscription_is_cft_supported() API, this adds isContentFilterSupported() to the Subscription class. This allows users to query at runtime whether the underlying RMW supports content-filtered topics for a given subscription, rather than relying on hardcoded RMW name checks. Currently gated to ROS 2 Rolling only (ROS_VERSION > 2505); on older distros it returns false.

Feature changes

src/rcl_subscription_bindings.cpp — Added IsContentFilterSupported() N-API function calling rcl_subscription_is_cft_supported(), guarded by #if ROS_VERSION > 2505. Added #include <rcl/subscription.h>. Registered as "isContentFilterSupported" in InitSubscriptionBindings().
lib/subscription.js — Added isContentFilterSupported() method with a distro guard (DistroUtils.getDistroId() < ROLLING returns false). Added DistroUtils import.
types/subscription.d.ts — Added isContentFilterSupported(): boolean to the Subscription interface.
test/test-subscription-content-filter.js — Added a separate top-level describe('subscription isContentFilterSupported') block that runs on all distros. Validates the return value accounts for both the rolling requirement and RMW capability.
test/types/index.test-d.ts — Added expectType<boolean> assertion for isContentFilterSupported().

CI changes

.github/workflows/linux-x64-build-and-test.yml — Replaced setup-ros with use-ros2-testing for rolling with a nightly binary tarball approach, following the official ROS 2 Rolling binary installation guide:
- Configures ROS apt sources via ros2-apt-source deb package.
- Installs apt packages (test-msgs, mrpt-msgs, colcon, rosdep) before extracting the nightly tarball, so the nightly's newer libs (e.g. librcutils.so with rcutils_decode_base64) overwrite apt's older versions.
- Extracts nightly tarball with --strip-components=1 into /opt/ros/rolling/ so everything lives under one prefix — single source /opt/ros/rolling/setup.bash like all other distros.
- Runs rosdep install for remaining runtime dependencies.
- Separated Electron test dependencies (xvfb, libgtk-3-0, etc.) into its own step that runs for all distros.
- Added -y to apt install for test-msgs.
- Removed stale use-ros2-testing parameter from setup-ros.

Fix: #1450

Copilot

Pull request overview

Adds a Subscription-level API to check whether content-filtering topics are supported by the underlying ROS 2 middleware, exposing it through the JS API, native bindings, and TypeScript typings, with accompanying tests.

Changes:

Add Subscription.isContentFilterSupported() to JS and TypeScript types.
Expose a new native binding isContentFilterSupported backed by rcl_subscription_is_cft_supported.
Add type-level and runtime tests for the new API.

Reviewed changes

Copilot reviewed 5 out of 6 changed files in this pull request and generated 3 comments.

Show a summary per file

File	Description
`lib/subscription.js`	Adds JS-facing `isContentFilterSupported()` method on `Subscription`.
`src/rcl_subscription_bindings.cpp`	Adds N-API binding that calls `rcl_subscription_is_cft_supported` and exports it.
`types/subscription.d.ts`	Extends the `Subscription` TypeScript interface with `isContentFilterSupported(): boolean`.
`test/types/index.test-d.ts`	Adds type assertion coverage for `isContentFilterSupported()`.
`test/test-subscription-content-filter.js`	Adds runtime test validating the method returns a boolean and matches expected capability heuristics.
`rosidl_parser/parser.py`	Modifies CLI arg-count error output/message (appears unrelated to PR goal).

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-23T06:07:30Z

rosidl_parser/parser.py

 if __name__ == '__main__':
    if len(sys.argv) < 4:
-        print('Usage: {} <command> <packageName> <filePath>'.format(sys.argv[0]), file=sys.stderr)
+        print('Wrong number of argments')


The argument-count check regresses error reporting: it now prints a generic message to stdout and drops the detailed usage message (previously printed to stderr). This makes failures from rosidl_parser/rosidl_parser.js much harder to diagnose because the Node-side error includes stderr was: ... and will now often be empty. Consider restoring the original Usage: <command> <packageName> <filePath> message and printing it to stderr.

Suggested change

print('Wrong number of argments')

print(f'Usage: {sys.argv[0]} <command> <packageName> <filePath>', file=sys.stderr)

Copilot · 2026-03-23T06:07:30Z

rosidl_parser/parser.py

 if __name__ == '__main__':
    if len(sys.argv) < 4:
-        print('Usage: {} <command> <packageName> <filePath>'.format(sys.argv[0]), file=sys.stderr)
+        print('Wrong number of argments')


Typo in the error message: "argments" should be "arguments".

Suggested change

print('Wrong number of argments')

print('Wrong number of arguments')

Copilot · 2026-03-23T06:07:30Z

rosidl_parser/parser.py

    if len(sys.argv) < 4:
-        print('Usage: {} <command> <packageName> <filePath>'.format(sys.argv[0]), file=sys.stderr)
+        print('Wrong number of argments')
        sys.exit(1)


This change in rosidl_parser/parser.py (updating the CLI arg-count error message) doesn’t appear related to the PR’s stated goal of adding a subscription content-filter support check. If it’s not required for the feature, consider reverting it or splitting it into a separate PR to keep changes focused.

coveralls · 2026-03-23T10:33:23Z

coverage: 85.973% (-0.003%) from 85.976%
when pulling a0cc7eb on minggangw:fix-1450
into 6e46c4e on RobotWebTools:develop.

Copilot

Pull request overview

Copilot reviewed 5 out of 6 changed files in this pull request and generated 2 comments.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2026-03-24T03:22:36Z

.github/workflows/linux-x64-build-and-test.yml

+        rosdep init || true
+        rosdep update
+        rosdep install --from-paths /opt/ros/rolling/share --ignore-src -y --skip-keys "cyclonedds fastcdr fastdds iceoryx_binding_c rmw_connextdds rti-connext-dds-7.3.0 urdfdom_headers"


In the Rolling nightly install step, rosdep install is run before any source /opt/ros/rolling/setup.bash (and without --rosdistro rolling). In a clean container this often means ROS_DISTRO is unset, causing rosdep install to fail or resolve against the wrong distro. Consider sourcing the newly extracted /opt/ros/rolling/setup.bash (or at least export ROS_DISTRO=rolling, or add --rosdistro rolling) before running rosdep update/install so dependency resolution is deterministic.

Suggested change

rosdep init || true

rosdep update

rosdep install --from-paths /opt/ros/rolling/share --ignore-src -y --skip-keys "cyclonedds fastcdr fastdds iceoryx_binding_c rmw_connextdds rti-connext-dds-7.3.0 urdfdom_headers"

export ROS_DISTRO=rolling

rosdep init || true

rosdep update

rosdep install --rosdistro rolling --from-paths /opt/ros/rolling/share --ignore-src -y --skip-keys "cyclonedds fastcdr fastdds iceoryx_binding_c rmw_connextdds rti-connext-dds-7.3.0 urdfdom_headers"

Copilot · 2026-03-24T03:22:36Z

src/rcl_subscription_bindings.cpp

+#if ROS_VERSION > 2505  // Rolling only
+Napi::Value IsContentFilterSupported(const Napi::CallbackInfo& info) {
+  Napi::Env env = info.Env();
+
+  RclHandle* subscription_handle =
+      RclHandle::Unwrap(info[0].As<Napi::Object>());
+  rcl_subscription_t* subscription =
+      reinterpret_cast<rcl_subscription_t*>(subscription_handle->ptr());
+
+  bool is_supported = rcl_subscription_is_cft_supported(subscription);
+  return Napi::Boolean::New(env, is_supported);
+}
+#endif


The preprocessor guard #if ROS_VERSION > 2505 is commented as “Rolling only”, but > 2505 will also evaluate true for any future non-Rolling distro IDs (e.g., 2605, 2705, …). This can (a) cause build failures if rcl_subscription_is_cft_supported() remains Rolling-only, and (b) diverge from the JS runtime guard which only enables the method on DistroId.ROLLING. Consider aligning both sides by guarding on the same threshold (e.g., ROS_VERSION >= 5000 if it’s truly Rolling-only, or updating the JS guard/comment if the API is expected to exist on post-Kilted stable distros too).

Add support check for content filter feature in subscription

a201c28

Copilot AI review requested due to automatic review settings March 23, 2026 06:04

Copilot started reviewing on behalf of minggangw March 23, 2026 06:05 View session

Copilot AI reviewed Mar 23, 2026

View reviewed changes

minggangw added 11 commits March 23, 2026 14:26

Address comments

73a4aaa

Address comments

cced547

Address comments

fdf6746

Address comments

a76200e

Address comments

ef17a64

Address comments

b228f52

Address comments

58a8aca

Address comments

a2bf1b5

Address comments

762b259

CI: fix rolling nightly source order and add colcon

f9a92d8

Address comments

c439dde

minggangw added 2 commits March 24, 2026 10:01

Address comments

0c534ae

Address comments

1205ac2

minggangw requested a review from Copilot March 24, 2026 03:19

Copilot started reviewing on behalf of minggangw March 24, 2026 03:19 View session

Copilot AI reviewed Mar 24, 2026

View reviewed changes

Address comments

a0cc7eb

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add support check for content filter feature in subscription#1451

Add support check for content filter feature in subscription#1451
minggangw wants to merge 15 commits intoRobotWebTools:developfrom
minggangw:fix-1450

minggangw commented Mar 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

Copilot AI Mar 23, 2026

Uh oh!

coveralls commented Mar 23, 2026 •

edited

Loading

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Copilot AI Mar 24, 2026

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

	print('Wrong number of argments')
	print(f'Usage: {sys.argv[0]} <command> <packageName> <filePath>', file=sys.stderr)

	print('Wrong number of argments')
	print('Wrong number of arguments')

Conversation

minggangw commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Feature changes

CI changes

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Reviewed changes

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 23, 2026

Choose a reason for hiding this comment

Uh oh!

coveralls commented Mar 23, 2026 • edited Loading Uh oh! There was an error while loading. Please reload this page.

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull request overview

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Copilot AI Mar 24, 2026

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

3 participants

minggangw commented Mar 23, 2026 •

edited

Loading

coveralls commented Mar 23, 2026 •

edited

Loading