bluetooth: Add control for reading/writing of characteristics to internals page.

chrome/browser/resources/bluetooth_internals/value_control.js

Index: chrome/browser/resources/bluetooth_internals/value_control.js
diff --git a/chrome/browser/resources/bluetooth_internals/value_control.js b/chrome/browser/resources/bluetooth_internals/value_control.js
new file mode 100644
index 0000000000000000000000000000000000000000..d2afd701f9d0106587de36a1c03362a862411194
--- /dev/null
+++ b/chrome/browser/resources/bluetooth_internals/value_control.js
@@ -0,0 +1,278 @@
+// Copyright 2017 The Chromium Authors. All rights reserved.
+// Use of this source code is governed by a BSD-style license that can be
+// found in the LICENSE file.
+
+/**
+ * Javascript for ValueControl, served from chrome://bluetooth-internals/.
+ */
+
+cr.define('value_control', function() {
+  /** @const */ var Snackbar = snackbar.Snackbar;
+  /** @const */ var SnackbarType = snackbar.SnackbarType;
+
+  /** @const {!Array<string>} */
+  var VALUE_DATA_TYPES = ['Hexadecimal', 'UTF-8', 'Decimal'];
+
+  /**
+   * A set of inputs that allow a user to request reads and writes of values.
+   * This control allows the value to be displayed in multiple forms
+   * as defined by the |VALUE_DATA_TYPES| array. Values must be written
+   * in these formats. Read and write capability is controlled by a
+   * 'permissions' bitfield provided by the characteristic.
+   * @constructor
+   */
+  var ValueControl = cr.ui.define('div');
+
+  ValueControl.prototype = {
+    __proto__: HTMLDivElement.prototype,
+
+    /**
+     * Decorates the element as a ValueControl. Creates the layout for the value
+     * control by creating a text input, select element, and two buttons for
+     * read/write requests. Event handlers are attached and references to these
+     * elements are stored for later use.
+     * @override
+     */
+    decorate: function() {
+      this.classList.add('value-control');
+
+      /** @private {!Array<number>} */
+      this.value_ = [];
+      /** @private {?string} */
+      this.deviceAddress_ = null;
+      /** @private {?string} */
+      this.serviceId_ = null;
+      /** @private {!interfaces.BluetoothDevice.CharacteristicInfo} */
+      this.characteristicInfo_ = null;
+
+      this.unavailableMessage_ = document.createElement('h3');
+      this.unavailableMessage_.textContent = 'Value cannot be read or written.';
+
+      this.valueInput_ = document.createElement('input');
+      this.valueInput_.addEventListener('change', function() {
+        try {
+          this.value_ = this.convertValueToArray_(this.valueInput_.value);

[dpapad, 2017/01/24 19:52:30]

Why are we converting the input to an array, and then using Array#reduce? Can't
we process it as a string instead? For example to convert to Hex by doing
this.valueInput_.value.toString(16).

[mbrunson, 2017/01/24 23:21:52]

Having the array seemed to be a good middle ground for both communication with
the Bluetooth API and conversions between formats.

The Bluetooth system expects an array of 8-bit integers for a value. I thought
it would be more complicated to check for the previous format and convert the
value accordingly when the user requested a write. If I type a value as hex
(e.g. 0x6162), I don't want the UTF-8 version of that to be "0x6162". I want it
to be the UTF-8 equivalent of that hex string (i.e. "ab"). This is preferred
because when a user requests a read from the Bluetooth system, they may want to
view the same value in different formats.

[dpapad, 2017/01/24 23:54:34]

Trying to understand the tradeoffs here, but still I don't think I am familiar
enough with the rest of the code. Here is a suggestion that might help:

How about encapsulating all the value conversion related stuff to a dedicated
Value class, and decouple it from the rest of this UI code. The API would looks
something as follows

Value#toHex()
Value#toDecimal()
Value#toUtf8()
Value#toArray()

Internally in Value I would store the actual value in a single format (not
Array, probably I would choose UTF8), and then I would convert on-the-fly as
needed, depending on which to*() method was called. toArray() would only be
called when passing data to the Blueotooth API. WDYT?

[mbrunson, 2017/01/25 02:43:36]

Ok. I've added Value at the top of this file.

+        } catch (e) {
+          Snackbar.show(e.message, SnackbarType.ERROR);
+        }
+      }.bind(this));
+
+      this.typeSelect_ = document.createElement('select');
+
+      VALUE_DATA_TYPES.forEach(function(type) {
+        var option = document.createElement('option');
+        option.value = type;
+        option.text = type;
+        this.typeSelect_.add(option);
+      }, this);
+
+      this.typeSelect_.addEventListener('change', this.redraw.bind(this));
+
+      var inputDiv = document.createElement('div');
+      inputDiv.appendChild(this.valueInput_);
+      inputDiv.appendChild(this.typeSelect_);
+
+      this.readBtn_ = document.createElement('button');
+      this.readBtn_.textContent = 'Read';
+      this.readBtn_.addEventListener('click', this.readValue_.bind(this));
+
+      this.writeBtn_ = document.createElement('button');
+      this.writeBtn_.textContent = 'Write';
+      this.writeBtn_.addEventListener('click', this.writeValue_.bind(this));
+
+      var buttonsDiv = document.createElement('div');
+      buttonsDiv.appendChild(this.readBtn_);
+      buttonsDiv.appendChild(this.writeBtn_);
+
+      this.appendChild(this.unavailableMessage_);
+      this.appendChild(inputDiv);
+      this.appendChild(buttonsDiv);
+    },
+
+    /**
+     * Sets the settings used by the value control and redraws the control to
+     * match the read/write settings provided in
+     * |characteristicInfo.permissions|.
+     * @param {string} deviceAddress
+     * @param {string} serviceId
+     * @param {!interfaces.BluetoothDevice.CharacteristicInfo}
+     *     characteristicInfo
+     */
+    load: function(deviceAddress, serviceId, characteristicInfo) {
+      this.deviceAddress_ = deviceAddress;
+      this.serviceId_ = serviceId;
+      this.characteristicInfo_ = characteristicInfo;
+
+      this.redraw();
+    },
+
+    /**
+     * Redraws the value control with updated layout depending on the
+     * availability of reads and writes and the current cached value.
+     */
+    redraw: function() {
+      this.readBtn_.hidden = (this.characteristicInfo_.properties &
+          interfaces.BluetoothDevice.Property.READ) === 0;
+      this.writeBtn_.hidden = (this.characteristicInfo_.properties &
+          interfaces.BluetoothDevice.Property.WRITE) === 0;
+
+      var isAvailable = !this.readBtn_.hidden || !this.writeBtn_.hidden;
+      this.unavailableMessage_.hidden = isAvailable;
+      this.valueInput_.hidden = !isAvailable;
+      this.typeSelect_.hidden = !isAvailable;
+
+      if (!isAvailable) return;
+
+      var type = this.typeSelect_.selectedOptions[0].value;
+
+      switch (type) {
+        case 'Hexadecimal':

[dpapad, 2017/01/24 19:52:30]

We are repeating those string literals multiple times in this file. Can we do
the following instead?

1. Change VALUE_DATA_TYPES to an enum instead of an array.

/** @enum {string} */
var ValueDataTypes = {
  HEXADECIMAL: 'Hexadecimal',
  ...
};

2. Use ValueDataTypes.HEXADECIMAL here and elsewhere (similar for other values).
3. At line 62, use

Object.keys(ValueDataTypes).forEach(function(key) {
  var type = ValueDataTypes[key];
  ...
}, this);

[mbrunson, 2017/01/24 23:21:52]

Done.

+          this.valueInput_.value = this.value_.reduce(
+              function(result, value, index) {
+                var answer = result + ('0' + value.toString(16)).substr(-2);
+                if (index === 0)

[dpapad, 2017/01/24 19:52:30]

Nit:

return index === 0 ? '0x' + answer : answer;

[mbrunson, 2017/01/24 23:21:52]

Done.

+                  return '0x' + answer;
+
+                return answer;
+              }, '');
+          break;
+
+        case 'UTF-8':
+          this.valueInput_.value = this.value_.reduce(function(result, value) {
+            return result + String.fromCharCode(value);
+          }, '');
+          break;
+
+        case 'Decimal':
+          this.valueInput_.value = this.value_.reduce(
+              function(result, value, index) {

[dpapad, 2017/01/24 19:52:30]

Nit:
return result + value.toString() +
    (index === this.value_.length - 1 ? '' : '-');

[mbrunson, 2017/01/24 23:21:52]

Done.

+                if (index === this.value_.length - 1)
+                  return result + value.toString();
+                else
+                  return result + value.toString() + '-';
+              }.bind(this), '');
+          break;
+      }
+    },
+
+    /**
+     * Sets the value of the control.
+     * @param {!Array<number>} value
+     */
+    setValue: function(value) {
+      this.value_ = value;
+      this.redraw();
+    },
+
+    /**
+     * Converts the current value of the text input into an array of numbers
+     * using the currently selected format in the type select element. The
+     * numbers are created such that they will not exceed 8-bits in length to be
+     * compatible with the expected data type of values set in a characteristic.
+     * @param {string} value
+     * @return {!Array<number>}
+     * @private
+     */
+    convertValueToArray_: function(value) {
+      if (!value)
+        return [];
+
+      var type = this.typeSelect_.selectedOptions[0].value;

[dpapad, 2017/01/24 19:52:30]

Isn't this the same as this.typeSelect_.value?

[mbrunson, 2017/01/24 23:21:52]

Done.

+      switch (type) {
+        case 'Hexadecimal':
+          if (!value.startsWith('0x'))
+            throw new Error('Expected value to start with "0x".');
+
+          var result = [];
+          for (var i = 2; i < value.length; i += 2) {
+            result.push(parseInt(value.substr(i, 2), 16));
+          }
+          return result;
+
+        case 'UTF-8':
+          return Array.from(value).map(function(char) {
+            return char.charCodeAt(0);
+          });
+
+        case 'Decimal':
+          if (!/^[0-9\-]*$/.test(value))
+            throw new Error('Value can only contain numbers and hyphens.');
+
+          return value.split('-').map(function(val) { return parseInt(val); });
+      }
+    },
+
+    /**
+     * Called when the read button is pressed. Connects to the device and
+     * retrieves the current value of the characteristic in the |service_id|
+     * with id |characteristic_id|
+     * @private
+     */
+    readValue_: function() {
+      this.readBtn_.disabled = true;
+
+      device_broker.connectToDevice(this.deviceAddress_).then(function(device) {
+        return device.readValueForCharacteristic(
+            this.serviceId_, this.characteristicInfo_.id);
+      }.bind(this)).then(function(response) {
+        this.readBtn_.disabled = false;
+        var GattResult = interfaces.BluetoothDevice.GattResult;
+
+        if (response.result === GattResult.SUCCESS) {
+          this.setValue(response.value);
+          Snackbar.show(
+              this.deviceAddress_ + ': Read succeeded', SnackbarType.SUCCESS);
+          return;
+        }
+
+        // TODO(crbug.com/663394): Replace with more descriptive error
+        // messages.
+        var errorString = Object.keys(GattResult).find(function(key) {
+          return GattResult[key] === response.result;
+        });
+
+        Snackbar.show(
+          this.deviceAddress_ + ': ' + errorString, SnackbarType.ERROR,
+          'Retry', this.readValue_.bind(this));
+      }.bind(this));
+    },
+
+    /**
+     * Called when the write button is pressed. Connects to the device and
+     * retrieves the current value of the characteristic in the |service_id|
+     * with id |characteristic_id|
+     * @private
+     */
+    writeValue_: function() {
+      this.writeBtn_.disabled = true;
+
+      device_broker.connectToDevice(this.deviceAddress_).then(function(device) {
+        return device.writeValueForCharacteristic(
+            this.serviceId_, this.characteristicInfo_.id, this.value_);
+      }.bind(this)).then(function(response) {
+        this.writeBtn_.disabled = false;
+        var GattResult = interfaces.BluetoothDevice.GattResult;
+
+        if (response.result === GattResult.SUCCESS) {
+          Snackbar.show(
+              this.deviceAddress_ + ': Write succeeded', SnackbarType.SUCCESS);
+          return;
+        }
+
+        // TODO(crbug.com/663394): Replace with more descriptive error
+        // messages.
+        var errorString = Object.keys(GattResult).find(function(key) {
+          return GattResult[key] === response.result;
+        });
+
+        Snackbar.show(
+            this.deviceAddress_ + ': ' + errorString, SnackbarType.ERROR,
+            'Retry', this.writeValue_.bind(this));
+      }.bind(this));
+    },
+  }
+
+  return {
+    ValueControl: ValueControl,
+  };
+});