Merge pull request IntelRealSense#1461 from tingshao/ROI

[Node.js] Fix the ROI not working issue
zokin · Apr 8, 2018 · 4e1426e · 4e1426e
2 parents fa97276 + fc13215
commit 4e1426e
Show file tree

Hide file tree

Showing 3 changed files with 51 additions and 10 deletions.
diff --git a/wrappers/nodejs/index.js b/wrappers/nodejs/index.js
@@ -40,8 +40,6 @@ class Device {
     sensors.forEach((s) => {
       if (s.isDepthSensor()) {
         array.push(new DepthSensor(s));
-      } else if (s.isROISensor()) {
-        array.push(new ROISensor(s));
       } else {
         array.push(new Sensor(s));
       }
@@ -675,12 +673,16 @@ class Options {
 class Sensor extends Options {
   /**
    * Construct a Sensor object, representing a RealSense camera subdevice
+   * By default, native resources associated with a Sensor object are freed
+   * automatically during cleanup.
    */
-  constructor(sensor) {
-    super(sensor);
-    this.cxxSensor = sensor;
+  constructor(cxxSensor, autoDelete = true) {
+    super(cxxSensor);
+    this.cxxSensor = cxxSensor;
     this._events = new EventEmitter();
-    internal.addObject(this);
+    if (autoDelete === true) {
+      internal.addObject(this);
+    }
   }
 
   /**
@@ -938,12 +940,27 @@ class Sensor extends Options {
  * Sensor for managing region of interest.
  */
 class ROISensor extends Sensor {
+  /**
+   * Create a ROISensor out of another sensor
+   * @param {Sensor} sensor a sensor object
+   * @return {ROISensor|undefined} return a ROISensor if the sensor can be
+   * treated as a ROISensor, otherwise return undefined.
+   */
+  static from(sensor) {
+    if (sensor.cxxSensor.isROISensor()) {
+      return new ROISensor(sensor.cxxSensor);
+    }
+    return undefined;
+  }
+
   /**
    * Construct a ROISensor object, representing a RealSense camera subdevice
-   *
+   * The newly created ROISensor object shares native resources with the sensor
+   * argument. So the new object shouldn't be freed automatically to make
+   * sure resources released only once during cleanup.
    */
-   constructor(sensor) {
-    super(sensor);
+   constructor(cxxSensor) {
+    super(cxxSensor, false);
   }
 
   /**
@@ -1005,11 +1022,11 @@ class ROISensor extends Sensor {
       minY = arguments[1];
       maxX = arguments[2];
       maxY = arguments[3];
-      this.cxxSensor.setRegionOfInterest(minX, minY, maxX, maxY);
     } else {
       throw new TypeError(
           'setRegionOfInterest(region) expects a RegionOfInterestObject as argument');
     }
+    this.cxxSensor.setRegionOfInterest(minX, minY, maxX, maxY);
   }
 }
 

diff --git a/wrappers/nodejs/src/addon.cpp b/wrappers/nodejs/src/addon.cpp
@@ -2281,6 +2281,7 @@ class RSSensor : public Nan::ObjectWrap, Options {
 
     CallNativeFunc(rs2_get_region_of_interest, &me->error_, me->sensor_, &minx,
         &miny, &maxx, &maxy, &me->error_);
+    if (me->error_) return;
     info.GetReturnValue().Set(
         RSRegionOfInterest(minx, miny, maxx, maxy).GetObject());
   }

diff --git a/wrappers/nodejs/test/test-functional-online.js b/wrappers/nodejs/test/test-functional-online.js
@@ -313,3 +313,26 @@ describe('Native error tests', function() {
     fs.unlinkSync(file);
   });
 });
+
+describe('ROI test', function() {
+  it('set/get ROI test', () => {
+    let ctx = new rs2.Context();
+    let sensors = ctx.querySensors();
+    sensors.forEach((s) => {
+      let roi = rs2.ROISensor.from(s);
+      if (roi) {
+        roi.setRegionOfInterest(1, 1, 10, 10);
+        let val = roi.getRegionOfInterest();
+        if (val) {
+          assert.equal(val.minX, 1);
+          assert.equal(val.minY, 1);
+          assert.equal(val.maxX, 10);
+          assert.equal(val.maxY, 10);
+        } else {
+          assert.equal(rs2.getError() instanceof Object, true);
+        }
+      }
+    });
+    rs2.cleanup();
+  });
+});