Adds Serial API to set data bits, parity, stop bits.

Adds Serial API to set data bits, parity, stop bits.

BUG=261688
TEST=BUILD Success, run serial.open() with options.

Committed: https://src.chromium.org/viewvc/chrome?view=rev&revision=219782

[limasdf, 2013-08-18 15:28:17]

.

[miket_OOO, 2013-08-21 19:43:44]

LGTM with a few comments. Thanks!

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/api/serial/serial_api.cc (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/api/serial/serial_api.cc:111:
EXTENSION_FUNCTION_VALIDATE(options->GetString(kDataBitKey, &data));
What exactly does E_F_V do here? You've checked that the key exists, and you've
gotten it as a string, and later you check whether the string is empty. Does
E_F_V add any further value here?

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/api/serial/serial_api.cc:113: databit_ =
serial::ParseDataBit(data);
no braces needed around these one-line blocks.

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/api/serial/serial_connection_posix.cc (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/api/serial/serial_connection_posix.cc:124:
options.c_cflag &= ~(PARENB | PARODD);
rather than listing the no-parity code twice, you can do this:

 case serial::PARITY_BIT_NOPARITY:
 default:
   options.c_cflag &= ~(PARENB | PARODD);

This avoids duplication of code and makes it easier on the reader to see which
case is picked as the default case.

https://codereview.chromium.org/22804008/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/api/serial.idl (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/api/serial.idl:22: long? bitrate;
It's OK if we make bitrate optional, but we should document here that the
default is 9600. Otherwise people will have to guess it.

[miket_OOO, 2013-08-21 19:45:17]

One more thing: please edit the CL description to be consistent about the order
of the three new fields. 8N1 8N2 etc. are the usual way people express these
settings, so instead of "data bits, stop bits, parity" it should always be "data
bits, parity, stop bits," even in the changelist description.

[limasdf, 2013-08-24 15:46:24]

Done.

and the document url is
http://chrome-apps-doc.appspot.com/_patch/22804008/apps/serial.html

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/api/serial/serial_api.cc (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/api/serial/serial_api.cc:111:
EXTENSION_FUNCTION_VALIDATE(options->GetString(kDataBitKey, &data));
On 2013/08/21 19:43:44, miket wrote:
> What exactly does E_F_V do here? You've checked that the key exists, and
you've
> gotten it as a string, and later you check whether the string is empty. Does
> E_F_V add any further value here?

It seems unnecessary. Removed.

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
File chrome/browser/extensions/api/serial/serial_connection_posix.cc (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/browser/extensions/...
chrome/browser/extensions/api/serial/serial_connection_posix.cc:124:
options.c_cflag &= ~(PARENB | PARODD);
On 2013/08/21 19:43:44, miket wrote:
> rather than listing the no-parity code twice, you can do this:
> 
>  case serial::PARITY_BIT_NOPARITY:
>  default:
>    options.c_cflag &= ~(PARENB | PARODD);
> 
> This avoids duplication of code and makes it easier on the reader to see which
> case is picked as the default case.

Done.

https://codereview.chromium.org/22804008/diff/8001/chrome/common/extensions/a...
File chrome/common/extensions/api/serial.idl (right):

https://codereview.chromium.org/22804008/diff/8001/chrome/common/extensions/a...
chrome/common/extensions/api/serial.idl:22: long? bitrate;
On 2013/08/21 19:43:44, miket wrote:
> It's OK if we make bitrate optional, but we should document here that the
> default is 9600. Otherwise people will have to guess it.

Done.

[commit-bot: I haz the power, 2013-08-25 15:23:15]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/limasdf@gmail.com/22804008/25001

[commit-bot: I haz the power, 2013-08-25 16:00:34]

Sorry for I got bad news for ya.
Compile failed with a clobber build on linux_chromeos.
http://build.chromium.org/p/tryserver.chromium/buildstatus?builder=linux_chro...
Your code is likely broken or HEAD is junk. Please ensure your
code is not broken then alert the build sheriffs.
Look at the try server FAQ for more details.

[commit-bot: I haz the power, 2013-08-27 06:40:50]

CQ is trying da patch. Follow status at
https://chromium-status.appspot.com/cq/limasdf@gmail.com/22804008/53001

[commit-bot: I haz the power, 2013-08-27 13:42:43]

Change committed as 219782