multiple context support #668

index.js
* Replaced _nodes array with a Map<context,Array<Node>. A list of nodes
is maintained for each context provided to rcl.init(context,argv).
* Moved to end of file the import of node.js and time_source.js, the
TimeSource property and ShadowNode inheritance function call. This
change was made to workaround a circular dependency in rate.js.
* Added context parameter to
`isShutdown(context=Context.defaultContext())`

node.js
* Changed createRate() to return Promise<Rate>
* Removed the context parameter from createTimer()

rate.js
* Reimplemented RateTimerServer to use public rclnodejs methods.
* Added `init(): Promise<void>` method which creates internal private
context and node

test-init-shutdown.js
* Updated to include a multiple-contexts testcase

test-rate.js
* Updated to use node.createRate(): Promise<Rate> correctly.

node.d.ts
* Removed context parameter from createTimer() method declaration.

main.ts
* Updated Rate tests for async node.createRate()

Author: wayneparrott

example/rate-example.js

@@ -35,7 +35,7 @@ async function main() {
     undefined,
     msg => console.log(`Received(${Date.now()}): ${msg.data}`)
   );
-  const rate = node.createRate(0.5);
+  const rate = await node.createRate(0.5);
 
   setInterval(() => publisher.publish(`hello ${Date.now()}`), 10);
 

index.js

@@ -24,7 +24,7 @@ const fs = require('fs');
 const generator = require('./rosidl_gen/index.js');
 const loader = require('./lib/interface_loader.js');
 const logging = require('./lib/logging.js');
-const Node = require('./lib/node.js');
+// const Node = require('./lib/node.js');
 const NodeOptions = require('./lib/node_options.js');
 const {
   FloatingPointRange,
@@ -40,7 +40,6 @@ const rclnodejs = require('bindings')('rclnodejs');
 const tsdGenerator = require('./rostsd_gen/index.js');
 const validator = require('./lib/validator.js');
 const Time = require('./lib/time.js');
-const TimeSource = require('./lib/time_source.js');
 const ActionClient = require('./lib/action/client.js');
 const ActionServer = require('./lib/action/server.js');
 const ClientGoalHandle = require('./lib/action/client_goal_handle.js');
@@ -59,7 +58,7 @@ function inherits(target, source) {
   });
 }
 
-inherits(rclnodejs.ShadowNode, Node);
+// inherits(rclnodejs.ShadowNode, Node);
 
 function getCurrentGeneratorVersion() {
   let jsonFilePath = path.join(generator.generatedRoot, 'generator.json');
@@ -90,8 +89,12 @@ function getCurrentGeneratorVersion() {
  * @exports rclnodejs
  */
 let rcl = {
-  _initialized: false,
-  _nodes: [],
+  
+  // flag identifies when module has been init'ed
+  initialized: false,
+  
+  // Map<Context,Array<Node>
+  _nodes: new Map(),
 
   /** {@link Clock} class */
   Clock: Clock,
@@ -144,9 +147,6 @@ let rcl = {
   /** {@link Time} class */
   Time: Time,
 
-  /** {@link TimeSource} class */
-  TimeSource: TimeSource,
-
   /** {@link module:validator|validator} object */
   validator: validator,
 
@@ -195,19 +195,22 @@ let rcl = {
       throw new TypeError('Invalid argument.');
     }
 
+    if (!this._nodes.has(context)) {
+      throw new Error('Invalid context. Must call rclnodejs(context) before using the context');
+    }
+
     let handle = rclnodejs.createNode(nodeName, namespace, context.handle());
     let node = new rclnodejs.ShadowNode();
     node.handle = handle;
     node.context = context;
-
     node.init(nodeName, namespace, context, options);
     debug(
       'Finish initializing node, name = %s and namespace = %s.',
       nodeName,
       namespace
     );
 
-    this._nodes.push(node);
+    this._nodes.get(context).push(node);
     return node;
   },
 
@@ -218,7 +221,13 @@ let rcl = {
    * @return {Promise<undefined>} A Promise.
    */
   init(context = Context.defaultContext(), argv = process.argv) {
+    
     return new Promise((resolve, reject) => {
+      // check if context has already been initialized
+      if (this._nodes.has(context)) {
+        throw new Error('The module rclnodejs has been initialized.');
+      };
+
       // check argv for correct value and state
       if (!Array.isArray(argv)) {
         throw new TypeError('argv must be an array.');
@@ -227,6 +236,10 @@ let rcl = {
         throw new TypeError('argv elements must not be null');
       }
 
+      // setup internal state for context outside promise. 
+      rclnodejs.init(context.handle(), argv);
+      this._nodes.set(context, []);
+
       let that = this;
       if (!this._initialized) {
         getCurrentGeneratorVersion()
@@ -245,8 +258,6 @@ let rcl = {
             generator
               .generateAll(forced)
               .then(() => {
-                this._context = context;
-                rclnodejs.init(context.handle(), argv);
                 this._initialized = true;
                 resolve();
               })
@@ -258,7 +269,7 @@ let rcl = {
             reject(e);
           });
       } else {
-        throw new Error('The module rclnodejs has been initialized.');
+        resolve();
       }
     });
   },
@@ -276,7 +287,7 @@ let rcl = {
     if (node.spinning) {
       throw new Error('The node is already spinning.');
     }
-    node.startSpinning(this._context.handle(), timeout);
+    node.startSpinning(timeout);
   },
 
   /**
@@ -292,37 +303,47 @@ let rcl = {
     if (node.spinning) {
       throw new Error('The node is already spinning.');
     }
-    node.spinOnce(this._context.handle(), timeout);
+    node.spinOnce(node.context.handle(), timeout);
   },
 
   /**
    * @param {Context} context - The context to be shutdown.
    * @return {undefined}
    */
-  shutdown(context) {
-    if (!this._initialized) {
-      throw new Error('The module rclnodejs has been shut.');
+  shutdown(context = Context.defaultContext()) {
+    if (this.isShutdown(context)) {
+      throw new Error('The module rclnodejs has been shutdown.');
+      return;
     }
 
-    this._nodes.forEach(node => {
+    // check for non-existant or deleted context
+    // if (context === Context.defaultContext()) {
+    //   console.log('default ctx', this._nodes.has(Context.defaultContext()));
+    // }
+    if (!this._nodes.has(context)) {console.log('xxx'); return;}
+
+    // shutdown and remove all nodes assigned to context
+    this._nodes.get(context).forEach(node => {
       node.stopSpinning();
       node.destroy();
     });
-    if (!context) {
+    this._nodes.delete(context);
+    
+    // shutdown context
+    if (context === Context.defaultContext()) {
       Context.shutdownDefaultContext();
     } else {
       context.shutdown();
     }
-    this._nodes = [];
-    this._initialized = false;
-    this._context = undefined;
   },
+  
   /**
-   * Return status that whether the module is shut down.
+   * A predictate for testing if a context has been shutdown.
+   * @param {Context} [context=defaultContext] - The context to inspect
    * @return {boolean} Return true if the module is shut down, otherwise return false.
    */
-  isShutdown() {
-    return !this._initialized;
+  isShutdown(context = Context.defaultContext()) {
+    return !this._nodes.has(context);
   },
 
   /**
@@ -414,3 +435,13 @@ process.on('SIGINT', () => {
 });
 
 module.exports = rcl;
+
+// The following statements are located here to work around a 
+// circular dependency issue occuring in rate.js
+const Node = require('./lib/node.js');
+const TimeSource = require('./lib/time_source.js');
+
+/** {@link TimeSource} class */
+rcl.TimeSource = TimeSource;
+
+inherits(rclnodejs.ShadowNode, Node);

lib/node.js

@@ -330,8 +330,8 @@ class Node {
     });
   }
 
-  startSpinning(context, timeout) {
-    this.start(context, timeout);
+  startSpinning(timeout) {
+    this.start(this.context.handle(), timeout);
     this.spinning = true;
   }
 
@@ -382,16 +382,20 @@ class Node {
    * Create a Timer.
    * @param {number} period - The number representing period in millisecond.
    * @param {function} callback - The callback to be called when timeout.
-   * @param {Context} context - The context, default is Context.defaultContext().
-   * @param {Clock} clock - The clock which the timer gets time from.
+   * @param {Clock} [clock] - The clock which the timer gets time from.
    * @return {Timer} - An instance of Timer.
    */
   createTimer(
     period,
     callback,
-    context = Context.defaultContext(),
     clock = null
   ) {
+    if (arguments.length === 3 && !(arguments[2] instanceof Clock)) {
+      clock = null;
+    } else if (arguments.length === 4) {
+      clock = arguments[3];
+    }
+
     if (typeof period !== 'number' || typeof callback !== 'function') {
       throw new TypeError('Invalid argument');
     }
@@ -413,7 +417,7 @@ class Node {
 
     let timerHandle = rclnodejs.createTimer(
       timerClock.handle,
-      context.handle(),
+      this.context.handle(),
       period
     );
     let timer = new Timer(timerHandle, period, callback);
@@ -428,9 +432,9 @@ class Node {
    * Create a Rate.
    *
    * @param {number} hz - The frequency of the rate timer; default is 1 hz.
-   * @returns {Rate} - New instance
+   * @returns {Promise<Rate>} - Promise resolving to new instance of Rate.
    */
-  createRate(hz = 1) {
+  async createRate(hz = 1) {
     if (typeof hz !== 'number') {
       throw new TypeError('Invalid argument');
     }
@@ -445,6 +449,7 @@ class Node {
     // lazy initialize rateTimerServer
     if (!this._rateTimerServer) {
       this._rateTimerServer = new Rates.RateTimerServer(this);
+      await this._rateTimerServer.init();
     }
 
     const period = Math.round(1000 / hz);

lib/rate.js

@@ -13,7 +13,7 @@
 
 'use strict';
 
-const rclnodejs = require('bindings')('rclnodejs');
+const rclnodejs = require('../index.js');
 const Context = require('./context.js');
 const NodeOptions = require('./node_options.js');
 
@@ -40,7 +40,7 @@ const NOP_FN = () => {};
  * async function run() {
  *   await rclnodejs.init();
  *   const node = rclnodejs.createNode('mynode');
- *   const rate = node.createRate(1); // 1 hz
+ *   const rate = await node.createRate(1); // 1 hz
  *   while (true) {
  *     doSomeStuff();
  *     await rate.sleep();
@@ -131,30 +131,32 @@ class RateTimerServer {
    *  supplies timers to.
    */
   constructor(parentNode) {
+    this._parentNode = parentNode;
     this._context = new Context();
+  }
 
-    // init rcl environment
-    rclnodejs.init(this._context.handle());
+  /**
+   * Setup the server's rcl context and node in preparation for creating
+   * rate timer instances.
+   * 
+   * @returns {undefined}
+   */
+  async init() {
+    await rclnodejs.init(this._context);
 
     // create hidden node
-    const nodeName = `_${parentNode.name()}_rate_timer_server`;
-    const nodeNamespace = parentNode.namespace();
-    this._node = new rclnodejs.ShadowNode();
-    this._node.handle = rclnodejs.createNode(
-      nodeName,
-      nodeNamespace,
-      this._context.handle()
-    );
-
+    const nodeName = `_${this._parentNode.name()}_rate_timer_server`;
+    const nodeNamespace = this._parentNode.namespace();
     const options = new NodeOptions();
     options.startParameterServices = false;
-    options.parameterOverrides = parentNode.getParameters();
+    options.parameterOverrides = this._parentNode.getParameters();
     options.automaticallyDeclareParametersFromOverrides = true;
 
-    this._node.init(nodeName, nodeNamespace, this._context, options);
+    this._node = 
+      rclnodejs.createNode(nodeName, nodeNamespace, this._context, options);
 
     // spin node
-    this._node.startSpinning(this._context.handle(), 10);
+    rclnodejs.spin(this._node, 10);
   }
 
   /**
@@ -164,7 +166,7 @@ class RateTimerServer {
    * @returns {Timer} - The new timer instance.
    */
   createTimer(period) {
-    const timer = this._node.createTimer(period, () => {}, this._context);
+    const timer = this._node.createTimer(period, () => {});
     return timer;
   }
 
@@ -178,8 +180,7 @@ class RateTimerServer {
    * @returns {undefined}
    */
   shutdown() {
-    this._node.destroy();
-    this._context.shutdown();
+    rclnodejs.shutdown(this._context);
   }
 }
 // module.exports = {Rate, RateTimerServer};

test/test-init-shutdown.js

@@ -160,4 +160,39 @@ describe('Node destroy testing', function() {
       rclnodejs.createNode('my_node');
     });
   });
+
+  it('rclnodejs multiple contexts init shutdown sequence', async function() {
+    await rclnodejs.init();
+    assert.ok(!rclnodejs.isShutdown());
+
+    let ctx = new rclnodejs.Context();
+    await rclnodejs.init(ctx);
+    assert.ok(!rclnodejs.isShutdown(ctx));
+    assert.ok(!rclnodejs.isShutdown());
+
+    assert.doesNotThrow(() => rclnodejs.shutdown());
+    assert.ok(rclnodejs.isShutdown());
+    assert.ok(!rclnodejs.isShutdown(ctx));
+
+    assert.doesNotThrow(() => rclnodejs.shutdown(ctx));
+    assert.ok(rclnodejs.isShutdown());
+    assert.ok(rclnodejs.isShutdown(ctx));
+
+    // repeat
+    await rclnodejs.init();
+    assert.ok(!rclnodejs.isShutdown());
+
+    ctx = new rclnodejs.Context();
+    await rclnodejs.init(ctx);
+    assert.ok(!rclnodejs.isShutdown(ctx));
+    assert.ok(!rclnodejs.isShutdown());
+
+    assert.doesNotThrow(() => rclnodejs.shutdown());
+    assert.ok(rclnodejs.isShutdown());
+    assert.ok(!rclnodejs.isShutdown(ctx));
+
+    assert.doesNotThrow(() => rclnodejs.shutdown(ctx));
+    assert.ok(rclnodejs.isShutdown());
+    assert.ok(rclnodejs.isShutdown(ctx));
+  });
 });