Skip to content
Merged
Show file tree
Hide file tree
Changes from 2 commits
Commits
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
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -140,6 +140,8 @@ To generate messages from IDL files, use the `generate-messages-idl` npm script:
npm run generate-messages-idl
```

\* This step is not needed for rclnodejs > 1.5.0

## Performance Benchmarks

Benchmark results for 1000 iterations with 1024KB messages (Ubuntu 24.04.3 WSL2, i7-1185G7):
Expand Down
70 changes: 69 additions & 1 deletion lib/interface_loader.js
Original file line number Diff line number Diff line change
Expand Up @@ -90,6 +90,67 @@ let interfaceLoader = {
return pkg;
},

_searchAndGenerateInterface(packageName, type, messageName, filePath) {
// Check if it's a valid package
for (const pkgPath of generator.getInstalledPackagePaths()) {
// We are going to ignore the path ROS2 installed.
if (pkgPath.includes('ros2-linux')) {
continue;
}
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded 'ros2-linux' string check is a magic value that lacks explanation. This filtering logic should be documented or made configurable to explain why certain paths are excluded.

Copilot uses AI. Check for mistakes.
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The hardcoded check for 'ros2-linux' is platform-specific and may not work on other operating systems or ROS2 installation methods. Consider using a more robust method to identify system ROS2 installations or make this configurable.

Copilot uses AI. Check for mistakes.

// Recursively search for files named messageName.* under pkgPath/
if (fs.existsSync(pkgPath)) {
// Recursive function to search for files
function searchForFile(dir) {
try {
const items = fs.readdirSync(dir, { withFileTypes: true });
for (const item of items) {
const fullPath = path.join(dir, item.name);

if (item.isFile()) {
const baseName = path.parse(item.name).name;
// Check if the base filename matches messageName
if (baseName === messageName) {
return fullPath;
}
} else if (item.isDirectory()) {
// Recursively search subdirectories
const result = searchForFile(fullPath);
if (result) {
return result;
}
}
}
} catch (err) {
// Skip directories we can't read
console.error('Error reading directory:', dir, err.message);
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This error message is logged to console.error but doesn't provide enough context about what operation was being performed. Consider using a more descriptive message like 'Failed to read directory during message discovery:' or use a proper logging mechanism.

Suggested change
console.error('Error reading directory:', dir, err.message);
console.error(
`Failed to read directory during message discovery for package "${packageName}", type "${type}", message "${messageName}". Directory: ${dir}. Error: ${err.message}`
);

Copilot uses AI. Check for mistakes.
}
return null;
}

const foundFilePath = searchForFile(
path.join(pkgPath, 'share', packageName)
);

if (foundFilePath && foundFilePath.length > 0) {
// Use worker thread to generate interfaces synchronously
try {
generator.generateInPathSyncWorker(pkgPath);
// Now try to load the interface again from the generated files
if (fs.existsSync(filePath)) {
return require(filePath);
}
} catch (err) {
console.error('Error in interface generation:', err);
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The error message lacks context about which package/interface failed to generate. Include packageName, type, and messageName in the error message for better debugging.

Suggested change
console.error('Error in interface generation:', err);
console.error(`Error in interface generation for package: ${packageName}, type: ${type}, message: ${messageName}:`, err);

Copilot uses AI. Check for mistakes.
}
}
}
}
throw new Error(
`The message required does not exist: ${packageName}, ${type}, ${messageName} at ${generator.generatedRoot}`
);
},

loadInterface(packageName, type, messageName) {
if (arguments.length === 1) {
const type = arguments[0];
Expand All @@ -100,7 +161,6 @@ let interfaceLoader = {
}
throw new Error(`The message required does not exist: ${type}`);
}

if (packageName && type && messageName) {
let filePath = path.join(
generator.generatedRoot,
Expand All @@ -110,8 +170,16 @@ let interfaceLoader = {

if (fs.existsSync(filePath)) {
return require(filePath);
} else {
return this._searchAndGenerateInterface(
packageName,
type,
messageName,
filePath
);
}
}
// We cannot parse `packageName`, `type` and `messageName` from the string passed.
throw new Error(
`The message required does not exist: ${packageName}, ${type}, ${messageName} at ${generator.generatedRoot}`
);
Expand Down
63 changes: 63 additions & 0 deletions rosidl_gen/generate_worker.js
Original file line number Diff line number Diff line change
@@ -0,0 +1,63 @@
// Copyright (c) 2025, The Robot Web Tools Contributors
//
// Licensed under the Apache License, Version 2.0 (the "License");
// you may not use this file except in compliance with the License.
// You may obtain a copy of the License at
//
// http://www.apache.org/licenses/LICENSE-2.0
//
// Unless required by applicable law or agreed to in writing, software
// distributed under the License is distributed on an "AS IS" BASIS,
// WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
// See the License for the specific language governing permissions and
// limitations under the License.

const fse = require('fs-extra');
const generateJSStructFromIDL = require('./idl_generator.js');
const packages = require('./packages.js');
const path = require('path');
const idlConvertor = require('../rosidl_convertor/idl_convertor.js');

const generatedRoot = path.join(__dirname, '../generated/');
const idlPath = path.join(generatedRoot, 'share');
const useIDL = !!process.argv.find((arg) => arg === '--idl');

// Get target path from environment variable instead of workerData
Copy link

Copilot AI Sep 10, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This comment references 'workerData' but the code uses child_process.spawnSync, not worker_threads. The comment should be updated to reflect the actual implementation using environment variables and spawn processes.

Suggested change
// Get target path from environment variable instead of workerData
// Get target path from the WORKER_TARGET_PATH environment variable

Copilot uses AI. Check for mistakes.
const targetPath = process.env.WORKER_TARGET_PATH;

async function generateInPath(targetPath) {
let pkgsInfo = null;
if (!useIDL) {
pkgsInfo = Array.from(
(await packages.findPackagesInDirectory(targetPath)).values()
);
} else {
const idlPkgs = await packages.findPackagesInDirectory(targetPath, useIDL);
await fse.ensureDir(idlPath);
const promises = [];
idlPkgs.forEach((pkg) => {
pkg.idls.forEach((idl) => {
promises.push(idlConvertor(idl.pkgName, idl.filePath, idlPath));
});
});
await Promise.all(promises);
const pkgsFromIdl = await packages.findPackagesInDirectory(idlPath, false);
pkgsInfo = Array.from(pkgsFromIdl.values());
}

await Promise.all(
pkgsInfo.map((pkgInfo) => generateJSStructFromIDL(pkgInfo, generatedRoot))
);
}

async function main() {
try {
await generateInPath(targetPath);
process.exit(0);
} catch (error) {
console.error('Worker generation failed:', error.message);
process.exit(1);
}
}

main();
31 changes: 31 additions & 0 deletions rosidl_gen/index.js
Original file line number Diff line number Diff line change
Expand Up @@ -53,6 +53,35 @@ async function generateInPath(path) {
);
}

function generateInPathSyncWorker(targetPath) {
try {
// Use child_process.spawnSync for truly synchronous execution
const result = require('child_process').spawnSync(
'node',
[path.join(__dirname, 'generate_worker.js')],
{
env: { ...process.env, WORKER_TARGET_PATH: targetPath },
encoding: 'utf8',
timeout: 30000,
}
);

if (result.error) {
throw result.error;
}

if (result.status !== 0) {
throw new Error(
`Worker process exited with code ${result.status}. stderr: ${result.stderr}`
);
}

return result.stdout;
} catch (error) {
throw error;
}
}

async function generateAll(forcedGenerating) {
// If we want to create the JavaScript files compulsively (|forcedGenerating| equals to true)
// or the JavaScript files have not been created (|exist| equals to false),
Expand Down Expand Up @@ -86,7 +115,9 @@ const generator = {

generateAll,
generateInPath,
generateInPathSyncWorker,
generatedRoot,
getInstalledPackagePaths,
};

module.exports = generator;
32 changes: 32 additions & 0 deletions test/custom_msg_test/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,32 @@
cmake_minimum_required(VERSION 3.8)
project(custom_msg_test)

if(CMAKE_COMPILER_IS_GNUCXX OR CMAKE_CXX_COMPILER_ID MATCHES "Clang")
add_compile_options(-Wall -Wextra -Wpedantic)
endif()

# find dependencies
find_package(ament_cmake REQUIRED)
find_package(std_msgs REQUIRED)
find_package(builtin_interfaces REQUIRED)
find_package(rosidl_default_generators REQUIRED)

# Generate interfaces
rosidl_generate_interfaces(${PROJECT_NAME}
"msg/Testing.msg"
DEPENDENCIES std_msgs builtin_interfaces
)

if(BUILD_TESTING)
find_package(ament_lint_auto REQUIRED)
# the following line skips the linter which checks for copyrights
# comment the line when a copyright and license is added to all source files
set(ament_cmake_copyright_FOUND TRUE)
# the following line skips cpplint (only works in a git repo)
# comment the line when this package is in a git repo and when
# a copyright and license is added to all source files
set(ament_cmake_cpplint_FOUND TRUE)
ament_lint_auto_find_test_dependencies()
endif()

ament_package()
31 changes: 31 additions & 0 deletions test/custom_msg_test/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,31 @@
# Custom Message Test Package

This ROS2 package contains custom message definitions for testing rclnodejs runtime message generation capabilities.

## Messages

### Testing.msg

A test message that contains the position of a point in free space with additional data:

- `float64 x` - X coordinate
- `float64 y` - Y coordinate
- `float64 z` - Z coordinate
- `string data` - Additional string data

## Usage

This package is used by the rclnodejs test suite to verify that custom messages can be generated and used at runtime.

## Building

```bash
# From the test directory
cd custom_msg_test
colcon build
source install/setup.bash
```

## Testing

The message definitions in this package are used by `test-rosidl-message-generator.js` to verify runtime message generation functionality.
5 changes: 5 additions & 0 deletions test/custom_msg_test/msg/Testing.msg
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
# This contains the position of a point in free space
float64 x
float64 y
float64 z
Comment on lines +2 to +4
Copy link

Copilot AI Sep 9, 2025

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

The message comment states 'This contains the position of a point in free space' but the test only verifies the x field and data field. The y and z fields are defined but not tested, creating a mismatch between the documented purpose and actual test coverage.

Copilot uses AI. Check for mistakes.
string data
23 changes: 23 additions & 0 deletions test/custom_msg_test/package.xml
Original file line number Diff line number Diff line change
@@ -0,0 +1,23 @@
<?xml version="1.0"?>
<?xml-model href="http://download.ros.org/schema/package_format3.xsd" schematypens="http://www.w3.org/2001/XMLSchema"?>
<package format="3">
<name>custom_msg_test</name>
<version>1.0.0</version>
<description>Custom message definitions for rclnodejs testing</description>
<maintainer email="[email protected]">Test Maintainer</maintainer>
<license>Apache-2.0</license>

<buildtool_depend>ament_cmake</buildtool_depend>

<depend>std_msgs</depend>
<depend>builtin_interfaces</depend>

<build_depend>rosidl_default_generators</build_depend>
<exec_depend>rosidl_default_runtime</exec_depend>

<member_of_group>rosidl_interface_packages</member_of_group>

<export>
<build_type>ament_cmake</build_type>
</export>
</package>
Loading
Loading