device/bluetooth: Update characteristic value D-Bus bindings.

The original BlueZ GATT API doc spec used DBus.Properties interface for
characteristic value access. Since we are modifying this API for Chrome OS,
this CL updates the Chrome D-Bus bindings and modifies the device/bluetooth
GATT code to use the new bindings for characteristic value reads, writes, and
updates.

BUG=378182
TEST=Tested against my custom bluetoothd build; device_unittests

Review URL: https://codereview.chromium.org/301093003

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@275693 0039d316-1c4b-4281-b951-d872f2087c98

10 files changed, 347 insertions, 198 deletions

diff --git a/chromeos/dbus/bluetooth_gatt_characteristic_client.cc b/chromeos/dbus/bluetooth_gatt_characteristic_client.cc
index e2e7c74..a510b91 100644
--- a/chromeos/dbus/bluetooth_gatt_characteristic_client.cc
+++ b/chromeos/dbus/bluetooth_gatt_characteristic_client.cc
@@ -13,6 +13,13 @@
 
 namespace chromeos {
 
+// static
+const char BluetoothGattCharacteristicClient::kNoResponseError[] =
+    "org.chromium.Error.NoResponse";
+// static
+const char BluetoothGattCharacteristicClient::kUnknownCharacteristicError[] =
+    "org.chromium.Error.UnknownCharacteristic";
+
 BluetoothGattCharacteristicClient::Properties::Properties(
     dbus::ObjectProxy* object_proxy,
     const std::string& interface_name,
@@ -20,7 +27,6 @@ BluetoothGattCharacteristicClient::Properties::Properties(
     : dbus::PropertySet(object_proxy, interface_name, callback) {
   RegisterProperty(bluetooth_gatt_characteristic::kUUIDProperty, &uuid);
   RegisterProperty(bluetooth_gatt_characteristic::kServiceProperty, &service);
-  RegisterProperty(bluetooth_gatt_characteristic::kValueProperty, &value);
   RegisterProperty(bluetooth_gatt_characteristic::kFlagsProperty, &flags);
 }
 
@@ -74,6 +80,61 @@ class BluetoothGattCharacteristicClientImpl
                 kBluetoothGattCharacteristicInterface));
   }
 
+  // BluetoothGattCharacteristicClient override.
+  virtual void ReadValue(const dbus::ObjectPath& object_path,
+                         const ValueCallback& callback,
+                         const ErrorCallback& error_callback) OVERRIDE {
+    dbus::ObjectProxy* object_proxy =
+        object_manager_->GetObjectProxy(object_path);
+    if (!object_proxy) {
+      error_callback.Run(kUnknownCharacteristicError, "");
+      return;
+    }
+
+    dbus::MethodCall method_call(
+        bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
+        bluetooth_gatt_characteristic::kReadValue);
+
+    object_proxy->CallMethodWithErrorCallback(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BluetoothGattCharacteristicClientImpl::OnValueSuccess,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   callback),
+        base::Bind(&BluetoothGattCharacteristicClientImpl::OnError,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   error_callback));
+  }
+
+  // BluetoothGattCharacteristicClient override.
+  virtual void WriteValue(const dbus::ObjectPath& object_path,
+                          const std::vector<uint8>& value,
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE {
+    dbus::ObjectProxy* object_proxy =
+        object_manager_->GetObjectProxy(object_path);
+    if (!object_proxy) {
+      error_callback.Run(kUnknownCharacteristicError, "");
+      return;
+    }
+
+    dbus::MethodCall method_call(
+        bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
+        bluetooth_gatt_characteristic::kWriteValue);
+    dbus::MessageWriter writer(&method_call);
+    writer.AppendArrayOfBytes(value.data(), value.size());
+
+    object_proxy->CallMethodWithErrorCallback(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BluetoothGattCharacteristicClientImpl::OnSuccess,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   callback),
+        base::Bind(&BluetoothGattCharacteristicClientImpl::OnError,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   error_callback));
+  }
+
   // dbus::ObjectManager::Interface override.
   virtual dbus::PropertySet* CreateProperties(
       dbus::ObjectProxy *object_proxy,
@@ -94,6 +155,21 @@ class BluetoothGattCharacteristicClientImpl
     VLOG(2) << "Remote GATT characteristic added: " << object_path.value();
     FOR_EACH_OBSERVER(BluetoothGattCharacteristicClient::Observer, observers_,
                       GattCharacteristicAdded(object_path));
+
+    // Connect the "ValueUpdated" signal.
+    dbus::ObjectProxy* object_proxy =
+        object_manager_->GetObjectProxy(object_path);
+    DCHECK(object_proxy);
+
+    object_proxy->ConnectToSignal(
+        bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
+        bluetooth_gatt_characteristic::kValueUpdatedSignal,
+        base::Bind(&BluetoothGattCharacteristicClientImpl::ValueUpdatedReceived,
+                   weak_ptr_factory_.GetWeakPtr(),
+                   object_path),
+        base::Bind(
+            &BluetoothGattCharacteristicClientImpl::ValueUpdatedConnected,
+            weak_ptr_factory_.GetWeakPtr()));
   }
 
   // dbus::ObjectManager::Interface override.
@@ -129,6 +205,78 @@ class BluetoothGattCharacteristicClientImpl
                                                         property_name));
   }
 
+  // Called by dbus:: when a "ValueUpdated" signal is received.
+  void ValueUpdatedReceived(const dbus::ObjectPath& object_path,
+                            dbus::Signal* signal) {
+    DCHECK(signal);
+    const uint8* bytes = NULL;
+    size_t length = 0;
+    dbus::MessageReader reader(signal);
+    if (!reader.PopArrayOfBytes(&bytes, &length)) {
+      LOG(WARNING) << "ValueUpdated signal has incorrect parameters: "
+                   << signal->ToString();
+      return;
+    }
+
+    std::vector<uint8> value;
+    if (bytes)
+      value.assign(bytes, bytes + length);
+
+    FOR_EACH_OBSERVER(BluetoothGattCharacteristicClient::Observer,
+                      observers_,
+                      GattCharacteristicValueUpdated(object_path, value));
+  }
+
+  // Called by dbus:: when the "ValueUpdated" signal is initially connected.
+  void ValueUpdatedConnected(const std::string& interface_name,
+                             const std::string& signal_name,
+                             bool success) {
+    LOG_IF(WARNING, !success) << "Failed to connect to the ValueUpdated signal";
+  }
+
+  // Called when a response for successful method call is received.
+  void OnSuccess(const base::Closure& callback, dbus::Response* response) {
+    DCHECK(response);
+    callback.Run();
+  }
+
+  // Called when a characteristic value response for a successful method call
+  // is received.
+  void OnValueSuccess(const ValueCallback& callback, dbus::Response* response) {
+    DCHECK(response);
+    dbus::MessageReader reader(response);
+
+    const uint8* bytes = NULL;
+    size_t length = 0;
+
+    if (!reader.PopArrayOfBytes(&bytes, &length))
+      VLOG(2) << "Error reading array of bytes in ValueCallback";
+
+    std::vector<uint8> value;
+
+    if (bytes)
+      value.assign(bytes, bytes + length);
+
+    callback.Run(value);
+  }
+
+  // Called when a response for a failed method call is received.
+  void OnError(const ErrorCallback& error_callback,
+               dbus::ErrorResponse* response) {
+    // Error response has optional error message argument.
+    std::string error_name;
+    std::string error_message;
+    if (response) {
+      dbus::MessageReader reader(response);
+      error_name = response->GetErrorName();
+      reader.PopString(&error_message);
+    } else {
+      error_name = kNoResponseError;
+      error_message = "";
+    }
+    error_callback.Run(error_name, error_message);
+  }
+
   dbus::ObjectManager* object_manager_;
 
   // List of observers interested in event notifications from us.
diff --git a/chromeos/dbus/bluetooth_gatt_characteristic_client.h b/chromeos/dbus/bluetooth_gatt_characteristic_client.h
index 2935e4a..e5d48ad 100644
--- a/chromeos/dbus/bluetooth_gatt_characteristic_client.h
+++ b/chromeos/dbus/bluetooth_gatt_characteristic_client.h
@@ -8,6 +8,7 @@
 #include <string>
 
 #include "base/basictypes.h"
+#include "base/callback.h"
 #include "chromeos/chromeos_export.h"
 #include "chromeos/dbus/dbus_client.h"
 #include "dbus/object_path.h"
@@ -28,10 +29,6 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     // [read-only]
     dbus::Property<dbus::ObjectPath> service;
 
-    // Characteristic value read from the remote Bluetooth device. Setting the
-    // value sends a write request to the remote device. [read-write]
-    dbus::Property<std::vector<uint8> > value;
-
     // List of flags representing the GATT "Characteristic Properties bit field"
     // and properties read from the GATT "Characteristic Extended Properties"
     // descriptor bit field. [read-only, optional]
@@ -62,8 +59,20 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     virtual void GattCharacteristicPropertyChanged(
         const dbus::ObjectPath& object_path,
         const std::string& property_name) {}
+
+    // Called when a "ValueUpdated" signal is received from the remote GATT
+    // characteristic with object path |object_path| with characteristic value
+    // |value|.
+    virtual void GattCharacteristicValueUpdated(
+        const dbus::ObjectPath& object_path,
+        const std::vector<uint8>& value) {}
   };
 
+  // Callbacks used to report the result of asynchronous methods.
+  typedef base::Callback<void(const std::string& error_name,
+                              const std::string& error_message)> ErrorCallback;
+  typedef base::Callback<void(const std::vector<uint8>& value)> ValueCallback;
+
   virtual ~BluetoothGattCharacteristicClient();
 
   // Adds and removes observers for events on all remote GATT characteristics.
@@ -79,9 +88,28 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
   // |object_path|. Values should be copied if needed.
   virtual Properties* GetProperties(const dbus::ObjectPath& object_path) = 0;
 
+  // Issues a request to read the value of GATT characteristic with object path
+  // |object_path| and returns the value in |callback| on success. On error,
+  // invokes |error_callback|.
+  virtual void ReadValue(const dbus::ObjectPath& object_path,
+                         const ValueCallback& callback,
+                         const ErrorCallback& error_callback) = 0;
+
+  // Issues a request to write the value of GATT characteristic with object path
+  // |object_path| with value |value|. Invokes |callback| on success and
+  // |error_callback| on failure.
+  virtual void WriteValue(const dbus::ObjectPath& object_path,
+                          const std::vector<uint8>& value,
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) = 0;
+
   // Creates the instance.
   static BluetoothGattCharacteristicClient* Create();
 
+  // Constants used to indicate exceptional error conditions.
+  static const char kNoResponseError[];
+  static const char kUnknownCharacteristicError[];
+
  protected:
   BluetoothGattCharacteristicClient();
 
diff --git a/chromeos/dbus/bluetooth_gatt_service_client.cc b/chromeos/dbus/bluetooth_gatt_service_client.cc
index 1888e51..83ce29a 100644
--- a/chromeos/dbus/bluetooth_gatt_service_client.cc
+++ b/chromeos/dbus/bluetooth_gatt_service_client.cc
@@ -13,15 +13,6 @@
 
 namespace chromeos {
 
-namespace {
-
-// TODO(armansito): Add these to service_constants.h when they are defined
-// in the BlueZ doc.
-const char kDeviceProperty[] = "Device";
-const char kPrimaryProperty[] = "Primary";
-
-}  // namespace
-
 BluetoothGattServiceClient::Properties::Properties(
     dbus::ObjectProxy* object_proxy,
     const std::string& interface_name,
@@ -29,8 +20,8 @@ BluetoothGattServiceClient::Properties::Properties(
     : dbus::PropertySet(object_proxy, interface_name, callback) {
   RegisterProperty(bluetooth_gatt_service::kUUIDProperty, &uuid);
   RegisterProperty(bluetooth_gatt_service::kIncludesProperty, &includes);
-  RegisterProperty(kDeviceProperty, &device);
-  RegisterProperty(kPrimaryProperty, &primary);
+  RegisterProperty(bluetooth_gatt_service::kDeviceProperty, &device);
+  RegisterProperty(bluetooth_gatt_service::kPrimaryProperty, &primary);
 }
 
 BluetoothGattServiceClient::Properties::~Properties() {
diff --git a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
index c822f95..0010ace 100644
--- a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
+++ b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc
@@ -51,9 +51,6 @@ void FakeBluetoothGattCharacteristicClient::Properties::Get(
     dbus::PropertyBase* property,
     dbus::PropertySet::GetCallback callback) {
   VLOG(1) << "Get " << property->name();
-
-  // TODO(armansito): Return success or failure here based on characteristic
-  // read permission.
   callback.Run(true);
 }
 
@@ -65,34 +62,7 @@ void FakeBluetoothGattCharacteristicClient::Properties::Set(
     dbus::PropertyBase* property,
     dbus::PropertySet::SetCallback callback) {
   VLOG(1) << "Set " << property->name();
-  if (property->name() != value.name()) {
-    callback.Run(false);
-    return;
-  }
-
-  // Allow writing to only certain characteristics that are defined with the
-  // write permission.
-  bool write = false;
-  for (std::vector<std::string>::const_iterator iter = flags.value().begin();
-       iter != flags.value().end();
-       ++iter) {
-    if (*iter == bluetooth_gatt_characteristic::kFlagWrite ||
-        *iter == bluetooth_gatt_characteristic::kFlagWriteWithoutResponse ||
-        *iter ==
-            bluetooth_gatt_characteristic::kFlagAuthenticatedSignedWrites ||
-        *iter == bluetooth_gatt_characteristic::kFlagReliableWrite) {
-      write = true;
-      break;
-    }
-  }
-
-  if (!write) {
-    callback.Run(false);
-    return;
-  }
-
-  callback.Run(true);
-  property->ReplaceValueWithSetValue();
+  callback.Run(false);
 }
 
 FakeBluetoothGattCharacteristicClient::FakeBluetoothGattCharacteristicClient()
@@ -145,6 +115,61 @@ FakeBluetoothGattCharacteristicClient::GetProperties(
   return NULL;
 }
 
+void FakeBluetoothGattCharacteristicClient::ReadValue(
+    const dbus::ObjectPath& object_path,
+    const ValueCallback& callback,
+    const ErrorCallback& error_callback) {
+  if (!IsHeartRateVisible()) {
+    error_callback.Run(kUnknownCharacteristicError, "");
+    return;
+  }
+
+  if (object_path.value() == heart_rate_measurement_path_ ||
+      object_path.value() == heart_rate_control_point_path_) {
+    error_callback.Run("org.bluez.Error.ReadNotPermitted",
+                       "Reads of this value are not allowed");
+    return;
+  }
+
+  if (object_path.value() != body_sensor_location_path_) {
+    error_callback.Run(kUnknownCharacteristicError, "");
+    return;
+  }
+
+  std::vector<uint8> value;
+  value.push_back(0x06);  // Location is "foot".
+  callback.Run(value);
+}
+
+void FakeBluetoothGattCharacteristicClient::WriteValue(
+    const dbus::ObjectPath& object_path,
+    const std::vector<uint8>& value,
+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  if (!IsHeartRateVisible()) {
+    error_callback.Run(kUnknownCharacteristicError, "");
+    return;
+  }
+
+  if (object_path.value() != heart_rate_control_point_path_) {
+    error_callback.Run("org.bluez.Error.WriteNotPermitted",
+                       "Writes of this value are not allowed");
+    return;
+  }
+
+  DCHECK(heart_rate_control_point_properties_.get());
+  if (value.size() != 1 || value[0] > 1) {
+    error_callback.Run("org.bluez.Error.Failed",
+                       "Invalid value given for write");
+    return;
+  }
+
+  if (value[0] == 1)
+    calories_burned_ = 0;
+
+  callback.Run();
+}
+
 void FakeBluetoothGattCharacteristicClient::ExposeHeartRateCharacteristics(
     const dbus::ObjectPath& service_path) {
   if (IsHeartRateVisible()) {
@@ -169,9 +194,6 @@ void FakeBluetoothGattCharacteristicClient::ExposeHeartRateCharacteristics(
   flags.push_back(bluetooth_gatt_characteristic::kFlagNotify);
   heart_rate_measurement_properties_->flags.ReplaceValue(flags);
 
-  std::vector<uint8> measurement_value = GetHeartRateMeasurementValue();
-  heart_rate_measurement_properties_->value.ReplaceValue(measurement_value);
-
   // ==== Body Sensor Location Characteristic ====
   body_sensor_location_path_ =
       service_path.value() + "/" + kBodySensorLocationPathComponent;
@@ -185,12 +207,6 @@ void FakeBluetoothGattCharacteristicClient::ExposeHeartRateCharacteristics(
   flags.push_back(bluetooth_gatt_characteristic::kFlagRead);
   body_sensor_location_properties_->flags.ReplaceValue(flags);
 
-  // The sensor is in the "Other" location.
-  std::vector<uint8> body_sensor_location_value;
-  body_sensor_location_value.push_back(0);
-  body_sensor_location_properties_->value.ReplaceValue(
-      body_sensor_location_value);
-
   // ==== Heart Rate Control Point Characteristic ====
   heart_rate_control_point_path_ =
       service_path.value() + "/" + kHeartRateControlPointPathComponent;
@@ -205,13 +221,6 @@ void FakeBluetoothGattCharacteristicClient::ExposeHeartRateCharacteristics(
   flags.push_back(bluetooth_gatt_characteristic::kFlagWrite);
   heart_rate_control_point_properties_->flags.ReplaceValue(flags);
 
-  // Set the initial value to 0. Whenever this gets set to 1, we will reset the
-  // total calories burned and change the value back to 0.
-  std::vector<uint8> heart_rate_control_point_value;
-  heart_rate_control_point_value.push_back(0);
-  heart_rate_control_point_properties_->value.ReplaceValue(
-      heart_rate_control_point_value);
-
   heart_rate_visible_ = true;
 
   NotifyCharacteristicAdded(dbus::ObjectPath(heart_rate_measurement_path_));
@@ -286,25 +295,6 @@ void FakeBluetoothGattCharacteristicClient::OnPropertyChanged(
   FOR_EACH_OBSERVER(BluetoothGattCharacteristicClient::Observer, observers_,
                     GattCharacteristicPropertyChanged(
                         object_path, property_name));
-
-  // If the heart rate control point was set, reset the calories burned.
-  if (object_path.value() != heart_rate_control_point_path_)
-    return;
-  DCHECK(heart_rate_control_point_properties_.get());
-  dbus::Property<std::vector<uint8> >* value_prop =
-      &heart_rate_control_point_properties_->value;
-  if (property_name != value_prop->name())
-    return;
-
-  std::vector<uint8> value = value_prop->value();
-  DCHECK(value.size() == 1);
-  if (value[0] == 0)
-    return;
-
-  DCHECK(value[0] == 1);
-  calories_burned_ = 0;
-  value[0] = 0;
-  value_prop->ReplaceValue(value);
 }
 
 void FakeBluetoothGattCharacteristicClient::NotifyCharacteristicAdded(
@@ -327,7 +317,12 @@ void FakeBluetoothGattCharacteristicClient::
     return;
   VLOG(2) << "Updating heart rate value.";
   std::vector<uint8> measurement = GetHeartRateMeasurementValue();
-  heart_rate_measurement_properties_->value.ReplaceValue(measurement);
+
+  FOR_EACH_OBSERVER(
+      BluetoothGattCharacteristicClient::Observer,
+      observers_,
+      GattCharacteristicValueUpdated(
+          dbus::ObjectPath(heart_rate_measurement_path_), measurement));
 
   base::MessageLoop::current()->PostDelayedTask(
       FROM_HERE,
diff --git a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h
index 1ae77cb..2f23586 100644
--- a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h
+++ b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.h
@@ -49,6 +49,13 @@ class CHROMEOS_EXPORT FakeBluetoothGattCharacteristicClient
   virtual std::vector<dbus::ObjectPath> GetCharacteristics() OVERRIDE;
   virtual Properties* GetProperties(const dbus::ObjectPath& object_path)
       OVERRIDE;
+  virtual void ReadValue(const dbus::ObjectPath& object_path,
+                         const ValueCallback& callback,
+                         const ErrorCallback& error_callback) OVERRIDE;
+  virtual void WriteValue(const dbus::ObjectPath& object_path,
+                          const std::vector<uint8>& value,
+                          const base::Closure& callback,
+                          const ErrorCallback& error_callback) OVERRIDE;
 
   // Makes the group of characteristics belonging to a particular GATT based
   // profile available under the GATT service with object path |service_path|.
diff --git a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
index 9b14809..814b163 100644
--- a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
+++ b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
@@ -806,9 +806,10 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
   EXPECT_EQ(2, error_callback_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_value_changed_count_);
 
-  // Issue write request to writeable characteristic. Writing "1" to the control
-  // point characteristic will immediately change its value back to "0", hence
-  // sending "ValueChanged" events twice.
+  // Issue write request to writeable characteristic. The "Body Sensor Location"
+  // characteristic does not send notifications and WriteValue does not result
+  // in a CharacteristicValueChanged event, thus no such event should be
+  // received.
   characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->
           GetHeartRateControlPointPath().value());
@@ -823,15 +824,14 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
                  base::Unretained(this)),
       base::Bind(&BluetoothGattChromeOSTest::ErrorCallback,
                  base::Unretained(this)));
-  EXPECT_EQ(characteristic->GetIdentifier(),
-            service_observer.last_gatt_characteristic_id_);
-  EXPECT_EQ(characteristic->GetUUID(),
-            service_observer.last_gatt_characteristic_uuid_);
+  EXPECT_TRUE(service_observer.last_gatt_characteristic_id_.empty());
+  EXPECT_FALSE(service_observer.last_gatt_characteristic_uuid_.IsValid());
   EXPECT_EQ(1, success_callback_count_);
   EXPECT_EQ(2, error_callback_count_);
-  EXPECT_EQ(5, service_observer.gatt_characteristic_value_changed_count_);
+  EXPECT_EQ(3, service_observer.gatt_characteristic_value_changed_count_);
 
-  // Issue a read request.
+  // Issue a read request. A successful read results in a
+  // CharacteristicValueChanged notification.
   characteristic = service->GetCharacteristic(
       fake_bluetooth_gatt_characteristic_client_->
           GetBodySensorLocationPath().value());
@@ -847,12 +847,12 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicValue) {
                  base::Unretained(this)));
   EXPECT_EQ(2, success_callback_count_);
   EXPECT_EQ(2, error_callback_count_);
-  EXPECT_EQ(5, service_observer.gatt_characteristic_value_changed_count_);
+  EXPECT_EQ(4, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_TRUE(ValuesEqual(characteristic->GetValue(), last_read_value_));
 
   // One last value changed notification.
   base::MessageLoop::current()->Run();
-  EXPECT_EQ(6, service_observer.gatt_characteristic_value_changed_count_);
+  EXPECT_EQ(5, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_EQ(kHeartRateMeasurementUUID,
             service_observer.last_gatt_characteristic_uuid_);
   EXPECT_EQ(fake_bluetooth_gatt_characteristic_client_->
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
index 3d3e15c..0a8a225 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
@@ -6,7 +6,6 @@
 
 #include "base/logging.h"
 #include "base/strings/stringprintf.h"
-#include "chromeos/dbus/bluetooth_gatt_characteristic_client.h"
 #include "chromeos/dbus/dbus_thread_manager.h"
 #include "device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h"
 #include "device/bluetooth/bluetooth_remote_gatt_service_chromeos.h"
@@ -37,6 +36,8 @@ BluetoothRemoteGattCharacteristicChromeOS::
           weak_ptr_factory_(this) {
   VLOG(1) << "Creating remote GATT characteristic with identifier: "
           << GetIdentifier() << ", UUID: " << GetUUID().canonical_value();
+  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
+      AddObserver(this);
   DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
       AddObserver(this);
 
@@ -53,6 +54,8 @@ BluetoothRemoteGattCharacteristicChromeOS::
     ~BluetoothRemoteGattCharacteristicChromeOS() {
   DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
       RemoveObserver(this);
+  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
+      RemoveObserver(this);
 
   // Clean up all the descriptors. There isn't much point in notifying service
   // observers for each descriptor that gets removed, so just delete them.
@@ -80,11 +83,7 @@ bool BluetoothRemoteGattCharacteristicChromeOS::IsLocal() const {
 
 const std::vector<uint8>&
 BluetoothRemoteGattCharacteristicChromeOS::GetValue() const {
-  BluetoothGattCharacteristicClient::Properties* properties =
-      DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-          GetProperties(object_path_);
-  DCHECK(properties);
-  return properties->value.value();
+  return cached_value_;
 }
 
 device::BluetoothGattService*
@@ -173,14 +172,15 @@ void BluetoothRemoteGattCharacteristicChromeOS::ReadRemoteCharacteristic(
   VLOG(1) << "Sending GATT characteristic read request to characteristic: "
           << GetIdentifier() << ", UUID: " << GetUUID().canonical_value()
           << ".";
-  BluetoothGattCharacteristicClient::Properties* properties =
-      DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-          GetProperties(object_path_);
-  DCHECK(properties);
-  properties->value.Get(
-      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnGetValue,
+
+  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->ReadValue(
+      object_path_,
+      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnValueSuccess,
                  weak_ptr_factory_.GetWeakPtr(),
-                 callback, error_callback));
+                 callback),
+      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnError,
+                 weak_ptr_factory_.GetWeakPtr(),
+                 error_callback));
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::WriteRemoteCharacteristic(
@@ -191,22 +191,27 @@ void BluetoothRemoteGattCharacteristicChromeOS::WriteRemoteCharacteristic(
           << GetIdentifier() << ", UUID: " << GetUUID().canonical_value()
           << ", with value: " << new_value << ".";
 
-  // Permission and bonding are handled by BlueZ so no need check it here.
-  if (new_value.empty()) {
-    VLOG(1) << "Nothing to write.";
-    error_callback.Run();
-    return;
-  }
-
-  BluetoothGattCharacteristicClient::Properties* properties =
-      DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-          GetProperties(object_path_);
-  DCHECK(properties);
-  properties->value.Set(
+  DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->WriteValue(
+      object_path_,
       new_value,
-      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnSetValue,
+      callback,
+      base::Bind(&BluetoothRemoteGattCharacteristicChromeOS::OnError,
                  weak_ptr_factory_.GetWeakPtr(),
-                 callback, error_callback));
+                 error_callback));
+}
+
+void BluetoothRemoteGattCharacteristicChromeOS::GattCharacteristicValueUpdated(
+    const dbus::ObjectPath& object_path,
+    const std::vector<uint8>& value) {
+  if (object_path != object_path_)
+    return;
+
+  cached_value_ = value;
+
+  VLOG(1) << "GATT characteristic value has changed: " << object_path.value()
+          << ": " << value;
+  DCHECK(service_);
+  service_->NotifyCharacteristicValueChanged(this, value);
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
@@ -287,36 +292,25 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorPropertyChanged(
       this, iter->second, properties->value.value());
 }
 
-void BluetoothRemoteGattCharacteristicChromeOS::OnGetValue(
+void BluetoothRemoteGattCharacteristicChromeOS::OnValueSuccess(
     const ValueCallback& callback,
-    const ErrorCallback& error_callback,
-    bool success) {
-  if (!success) {
-    VLOG(1) << "Failed to read the value from the remote characteristic.";
-    error_callback.Run();
-    return;
-  }
+    const std::vector<uint8>& value) {
+  VLOG(1) << "Characteristic value read: " << value;
+  cached_value_ = value;
 
-  VLOG(1) << "Read value of remote characteristic.";
-  BluetoothGattCharacteristicClient::Properties* properties =
-      DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-          GetProperties(object_path_);
-  DCHECK(properties);
-  callback.Run(properties->value.value());
+  DCHECK(service_);
+  service_->NotifyCharacteristicValueChanged(this, cached_value_);
+
+  callback.Run(value);
 }
 
-void BluetoothRemoteGattCharacteristicChromeOS::OnSetValue(
-    const base::Closure& callback,
+void BluetoothRemoteGattCharacteristicChromeOS::OnError(
     const ErrorCallback& error_callback,
-    bool success) {
-  if (!success) {
-    VLOG(1) << "Failed to write the value of remote characteristic.";
-    error_callback.Run();
-    return;
-  }
-
-  VLOG(1) << "Wrote value of remote characteristic.";
-  callback.Run();
+    const std::string& error_name,
+    const std::string& error_message) {
+  VLOG(1) << "Operation failed: " << error_name << ", message: "
+          << error_message;
+  error_callback.Run();
 }
 
 }  // namespace chromeos
diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
index 849655a..91c3f62 100644
--- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
+++ b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
@@ -10,6 +10,7 @@
 #include <vector>
 
 #include "base/memory/weak_ptr.h"
+#include "chromeos/dbus/bluetooth_gatt_characteristic_client.h"
 #include "chromeos/dbus/bluetooth_gatt_descriptor_client.h"
 #include "dbus/object_path.h"
 #include "device/bluetooth/bluetooth_gatt_characteristic.h"
@@ -32,6 +33,7 @@ class BluetoothRemoteGattServiceChromeOS;
 // platform.
 class BluetoothRemoteGattCharacteristicChromeOS
     : public device::BluetoothGattCharacteristic,
+      public BluetoothGattCharacteristicClient::Observer,
       public BluetoothGattDescriptorClient::Observer {
  public:
   // device::BluetoothGattCharacteristic overrides.
@@ -68,6 +70,11 @@ class BluetoothRemoteGattCharacteristicChromeOS
       const dbus::ObjectPath& object_path);
   virtual ~BluetoothRemoteGattCharacteristicChromeOS();
 
+  // BluetoothGattCharacteristicClient::Observer overrides.
+  virtual void GattCharacteristicValueUpdated(
+      const dbus::ObjectPath& object_path,
+      const std::vector<uint8>& value) OVERRIDE;
+
   // BluetoothGattDescriptorClient::Observer overrides.
   virtual void GattDescriptorAdded(
       const dbus::ObjectPath& object_path) OVERRIDE;
@@ -77,17 +84,16 @@ class BluetoothRemoteGattCharacteristicChromeOS
       const dbus::ObjectPath& object_path,
       const std::string& property_name) OVERRIDE;
 
-  // Called by dbus:: on completion of the request to get the characteristic
-  // value.
-  void OnGetValue(const ValueCallback& callback,
-                  const ErrorCallback& error_callback,
-                  bool success);
+  // Called by dbus:: on successful completion of a request to read
+  // the characteristic value.
+  void OnValueSuccess(const ValueCallback& callback,
+                      const std::vector<uint8>& value);
 
-  // Called by dbus:: on completion of the request to set the characteristic
-  // value.
-  void OnSetValue(const base::Closure& callback,
-                  const ErrorCallback& error_callback,
-                  bool success);
+  // Called by dbus:: on unsuccessful completion of a request to read or write
+  // the characteristic value.
+  void OnError(const ErrorCallback& error_callback,
+               const std::string& error_name,
+               const std::string& error_message);
 
   // Object path of the D-Bus characteristic object.
   dbus::ObjectPath object_path_;
@@ -95,6 +101,10 @@ class BluetoothRemoteGattCharacteristicChromeOS
   // The GATT service this GATT characteristic belongs to.
   BluetoothRemoteGattServiceChromeOS* service_;
 
+  // The cached characteristic value based on the most recent read or
+  // notification.
+  std::vector<uint8> cached_value_;
+
   // Mapping from GATT descriptor object paths to descriptor objects owned by
   // this characteristic. Since the Chrome OS implementation uses object paths
   // as unique identifiers, we also use this mapping to return descriptors by
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
index e4b0ae2..81aaa3d 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
@@ -14,20 +14,6 @@
 
 namespace chromeos {
 
-namespace {
-
-// Stream operator for logging vector<uint8>.
-std::ostream& operator<<(std::ostream& out, const std::vector<uint8> bytes) {
-  out << "[";
-  for (std::vector<uint8>::const_iterator iter = bytes.begin();
-       iter != bytes.end(); ++iter) {
-    out << base::StringPrintf("%02X", *iter);
-  }
-  return out << "]";
-}
-
-}  // namespace
-
 BluetoothRemoteGattServiceChromeOS::BluetoothRemoteGattServiceChromeOS(
     BluetoothDeviceChromeOS* device,
     const dbus::ObjectPath& object_path)
@@ -165,6 +151,16 @@ void BluetoothRemoteGattServiceChromeOS::NotifyServiceChanged() {
                     GattServiceChanged(this));
 }
 
+void BluetoothRemoteGattServiceChromeOS::NotifyCharacteristicValueChanged(
+    BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+    const std::vector<uint8>& value) {
+  DCHECK(characteristic->GetService() == this);
+  FOR_EACH_OBSERVER(
+      device::BluetoothGattService::Observer,
+      observers_,
+      GattCharacteristicValueChanged(this, characteristic, value));
+}
+
 void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorAddedOrRemoved(
     BluetoothRemoteGattCharacteristicChromeOS* characteristic,
     BluetoothRemoteGattDescriptorChromeOS* descriptor,
@@ -254,29 +250,4 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicRemoved(
   delete characteristic;
 }
 
-void BluetoothRemoteGattServiceChromeOS::GattCharacteristicPropertyChanged(
-    const dbus::ObjectPath& object_path,
-    const std::string& property_name) {
-  CharacteristicMap::iterator iter = characteristics_.find(object_path);
-  if (iter == characteristics_.end()) {
-    VLOG(2) << "Unknown GATT characteristic property changed: "
-            << object_path.value();
-    return;
-  }
-
-  // Ignore all property changes except for "Value".
-  BluetoothGattCharacteristicClient::Properties* properties =
-      DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient()->
-          GetProperties(object_path);
-  DCHECK(properties);
-  if (property_name != properties->value.name())
-    return;
-
-  VLOG(1) << "GATT characteristic value has changed: " << object_path.value()
-          << ": " << properties->value.value();
-  FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
-                    GattCharacteristicValueChanged(this, iter->second,
-                                                   properties->value.value()));
-}
-
 }  // namespace chromeos
diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
index f86c2ac..db42ecf 100644
--- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
+++ b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
@@ -69,6 +69,14 @@ class BluetoothRemoteGattServiceChromeOS
   // service observers when characteristic descriptors get added and removed.
   void NotifyServiceChanged();
 
+  // Notifies its observers that the value of a characteristic has changed.
+  // Called by BluetoothRemoteGattCharacteristicChromeOS instances to notify
+  // service observers when their cached value is updated after a successful
+  // read request or when a "ValueUpdated" signal is received.
+  void NotifyCharacteristicValueChanged(
+      BluetoothRemoteGattCharacteristicChromeOS* characteristic,
+      const std::vector<uint8>& value);
+
   // Notifies its observers that a descriptor |descriptor| belonging to
   // characteristic |characteristic| has been added or removed. This is used
   // by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
@@ -105,9 +113,6 @@ class BluetoothRemoteGattServiceChromeOS
       const dbus::ObjectPath& object_path) OVERRIDE;
   virtual void GattCharacteristicRemoved(
       const dbus::ObjectPath& object_path) OVERRIDE;
-  virtual void GattCharacteristicPropertyChanged(
-      const dbus::ObjectPath& object_path,
-      const std::string& property_name) OVERRIDE;
 
   // Object path of the GATT service.
   dbus::ObjectPath object_path_;