[unix] Flush now gives errors and flushes tx and rx (#900)

- Bonus less structs as many functions return nothing or error
serialport · Aug 8, 2016 · 3400822 · 3400822
1 parent b340407
commit 3400822
Show file tree

Hide file tree

Showing 7 changed files with 98 additions and 74 deletions.
diff --git a/README.md b/README.md
@@ -56,8 +56,8 @@ For getting started with node-serialport, we recommend you begin with the follow
         * [`.pause()`](#module_serialport--SerialPort+pause)
         * [`.resume()`](#module_serialport--SerialPort+resume)
         * [`.close(callback)`](#module_serialport--SerialPort+close)
-        * [`.flush([callback])`](#module_serialport--SerialPort+flush)
         * [`.set([options], [callback])`](#module_serialport--SerialPort+set)
+        * [`.flush([callback])`](#module_serialport--SerialPort+flush)
         * [`.drain([callback])`](#module_serialport--SerialPort+drain)
         * [`Event: "data"`](#module_serialport--SerialPort+event_data)
         * [`Event: "error"`](#module_serialport--SerialPort+event_error)
@@ -399,20 +399,6 @@ Closes an open connection
 | callback | <code>errorCallback</code> | Called once a connection is closed. |
 
 
--
-
-<a name="module_serialport--SerialPort+flush"></a>
-
-#### `serialPort.flush([callback])`
-Flushes data received but not read. See [`tcflush()`](http://linux.die.net/man/3/tcflush) for Mac/Linux and [`FlushFileBuffers`](http://msdn.microsoft.com/en-us/library/windows/desktop/aa364439) for Windows.
-
-**Kind**: instance method of <code>[SerialPort](#exp_module_serialport--SerialPort)</code>  
-
-| Param | Type | Description |
-| --- | --- | --- |
-| [callback] | <code>[errorCallback](#module_serialport--SerialPort..errorCallback)</code> | Called once the flush operation finishes. |
-
-
 -
 
 <a name="module_serialport--SerialPort+set"></a>
@@ -433,6 +419,20 @@ Sets flags on an open port. Uses [`SetCommMask`](https://msdn.microsoft.com/en-u
 | [callback] | <code>[errorCallback](#module_serialport--SerialPort..errorCallback)</code> |  | Called once the port's flags have been set. |
 
 
+-
+
+<a name="module_serialport--SerialPort+flush"></a>
+
+#### `serialPort.flush([callback])`
+Flush discards data received but not read and written but not transmitted. For more technical details see [`tcflush(fd, TCIFLUSH)`](http://linux.die.net/man/3/tcflush) for Mac/Linux and [`FlushFileBuffers`](http://msdn.microsoft.com/en-us/library/windows/desktop/aa364439) for Windows.
+
+**Kind**: instance method of <code>[SerialPort](#exp_module_serialport--SerialPort)</code>  
+
+| Param | Type | Description |
+| --- | --- | --- |
+| [callback] | <code>[errorCallback](#module_serialport--SerialPort..errorCallback)</code> | Called once the flush operation finishes. |
+
+
 -
 
 <a name="module_serialport--SerialPort+drain"></a>

diff --git a/lib/serialport.js b/lib/serialport.js
@@ -478,25 +478,6 @@ SerialPort.prototype.close = function(callback) {
   }.bind(this));
 };
 
-/**
- * Flushes data received but not read. See [`tcflush()`](http://linux.die.net/man/3/tcflush) for Mac/Linux and [`FlushFileBuffers`](http://msdn.microsoft.com/en-us/library/windows/desktop/aa364439) for Windows.
- * @param  {module:serialport~errorCallback=} callback Called once the flush operation finishes.
- */
-SerialPort.prototype.flush = function(callback) {
-  if (!this.isOpen) {
-    debug('flush attempted, but port is not open');
-    return this._error(new Error('Port is not open'), callback);
-  }
-
-  SerialPortBinding.flush(this.fd, function(err, result) {
-    if (err) {
-      debug('SerialPortBinding.flush had an error', err);
-      return this._error(err, callback);
-    }
-    if (callback) { callback.call(this, null, result) }
-  }.bind(this));
-};
-
 /**
  * Sets flags on an open port. Uses [`SetCommMask`](https://msdn.microsoft.com/en-us/library/windows/desktop/aa363257(v=vs.85).aspx) for windows and [`ioctl`](http://linux.die.net/man/4/tty_ioctl) for mac and linux.
  * @param {object=} options All options are operating system default when the port is opened. Every flag is set on each call to the provided or default values. If options isn't provided default options will be used.
@@ -538,6 +519,25 @@ SerialPort.prototype.set = function(options, callback) {
   }.bind(this));
 };
 
+/**
+ * Flush discards data received but not read and written but not transmitted. For more technical details see [`tcflush(fd, TCIFLUSH)`](http://linux.die.net/man/3/tcflush) for Mac/Linux and [`FlushFileBuffers`](http://msdn.microsoft.com/en-us/library/windows/desktop/aa364439) for Windows.
+ * @param  {module:serialport~errorCallback=} callback Called once the flush operation finishes.
+ */
+SerialPort.prototype.flush = function(callback) {
+  if (!this.isOpen) {
+    debug('flush attempted, but port is not open');
+    return this._error(new Error('Port is not open'), callback);
+  }
+
+  SerialPortBinding.flush(this.fd, function(err, result) {
+    if (err) {
+      debug('SerialPortBinding.flush had an error', err);
+      return this._error(err, callback);
+    }
+    if (callback) { callback.call(this, null, result) }
+  }.bind(this));
+};
+
 /**
  * Waits until all output data has been transmitted to the serial port. See [`tcdrain()`](http://linux.die.net/man/3/tcdrain) or [FlushFileBuffers()](https://msdn.microsoft.com/en-us/library/windows/desktop/aa364439(v=vs.85).aspx) for more information.
  * @param {module:serialport~errorCallback=} callback Called once the drain operation returns.

diff --git a/src/serialport.cpp b/src/serialport.cpp
@@ -371,8 +371,8 @@ NAN_METHOD(Close) {
   }
   v8::Local<v8::Function> callback = info[1].As<v8::Function>();
 
-  CloseBaton* baton = new CloseBaton();
-  memset(baton, 0, sizeof(CloseBaton));
+  VoidBaton* baton = new VoidBaton();
+  memset(baton, 0, sizeof(VoidBaton));
   baton->fd = info[0]->ToInt32()->Int32Value();
   baton->callback = new Nan::Callback(callback);
 
@@ -385,7 +385,7 @@ NAN_METHOD(Close) {
 
 void EIO_AfterClose(uv_work_t* req) {
   Nan::HandleScope scope;
-  CloseBaton* data = static_cast<CloseBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   v8::Local<v8::Value> argv[1];
   if (data->errorString[0]) {
@@ -496,8 +496,8 @@ NAN_METHOD(Flush) {
   }
   v8::Local<v8::Function> callback = info[1].As<v8::Function>();
 
-  FlushBaton* baton = new FlushBaton();
-  memset(baton, 0, sizeof(FlushBaton));
+  VoidBaton* baton = new VoidBaton();
+  memset(baton, 0, sizeof(VoidBaton));
   baton->fd = fd;
   baton->callback = new Nan::Callback(callback);
 
@@ -511,18 +511,17 @@ NAN_METHOD(Flush) {
 void EIO_AfterFlush(uv_work_t* req) {
   Nan::HandleScope scope;
 
-  FlushBaton* data = static_cast<FlushBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
-  v8::Local<v8::Value> argv[2];
+  v8::Local<v8::Value> argv[1];
 
   if (data->errorString[0]) {
     argv[0] = v8::Exception::Error(Nan::New<v8::String>(data->errorString).ToLocalChecked());
-    argv[1] = Nan::Undefined();
   } else {
-    argv[0] = Nan::Undefined();
-    argv[1] = Nan::New<v8::Int32>(data->result);
+    argv[0] = Nan::Null();
   }
-  data->callback->Call(2, argv);
+
+  data->callback->Call(1, argv);
 
   delete data->callback;
   delete data;
@@ -602,8 +601,8 @@ NAN_METHOD(Drain) {
   }
   v8::Local<v8::Function> callback = info[1].As<v8::Function>();
 
-  DrainBaton* baton = new DrainBaton();
-  memset(baton, 0, sizeof(DrainBaton));
+  VoidBaton* baton = new VoidBaton();
+  memset(baton, 0, sizeof(VoidBaton));
   baton->fd = fd;
   baton->callback = new Nan::Callback(callback);
 
@@ -617,7 +616,7 @@ NAN_METHOD(Drain) {
 void EIO_AfterDrain(uv_work_t* req) {
   Nan::HandleScope scope;
 
-  DrainBaton* data = static_cast<DrainBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   v8::Local<v8::Value> argv[1];
 

diff --git a/src/serialport.h b/src/serialport.h
@@ -142,12 +142,6 @@ struct QueuedWrite {
   }
 };
 
-struct CloseBaton {
-  int fd;
-  Nan::Callback* callback;
-  char errorString[ERROR_STRING_SIZE];
-};
-
 struct ListResultItem {
   std::string comName;
   std::string manufacturer;
@@ -164,13 +158,6 @@ struct ListBaton {
   char errorString[ERROR_STRING_SIZE];
 };
 
-struct FlushBaton {
-  int fd;
-  Nan::Callback* callback;
-  int result;
-  char errorString[ERROR_STRING_SIZE];
-};
-
 struct SetBaton {
   int fd;
   Nan::Callback* callback;
@@ -183,10 +170,9 @@ struct SetBaton {
   bool brk;
 };
 
-struct DrainBaton {
+struct VoidBaton {
   int fd;
   Nan::Callback* callback;
-  int result;
   char errorString[ERROR_STRING_SIZE];
 };
 

diff --git a/src/serialport_unix.cpp b/src/serialport_unix.cpp
@@ -375,7 +375,8 @@ void EIO_Write(uv_work_t* req) {
 }
 
 void EIO_Close(uv_work_t* req) {
-  CloseBaton* data = static_cast<CloseBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
+
   if (-1 == close(data->fd)) {
     snprintf(data->errorString, sizeof(data->errorString), "Error: %s, unable to close fd %d", strerror(errno), data->fd);
   }
@@ -671,12 +672,6 @@ void EIO_List(uv_work_t* req) {
 #endif
 }
 
-void EIO_Flush(uv_work_t* req) {
-  FlushBaton* data = static_cast<FlushBaton*>(req->data);
-
-  data->result = tcflush(data->fd, TCIFLUSH);
-}
-
 void EIO_Set(uv_work_t* req) {
   SetBaton* data = static_cast<SetBaton*>(req->data);
 
@@ -719,8 +714,17 @@ void EIO_Set(uv_work_t* req) {
   }
 }
 
+void EIO_Flush(uv_work_t* req) {
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
+
+  if (-1 == tcflush(data->fd, TCIOFLUSH)) {
+    snprintf(data->errorString, sizeof(data->errorString), "Error: %s, cannot flush", strerror(errno));
+    return;
+  }
+}
+
 void EIO_Drain(uv_work_t* req) {
-  DrainBaton* data = static_cast<DrainBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   if (-1 == tcdrain(data->fd)) {
     snprintf(data->errorString, sizeof(data->errorString), "Error: %s, cannot drain", strerror(errno));

diff --git a/src/serialport_win.cpp b/src/serialport_win.cpp
@@ -432,7 +432,7 @@ void EIO_Write(uv_work_t* req) {
 }
 
 void EIO_Close(uv_work_t* req) {
-  CloseBaton* data = static_cast<CloseBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   g_closingHandles.push_back(data->fd);
 
@@ -537,7 +537,7 @@ void EIO_List(uv_work_t* req) {
 }
 
 void EIO_Flush(uv_work_t* req) {
-  FlushBaton* data = static_cast<FlushBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   if (!FlushFileBuffers((HANDLE)data->fd)) {
     ErrorCodeToString("flushing connection (FlushFileBuffers)", GetLastError(), data->errorString);
@@ -546,7 +546,7 @@ void EIO_Flush(uv_work_t* req) {
 }
 
 void EIO_Drain(uv_work_t* req) {
-  DrainBaton* data = static_cast<DrainBaton*>(req->data);
+  VoidBaton* data = static_cast<VoidBaton*>(req->data);
 
   if (!FlushFileBuffers((HANDLE)data->fd)) {
     ErrorCodeToString("draining connection (FlushFileBuffers)", GetLastError(), data->errorString);

diff --git a/test/bindings.js b/test/bindings.js
@@ -306,6 +306,41 @@ describe('SerialPortBinding', function() {
     });
   });
 
+  describe('#flush', function() {
+    it('errors when given a bad fd', function(done) {
+      SerialPortBinding.flush(44, function(err) {
+        assert.instanceOf(err, Error);
+        done();
+      });
+    });
+
+    if (!testPort) {
+      it('Cannot be tested further. Set the TEST_PORT env var with an available serialport for more testing.');
+      return;
+    }
+
+    beforeEach(function(done) {
+      SerialPortBinding.open(testPort, defaultPortOpenOptions, function(err, fd) {
+        assert.isNull(err);
+        assert.isNumber(fd);
+        this.fd = fd;
+        done();
+      }.bind(this));
+    });
+
+    afterEach(function(done) {
+      SerialPortBinding.close(this.fd, done);
+      this.fd = null;
+    });
+
+    it('flushes the port', function(done) {
+      SerialPortBinding.flush(this.fd, function(err) {
+        assert.isNull(err);
+        done();
+      });
+    });
+  });
+
   describe('#set', function() {
     it('errors when given a bad fd', function(done) {
       SerialPortBinding.drain(44, function(err) {