Skip to content
Merged
Show file tree
Hide file tree
Changes from 1 commit
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: 1 addition & 1 deletion example/action-client-cancel-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -49,7 +49,7 @@ class FibonacciActionClient {
this._node.getLogger().info('Goal accepted');

// Start a 2 second timer
this._timer = this._node.createTimer(2000, () =>
this._timer = this._node.createTimer(BigInt(2000000), () =>
this.timerCallback(goalHandle)
);
}
Expand Down
4 changes: 3 additions & 1 deletion example/action-server-defer-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -83,7 +83,9 @@ class FibonacciActionServer {
handleAcceptedCallback(goalHandle) {
this._node.getLogger().info('Deferring execution...');
this._goalHandle = goalHandle;
this._timer = this._node.createTimer(3000, () => this.timerCallback());
this._timer = this._node.createTimer(BigInt(3000000), () =>
this.timerCallback()
);
}

cancelCallback(goalHandle) {
Expand Down
2 changes: 1 addition & 1 deletion example/lifecycle-node-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ class App {
onActivate() {
console.log('Lifecycle: ACTIVATE');
this._publisher.activate();
this._timer = this._node.createTimer(1000, () => {
this._timer = this._node.createTimer(BigInt(1000000), () => {
this._publisher.publish(`${this._count--}`);
});
return rclnodejs.lifecycle.CallbackReturnCode.SUCCESS;
Expand Down
2 changes: 1 addition & 1 deletion example/timer-example.js
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,7 @@ rclnodejs
.then(() => {
let node = rclnodejs.createNode('timer_example_node');

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

console.log('Cancel this timer.');
Expand Down
17 changes: 3 additions & 14 deletions lib/node.js
Original file line number Diff line number Diff line change
Expand Up @@ -507,7 +507,7 @@ class Node extends rclnodejs.ShadowNode {

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

Copy link

Copilot AI Feb 20, 2025

Choose a reason for hiding this comment

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

With the new bigint-based period in nanoseconds, the previous range validation (designed for millisecond values) has been removed. Consider adding a new range check that verifies the period is within acceptable nanosecond limits to avoid unintended timer values.

Suggested change
const MIN_PERIOD = 1n; // 1 nanosecond
const MAX_PERIOD = 3600000000000n; // 1 hour in nanoseconds

Copilot uses AI. Check for mistakes.
if (typeof period !== 'number' || typeof callback !== 'function') {
if (typeof period !== 'bigint' || typeof callback !== 'function') {
throw new TypeError('Invalid argument');
}

// The period unit is millisecond in JavaScript side. When being passed to the
// C++ side, the value will be converted to nanosecond, which goes into a uint64_t
// with maxmium value of 2^64-1. So the maxmium is UINT64_MAX in ns, that's 0x10c6f7a0b5ed in ms.
const MAX_TIMER_PERIOD_IN_MILLISECOND = 0x10c6f7a0b5ed;
if (period > 0x10c6f7a0b5ed || period < 0) {
throw new RangeError(
`Parameter must be between 0.0 and ${MAX_TIMER_PERIOD_IN_MILLISECOND}`
);
}

const timerClock = clock || this._clock;

let timerHandle = rclnodejs.createTimer(
timerClock.handle,
this.context.handle,
Expand Down Expand Up @@ -573,7 +562,7 @@ class Node extends rclnodejs.ShadowNode {
}

const period = Math.round(1000 / hz);
const timer = this._rateTimerServer.createTimer(period);
const timer = this._rateTimerServer.createTimer(BigInt(period) * 1000000n);
const rate = new Rates.Rate(hz, timer);

return rate;
Expand Down
2 changes: 1 addition & 1 deletion lib/rate.js
Original file line number Diff line number Diff line change
Expand Up @@ -166,7 +166,7 @@ class RateTimerServer {
/**
* Create a new timer instance with callback set to NOP.
*
* @param {number} period - The period in milliseconds
* @param {bigint} period - The period in nanoseconds.
* @returns {Timer} - The new timer instance.
*/
createTimer(period) {
Expand Down
10 changes: 5 additions & 5 deletions lib/timer.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ class Timer {
}

/**
* @type {number}
* @type {bigint} - The period of the timer in nanoseconds.
*/
get period() {
return this._period;
Expand Down Expand Up @@ -73,18 +73,18 @@ class Timer {

/**
* Get the interval since the last call of this timer.
* @return {number} - the interval value - ms.
* @return {bigint} - the interval value in nanoseconds.
*/
timeSinceLastCall() {
return parseInt(rclnodejs.timerGetTimeSinceLastCall(this._handle), 10);
return rclnodejs.timerGetTimeSinceLastCall(this._handle);
}

/**
* Get the interval until the next call will happen.
* @return {number} - the interval value - ms.
* @return {bigint} - the interval value in nanoseconds.
*/
timeUntilNextCall() {
return parseInt(rclnodejs.timerGetTimeUntilNextCall(this._handle), 10);
return rclnodejs.timerGetTimeUntilNextCall(this._handle);
}
}

Expand Down
31 changes: 17 additions & 14 deletions src/rcl_bindings.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -319,24 +319,27 @@ NAN_METHOD(CreateTimer) {
Nan::To<v8::Object>(info[1]).ToLocalChecked());
rcl_context_t* context =
reinterpret_cast<rcl_context_t*>(context_handle->ptr());
int64_t period_ms = Nan::To<int64_t>(info[2]).FromJust();

if (!info[2]->IsBigInt()) {
Nan::ThrowTypeError("Timer period must be a BigInt");
return;
}
v8::Local<v8::BigInt> bigInt = info[2].As<v8::BigInt>();
int64_t period_nsec = bigInt->Int64Value();
rcl_timer_t* timer =
reinterpret_cast<rcl_timer_t*>(malloc(sizeof(rcl_timer_t)));
*timer = rcl_get_zero_initialized_timer();

#if ROS_VERSION > 2305 // After Iron.
THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK,
rcl_timer_init2(timer, clock, context, RCL_MS_TO_NS(period_ms), nullptr,
rcl_timer_init2(timer, clock, context, period_nsec, nullptr,
rcl_get_default_allocator(), /*autostart=*/true),
rcl_get_error_string().str);
#else
THROW_ERROR_IF_NOT_EQUAL(
RCL_RET_OK,
rcl_timer_init(timer, clock, context, RCL_MS_TO_NS(period_ms), nullptr,
rcl_get_default_allocator()),
rcl_get_error_string().str);
THROW_ERROR_IF_NOT_EQUAL(RCL_RET_OK,
rcl_timer_init(timer, clock, context, period_nsec,
nullptr, rcl_get_default_allocator()),
rcl_get_error_string().str);
#endif

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

info.GetReturnValue().Set(
Nan::New<v8::String>(std::to_string(RCL_NS_TO_MS(remaining_time)))
.ToLocalChecked());
v8::Local<v8::BigInt> bigInt =
v8::BigInt::New(v8::Isolate::GetCurrent(), remaining_time);
info.GetReturnValue().Set(bigInt);
}

NAN_METHOD(TimerGetTimeSinceLastCall) {
Expand All @@ -425,9 +428,9 @@ NAN_METHOD(TimerGetTimeSinceLastCall) {
RCL_RET_OK, rcl_timer_get_time_since_last_call(timer, &elapsed_time),
rcl_get_error_string().str);

info.GetReturnValue().Set(
Nan::New<v8::String>(std::to_string(RCL_NS_TO_MS(elapsed_time)))
.ToLocalChecked());
v8::Local<v8::BigInt> bigInt =
v8::BigInt::New(v8::Isolate::GetCurrent(), elapsed_time);
info.GetReturnValue().Set(bigInt);
}

NAN_METHOD(CreateTimePoint) {
Expand Down
2 changes: 1 addition & 1 deletion test/publisher_array_setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ rclnodejs
effort: [4, 5, 6],
};

var timer = node.createTimer(100, () => {
var timer = node.createTimer(BigInt('100000000'), () => {
publisher.publish(state);
});
rclnodejs.spin(node);
Expand Down
2 changes: 1 addition & 1 deletion test/publisher_msg.js
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ rclnodejs
msg.data = rclValue;

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

Expand Down
2 changes: 1 addition & 1 deletion test/publisher_msg_colorrgba.js
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,7 @@ rclnodejs
};

var publisher = node.createPublisher(ColorRGBA, 'ColorRGBA_channel');
var timer = node.createTimer(100, () => {
var timer = node.createTimer(BigInt('100000000'), () => {
publisher.publish(msg);
});

Expand Down
2 changes: 1 addition & 1 deletion test/publisher_msg_header.js
Original file line number Diff line number Diff line change
Expand Up @@ -33,7 +33,7 @@ rclnodejs
};

var publisher = node.createPublisher(Header, 'Header_channel');
var timer = node.createTimer(100, () => {
var timer = node.createTimer(BigInt('100000000'), () => {
publisher.publish(header);
});

Expand Down
2 changes: 1 addition & 1 deletion test/publisher_msg_jointstate.js
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,7 @@ rclnodejs
};

var publisher = node.createPublisher(JointState, 'JointState_channel');
var timer = node.createTimer(100, () => {
var timer = node.createTimer(BigInt('100000000'), () => {
publisher.publish(state);
});

Expand Down
2 changes: 1 addition & 1 deletion test/publisher_setup.js
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ rclnodejs
const msg = 'Greeting from publisher';

var publisher = node.createPublisher(RclString, 'topic');
var timer = node.createTimer(100, () => {
var timer = node.createTimer(BigInt('100000000'), () => {
publisher.publish(msg);
});
rclnodejs.spin(node);
Expand Down
6 changes: 3 additions & 3 deletions test/test-destruction.js
Original file line number Diff line number Diff line change
Expand Up @@ -61,8 +61,8 @@ describe('Node & Entity destroy testing', function () {
var defaultServicesCount = node._services.length;

// timers
var timer1 = node.createTimer(0.1, () => {});
var timer2 = node.createTimer(1, () => {});
var timer1 = node.createTimer(BigInt(1000), () => {});
var timer2 = node.createTimer(BigInt(100), () => {});
assert.deepStrictEqual(2, node._timers.length);
node.destroyTimer(timer1);
assert.deepStrictEqual(1, node._timers.length);
Expand Down Expand Up @@ -129,7 +129,7 @@ describe('Node & Entity destroy testing', function () {
var node = rclnodejs.createNode('my_node7');

// timers
var timer = node.createTimer(0.1, () => {});
var timer = node.createTimer(BigInt(100), () => {});
node.destroyTimer(timer);
node.destroyTimer(timer);

Expand Down
6 changes: 3 additions & 3 deletions test/test-existance.js
Original file line number Diff line number Diff line change
Expand Up @@ -246,7 +246,7 @@ describe('rclnodejs class existance testing', function () {

before(function () {
node = rclnodejs.createNode('Timer');
timer = node.createTimer(10, () => {});
timer = node.createTimer(BigInt(10000), () => {});
});

after(function () {
Expand All @@ -255,8 +255,8 @@ describe('rclnodejs class existance testing', function () {
});

it('period property should exist', function () {
assertMember('period', timer, timer.period, 'number');
assert.deepStrictEqual(timer.period, 10);
assertMember('period', timer, timer.period, 'bigint');
assert.deepStrictEqual(timer.period, BigInt(10000));
});

it('cancel method should exist', function () {
Expand Down
2 changes: 1 addition & 1 deletion test/test-extra-destroy-methods.js
Original file line number Diff line number Diff line change
Expand Up @@ -110,7 +110,7 @@ describe('Node extra destroy methods testing', function () {

it('destroyTimer()', function () {
var node = rclnodejs.createNode('node5');
var timer = node.createTimer(1000, () => {});
var timer = node.createTimer(BigInt(1000000), () => {});
assert.deepStrictEqual(node._timers.length, 1);

assertThrowsError(
Expand Down
Loading
Loading