Skip to content

Commit a2209ff

Browse files
authored
Change timer period to type of bigint in nanoseconds (#1038)
Per current implementation, the timer period is in milliseconds, 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 the timer period to type of `bigint` in nanoseconds, which aligns with other ROS2 clients, like `rclpy`. This patch implements: 1. Change the timer period to type of `bigint` in nanoseconds for JavaScript side. 2. Get the period from `v8::BigInt` object as `int64_t` and pass it to `rcl` from C++ side. 3. Update the related unit tests accordingly. Fix: #1037
1 parent f47305a commit a2209ff

27 files changed

+100
-104
lines changed

example/action-client-cancel-example.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -49,7 +49,7 @@ class FibonacciActionClient {
4949
this._node.getLogger().info('Goal accepted');
5050

5151
// Start a 2 second timer
52-
this._timer = this._node.createTimer(2000, () =>
52+
this._timer = this._node.createTimer(BigInt(2000000), () =>
5353
this.timerCallback(goalHandle)
5454
);
5555
}

example/action-server-defer-example.js

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -83,7 +83,9 @@ class FibonacciActionServer {
8383
handleAcceptedCallback(goalHandle) {
8484
this._node.getLogger().info('Deferring execution...');
8585
this._goalHandle = goalHandle;
86-
this._timer = this._node.createTimer(3000, () => this.timerCallback());
86+
this._timer = this._node.createTimer(BigInt(3000000), () =>
87+
this.timerCallback()
88+
);
8789
}
8890

8991
cancelCallback(goalHandle) {

example/lifecycle-node-example.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -88,7 +88,7 @@ class App {
8888
onActivate() {
8989
console.log('Lifecycle: ACTIVATE');
9090
this._publisher.activate();
91-
this._timer = this._node.createTimer(1000, () => {
91+
this._timer = this._node.createTimer(BigInt('1000000000'), () => {
9292
this._publisher.publish(`${this._count--}`);
9393
});
9494
return rclnodejs.lifecycle.CallbackReturnCode.SUCCESS;

example/timer-example.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -21,7 +21,7 @@ rclnodejs
2121
.then(() => {
2222
let node = rclnodejs.createNode('timer_example_node');
2323

24-
let timer = node.createTimer(1000, () => {
24+
let timer = node.createTimer(BigInt(1000000), () => {
2525
console.log('One second escaped!');
2626

2727
console.log('Cancel this timer.');

lib/node.js

Lines changed: 3 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -507,7 +507,7 @@ class Node extends rclnodejs.ShadowNode {
507507

508508
/**
509509
* Create a Timer.
510-
* @param {number} period - The number representing period in millisecond.
510+
* @param {bigint} period - The number representing period in nanoseconds.
511511
* @param {function} callback - The callback to be called when timeout.
512512
* @param {Clock} [clock] - The clock which the timer gets time from.
513513
* @return {Timer} - An instance of Timer.
@@ -519,22 +519,11 @@ class Node extends rclnodejs.ShadowNode {
519519
clock = arguments[3];
520520
}
521521

522-
if (typeof period !== 'number' || typeof callback !== 'function') {
522+
if (typeof period !== 'bigint' || typeof callback !== 'function') {
523523
throw new TypeError('Invalid argument');
524524
}
525525

526-
// The period unit is millisecond in JavaScript side. When being passed to the
527-
// C++ side, the value will be converted to nanosecond, which goes into a uint64_t
528-
// with maxmium value of 2^64-1. So the maxmium is UINT64_MAX in ns, that's 0x10c6f7a0b5ed in ms.
529-
const MAX_TIMER_PERIOD_IN_MILLISECOND = 0x10c6f7a0b5ed;
530-
if (period > 0x10c6f7a0b5ed || period < 0) {
531-
throw new RangeError(
532-
`Parameter must be between 0.0 and ${MAX_TIMER_PERIOD_IN_MILLISECOND}`
533-
);
534-
}
535-
536526
const timerClock = clock || this._clock;
537-
538527
let timerHandle = rclnodejs.createTimer(
539528
timerClock.handle,
540529
this.context.handle,
@@ -573,7 +562,7 @@ class Node extends rclnodejs.ShadowNode {
573562
}
574563

575564
const period = Math.round(1000 / hz);
576-
const timer = this._rateTimerServer.createTimer(period);
565+
const timer = this._rateTimerServer.createTimer(BigInt(period) * 1000000n);
577566
const rate = new Rates.Rate(hz, timer);
578567

579568
return rate;

lib/rate.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -166,7 +166,7 @@ class RateTimerServer {
166166
/**
167167
* Create a new timer instance with callback set to NOP.
168168
*
169-
* @param {number} period - The period in milliseconds
169+
* @param {bigint} period - The period in nanoseconds.
170170
* @returns {Timer} - The new timer instance.
171171
*/
172172
createTimer(period) {

lib/timer.js

Lines changed: 5 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -29,7 +29,7 @@ class Timer {
2929
}
3030

3131
/**
32-
* @type {number}
32+
* @type {bigint} - The period of the timer in nanoseconds.
3333
*/
3434
get period() {
3535
return this._period;
@@ -73,18 +73,18 @@ class Timer {
7373

7474
/**
7575
* Get the interval since the last call of this timer.
76-
* @return {number} - the interval value - ms.
76+
* @return {bigint} - the interval value in nanoseconds.
7777
*/
7878
timeSinceLastCall() {
79-
return parseInt(rclnodejs.timerGetTimeSinceLastCall(this._handle), 10);
79+
return rclnodejs.timerGetTimeSinceLastCall(this._handle);
8080
}
8181

8282
/**
8383
* Get the interval until the next call will happen.
84-
* @return {number} - the interval value - ms.
84+
* @return {bigint} - the interval value in nanoseconds.
8585
*/
8686
timeUntilNextCall() {
87-
return parseInt(rclnodejs.timerGetTimeUntilNextCall(this._handle), 10);
87+
return rclnodejs.timerGetTimeUntilNextCall(this._handle);
8888
}
8989
}
9090

src/rcl_bindings.cpp

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -319,24 +319,27 @@ NAN_METHOD(CreateTimer) {
319319
Nan::To<v8::Object>(info[1]).ToLocalChecked());
320320
rcl_context_t* context =
321321
reinterpret_cast<rcl_context_t*>(context_handle->ptr());
322-
int64_t period_ms = Nan::To<int64_t>(info[2]).FromJust();
323-
322+
if (!info[2]->IsBigInt()) {
323+
Nan::ThrowTypeError("Timer period must be a BigInt");
324+
return;
325+
}
326+
v8::Local<v8::BigInt> bigInt = info[2].As<v8::BigInt>();
327+
int64_t period_nsec = bigInt->Int64Value();
324328
rcl_timer_t* timer =
325329
reinterpret_cast<rcl_timer_t*>(malloc(sizeof(rcl_timer_t)));
326330
*timer = rcl_get_zero_initialized_timer();
327331

328332
#if ROS_VERSION > 2305 // After Iron.
329333
THROW_ERROR_IF_NOT_EQUAL(
330334
RCL_RET_OK,
331-
rcl_timer_init2(timer, clock, context, RCL_MS_TO_NS(period_ms), nullptr,
335+
rcl_timer_init2(timer, clock, context, period_nsec, nullptr,
332336
rcl_get_default_allocator(), /*autostart=*/true),
333337
rcl_get_error_string().str);
334338
#else
335-
THROW_ERROR_IF_NOT_EQUAL(
336-
RCL_RET_OK,
337-
rcl_timer_init(timer, clock, context, RCL_MS_TO_NS(period_ms), nullptr,
338-
rcl_get_default_allocator()),
339-
rcl_get_error_string().str);
339+
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
340+
rcl_timer_init(timer, clock, context, period_nsec,
341+
nullptr, rcl_get_default_allocator()),
342+
rcl_get_error_string().str);
340343
#endif
341344

342345
auto js_obj = RclHandle::NewInstance(timer, clock_handle, [](void* ptr) {
@@ -410,9 +413,9 @@ NAN_METHOD(TimerGetTimeUntilNextCall) {
410413
RCL_RET_OK, rcl_timer_get_time_until_next_call(timer, &remaining_time),
411414
rcl_get_error_string().str);
412415

413-
info.GetReturnValue().Set(
414-
Nan::New<v8::String>(std::to_string(RCL_NS_TO_MS(remaining_time)))
415-
.ToLocalChecked());
416+
v8::Local<v8::BigInt> bigInt =
417+
v8::BigInt::New(v8::Isolate::GetCurrent(), remaining_time);
418+
info.GetReturnValue().Set(bigInt);
416419
}
417420

418421
NAN_METHOD(TimerGetTimeSinceLastCall) {
@@ -425,9 +428,9 @@ NAN_METHOD(TimerGetTimeSinceLastCall) {
425428
RCL_RET_OK, rcl_timer_get_time_since_last_call(timer, &elapsed_time),
426429
rcl_get_error_string().str);
427430

428-
info.GetReturnValue().Set(
429-
Nan::New<v8::String>(std::to_string(RCL_NS_TO_MS(elapsed_time)))
430-
.ToLocalChecked());
431+
v8::Local<v8::BigInt> bigInt =
432+
v8::BigInt::New(v8::Isolate::GetCurrent(), elapsed_time);
433+
info.GetReturnValue().Set(bigInt);
431434
}
432435

433436
NAN_METHOD(CreateTimePoint) {

test/publisher_array_setup.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -39,7 +39,7 @@ rclnodejs
3939
effort: [4, 5, 6],
4040
};
4141

42-
var timer = node.createTimer(100, () => {
42+
var timer = node.createTimer(BigInt('100000000'), () => {
4343
publisher.publish(state);
4444
});
4545
rclnodejs.spin(node);

test/publisher_msg.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -32,7 +32,7 @@ rclnodejs
3232
msg.data = rclValue;
3333

3434
var publisher = node.createPublisher(msgType, rclType + '_channel');
35-
var timer = node.createTimer(100, () => {
35+
var timer = node.createTimer(BigInt('100000000'), () => {
3636
publisher.publish(msg);
3737
});
3838

0 commit comments

Comments
 (0)