Skip to content

Commit 9971c71

Browse files
committed
Address PR 1478 review comments
1 parent 40127e5 commit 9971c71

7 files changed

Lines changed: 122 additions & 39 deletions

File tree

electron_demo/manipulator/README.md

Lines changed: 9 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -282,10 +282,10 @@ manipulator/
282282

283283
### Key Dependencies
284284

285-
- **electron**: `^31.7.7` - Desktop application framework
286-
- **rclnodejs**: `^1.5.1` - ROS2 JavaScript client library (latest compatible)
287-
- **@electron/rebuild**: `^3.7.2` - Native module rebuilding tool
288-
- **three.js**: `r128` - 3D graphics library (loaded via CDN)
285+
- **electron**: `^40.1.0` - Desktop application framework
286+
- **rclnodejs**: `^1.8.1` - ROS2 JavaScript client library
287+
- **@electron/rebuild**: `^4.0.3` - Native module rebuilding tool
288+
- **three**: `^0.182.0` - 3D graphics library
289289

290290
### Debugging
291291

@@ -305,10 +305,11 @@ manipulator/
305305
npm start
306306
```
307307

308-
2. **Build errors with latest Electron**
309-
- This demo uses Electron 31.7.7 for rclnodejs compatibility
310-
- Electron 38+ requires C++20, which rclnodejs doesn't support yet
311-
- The current versions are tested and stable
308+
2. **Build errors with Electron**
309+
310+
- This demo currently uses Electron 40.1.0
311+
- If you change Electron or other native-module dependencies, rerun `npm run rebuild`
312+
- The versions recorded in `package.json` and `package-lock.json` are the tested baseline for this demo
312313

313314
3. **No ROS2 messages received**
314315
- Check if ROS2 daemon is running: `ros2 daemon start`

electron_demo/turtle_tf2/README.md

Lines changed: 2 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -51,7 +51,7 @@ The turtle_tf2 demo showcases:
5151
### System Requirements
5252

5353
- **ROS2**: Humble, Iron, or Rolling distribution
54-
- **Node.js**: Version 18 or higher (for compatibility with latest Electron)
54+
- **Node.js**: Version 20.20.2 or higher
5555
- **turtlesim**: ROS2 turtle simulation package
5656
- **Electron**: For desktop application framework
5757

@@ -317,46 +317,39 @@ You can observe the following behavior by:
317317
### Common Issues
318318

319319
1. **"Cannot connect to ROS2" or "librcl.so: cannot open shared object file"**
320-
321320
- Ensure ROS2 is sourced: `source /opt/ros/$ROS_DISTRO/setup.bash`
322321
- **Critical**: Source ROS2 in the SAME terminal where you run `npm start`
323322
- Check if ROS2 daemon is running: `ros2 daemon status`
324323
- Verify ROS2 installation: `ros2 --version`
325324

326325
2. **"Turtlesim not responding" or "Failed to spawn turtle2"**
327-
328326
- Verify turtlesim is running: `ros2 run turtlesim turtlesim_node`
329327
- Check available topics: `ros2 topic list`
330328
- Ensure spawn service is available: `ros2 service list | grep spawn`
331329
- Try restarting turtlesim_node if spawn calls fail
332330

333331
3. **"No transforms detected"**
334-
335332
- Ensure demo is started: Click "Start Demo" button
336333
- Check TF2 tree: `ros2 run tf2_tools view_frames`
337334

338335
4. **"Dynamic frame not visible when toggling"**
339-
340336
- **Check if the demo is started**: Click "Start Demo" button first to initialize all broadcasters
341337
- **Look for an orange sphere near coordinates (2,3)**: The dynamic frame appears as an orange sphere orbiting around the red static frame
342338
- **Wait for circular motion**: The dynamic frame moves in a 2-unit radius circle, taking about 6 seconds for a full rotation
343339
- **The orange sphere is now bigger**: The dynamic frame has been made 3x larger for better visibility
344340
- **Check the transform list**: The dynamic frame should appear in the left panel's transform list with changing coordinates around (2±2, 3±2, 0)
345341

346342
5. **"3D visualization not loading"**
347-
348343
- Check browser console for WebGL errors
349344
- Ensure hardware acceleration is enabled
350345
- Try restarting the Electron application
351346

352347
6. **"electron: not found" or native module errors**
353-
354348
- Make sure you ran `npm run rebuild` after `npm install`
355-
- Ensure Node.js version is compatible (18 or higher)
349+
- Ensure Node.js version is compatible (20.20.2 or higher)
356350
- Try deleting `node_modules` and running `npm install && npm run rebuild` again
357351

358352
7. **"THREE is not defined" or script loading errors**
359-
360353
- Ensure Three.js is properly installed: `npm install three@0.155.0`
361354
- Check that `node_modules/three/build/three.min.js` exists
362355
- If issues persist, try reinstalling: `rm -rf node_modules && npm install && npm run rebuild`

lib/native_loader.js

Lines changed: 21 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,10 @@ const { execSync } = require('child_process');
2020
const { NativeError } = require('./errors.js');
2121
const bindings = require('bindings');
2222
const debug = require('debug')('rclnodejs');
23+
const {
24+
detectPrebuildRuntime,
25+
getTaggedPrebuildFilename,
26+
} = require('./prebuilds');
2327
const { detectUbuntuCodename } = require('./utils');
2428

2529
let nativeModule = null;
@@ -35,6 +39,7 @@ function customFallbackLoader() {
3539

3640
const rosDistro = process.env.ROS_DISTRO;
3741
const arch = process.arch;
42+
const runtime = detectPrebuildRuntime();
3843
const ubuntuCodename = detectUbuntuCodename();
3944

4045
// Require all three components for exact match
@@ -58,16 +63,22 @@ function customFallbackLoader() {
5863
}
5964

6065
try {
61-
// Look for exact match binary: {ros_distro}-{ubuntu_codename}-{arch}-rclnodejs.node
62-
const exactMatchFilename = `${rosDistro}-${ubuntuCodename}-${arch}-rclnodejs.node`;
63-
const exactMatchPath = path.join(prebuildDir, exactMatchFilename);
64-
65-
if (fs.existsSync(exactMatchPath)) {
66-
debug(`Found exact match binary: ${exactMatchFilename}`);
67-
return require(exactMatchPath);
66+
const candidate = getTaggedPrebuildFilename({
67+
rosDistro,
68+
ubuntuCodename,
69+
arch,
70+
runtime,
71+
});
72+
const candidatePath = path.join(prebuildDir, candidate);
73+
74+
if (fs.existsSync(candidatePath)) {
75+
debug(`Found ${runtime} prebuilt binary: ${candidate}`);
76+
return require(candidatePath);
6877
}
6978

70-
debug(`No exact match found for: ${exactMatchFilename}`);
79+
debug(
80+
`No matching ${runtime} prebuilt binary found for ${rosDistro}-${ubuntuCodename}-${arch}`
81+
);
7182
return null;
7283
} catch (e) {
7384
debug('Error in simplified prebuilt loader:', e.message);
@@ -110,10 +121,11 @@ function loadNativeAddon() {
110121
}
111122

112123
const rosDistro = process.env.ROS_DISTRO;
124+
const runtime = detectPrebuildRuntime();
113125
const ubuntuCodename = detectUbuntuCodename();
114126

115127
debug(
116-
`Platform: ${process.platform}, Arch: ${process.arch}, Ubuntu: ${ubuntuCodename || 'unknown'}, ROS: ${rosDistro || 'unknown'}`
128+
`Platform: ${process.platform}, Arch: ${process.arch}, Runtime: ${runtime}, Ubuntu: ${ubuntuCodename || 'unknown'}, ROS: ${rosDistro || 'unknown'}`
117129
);
118130

119131
// Prebuilt binaries are only supported on Linux (Ubuntu)

lib/prebuilds.js

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
// Copyright (c) 2026, 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 PREBUILD_PACKAGE_NAME = 'rclnodejs';
18+
const SUPPORTED_PREBUILD_RUNTIMES = new Set(['node', 'electron']);
19+
20+
function detectPrebuildRuntime() {
21+
if (process.env.npm_config_runtime === 'electron') {
22+
return 'electron';
23+
}
24+
25+
return process.versions.electron ? 'electron' : 'node';
26+
}
27+
28+
function getTaggedPrebuildFilename({
29+
rosDistro,
30+
ubuntuCodename,
31+
arch,
32+
runtime,
33+
}) {
34+
return `${rosDistro}-${ubuntuCodename}-${arch}-${runtime}-${PREBUILD_PACKAGE_NAME}.node`;
35+
}
36+
37+
function getRuntimeFromGeneratedPrebuild(fileName) {
38+
const runtime = fileName.split('.')[0];
39+
return SUPPORTED_PREBUILD_RUNTIMES.has(runtime) ? runtime : null;
40+
}
41+
42+
module.exports = {
43+
detectPrebuildRuntime,
44+
getRuntimeFromGeneratedPrebuild,
45+
getTaggedPrebuildFilename,
46+
PREBUILD_PACKAGE_NAME,
47+
};

package.json

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -34,7 +34,9 @@
3434
"format": "clang-format -i -style=file ./src/*.cpp ./src/*.h && npx --yes prettier --write \"{lib,rosidl_gen,rostsd_gen,rosidl_parser,types,example,test,scripts,benchmark,rostsd_gen}/**/*.{js,md,ts}\" ./*.{js,md,ts}",
3535
"prepare": "husky",
3636
"coverage": "cat ./coverage/lcov.info | ./node_modules/coveralls/bin/coveralls.js",
37-
"prebuild": "prebuildify --napi --strip --target 20.20.2 --target electron@23.0.0 && node scripts/tag_prebuilds.js"
37+
"prebuild:node": "prebuildify --napi --strip --name node --target 20.20.2",
38+
"prebuild:electron": "prebuildify --napi --strip --name electron --target electron@23.0.0",
39+
"prebuild": "npm run prebuild:node && npm run prebuild:electron && node scripts/tag_prebuilds.js"
3840
},
3941
"bin": {
4042
"generate-ros-messages": "./scripts/generate_messages.js"

scripts/install.js

Lines changed: 14 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,10 @@
1515
const fs = require('fs');
1616
const path = require('path');
1717
const { execSync } = require('child_process');
18+
const {
19+
detectPrebuildRuntime,
20+
getTaggedPrebuildFilename,
21+
} = require('../lib/prebuilds');
1822
const { detectUbuntuCodename } = require('../lib/utils');
1923

2024
function getRosDistro() {
@@ -24,6 +28,7 @@ function getRosDistro() {
2428
function checkPrebuiltBinary() {
2529
const platform = process.platform;
2630
const arch = process.arch;
31+
const runtime = detectPrebuildRuntime();
2732

2833
// Only Linux has prebuilt binaries
2934
if (platform !== 'linux') {
@@ -50,18 +55,24 @@ function checkPrebuiltBinary() {
5055
'prebuilds',
5156
`${platform}-${arch}`
5257
);
53-
const expectedBinary = `${rosDistro}-${ubuntuCodename}-${arch}-rclnodejs.node`;
58+
const expectedBinary = getTaggedPrebuildFilename({
59+
rosDistro,
60+
ubuntuCodename,
61+
arch,
62+
runtime,
63+
});
64+
5465
const binaryPath = path.join(prebuildDir, expectedBinary);
5566

5667
if (fs.existsSync(binaryPath)) {
5768
console.log(`✓ Found prebuilt binary: ${expectedBinary}`);
58-
console.log(` Platform: ${platform}, Arch: ${arch}`);
69+
console.log(` Platform: ${platform}, Arch: ${arch}, Runtime: ${runtime}`);
5970
console.log(` Ubuntu: ${ubuntuCodename}, ROS: ${rosDistro}`);
6071
return true;
6172
}
6273

6374
console.log(
64-
`✗ No prebuilt binary found for ${rosDistro}-${ubuntuCodename}-${arch}`
75+
`✗ No ${runtime} prebuilt binary found for ${rosDistro}-${ubuntuCodename}-${arch}`
6576
);
6677

6778
// List available binaries for debugging

scripts/tag_prebuilds.js

Lines changed: 26 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -14,6 +14,10 @@
1414

1515
const fs = require('fs');
1616
const path = require('path');
17+
const {
18+
getRuntimeFromGeneratedPrebuild,
19+
getTaggedPrebuildFilename,
20+
} = require('../lib/prebuilds');
1721
const { detectUbuntuCodename } = require('../lib/utils');
1822

1923
function tagPrebuilds() {
@@ -38,23 +42,36 @@ function tagPrebuilds() {
3842
return;
3943
}
4044

41-
const files = fs.readdirSync(prebuildDir).filter((f) => f.endsWith('.node'));
45+
const files = fs
46+
.readdirSync(prebuildDir)
47+
.filter(
48+
(file) => file.endsWith('.node') && getRuntimeFromGeneratedPrebuild(file)
49+
);
4250

4351
for (const file of files) {
4452
const filePath = path.join(prebuildDir, file);
53+
const runtime = getRuntimeFromGeneratedPrebuild(file);
4554

46-
// Create tagged version with format: {ros_distro}-{linux-codename}-{cpu-arch}-rclnodejs.node
47-
if (rosDistro && ubuntuCodename) {
48-
const taggedName = `${rosDistro}-${ubuntuCodename}-${arch}-rclnodejs.node`;
55+
// Create tagged version with format:
56+
// {ros_distro}-{linux-codename}-{cpu-arch}-{runtime}-rclnodejs.node
57+
if (rosDistro && ubuntuCodename && runtime) {
58+
const taggedName = getTaggedPrebuildFilename({
59+
rosDistro,
60+
ubuntuCodename,
61+
arch,
62+
runtime,
63+
});
4964
const taggedPath = path.join(prebuildDir, taggedName);
65+
66+
if (fs.existsSync(taggedPath)) {
67+
fs.unlinkSync(taggedPath);
68+
}
69+
5070
fs.copyFileSync(filePath, taggedPath);
5171
console.log(`Created tagged binary: ${taggedName}`);
5272

53-
// Remove the original generic binary file if it's the basic rclnodejs.node
54-
if (file === 'rclnodejs.node') {
55-
fs.unlinkSync(filePath);
56-
console.log(`Removed generic binary: ${file}`);
57-
}
73+
fs.unlinkSync(filePath);
74+
console.log(`Removed generated binary: ${file}`);
5875
} else {
5976
console.log(
6077
`Skipping tagging for ${file} - missing ROS_DISTRO or Ubuntu codename`

0 commit comments

Comments
 (0)