Add functions to run ros2 run/launch #1222
Conversation
There was a problem hiding this comment.
Pull Request Overview
This PR adds functions to run ROS2 package executables and launch files programmatically from within Node.js applications using the ros2 run and ros2 launch commands.
- Adds
ros2Run()function to execute ROS2 package executables with optional arguments - Adds
ros2Launch()function to run ROS2 launch files with optional arguments - Includes comprehensive input validation for both functions
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 6 comments.
| File | Description |
|---|---|
| index.js | Implements ros2Run and ros2Launch functions with input validation and child process spawning |
| test/types/index.test-d.ts | Adds type definitions for the new functions to ensure proper TypeScript support |
|
|
||
| const command = 'ros2'; | ||
| const cmdArgs = ['run', packageName, executableName, ...args]; | ||
| const childProcess = spawn(command, cmdArgs); |
There was a problem hiding this comment.
The spawn call lacks proper validation of the command and arguments, which could lead to command injection if packageName, executableName, or args contain shell metacharacters. Consider sanitizing inputs or using shell: false option explicitly.
| const childProcess = spawn(command, cmdArgs); | |
| const childProcess = spawn(command, cmdArgs, { shell: false }); |
| } | ||
| const command = 'ros2'; | ||
| const cmdArgs = ['launch', packageName, launchFile, ...args]; | ||
| const childProcess = spawn(command, cmdArgs); |
There was a problem hiding this comment.
The spawn call lacks proper validation of the command and arguments, which could lead to command injection if packageName, launchFile, or args contain shell metacharacters. Consider sanitizing inputs or using shell: false option explicitly.
| const childProcess = spawn(command, cmdArgs); | |
| const childProcess = spawn(command, cmdArgs, { shell: false }); |
index.js
Outdated
| }); | ||
|
|
||
| resolve({ | ||
| process: childProcess, |
There was a problem hiding this comment.
The function resolves immediately after spawning the process without waiting for it to start successfully. Consider waiting for the 'spawn' event or checking if the process is running before resolving to provide better error handling.
| process: childProcess, | |
| childProcess.once('error', (error) => { | |
| reject(new Error(`Failed to start ros2 run: ${error.message}`)); | |
| }); | |
| childProcess.once('spawn', () => { | |
| resolve({ | |
| process: childProcess, | |
| }); |
|
|
||
| resolve({ | ||
| process: childProcess, | ||
| }); |
There was a problem hiding this comment.
The function resolves immediately after spawning the process without waiting for it to start successfully. Consider waiting for the 'spawn' event or checking if the process is running before resolving to provide better error handling.
| }); | |
| // Wait for immediate errors before resolving | |
| childProcess.once('error', (error) => { | |
| reject(new Error(`Failed to start ros2 launch: ${error.message}`)); | |
| }); | |
| setImmediate(() => { | |
| // If no error occurred, resolve | |
| resolve({ | |
| process: childProcess, | |
| }); | |
| }); |
index.js
Outdated
| */ | ||
| ros2Launch(packageName, launchFile, args = []) { | ||
| return ros2Launch(packageName, launchFile, args); | ||
| }, |
There was a problem hiding this comment.
[nitpick] The method is a simple wrapper around the standalone function. Consider removing the duplication by either using the standalone function directly or implementing the logic only in the object method.
| }, |
index.js
Outdated
| */ | ||
| ros2Launch(packageName, launchFile, args = []) { | ||
| return ros2Launch(packageName, launchFile, args); | ||
| }, |
There was a problem hiding this comment.
[nitpick] The method is a simple wrapper around the standalone function. Consider removing the duplication by either using the standalone function directly or implementing the logic only in the object method.
| }, | |
| ros2Launch: ros2Launch, |
There was a problem hiding this comment.
Pull Request Overview
This PR adds functionality to run ROS2 package executables and launch files programmatically from Node.js applications using the ros2 run and ros2 launch commands.
- Adds
ros2Run()function to execute ROS2 package executables with optional arguments and input validation - Adds
ros2Launch()function to run ROS2 launch files with optional arguments and input validation - Includes TypeScript type definitions for both new functions
Reviewed Changes
Copilot reviewed 2 out of 3 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| index.js | Implements ros2Run and ros2Launch functions with validation and spawns child processes |
| test/types/index.test-d.ts | Adds TypeScript type tests for the new ros2Run and ros2Launch functions |
| childProcess.on('error', (error) => { | ||
| reject(new Error(`Failed to start ros2 run: ${error.message}`)); | ||
| }); | ||
| childProcess.on('spawn', () => { |
There was a problem hiding this comment.
The 'spawn' event is not standard in Node.js child_process. Use 'childProcess.pid' check or a timeout to confirm successful spawn instead of relying on a non-existent event.
| resolve({ | ||
| process: childProcess, | ||
| }); | ||
| }); |
There was a problem hiding this comment.
The 'spawn' event is not standard in Node.js child_process. Use 'childProcess.pid' check or a timeout to confirm successful spawn instead of relying on a non-existent event.
| }); | |
| // The 'spawn' event is not standard; instead, check for childProcess.pid | |
| if (childProcess.pid) { | |
| resolve({ | |
| process: childProcess, | |
| }); | |
| } else { | |
| // Fallback: wait a short time to see if pid is set | |
| setTimeout(() => { | |
| if (childProcess.pid) { | |
| resolve({ | |
| process: childProcess, | |
| }); | |
| } else { | |
| reject(new Error('Failed to spawn ros2 process')); | |
| } | |
| }, 50); | |
| } |
This PR adds functions to run ROS2 package executables and launch files programmatically from within Node.js applications using the `ros2 run` and `ros2 launch` commands. - Adds `ros2Run()` function to execute ROS2 package executables with optional arguments - Adds `ros2Launch()` function to run ROS2 launch files with optional arguments - Includes comprehensive input validation for both functions Fix: #1220
2 participants
This PR adds functions to run ROS2 package executables and launch files programmatically from within Node.js applications using the
ros2 runandros2 launchcommands.ros2Run()function to execute ROS2 package executables with optional argumentsros2Launch()function to run ROS2 launch files with optional argumentsFix: #1220