Skip to content

Fix subscription content-filtering flakiness #1369

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
minggangw merged 6 commits into from
Jan 14, 2026
+143 −80
Merged

Fix subscription content-filtering flakiness #1369

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
6 commits
Select commit Hold shift + click to select a range
28be546
Fix subscription content-filtering flakiness
minggangw Jan 14, 2026
ca058bf
Address comments
minggangw Jan 14, 2026
cd2d89f
Address comments
minggangw Jan 14, 2026
d82195f
Address comments
minggangw Jan 14, 2026
4267f85
Address comments
minggangw Jan 14, 2026
c043b78
Address comments
minggangw Jan 14, 2026
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
131 changes: 95 additions & 36 deletions src/rcl_subscription_bindings.cpp
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -38,9 +38,9 @@ Napi::Value RclTake(const Napi::CallbackInfo& info) {
rcl_ret_t ret = rcl_take(subscription, msg_taken, nullptr, nullptr);

if (ret != RCL_RET_OK && ret != RCL_RET_SUBSCRIPTION_TAKE_FAILED) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, rcl_get_error_string().str)
.ThrowAsJavaScriptException();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return Napi::Boolean::New(env, false);
}

Expand Down Expand Up @@ -99,7 +99,7 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) {
for (int i = 0; i < argc; i++) {
std::string arg = jsArgv.Get(i).As<Napi::String>().Utf8Value();
int len = arg.length() + 1;
argv[i] = reinterpret_cast<char*>(malloc(len * sizeof(char*)));
argv[i] = reinterpret_cast<char*>(malloc(len * sizeof(char)));
snprintf(argv[i], len, "%s", arg.c_str());
}
}
Expand All @@ -109,9 +109,9 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) {
expression.c_str(), argc, (const char**)argv, &subscription_ops);

if (ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, rcl_get_error_string().str)
.ThrowAsJavaScriptException();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
}

if (argc) {
Expand All @@ -120,33 +120,46 @@ Napi::Value CreateSubscription(const Napi::CallbackInfo& info) {
}
free(argv);
}

if (ret != RCL_RET_OK) {
free(subscription);
return env.Undefined();
}
}
}

const rosidl_message_type_support_t* ts =
GetMessageTypeSupport(package_name, message_sub_folder, message_name);

if (ts) {
THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK,
rcl_subscription_init(subscription, node, ts, topic.c_str(),
&subscription_ops),
rcl_get_error_string().str);
rcl_ret_t ret = rcl_subscription_init(subscription, node, ts, topic.c_str(),
&subscription_ops);
if (ret != RCL_RET_OK) {
std::string error_msg = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
free(subscription);
return env.Undefined();
}

auto js_obj = RclHandle::NewInstance(
env, subscription, node_handle, [node, env](void* ptr) {
rcl_subscription_t* subscription =
reinterpret_cast<rcl_subscription_t*>(ptr);
rcl_ret_t ret = rcl_subscription_fini(subscription, node);
free(ptr);
THROW_ERROR_IF_NOT_EQUAL_NO_RETURN(RCL_RET_OK, ret,
rcl_get_error_string().str);
if (ret != RCL_RET_OK) {
std::string error_msg = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
}
});

return js_obj;
} else {
Napi::Error::New(env, GetErrorMessageAndClear())
.ThrowAsJavaScriptException();
free(subscription);
return env.Undefined();
}
}
Expand Down Expand Up @@ -235,7 +248,7 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) {
for (int i = 0; i < argc; i++) {
std::string arg = jsArgv.Get(i).As<Napi::String>().Utf8Value();
int len = arg.length() + 1;
argv[i] = reinterpret_cast<char*>(malloc(len * sizeof(char*)));
argv[i] = reinterpret_cast<char*>(malloc(len * sizeof(char)));
snprintf(argv[i], len, "%s", arg.c_str());
}
}
Expand All @@ -245,15 +258,23 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) {
rcl_subscription_content_filter_options_t options =
rcl_get_zero_initialized_subscription_content_filter_options();

THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK,
rcl_subscription_content_filter_options_set(
subscription, expression.c_str(), argc, (const char**)argv, &options),
rcl_get_error_string().str);
rcl_ret_t ret = rcl_subscription_content_filter_options_set(
subscription, expression.c_str(), argc, (const char**)argv, &options);

THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK, rcl_subscription_set_content_filter(subscription, &options),
rcl_get_error_string().str);
if (ret != RCL_RET_OK) {
if (argc) {
for (int i = 0; i < argc; i++) {
free(argv[i]);
}
free(argv);
}
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

ret = rcl_subscription_set_content_filter(subscription, &options);

if (argc) {
for (int i = 0; i < argc; i++) {
Expand All @@ -262,6 +283,23 @@ Napi::Value SetContentFilter(const Napi::CallbackInfo& info) {
free(argv);
}

rcl_ret_t fini_ret =
rcl_subscription_content_filter_options_fini(subscription, &options);

if (ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

if (fini_ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

return Napi::Boolean::New(env, true);
}

Expand All @@ -277,15 +315,33 @@ Napi::Value ClearContentFilter(const Napi::CallbackInfo& info) {
rcl_subscription_content_filter_options_t options =
rcl_get_zero_initialized_subscription_content_filter_options();

THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK,
rcl_subscription_content_filter_options_init(
subscription, "", 0, (const char**)nullptr, &options),
rcl_get_error_string().str);
rcl_ret_t ret = rcl_subscription_content_filter_options_init(
subscription, "", 0, (const char**)nullptr, &options);

if (ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

ret = rcl_subscription_set_content_filter(subscription, &options);
rcl_ret_t fini_ret =
rcl_subscription_content_filter_options_fini(subscription, &options);

if (ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK, rcl_subscription_set_content_filter(subscription, &options),
rcl_get_error_string().str);
if (fini_ret != RCL_RET_OK) {
std::string error_string = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_string).ThrowAsJavaScriptException();
return env.Undefined();
}

return Napi::Boolean::New(env, true);
}
Expand All @@ -303,9 +359,9 @@ Napi::Value GetContentFilter(const Napi::CallbackInfo& info) {

rcl_ret_t ret = rcl_subscription_get_content_filter(subscription, &options);
if (ret != RCL_RET_OK) {
Napi::Error::New(env, rcl_get_error_string().str)
.ThrowAsJavaScriptException();
std::string error_msg = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
return env.Undefined();
}

Expand All @@ -331,9 +387,9 @@ Napi::Value GetContentFilter(const Napi::CallbackInfo& info) {
rcl_ret_t fini_ret =
rcl_subscription_content_filter_options_fini(subscription, &options);
if (fini_ret != RCL_RET_OK) {
Napi::Error::New(env, rcl_get_error_string().str)
.ThrowAsJavaScriptException();
std::string error_msg = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
return env.Undefined();
}

Expand All @@ -347,9 +403,12 @@ Napi::Value GetPublisherCount(const Napi::CallbackInfo& info) {
RclHandle::Unwrap(info[0].As<Napi::Object>())->ptr());

size_t count = 0;
THROW_ERROR_IF_NOT_EQUAL(
rcl_subscription_get_publisher_count(subscription, &count), RCL_RET_OK,
rcl_get_error_string().str);
rcl_ret_t ret = rcl_subscription_get_publisher_count(subscription, &count);
if (ret != RCL_RET_OK) {
std::string error_msg = rcl_get_error_string().str;
rcl_reset_error();
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
}

return Napi::Number::New(env, count);
}
Expand Down
6 changes: 3 additions & 3 deletions test/test-clock-event.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -42,7 +42,7 @@ describe('ClockEvent', function () {
await event.waitUntilSteady(clock, until.nanoseconds);
const end = Date.now();

assert(end - start >= 1000);
assert(end - start >= 950);
});

it('should wait until system time', async function () {
Expand All @@ -55,7 +55,7 @@ describe('ClockEvent', function () {
await event.waitUntilSystem(clock, until.nanoseconds);
const end = Date.now();

assert(end - start >= 1000);
assert(end - start >= 950);
});

it('should wait until ros time', async function () {
Expand All @@ -72,7 +72,7 @@ describe('ClockEvent', function () {
await event.waitUntilRos(clock, until.nanoseconds);
const end = Date.now();

assert(end - start >= 1000);
assert(end - start >= 950);
});

it('should set and clear event', function () {
Expand Down
86 changes: 45 additions & 41 deletions test/test-type-description-service.js
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -85,51 +85,55 @@ describe('type description service test suite', function () {
});

it('Test type description service configured by parameter', function (done) {
exec(
'ros2 param list /test_type_description_service',
(error, stdout, stderr) => {
if (error || stderr) {
done(
new Error(
`Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}`
)
);
return;
}
if (stdout.includes('start_type_description_service')) {
done();
} else {
done(
new Error("'start_type_description_service' not found in stdout.")
);
setTimeout(() => {
exec(
'ros2 param list /test_type_description_service',
(error, stdout, stderr) => {
if (error || stderr) {
done(
new Error(
`Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}`
)
);
return;
}
if (stdout.includes('start_type_description_service')) {
done();
} else {
done(
new Error("'start_type_description_service' not found in stdout.")
);
}
}
}
);
);
}, 1000);
});

it('Test start_type_description_service parameter value', function (done) {
exec(
'ros2 param get /test_type_description_service start_type_description_service',
(error, stdout, stderr) => {
if (error || stderr) {
done(
new Error(
`Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}`
)
);
return;
}
if (stdout.includes('Boolean value is: True')) {
done();
} else {
console.log(stdout);
done(
new Error(
"'start_type_description_service param value' not found in stdout."
)
);
setTimeout(() => {
exec(
'ros2 param get /test_type_description_service start_type_description_service',
(error, stdout, stderr) => {
if (error || stderr) {
done(
new Error(
`Test type description service configured by parameter failed. Error: ${error}, Stderr: ${stderr}`
)
);
return;
}
if (stdout.includes('Boolean value is: True')) {
done();
} else {
console.log(stdout);
done(
new Error(
"'start_type_description_service param value' not found in stdout."
)
);
}
}
}
);
);
}, 1000);
});
});
Loading