Skip to content

Commit fb4597a

Browse files
authored
Add removeROSArgs() function (#1339)
This PR adds a new `removeROSArgs()` function to rclnodejs that filters out ROS-specific command-line arguments from a given argument list, returning only the non-ROS arguments. - Implements `removeROSArgs()` function with complete TypeScript type definitions - Adds comprehensive test coverage including edge cases for invalid inputs - Uses native RCL functions (`rcl_parse_arguments` and `rcl_remove_ros_arguments`) for correct ROS argument parsing Fix: #1330
1 parent 2b44f3b commit fb4597a

File tree

5 files changed

+147
-0
lines changed

5 files changed

+147
-0
lines changed

index.js

Lines changed: 15 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -386,6 +386,21 @@ let rcl = {
386386
_rosVersionChecked = true;
387387
},
388388

389+
/**
390+
* Remove ROS-specific arguments from the given argument list.
391+
* @param {string[]} argv - The argument list to process.
392+
* @return {string[]} The argument list with ROS arguments removed.
393+
*/
394+
removeROSArgs(argv) {
395+
if (!Array.isArray(argv)) {
396+
throw new TypeError('argv must be an array.');
397+
}
398+
if (!argv.every((argument) => typeof argument === 'string')) {
399+
throw new TypeError('argv elements must be strings (and not null).');
400+
}
401+
return rclnodejs.removeROSArgs(argv);
402+
},
403+
389404
/**
390405
* Start detection and processing of units of work.
391406
* @param {Node} node - The node to be spun up.

src/rcl_bindings.cpp

Lines changed: 59 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,11 @@
1515
#include "rcl_bindings.h"
1616

1717
#include <node.h>
18+
#include <rcl/arguments.h>
19+
#include <rcl/rcl.h>
20+
21+
#include <rcpputils/scope_exit.hpp>
22+
1823
#if ROS_VERSION >= 2006
1924
#include <rosidl_runtime_c/string_functions.h>
2025
#else
@@ -23,11 +28,64 @@
2328

2429
#include <memory>
2530
#include <string>
31+
#include <vector>
2632

2733
#include "rcl_handle.h"
34+
#include "rcl_utilities.h"
2835

2936
namespace rclnodejs {
3037

38+
Napi::Value RemoveROSArgs(const Napi::CallbackInfo& info) {
39+
Napi::Env env = info.Env();
40+
Napi::Array jsArgv = info[0].As<Napi::Array>();
41+
size_t argc = jsArgv.Length();
42+
char** argv = AbstractArgsFromNapiArray(jsArgv);
43+
44+
rcl_allocator_t allocator = rcl_get_default_allocator();
45+
rcl_arguments_t parsed_args = rcl_get_zero_initialized_arguments();
46+
47+
rcl_ret_t ret = rcl_parse_arguments(argc, const_cast<const char**>(argv),
48+
allocator, &parsed_args);
49+
50+
if (RCL_RET_OK != ret) {
51+
FreeArgs(argv, argc);
52+
Napi::Error::New(env, "Failed to parse arguments")
53+
.ThrowAsJavaScriptException();
54+
return env.Undefined();
55+
}
56+
57+
RCPPUTILS_SCOPE_EXIT({
58+
rcl_ret_t fini_ret = rcl_arguments_fini(&parsed_args);
59+
if (RCL_RET_OK != fini_ret) {
60+
// Log error but don't throw since we might be already throwing
61+
}
62+
});
63+
64+
int nonros_argc = 0;
65+
const char** nonros_argv = nullptr;
66+
67+
ret = rcl_remove_ros_arguments(const_cast<const char**>(argv), &parsed_args,
68+
allocator, &nonros_argc, &nonros_argv);
69+
70+
if (RCL_RET_OK != ret) {
71+
FreeArgs(argv, argc);
72+
Napi::Error::New(env, "Failed to remove ROS arguments")
73+
.ThrowAsJavaScriptException();
74+
return env.Undefined();
75+
}
76+
77+
RCPPUTILS_SCOPE_EXIT({ allocator.deallocate(nonros_argv, allocator.state); });
78+
79+
Napi::Array result = Napi::Array::New(env, nonros_argc);
80+
for (int i = 0; i < nonros_argc; ++i) {
81+
result.Set(i, Napi::String::New(env, nonros_argv[i]));
82+
}
83+
84+
FreeArgs(argv, argc);
85+
86+
return result;
87+
}
88+
3189
Napi::Value InitString(const Napi::CallbackInfo& info) {
3290
Napi::Env env = info.Env();
3391

@@ -110,6 +168,7 @@ Napi::Object InitBindings(Napi::Env env, Napi::Object exports) {
110168
Napi::Function::New(env, CreateArrayBufferFromAddress));
111169
exports.Set("createArrayBufferCleaner",
112170
Napi::Function::New(env, CreateArrayBufferCleaner));
171+
exports.Set("removeROSArgs", Napi::Function::New(env, RemoveROSArgs));
113172
return exports;
114173
}
115174

test/test-remove-ros-args.js

Lines changed: 63 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,63 @@
1+
// Copyright (c) 2025, The Robot Web Tools Contributors
2+
//
3+
// Licensed under the Apache License, Version 2.0 (the "License");
4+
// you may not use this file except in compliance with the License.
5+
// You may obtain a copy of the License at
6+
//
7+
// http://www.apache.org/licenses/LICENSE-2.0
8+
//
9+
// Unless required by applicable law or agreed to in writing, software
10+
// distributed under the License is distributed on an "AS IS" BASIS,
11+
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
12+
// See the License for the specific language governing permissions and
13+
// limitations under the License.
14+
15+
'use strict';
16+
17+
const assert = require('assert');
18+
const rclnodejs = require('../index.js');
19+
20+
describe('rclnodejs removeROSArgs', function () {
21+
it('should remove ROS arguments', function () {
22+
const args = ['node', 'script.js', '--ros-args', '-r', '__node:=my_node'];
23+
const nonRosArgs = rclnodejs.removeROSArgs(args);
24+
assert.deepStrictEqual(nonRosArgs, ['node', 'script.js']);
25+
});
26+
27+
it('should keep non-ROS arguments', function () {
28+
const args = ['node', 'script.js', 'arg1', 'arg2'];
29+
const nonRosArgs = rclnodejs.removeROSArgs(args);
30+
assert.deepStrictEqual(nonRosArgs, ['node', 'script.js', 'arg1', 'arg2']);
31+
});
32+
33+
it('should handle mixed arguments', function () {
34+
const args = [
35+
'node',
36+
'script.js',
37+
'arg1',
38+
'--ros-args',
39+
'-r',
40+
'__node:=my_node',
41+
'--',
42+
'arg2',
43+
];
44+
const nonRosArgs = rclnodejs.removeROSArgs(args);
45+
46+
assert.ok(nonRosArgs.includes('arg1'));
47+
assert.ok(nonRosArgs.includes('arg2'));
48+
assert.ok(!nonRosArgs.includes('--ros-args'));
49+
assert.ok(!nonRosArgs.includes('__node:=my_node'));
50+
});
51+
52+
it('should throw if argv is not an array', function () {
53+
assert.throws(() => {
54+
rclnodejs.removeROSArgs('not an array');
55+
}, TypeError);
56+
});
57+
58+
it('should throw if argv elements are not strings', function () {
59+
assert.throws(() => {
60+
rclnodejs.removeROSArgs(['string', 123]);
61+
}, TypeError);
62+
});
63+
});

test/types/index.test-d.ts

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,9 @@ MSG.data = '';
1414

1515
// ---- rclnodejs -----
1616
expectType<Promise<void>>(rclnodejs.init());
17+
expectType<string[]>(
18+
rclnodejs.removeROSArgs(['--ros-args', '-r', '__node:=my_node'])
19+
);
1720
expectType<string | undefined>(rclnodejs.DistroUtils.getDistroName());
1821
expectType<boolean>(rclnodejs.isShutdown());
1922
expectType<void>(rclnodejs.shutdown());

types/index.d.ts

Lines changed: 7 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -58,6 +58,13 @@ declare module 'rclnodejs' {
5858
*/
5959
function init(context?: Context, argv?: string[]): Promise<void>;
6060

61+
/**
62+
* Remove ROS-specific arguments from the given argument list.
63+
* @param argv - The argument list to process.
64+
* @returns The argument list with ROS arguments removed.
65+
*/
66+
function removeROSArgs(argv: string[]): string[];
67+
6168
/**
6269
* Start detection and processing of units of work.
6370
*

0 commit comments

Comments
 (0)