Implement the client side of Serial I/O on data pipe.

extensions/test/data/serial_unittest.js

Index: extensions/test/data/serial_unittest.js
diff --git a/extensions/test/data/serial_unittest.js b/extensions/test/data/serial_unittest.js
index dc7bbea56e06cddd8453f3a73358d177fe90f9fb..bd244c3be8c83438483e949aaab3a67ddd60f612 100644
--- a/extensions/test/data/serial_unittest.js
+++ b/extensions/test/data/serial_unittest.js
@@ -13,22 +13,50 @@ var test = require('test').binding;
 var serial = require('serial').binding;
 var unittestBindings = require('test_environment_specific_bindings');
 
+var timeoutManager = new unittestBindings.TimeoutManager();
+timeoutManager.installGlobals();
+
+var BUFFER_SIZE = 10;
+
 var connectionId = null;
 
 function connect(callback, options) {
   options = options || {
     name: 'test connection',
-    bufferSize: 8192,
+    bufferSize: BUFFER_SIZE,
     receiveTimeout: 12345,
     sendTimeout: 6789,
     persistent: true,
-  }
+  };
   serial.connect('device', options, test.callbackPass(function(connectionInfo) {
     connectionId = connectionInfo.connectionId;
-    callback(connectionInfo);
+    if (callback)
+      callback(connectionInfo);
   }));
 }
 
+function addReceiveHook(callback) {

[raymes, 2014/08/29 06:07:16]

Please add a comment for these functions. Saves people from having to dig into
what they actually do.

[Sam McNally, 2014/09/01 06:35:19]

Done.

+  return requireAsync('serial_service').then(function(serialService) {
+    var dataReceived = serialService.Connection.prototype.onDataReceived_;
+    serialService.Connection.prototype.onDataReceived_ = function() {
+      var result = $Function.apply(dataReceived, this, arguments);
+      callback();
+      return result;
+    };
+  });
+}
+
+function addReceiveErrorHook(callback) {
+  return requireAsync('serial_service').then(function(serialService) {
+    var receiveError = serialService.Connection.prototype.onReceiveError_;
+    serialService.Connection.prototype.onReceiveError_ = function() {
+      var result = $Function.apply(receiveError, this, arguments);
+      callback();
+      return result;
+    };
+  });
+}
+
 function disconnect() {
   serial.disconnect(connectionId, test.callbackPass(function(success) {
     test.assertTrue(success);
@@ -41,7 +69,7 @@ function checkClientConnectionInfo(connectionInfo) {
   test.assertEq('test connection', connectionInfo.name);
   test.assertEq(12345, connectionInfo.receiveTimeout);
   test.assertEq(6789, connectionInfo.sendTimeout);
-  test.assertEq(8192, connectionInfo.bufferSize);
+  test.assertEq(BUFFER_SIZE, connectionInfo.bufferSize);
   test.assertFalse(connectionInfo.paused);
 }
 
@@ -58,6 +86,29 @@ function checkConnectionInfo(connectionInfo) {
   checkServiceConnectionInfo(connectionInfo);
 }
 
+function runReceiveErrorTest(expectedError) {
+  connect();
+  test.listenOnce(serial.onReceiveError, function(result) {
+    serial.getInfo(connectionId, test.callbackPass(function(connectionInfo) {
+      disconnect();
+      test.assertTrue(connectionInfo.paused);
+    }));
+    test.assertEq(connectionId, result.connectionId);
+    test.assertEq(expectedError, result.error);
+  });
+}
+
+function runSendErrorTest(expectedError) {
+  connect(function() {
+    var buffer = new ArrayBuffer(1);
+    serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+      disconnect();
+      test.assertEq(0, sendInfo.bytesSent);
+      test.assertEq(expectedError, sendInfo.error);
+    }));
+  });
+}
+
 unittestBindings.exportTests([
   function testGetDevices() {
     serial.getDevices(test.callbackPass(function(devices) {
@@ -277,6 +328,272 @@ unittestBindings.exportTests([
     });
   },
 
+  function testEcho() {

[raymes, 2014/08/29 06:07:16]

This looks like it does more than a simple echo. The nesting of sends makes this
particularly difficult to read. Could you add comments about what the different
cases it tests are?

[Sam McNally, 2014/09/01 06:35:18]

Split this test into one test for each thing being tested here.

+    connect(function() {
+      var data = 'data';
+      var buffer = new ArrayBuffer(data.length);
+      var byteBuffer = new Int8Array(buffer);
+      for (var i = 0; i < data.length; i++) {
+        byteBuffer[i] = data.charCodeAt(i);
+      }
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        test.assertEq(4, sendInfo.bytesSent);
+        test.assertEq(undefined, sendInfo.error);
+      }));
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        test.assertEq(0, sendInfo.bytesSent);
+        test.assertEq('pending', sendInfo.error);
+      }));

[raymes, 2014/08/29 06:07:16]

It would be useful to comment about what the second send is for.

[Sam McNally, 2014/09/01 06:35:18]

Done.

+      test.listenOnce(serial.onReceive, function(result) {
+        serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+          test.listenOnce(serial.onReceive, function(result) {
+            test.assertEq(connectionId, result.connectionId);
+            test.assertEq(4, result.data.byteLength);
+            var resultByteBuffer = new Int8Array(result.data);
+            for (var i = 0; i < byteBuffer.byteLength; i++) {
+              test.assertEq(byteBuffer[i], resultByteBuffer[i]);
+            }
+            disconnect();
+          });
+          test.assertEq(4, sendInfo.bytesSent);
+          test.assertEq(undefined, sendInfo.error);
+        }));
+        test.assertEq(connectionId, result.connectionId);
+        test.assertEq(4, result.data.byteLength);
+        var resultByteBuffer = new Int8Array(result.data);
+        for (var i = 0; i < byteBuffer.byteLength; i++) {
+          test.assertEq(byteBuffer[i], resultByteBuffer[i]);
+        }

[raymes, 2014/08/29 06:07:16]

maybe move this up above the send.

[Sam McNally, 2014/09/01 06:35:19]

Done.

+      });
+    });
+  },
+
+  function testPausedEcho() {

[raymes, 2014/08/29 06:07:16]

Change this to be more like testPausedReceiveError with a custom IO handler

[Sam McNally, 2014/09/01 06:35:19]

Done.

+    var unpaused = false;
+    addReceiveHook(function() {
+      if (unpaused)
+        return;
+      unpaused = true;
+      serial.setPaused(connectionId, false, test.callbackPass());
+      serial.setPaused(connectionId, false, test.callbackPass());

[raymes, 2014/08/29 06:07:16]

Why do we need to call this twice?

[Sam McNally, 2014/09/01 06:35:19]

Done.

+    }).then(test.callbackPass(function() {
+      connect(function() {
+        var data = 'data';
+        var buffer = new ArrayBuffer(data.length);
+        var byteBuffer = new Int8Array(buffer);
+        for (var i = 0; i < data.length; i++) {
+          byteBuffer[i] = data.charCodeAt(i);
+        }
+        serial.setPaused(connectionId, true, test.callbackPass(function() {
+          serial.send(connectionId, buffer, test.callbackPass(
+              function(sendInfo) {
+            test.assertEq(4, sendInfo.bytesSent);
+            test.assertEq(undefined, sendInfo.error);
+          }));
+        }));
+        serial.setPaused(connectionId, true, test.callbackPass());
+        test.listenOnce(serial.onReceive, function(result) {
+          disconnect();
+          test.assertEq(connectionId, result.connectionId);
+          test.assertEq(4, result.data.byteLength);
+          var resultByteBuffer = new Int8Array(result.data);
+          for (var i = 0; i < byteBuffer.byteLength; i++) {
+            test.assertEq(byteBuffer[i], resultByteBuffer[i]);
+          }
+        });
+      });
+    }));
+  },
+
+  function testPausedReceiveError() {
+    var unpaused = false;
+    addReceiveErrorHook(function() {
+      if (unpaused)
+        return;
+      unpaused = true;
+      serial.setPaused(connectionId, false, test.callbackPass());
+      serial.setPaused(connectionId, false, test.callbackPass());

[raymes, 2014/08/29 06:07:16]

If we've already tested this case, no need to test again.

[Sam McNally, 2014/09/01 06:35:19]

Done.

+    }).then(test.callbackPass(function() {
+      connect(function() {
+        serial.setPaused(connectionId, true, test.callbackPass());
+        serial.setPaused(connectionId, true, test.callbackPass());
+      });
+    }));
+
+    test.listenOnce(serial.onReceiveError, function(result) {
+      serial.getInfo(connectionId, test.callbackPass(function(connectionInfo) {
+        disconnect();
+        test.assertTrue(connectionInfo.paused);
+      }));
+      test.assertEq(connectionId, result.connectionId);
+      test.assertEq('device_lost', result.error);
+    });
+    serial.onReceive.addListener(function() {
+      test.fail('unexpected onReceive event');
+    });
+  },
+
+  function testLargeSend() {
+    connect(function() {
+      var buffer = new ArrayBuffer(BUFFER_SIZE * 3);
+      var byteBuffer = new Int8Array(buffer);
+      for (var i = 0; i < buffer.byteLength; i++) {
+        byteBuffer[i] = i;
+      }
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        disconnect();
+        test.assertEq(BUFFER_SIZE * 3, sendInfo.bytesSent);
+        test.assertEq(undefined, sendInfo.error);
+      }));
+    });
+  },

[raymes, 2014/08/29 06:07:16]

Consider removing this test as dicussed since it's not testing anything in
serial.

[Sam McNally, 2014/09/01 06:35:19]

Done.

+
+  function testSendPartialSuccessWithError() {
+    connect(function() {
+      var data = 'data';
+      var buffer = new ArrayBuffer(data.length);
+      var byteBuffer = new Int8Array(buffer);
+      for (var i = 0; i < data.length; i++) {
+        byteBuffer[i] = data.charCodeAt(i);
+      }
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+          test.assertEq(4, sendInfo.bytesSent);
+          test.assertEq(undefined, sendInfo.error);
+          disconnect();
+        }));
+        test.assertEq(2, sendInfo.bytesSent);
+        test.assertEq('system_error', sendInfo.error);

[raymes, 2014/08/29 06:07:16]

Move this above the send just for clarity.

[Sam McNally, 2014/09/01 06:35:19]

Done.

+      }));
+    });
+  },
+
+  function testSendTimeout() {
+    connect(function() {
+      var data = 'data';
+      var buffer = new ArrayBuffer(data.length);
+      var byteBuffer = new Int8Array(buffer);
+      for (var i = 0; i < data.length; i++) {
+        byteBuffer[i] = data.charCodeAt(i);
+      }
+      timeoutManager.clearTimeoutCreated();
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        disconnect();
+        test.assertEq(0, sendInfo.bytesSent);
+        test.assertEq('timeout', sendInfo.error);
+        test.assertEq(6789, timeoutManager.currentTime);
+      }));
+      // Changing the timeout does not effect a send in progress.
+      serial.update(connectionId, {sendTimeout: 10}, test.callbackPass(
+          function() {
+        timeoutManager.timeoutCreated().then(function() {
+          timeoutManager.advanceTime(10);
+          timeoutManager.advanceTime(6779);
+        });
+      }));
+    });
+  },
+
+  function testDisableSendTimeout() {
+    connect(function() {
+      var data = 'data';
+      var buffer = new ArrayBuffer(data.length);
+      var byteBuffer = new Int8Array(buffer);
+      for (var i = 0; i < data.length; i++) {
+        byteBuffer[i] = data.charCodeAt(i);
+      }
+      timeoutManager.clearTimeoutCreated();
+      serial.send(connectionId, buffer, test.callbackPass(function(sendInfo) {
+        disconnect();
+        test.assertEq(0, sendInfo.bytesSent);
+        test.assertEq('timeout', sendInfo.error);
+        test.assertEq(6789, timeoutManager.currentTime);
+      }));
+      // Disabling the timeout does not effect a send in progress.
+      serial.update(connectionId, {sendTimeout: 0}, test.callbackPass(
+          function() {
+        timeoutManager.timeoutCreated().then(function() {
+          timeoutManager.advanceTime(6789);
+        });
+      }));
+    });
+  },
+
+  function testReceiveTimeout() {
+    connect(function() {
+      test.listenOnce(serial.onReceiveError, function(result) {
+        serial.getInfo(connectionId, test.callbackPass(
+            function(connectionInfo) {
+          disconnect();
+          test.assertFalse(connectionInfo.paused);
+        }));
+        test.assertEq(connectionId, result.connectionId);
+        test.assertEq('timeout', result.error);
+        test.assertEq(12345, timeoutManager.currentTime);

[raymes, 2014/08/29 06:07:16]

Maybe move these above the call to the timeout

[Sam McNally, 2014/09/01 06:35:19]

Done.

+      });
+      // Changing the timeout does not take effect until the current timeout
+      // expires or a receive completes.
+      serial.update(connectionId, {receiveTimeout: 10}, test.callbackPass(
+          function() {
+        timeoutManager.timeoutCreated().then(function() {
+          timeoutManager.advanceTime(10);
+          timeoutManager.advanceTime(12335);
+        });
+      }));
+    });
+  },
+
+  function testDisableReceiveTimeout() {
+    connect(function() {
+      test.listenOnce(serial.onReceiveError, function(result) {
+        serial.getInfo(connectionId, test.callbackPass(
+            function(connectionInfo) {
+          disconnect();
+          test.assertFalse(connectionInfo.paused);
+        }));
+        test.assertEq(connectionId, result.connectionId);
+        test.assertEq('timeout', result.error);
+        test.assertEq(12345, timeoutManager.currentTime);
+      });
+      // Disabling the timeout does not take effect until the current timeout
+      // expires or a receive completes.
+      serial.update(connectionId, {receiveTimeout: 0}, test.callbackPass(
+          function() {
+        timeoutManager.timeoutCreated().then(function() {
+          timeoutManager.advanceTime(12345);
+        });
+      }));
+    });
+  },
+
+  function testReceiveErrorDisconnected() {
+    runReceiveErrorTest('disconnected');
+  },
+
+  function testReceiveErrorTimeout() {
+    runReceiveErrorTest('timeout');
+  },
+
+  function testReceiveErrorDeviceLost() {
+    runReceiveErrorTest('device_lost');
+  },
+
+  function testReceiveErrorSystemError() {
+    runReceiveErrorTest('system_error');
+  },
+
+  function testSendErrorDisconnected() {
+    runSendErrorTest('disconnected');
+  },
+
+  function testSendErrorTimeout() {
+    runSendErrorTest('timeout');

[raymes, 2014/08/29 06:07:16]

make a note that this test should pause receiving but wouldn't in production?

[Sam McNally, 2014/09/01 06:35:19]

Done.

+  },
+
+  function testSendErrorSystemError() {
+    runSendErrorTest('system_error');
+  },
+
   function testDisconnectUnknownConnectionId() {
     serial.disconnect(-1, test.callbackFail('Serial connection not found.'));
   },
@@ -309,4 +626,9 @@ unittestBindings.exportTests([
     serial.setPaused(
         -1, false, test.callbackFail('Serial connection not found.'));
   },
+
+  function testSendUnknownConnectionId() {
+    var buffer = new ArrayBuffer(1);
+    serial.send(-1, buffer, test.callbackFail('Serial connection not found.'));
+  },
 ], test.runTests, exports);