Address comments

Author: minggangw

README.md

@@ -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):

lib/interface_loader.js

@@ -93,6 +93,7 @@ let interfaceLoader = {
   _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;
       }
@@ -122,14 +123,15 @@ let interfaceLoader = {
             }
           } catch (err) {
             // Skip directories we can't read
-            console.log('Error reading directory:', dir, err.message);
+            console.error('Error reading directory:', dir, err.message);
           }
           return null;
         }
 
         const foundFilePath = searchForFile(
           path.join(pkgPath, 'share', packageName)
         );
+
         if (foundFilePath && foundFilePath.length > 0) {
           // Use worker thread to generate interfaces synchronously
           try {
@@ -141,10 +143,6 @@ let interfaceLoader = {
           } catch (err) {
             console.error('Error in interface generation:', err);
           }
-        } else {
-          throw new Error(
-            `The message required does not exist: ${packageName}, ${type}, ${messageName} at ${generator.generatedRoot}`
-          );
         }
       }
     }

test/test-rosidl-message-generator.js

@@ -19,34 +19,11 @@ const os = require('os');
 const rclnodejs = require('../index.js');
 const path = require('path');
 
-function buildTestMessage() {
-  // Build the custom_msg_test package synchronously before running the test
-  const customMsgTestPath = path.join(__dirname, 'custom_msg_test');
-  const buildResult = require('child_process').spawnSync('colcon', ['build'], {
-    cwd: customMsgTestPath,
-    stdio: 'inherit',
-    timeout: 60000, // 60 second timeout
-  });
-
-  if (buildResult.error) {
-    throw new Error(
-      `Failed to build custom_msg_test package: ${buildResult.error.message}`
-    );
-  }
-
-  if (buildResult.status !== 0) {
-    throw new Error(`colcon build failed with exit code ${buildResult.status}`);
-  }
-
+function sourceSetupScript(setupPath) {
   // Source the local_setup.sh to get environment variables
-  const setupScriptPath = path.join(
-    customMsgTestPath,
-    'install',
-    'local_setup.sh'
-  );
   const sourceResult = require('child_process').spawnSync(
     'bash',
-    ['-c', `source ${setupScriptPath} && env`],
+    ['-c', `source ${setupPath} && env`],
     {
       encoding: 'utf8',
       timeout: 10000, // 10 second timeout
@@ -83,6 +60,34 @@ function buildTestMessage() {
   });
 }
 
+function buildTestMessage() {
+  // Build the custom_msg_test package synchronously before running the test
+  const customMsgTestPath = path.join(__dirname, 'custom_msg_test');
+  const buildResult = require('child_process').spawnSync('colcon', ['build'], {
+    cwd: customMsgTestPath,
+    stdio: 'inherit',
+    timeout: 60000, // 60 second timeout
+  });
+
+  if (buildResult.error) {
+    throw new Error(
+      `Failed to build custom_msg_test package: ${buildResult.error.message}`
+    );
+  }
+
+  if (buildResult.status !== 0) {
+    throw new Error(`colcon build failed with exit code ${buildResult.status}`);
+  }
+
+  // Source the local_setup.sh to get environment variables
+  const setupScriptPath = path.join(
+    customMsgTestPath,
+    'install',
+    'local_setup.sh'
+  );
+  sourceSetupScript(setupScriptPath);
+}
+
 describe('ROSIDL Node.js message generator test suite', function () {
   this.timeout(60 * 1000);
 
@@ -289,15 +294,18 @@ describe('ROSIDL Node.js message generator test suite', function () {
 
   it('Generate message at runtime', function () {
     const amentPrefixPathOriginal = process.env.AMENT_PREFIX_PATH;
-    buildTestMessage();
-
-    assert.doesNotThrow(() => {
-      const Testing = rclnodejs.require('custom_msg_test/msg/Testing');
-      const t = new Testing();
-      assert.equal(typeof t, 'object');
-      assert.equal(typeof t.x, 'number');
-      assert.equal(typeof t.data, 'string');
-    }, 'This function should not throw');
-    process.env.AMENT_PREFIX_PATH = amentPrefixPathOriginal;
+    try {
+      buildTestMessage();
+
+      assert.doesNotThrow(() => {
+        const Testing = rclnodejs.require('custom_msg_test/msg/Testing');
+        const t = new Testing();
+        assert.equal(typeof t, 'object');
+        assert.equal(typeof t.x, 'number');
+        assert.equal(typeof t.data, 'string');
+      }, 'This function should not throw');
+    } finally {
+      process.env.AMENT_PREFIX_PATH = amentPrefixPathOriginal;
+    }
   });
 });