Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
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('1000000000'), () => {
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