Skip to content

Commit 5ac338a

Browse files
committed
Add timer autostart and TimerInfo callback support (#1472)
- extend `Node.createTimer()` to accept `{ autostart?: boolean }` while preserving existing call signatures - inject `TimerInfo` metadata into JS timer callbacks when native support is available, exposing `expectedCallTime` and `actualCallTime` - wire native timer creation to honor `autostart`, with a compatible fallback for older ROS distros - update timer typings for `TimerInfo`, `TimerOptions`, and the optional callback argument - add runtime and type tests covering `autostart: false`, callback metadata, and option validation - refresh timer API comments to match the new behavior Fix: #1471
1 parent 9960c3d commit 5ac338a

7 files changed

Lines changed: 200 additions & 22 deletions

File tree

lib/node.js

Lines changed: 59 additions & 10 deletions
Original file line numberDiff line numberDiff line change
@@ -256,8 +256,14 @@ class Node extends rclnodejs.ShadowNode {
256256

257257
timersReady.forEach((timer) => {
258258
if (timer.isReady()) {
259-
rclnodejs.callTimer(timer.handle);
260-
timer.callback();
259+
let timerInfo;
260+
if (typeof rclnodejs.callTimerWithInfo === 'function') {
261+
timerInfo = rclnodejs.callTimerWithInfo(timer.handle);
262+
timer.callback(timerInfo);
263+
} else {
264+
rclnodejs.callTimer(timer.handle);
265+
timer.callback();
266+
}
261267
}
262268
});
263269

@@ -628,15 +634,43 @@ class Node extends rclnodejs.ShadowNode {
628634
/**
629635
* Create a Timer.
630636
* @param {bigint} period - The number representing period in nanoseconds.
631-
* @param {function} callback - The callback to be called when timeout.
632-
* @param {Clock} [clock] - The clock which the timer gets time from.
637+
* @param {function} callback - The callback to be called when the timer fires.
638+
* On distros with native support, the callback receives a `TimerInfo` object
639+
* describing the expected and actual call time.
640+
* @param {object|Clock} [optionsOrClock] - Timer options or the clock which the timer gets time from.
641+
* Supported options: `{ autostart?: boolean }`.
642+
* @param {Clock} [clock] - The clock which the timer gets time from when options are provided.
633643
* @return {Timer} - An instance of Timer.
634644
*/
635-
createTimer(period, callback, clock = null) {
636-
if (arguments.length === 3 && !(arguments[2] instanceof Clock)) {
637-
clock = null;
638-
} else if (arguments.length === 4) {
639-
clock = arguments[3];
645+
createTimer(period, callback, optionsOrClock = null, clock = null) {
646+
let options = {};
647+
648+
if (optionsOrClock instanceof Clock.Clock) {
649+
clock = optionsOrClock;
650+
} else if (optionsOrClock === null || optionsOrClock === undefined) {
651+
// Keep the 4th argument as the clock when the 3rd argument is omitted or explicitly null.
652+
} else {
653+
if (typeof optionsOrClock !== 'object' || Array.isArray(optionsOrClock)) {
654+
throw new TypeValidationError(
655+
'options',
656+
optionsOrClock,
657+
'object or Clock',
658+
{
659+
nodeName: this.name(),
660+
}
661+
);
662+
}
663+
options = optionsOrClock;
664+
}
665+
666+
if (
667+
arguments.length === 4 &&
668+
clock !== null &&
669+
!(clock instanceof Clock.Clock)
670+
) {
671+
throw new TypeValidationError('clock', clock, 'Clock', {
672+
nodeName: this.name(),
673+
});
640674
}
641675

642676
if (typeof period !== 'bigint') {
@@ -649,12 +683,27 @@ class Node extends rclnodejs.ShadowNode {
649683
nodeName: this.name(),
650684
});
651685
}
686+
if (
687+
options.autostart !== undefined &&
688+
typeof options.autostart !== 'boolean'
689+
) {
690+
throw new TypeValidationError(
691+
'options.autostart',
692+
options.autostart,
693+
'boolean',
694+
{
695+
nodeName: this.name(),
696+
}
697+
);
698+
}
652699

653700
const timerClock = clock || this._clock;
701+
const autostart = options.autostart ?? true;
654702
let timerHandle = rclnodejs.createTimer(
655703
timerClock.handle,
656704
this.context.handle,
657-
period
705+
period,
706+
autostart
658707
);
659708
let timer = new Timer(timerHandle, period, callback);
660709
debug('Finish creating timer, period = %d.', period);

lib/timer.js

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -150,7 +150,7 @@ class Timer {
150150

151151
/**
152152
* Call a timer and starts counting again, retrieves actual and expected call time.
153-
* @return {object} - The timer information.
153+
* @return {{expectedCallTime: bigint, actualCallTime: bigint}} - The timer information.
154154
*/
155155
callTimerWithInfo() {
156156
if (DistroUtils.getDistroId() <= DistroUtils.getDistroId('humble')) {

src/rcl_timer_bindings.cpp

Lines changed: 21 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,15 +78,23 @@ Napi::Value CreateTimer(const Napi::CallbackInfo& info) {
7878

7979
bool lossless;
8080
int64_t period_nsec = info[2].As<Napi::BigInt>().Int64Value(&lossless);
81+
bool autostart = true;
82+
if (info.Length() > 3) {
83+
if (!info[3].IsBoolean()) {
84+
Napi::TypeError::New(env, "Timer autostart must be a boolean")
85+
.ThrowAsJavaScriptException();
86+
return env.Undefined();
87+
}
88+
autostart = info[3].As<Napi::Boolean>().Value();
89+
}
8190
rcl_timer_t* timer =
8291
reinterpret_cast<rcl_timer_t*>(malloc(sizeof(rcl_timer_t)));
8392
*timer = rcl_get_zero_initialized_timer();
8493

8594
#if ROS_VERSION > 2305 // After Iron.
8695
{
8796
rcl_ret_t ret = rcl_timer_init2(timer, clock, context, period_nsec, nullptr,
88-
rcl_get_default_allocator(),
89-
/*autostart=*/true);
97+
rcl_get_default_allocator(), autostart);
9098
if (RCL_RET_OK != ret) {
9199
std::string error_msg = rcl_get_error_string().str;
92100
rcl_reset_error();
@@ -106,6 +114,17 @@ Napi::Value CreateTimer(const Napi::CallbackInfo& info) {
106114
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
107115
return env.Undefined();
108116
}
117+
if (!autostart) {
118+
rcl_ret_t cancel_ret = rcl_timer_cancel(timer);
119+
if (RCL_RET_OK != cancel_ret) {
120+
std::string error_msg = rcl_get_error_string().str;
121+
rcl_reset_error();
122+
rcl_timer_fini(timer);
123+
free(timer);
124+
Napi::Error::New(env, error_msg).ThrowAsJavaScriptException();
125+
return env.Undefined();
126+
}
127+
}
109128
}
110129
#endif
111130

test/test-timer.js

Lines changed: 87 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616

1717
const assert = require('assert');
1818
const rclnodejs = require('../index.js');
19+
const rclnodejsNative = require('../lib/native_loader.js');
1920
const DistroUtils = require('../lib/distro.js');
2021
const sinon = require('sinon');
2122

@@ -186,6 +187,75 @@ describe('rclnodejs Timer class testing', function () {
186187
done();
187188
});
188189

190+
it('node.createTimer supports autostart false', function (done) {
191+
let called = false;
192+
const timer = node.createTimer(
193+
TIMER_INTERVAL,
194+
() => {
195+
called = true;
196+
timer.cancel();
197+
done();
198+
},
199+
{ autostart: false }
200+
);
201+
202+
rclnodejs.spin(node);
203+
204+
setTimeout(() => {
205+
assert.strictEqual(called, false);
206+
timer.reset();
207+
}, 150);
208+
});
209+
210+
it('timer callback receives TimerInfo when available', function (done) {
211+
if (DistroUtils.getDistroId() <= DistroUtils.getDistroId('humble')) {
212+
this.skip();
213+
return;
214+
}
215+
216+
const timer = node.createTimer(TIMER_INTERVAL, (info) => {
217+
assert.deepStrictEqual(typeof info.expectedCallTime, 'bigint');
218+
assert.deepStrictEqual(typeof info.actualCallTime, 'bigint');
219+
timer.cancel();
220+
done();
221+
});
222+
223+
rclnodejs.spin(node);
224+
});
225+
226+
it('timer callback preserves zero arguments when TimerInfo is unavailable', function (done) {
227+
const originalFunc = rclnodejsNative.callTimerWithInfo;
228+
229+
try {
230+
Object.defineProperty(rclnodejsNative, 'callTimerWithInfo', {
231+
value: undefined,
232+
configurable: true,
233+
writable: true,
234+
});
235+
} catch (e) {
236+
this.skip();
237+
return;
238+
}
239+
240+
const timer = node.createTimer(TIMER_INTERVAL, function () {
241+
try {
242+
assert.strictEqual(arguments.length, 0);
243+
timer.cancel();
244+
done();
245+
} finally {
246+
try {
247+
Object.defineProperty(rclnodejsNative, 'callTimerWithInfo', {
248+
value: originalFunc,
249+
configurable: true,
250+
writable: true,
251+
});
252+
} catch (e) {}
253+
}
254+
});
255+
256+
rclnodejs.spin(node);
257+
});
258+
189259
it('timer.setOnResetCallback', function (done) {
190260
if (DistroUtils.getDistroId() <= DistroUtils.getDistroId('humble')) {
191261
this.skip();
@@ -340,4 +410,21 @@ describe('rclnodejs Timer class coverage testing', function () {
340410
consoleSpy.restore();
341411
}
342412
});
413+
414+
it('node.createTimer validates autostart option type', function () {
415+
assert.throws(() => {
416+
node.createTimer(TIMER_INTERVAL, () => {}, { autostart: 'false' });
417+
}, /options\.autostart/);
418+
});
419+
420+
it('native createTimer validates autostart argument type', function () {
421+
assert.throws(() => {
422+
rclnodejsNative.createTimer(
423+
node.getClock().handle,
424+
node.context.handle,
425+
TIMER_INTERVAL,
426+
'false'
427+
);
428+
}, /Timer autostart must be a boolean/);
429+
});
343430
});

test/types/index.test-d.ts

Lines changed: 11 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -321,11 +321,20 @@ expectType<boolean>(client.isDestroyed());
321321
expectType<string>(client.loggerName);
322322

323323
// ---- Timer ----
324-
const timerCallback = () => {};
324+
const timerCallback: rclnodejs.TimerRequestCallback = (timerInfo) => {
325+
if (timerInfo) {
326+
expectType<bigint>(timerInfo.expectedCallTime);
327+
expectType<bigint>(timerInfo.actualCallTime);
328+
}
329+
};
325330
expectType<rclnodejs.TimerRequestCallback>(timerCallback);
326331

327332
const timer = node.createTimer(BigInt(100000), timerCallback);
333+
const delayedTimer = node.createTimer(BigInt(100000), timerCallback, {
334+
autostart: false,
335+
});
328336
expectType<rclnodejs.Timer>(timer);
337+
expectType<rclnodejs.Timer>(delayedTimer);
329338
expectType<bigint>(timer.period);
330339
expectType<boolean>(timer.isReady());
331340
expectType<bigint>(timer.timeSinceLastCall());
@@ -337,7 +346,7 @@ expectType<void>(timer.changeTimerPeriod(BigInt(100000)));
337346
expectType<bigint>(timer.timerPeriod());
338347
expectType<void>(timer.setOnResetCallback((_events: number) => {}));
339348
expectType<void>(timer.clearOnResetCallback());
340-
expectType<object>(timer.callTimerWithInfo());
349+
expectType<rclnodejs.TimerInfo>(timer.callTimerWithInfo());
341350

342351
// ---- Rate ----
343352
const rate = await node.createRate(1);

types/node.d.ts

Lines changed: 18 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -104,14 +104,24 @@ declare module 'rclnodejs' {
104104
*/
105105
const DEFAULT_OPTIONS: Options;
106106

107+
interface TimerInfo {
108+
expectedCallTime: bigint;
109+
actualCallTime: bigint;
110+
}
111+
112+
interface TimerOptions {
113+
autostart?: boolean;
114+
}
115+
107116
/**
108117
* Callback for receiving periodic interrupts from a Timer.
118+
* Receives timer metadata when the underlying ROS distro exposes it.
109119
*
110120
* @remarks
111121
* See {@link Node.createTimer | Node.createTimer}
112122
* See {@link Timer}
113123
*/
114-
type TimerRequestCallback = () => void;
124+
type TimerRequestCallback = (timerInfo?: TimerInfo) => void;
115125

116126
/**
117127
* Callback indicating parameters are about to be declared or set.
@@ -313,15 +323,18 @@ declare module 'rclnodejs' {
313323
/**
314324
* Create a Timer.
315325
*
316-
* @param period - Elapsed time between interrupt events (milliseconds).
317-
* @param callback - Called on timeout interrupt.
318-
* @param clock - Optional clock to use for the timer.
326+
* @param period - Elapsed time between interrupt events in nanoseconds.
327+
* @param callback - Called when the timer fires. Receives a `TimerInfo` argument when available.
328+
* @param optionsOrClock - Optional timer options or clock to use for the timer.
329+
* Supports `{ autostart?: boolean }` when an options object is provided.
330+
* @param clock - Optional clock to use for the timer when options are provided.
319331
* @returns New instance of Timer.
320332
*/
321333
createTimer(
322334
period: bigint,
323335
callback: TimerRequestCallback,
324-
clock?: Clock
336+
optionsOrClock?: TimerOptions | Clock | null,
337+
clock?: Clock | null
325338
): Timer;
326339

327340
/**

types/timer.d.ts

Lines changed: 3 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -78,8 +78,9 @@ declare module 'rclnodejs' {
7878

7979
/**
8080
* Call a timer and starts counting again, retrieves actual and expected call time.
81-
* @return - The timer information.
81+
*
82+
* @return The timer information with expected and actual call timestamps.
8283
*/
83-
callTimerWithInfo(): object;
84+
callTimerWithInfo(): TimerInfo;
8485
}
8586
}

0 commit comments

Comments
 (0)