Skip to content

Commit 3e57ac8

Browse files
minggangwminggangw
committed
Leverage BigInt for Time/Duration (#1039)
Per current implementation, the `Time`/`Duration` in rclnodejs module may be initialized with two parts, which are `seconds` and `nanoseconds` using type of `number` or `string`, this is because the range limitation of integer that JavaScript can [represents](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER). While, after [`BigInt`](https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/BigInt) introduced, we can change it to type of `bigint` only, which aligns with other ROS2 clients, like `rclpy`. This patch implements: 1. Leverage `BigInt` to initialize `Time`/`Duration` in nanoseconds from JavaScript side. 2. Change `Clock` and `TimeSource` accordingly. 3. Get the period from `v8::BigInt` object as `int64_t` and pass it to `rcl` from C++ side. 4. Update the related unit tests accordingly. 5. Remove dependency, `int64-napi`, which was used to pass `int64_t` from JavaScript to C++. Fix: #1040
1 parent 7834387 commit 3e57ac8

File tree

12 files changed

+151
-244
lines changed

12 files changed

+151
-244
lines changed

‎lib/action/server.js‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -266,10 +266,12 @@ class ActionServer extends Entity {
266266
let goalHandle;
267267
if (accepted) {
268268
// Stamp time of acceptance
269-
const secondsAndNanos = this._node.getClock().now().secondsAndNanoseconds;
269+
const { seconds, nanoseconds } = this._node
270+
.getClock()
271+
.now().secondsAndNanoseconds;
270272
goalInfo.stamp = {
271-
sec: secondsAndNanos.seconds,
272-
nanosec: secondsAndNanos.nanoseconds,
273+
sec: Number(seconds),
274+
nanosec: Number(nanoseconds),
273275
};
274276

275277
try {

‎lib/clock.js‎

Lines changed: 2 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -52,8 +52,8 @@ class Clock {
5252
* @return {Time} Return the current time.
5353
*/
5454
now() {
55-
let time = rclnodejs.clockGetNow(this._handle);
56-
return new Time(time.sec, time.nanosec, this._clockType);
55+
const nowInNanosec = rclnodejs.clockGetNow(this._handle);
56+
return new Time(0n, nowInNanosec, this._clockType);
5757
}
5858
}
5959

‎lib/duration.js‎

Lines changed: 22 additions & 35 deletions
Original file line numberDiff line numberDiff line change
@@ -15,7 +15,7 @@
1515
'use strict';
1616

1717
const rclnodejs = require('bindings')('rclnodejs');
18-
const int64 = require('int64-napi');
18+
const S_TO_NS = 10n ** 9n;
1919

2020
/**
2121
* @class - Class representing a Duration in ROS
@@ -24,51 +24,38 @@ const int64 = require('int64-napi');
2424
class Duration {
2525
/**
2626
* Create a Duration.
27-
* @param {number|string} [seconds=0] - The second part of the duration.
28-
* @param {number|string} [nanoseconds=0] - The nanosecond part of the duration.
27+
* @param {bigint} [seconds=0] - The second part of the duration.
28+
* @param {bigint} [nanoseconds=0] - The nanosecond part of the duration.
2929
*/
30-
constructor(seconds = 0, nanoseconds = 0) {
31-
if (typeof seconds !== 'number' && typeof seconds !== 'string') {
30+
constructor(seconds = 0n, nanoseconds = 0n) {
31+
if (typeof seconds !== 'bigint') {
3232
throw new TypeError('Invalid argument of seconds');
3333
}
3434

35-
if (typeof nanoseconds !== 'number' && typeof nanoseconds !== 'string') {
35+
if (typeof nanoseconds !== 'bigint') {
3636
throw new TypeError('Invalid argument of nanoseconds');
3737
}
3838

39-
let secondInt64 = int64.from(seconds);
40-
let nanoInt64 = int64.from(nanoseconds);
41-
42-
if (typeof seconds === 'string' && seconds.startsWith('-')) {
43-
secondInt64 = int64.negative(secondInt64);
44-
}
45-
if (typeof nanoseconds === 'string' && nanoseconds.startsWith('-')) {
46-
nanoInt64 = int64.negative(nanoInt64);
39+
const total = seconds * S_TO_NS + nanoseconds;
40+
if (total >= 2n ** 63n) {
41+
throw new RangeError(
42+
'Total nanoseconds value is too large to store in C time point.'
43+
);
4744
}
48-
this._nanoseconds = secondInt64.multiply(1e9).add(nanoInt64);
49-
this._handle = rclnodejs.createDuration(this._nanoseconds.toString());
45+
46+
this._nanoseconds = total;
47+
this._handle = rclnodejs.createDuration(this._nanoseconds);
5048
}
5149

5250
/**
5351
* Get the nanosecond part of the Duration.
5452
* @name Duration#get:nanoseconds
5553
* @function
56-
* @return {number|string} - value in nanosecond, if the value is greater than Number.MAX_SAFE_INTEGER (2^53-1), will be presented in string of decimal format.
54+
* @return {bigint} - value in nanosecond.
5755
*/
5856

5957
get nanoseconds() {
60-
let nanoStr = rclnodejs.getDurationNanoseconds(this._handle);
61-
let nano;
62-
63-
if (nanoStr.startsWith('-')) {
64-
nano = int64.negative(int64.from(nanoStr));
65-
} else {
66-
nano = int64.from(nanoStr);
67-
}
68-
if (Number.isFinite(nano.toNumber())) {
69-
return nano.toNumber();
70-
}
71-
return nano.toString();
58+
return rclnodejs.getDurationNanoseconds(this._handle);
7259
}
7360

7461
/**
@@ -78,7 +65,7 @@ class Duration {
7865
*/
7966
eq(other) {
8067
if (other instanceof Duration) {
81-
return this._nanoseconds.eq(other.nanoseconds);
68+
return this._nanoseconds === other.nanoseconds;
8269
}
8370
throw new TypeError(
8471
`Can't compare duration with object of type: ${other.constructor.name}`
@@ -92,7 +79,7 @@ class Duration {
9279
*/
9380
ne(other) {
9481
if (other instanceof Duration) {
95-
return this._nanoseconds.ne(other.nanoseconds);
82+
return this._nanoseconds !== other.nanoseconds;
9683
}
9784
throw new TypeError('Invalid argument');
9885
}
@@ -104,7 +91,7 @@ class Duration {
10491
*/
10592
lt(other) {
10693
if (other instanceof Duration) {
107-
return this._nanoseconds.lt(other.nanoseconds);
94+
return this._nanoseconds < other.nanoseconds;
10895
}
10996
throw new TypeError('Invalid argument');
11097
}
@@ -116,7 +103,7 @@ class Duration {
116103
*/
117104
lte(other) {
118105
if (other instanceof Duration) {
119-
return this._nanoseconds.lte(other.nanoseconds);
106+
return this._nanoseconds <= other.nanoseconds;
120107
}
121108
throw new TypeError('Invalid argument');
122109
}
@@ -128,7 +115,7 @@ class Duration {
128115
*/
129116
gt(other) {
130117
if (other instanceof Duration) {
131-
return this._nanoseconds.gt(other.nanoseconds);
118+
return this._nanoseconds > other.nanoseconds;
132119
}
133120
throw new TypeError('Invalid argument');
134121
}
@@ -140,7 +127,7 @@ class Duration {
140127
*/
141128
gte(other) {
142129
if (other instanceof Duration) {
143-
return this._nanoseconds.gte(other.nanoseconds);
130+
return this._nanoseconds >= other.nanoseconds;
144131
}
145132
throw new TypeError('Invalid argument');
146133
}

‎lib/node.js‎

Lines changed: 3 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -1469,10 +1469,10 @@ class Node extends rclnodejs.ShadowNode {
14691469
PARAMETER_EVENT_MSG_TYPE
14701470
))();
14711471

1472-
const secondsAndNanos = this._clock.now().secondsAndNanoseconds;
1472+
const { seconds, nanoseconds } = this._clock.now().secondsAndNanoseconds;
14731473
parameterEvent.stamp = {
1474-
sec: secondsAndNanos.seconds,
1475-
nanosec: secondsAndNanos.nanoseconds,
1474+
sec: Number(seconds),
1475+
nanosec: Number(nanoseconds),
14761476
};
14771477

14781478
parameterEvent.node =

‎lib/time.js‎

Lines changed: 39 additions & 50 deletions
Original file line numberDiff line numberDiff line change
@@ -17,7 +17,7 @@
1717
const rclnodejs = require('bindings')('rclnodejs');
1818
const Duration = require('./duration.js');
1919
const ClockType = require('./clock_type.js');
20-
const int64 = require('int64-napi');
20+
const S_TO_NS = 10n ** 9n;
2121

2222
/**
2323
* @class - Class representing a Time in ROS
@@ -26,42 +26,43 @@ const int64 = require('int64-napi');
2626
class Time {
2727
/**
2828
* Create a Time.
29-
* @param {number|string} [seconds=0] - The second part of the time.
30-
* @param {number|string} [nanoseconds=0] - The nanosecond part of the time.
29+
* @param {bigint} [seconds=0] - The second part of the time.
30+
* @param {bigint} [nanoseconds=0] - The nanosecond part of the time.
3131
* @param {ClockType} [clockType=Clock.ClockType.SYSTEM_TIME] - The clock type.
3232
*/
33-
constructor(seconds = 0, nanoseconds = 0, clockType = ClockType.SYSTEM_TIME) {
34-
if (typeof seconds !== 'number' && typeof seconds !== 'string') {
33+
constructor(
34+
seconds = 0n,
35+
nanoseconds = 0n,
36+
clockType = ClockType.SYSTEM_TIME
37+
) {
38+
if (typeof seconds !== 'bigint') {
3539
throw new TypeError('Invalid argument of seconds');
3640
}
3741

38-
if (typeof nanoseconds !== 'number' && typeof nanoseconds !== 'string') {
42+
if (typeof nanoseconds !== 'bigint') {
3943
throw new TypeError('Invalid argument of nanoseconds');
4044
}
4145

4246
if (typeof clockType !== 'number') {
4347
throw new TypeError('Invalid argument of clockType');
4448
}
4549

46-
if (
47-
int64.lt(seconds, 0) ||
48-
(typeof seconds === 'string' && seconds.startsWith('-'))
49-
) {
50+
if (seconds < 0n) {
5051
throw new RangeError('seconds value must not be negative');
5152
}
5253

53-
if (
54-
int64.lt(nanoseconds, 0) ||
55-
(typeof nanoseconds === 'string' && nanoseconds.startsWith('-'))
56-
) {
54+
if (nanoseconds < 0n) {
5755
throw new RangeError('nanoseconds value must not be negative');
5856
}
5957

60-
this._nanoseconds = int64.from(seconds).multiply(1e9).add(nanoseconds);
61-
this._handle = rclnodejs.createTimePoint(
62-
this._nanoseconds.toString(),
63-
clockType
64-
);
58+
const total = seconds * S_TO_NS + nanoseconds;
59+
if (total >= 2n ** 63n) {
60+
throw new RangeError(
61+
'Total nanoseconds value is too large to store in C time point.'
62+
);
63+
}
64+
this._nanoseconds = total;
65+
this._handle = rclnodejs.createTimePoint(this._nanoseconds, clockType);
6566
this._clockType = clockType;
6667
}
6768

@@ -80,22 +81,11 @@ class Time {
8081
* Get the nanosecond part of the time.
8182
* @name Time#get:nanoseconds
8283
* @function
83-
* @return {number|string} - value in nanosecond, if the value is greater than Number.MAX_SAFE_INTEGER (2^53-1), will be presented in string of decimal format.
84+
* @return {bigint} - value in nanosecond.
8485
*/
8586

8687
get nanoseconds() {
87-
let str = rclnodejs.getNanoseconds(this._handle);
88-
let nano;
89-
90-
if (str.startsWith('-')) {
91-
nano = int64.negative(int64.from(str));
92-
} else {
93-
nano = int64.from(str);
94-
}
95-
if (Number.isFinite(nano.toNumber())) {
96-
return nano.toNumber();
97-
}
98-
return nano.toString();
88+
return rclnodejs.getNanoseconds(this._handle);
9989
}
10090

10191
/**
@@ -106,9 +96,11 @@ class Time {
10696
*/
10797

10898
get secondsAndNanoseconds() {
109-
const seconds = int64.from(this._nanoseconds).divide(1e9).toNumber();
110-
const nanoseconds = int64.from(this._nanoseconds).mod(1e9).toNumber();
111-
return { seconds, nanoseconds };
99+
const nanoseconds = this._nanoseconds;
100+
return {
101+
seconds: nanoseconds / S_TO_NS,
102+
nanoseconds: nanoseconds % S_TO_NS,
103+
};
112104
}
113105

114106
/**
@@ -119,8 +111,8 @@ class Time {
119111
add(other) {
120112
if (other instanceof Duration) {
121113
return new Time(
122-
0,
123-
int64.add(this._nanoseconds, other.nanoseconds).toString(),
114+
0n,
115+
this._nanoseconds + other.nanoseconds,
124116
this._clockType
125117
);
126118
}
@@ -137,14 +129,11 @@ class Time {
137129
if (other._clockType !== this._clockType) {
138130
throw new TypeError("Can't subtract times with different clock types");
139131
}
140-
return new Duration(
141-
0,
142-
int64.subtract(this._nanoseconds, other._nanoseconds).toString()
143-
);
132+
return new Duration(0n, this._nanoseconds - other._nanoseconds);
144133
} else if (other instanceof Duration) {
145134
return new Time(
146-
0,
147-
int64.subtract(this._nanoseconds, other._nanoseconds).toString(),
135+
0n,
136+
this._nanoseconds - other._nanoseconds,
148137
this._clockType
149138
);
150139
}
@@ -161,7 +150,7 @@ class Time {
161150
if (other._clockType !== this._clockType) {
162151
throw new TypeError("Can't compare times with different clock types");
163152
}
164-
return this._nanoseconds.eq(other.nanoseconds);
153+
return this._nanoseconds === other.nanoseconds;
165154
}
166155
throw new TypeError('Invalid argument');
167156
}
@@ -176,7 +165,7 @@ class Time {
176165
if (other._clockType !== this._clockType) {
177166
throw new TypeError("Can't compare times with different clock types");
178167
}
179-
return this._nanoseconds.ne(other.nanoseconds);
168+
return this._nanoseconds !== other.nanoseconds;
180169
}
181170
}
182171

@@ -190,7 +179,7 @@ class Time {
190179
if (other._clockType !== this._clockType) {
191180
throw new TypeError("Can't compare times with different clock types");
192181
}
193-
return this._nanoseconds.lt(other.nanoseconds);
182+
return this._nanoseconds < other.nanoseconds;
194183
}
195184
throw new TypeError('Invalid argument');
196185
}
@@ -205,7 +194,7 @@ class Time {
205194
if (other._clockType !== this._clockType) {
206195
throw new TypeError("Can't compare times with different clock types");
207196
}
208-
return this._nanoseconds.lte(other.nanoseconds);
197+
return this._nanoseconds <= other.nanoseconds;
209198
}
210199
throw new TypeError('Invalid argument');
211200
}
@@ -220,7 +209,7 @@ class Time {
220209
if (other._clockType !== this._clockType) {
221210
throw new TypeError("Can't compare times with different clock types");
222211
}
223-
return this._nanoseconds.gt(other.nanoseconds);
212+
return this._nanoseconds > other.nanoseconds;
224213
}
225214
throw new TypeError('Invalid argument');
226215
}
@@ -235,7 +224,7 @@ class Time {
235224
if (other._clockType !== this._clockType) {
236225
throw new TypeError("Can't compare times with different clock types");
237226
}
238-
return this._nanoseconds.gte(other.nanoseconds);
227+
return this._nanoseconds >= other.nanoseconds;
239228
}
240229
throw new TypeError('Invalid argument');
241230
}
@@ -261,7 +250,7 @@ class Time {
261250
* @return {Time} Return the created Time object.
262251
*/
263252
static fromMsg(msg, clockType = ClockType.ROS_TIME) {
264-
return new Time(msg.sec, msg.nanosec, clockType);
253+
return new Time(BigInt(msg.sec), BigInt(msg.nanosec), clockType);
265254
}
266255
}
267256

‎lib/time_source.js‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -37,7 +37,7 @@ class TimeSource {
3737
this._node = node;
3838
this._associatedClocks = [];
3939
this._clockSubscription = undefined;
40-
this._lastTimeSet = new Time(0, 0, ClockType.ROS_TIME);
40+
this._lastTimeSet = new Time(0n, 0n, ClockType.ROS_TIME);
4141
this._isRosTimeActive = false;
4242

4343
if (this._node) {

‎package.json‎

Lines changed: 0 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,7 +63,6 @@
6363
"tree-kill": "^1.2.2",
6464
"typescript": "^5.7.2"
6565
},
66-
"//": "Pin int64-napi to ^1.0.2",
6766
"dependencies": {
6867
"@rclnodejs/ref-array-di": "^1.2.2",
6968
"@rclnodejs/ref-napi": "^4.0.0",
@@ -76,7 +75,6 @@
7675
"dtslint": "^4.2.1",
7776
"fs-extra": "^11.2.0",
7877
"json-bigint": "^1.0.0",
79-
"int64-napi": "^1.0.2",
8078
"is-close": "^1.3.3",
8179
"mkdirp": "^3.0.1",
8280
"mz": "^2.7.0",

0 commit comments

Comments
 (0)