Add GetConnectionInfo function for BluetoothDevice, replacing the existing

chromeos/dbus/bluetooth_device_client.cc

Index: chromeos/dbus/bluetooth_device_client.cc
diff --git a/chromeos/dbus/bluetooth_device_client.cc b/chromeos/dbus/bluetooth_device_client.cc
index 777b1aa1cfb9307f36cbd9ec4240bdc997a1af80..4128c6f37677b3d67dabac7afc7708d961be42e7 100644
--- a/chromeos/dbus/bluetooth_device_client.cc
+++ b/chromeos/dbus/bluetooth_device_client.cc
@@ -16,6 +16,13 @@
 
 namespace chromeos {
 
+namespace {
+
+// Value returned for the the RSSI or TX power if it cannot be read.
+const int kUnknownPower = 127;

[Ilya Sherman, 2014/11/19 22:18:07]

nit: It seems a shame to define this in two separate places -- any chance of
sharing the code somehow?

[Tim Song, 2014/12/06 00:51:37]

Sharing this constant would be pretty hard to do. chromeos/ can't depend on
device/ and device/ can't depend only on chromeos/ for this constant because it
also needs to use it for other platforms. It's not worth making a separate
shared target.

+
+}  // namespace
+
 const char BluetoothDeviceClient::kNoResponseError[] =
     "org.chromium.Error.NoResponse";
 const char BluetoothDeviceClient::kUnknownDeviceError[] =
@@ -41,10 +48,6 @@ BluetoothDeviceClient::Properties::Properties(
   RegisterProperty(bluetooth_device::kLegacyPairingProperty, &legacy_pairing);
   RegisterProperty(bluetooth_device::kModaliasProperty, &modalias);
   RegisterProperty(bluetooth_device::kRSSIProperty, &rssi);
-  RegisterProperty(bluetooth_device::kConnectionRSSI, &connection_rssi);
-  RegisterProperty(bluetooth_device::kConnectionTXPower, &connection_tx_power);
-  RegisterProperty(bluetooth_device::kConnectionTXPowerMax,
-                   &connection_tx_power_max);
 }
 
 BluetoothDeviceClient::Properties::~Properties() {
@@ -273,37 +276,12 @@ class BluetoothDeviceClientImpl
   }
 
   // BluetoothDeviceClient override.
-  virtual void StartConnectionMonitor(
-      const dbus::ObjectPath& object_path,
-      const base::Closure& callback,
-      const ErrorCallback& error_callback) override {
-    dbus::MethodCall method_call(bluetooth_device::kBluetoothDeviceInterface,
-                                 bluetooth_device::kStartConnectionMonitor);
-
-    dbus::ObjectProxy* object_proxy =
-        object_manager_->GetObjectProxy(object_path);
-    if (!object_proxy) {
-      error_callback.Run(kUnknownDeviceError, "");
-      return;
-    }
-    object_proxy->CallMethodWithErrorCallback(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&BluetoothDeviceClientImpl::OnSuccess,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   callback),
-        base::Bind(&BluetoothDeviceClientImpl::OnError,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   error_callback));
-  }
-
-  // BluetoothDeviceClient override.
-  virtual void StopConnectionMonitor(
-      const dbus::ObjectPath& object_path,
-      const base::Closure& callback,
-      const ErrorCallback& error_callback) override {
-    dbus::MethodCall method_call(bluetooth_device::kBluetoothDeviceInterface,
-                                 bluetooth_device::kStopConnectionMonitor);
+  void GetConnInfo(const dbus::ObjectPath& object_path,
+                   const ConnInfoCallback& callback,
+                   const ErrorCallback& error_callback) override {
+    dbus::MethodCall method_call(
+        bluetooth_plugin_device::kBluetoothPluginInterface,
+        bluetooth_plugin_device::kGetConnInfo);
 
     dbus::ObjectProxy* object_proxy =
         object_manager_->GetObjectProxy(object_path);
@@ -312,14 +290,11 @@ class BluetoothDeviceClientImpl
       return;
     }
     object_proxy->CallMethodWithErrorCallback(
-        &method_call,
-        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
-        base::Bind(&BluetoothDeviceClientImpl::OnSuccess,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   callback),
+        &method_call, dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BluetoothDeviceClientImpl::OnGetConnInfoSuccess,
+                   weak_ptr_factory_.GetWeakPtr(), callback),
         base::Bind(&BluetoothDeviceClientImpl::OnError,
-                   weak_ptr_factory_.GetWeakPtr(),
-                   error_callback));
+                   weak_ptr_factory_.GetWeakPtr(), error_callback));
   }
 
  protected:
@@ -365,6 +340,25 @@ class BluetoothDeviceClientImpl
     callback.Run();
   }
 
+  // Called when a response for the GetConnInfo method is received.
+  void OnGetConnInfoSuccess(const ConnInfoCallback& callback,
+                            dbus::Response* response) {
+    int16 rssi = kUnknownPower;
+    int16 transmit_power = kUnknownPower;
+    int16 max_transmit_power = kUnknownPower;
+
+    if (response) {
+      dbus::MessageReader reader(response);
+      reader.PopInt16(&rssi);
+      reader.PopInt16(&transmit_power);
+      reader.PopInt16(&max_transmit_power);

[Ilya Sherman, 2014/11/19 22:18:07]

Are these PopInt16 calls guaranteed to succeed, or should there be
error-handling code?

[Tim Song, 2014/12/06 00:51:37]

The return value should always follow this format or there is something
seriously wrong. I added a DCHECK.

+    } else {
+      LOG(ERROR) << "GetConnInfo suceeded, but no response recieved.";
+    }
+
+    callback.Run(rssi, transmit_power, max_transmit_power);
+  }
+
   // Called when a response for a failed method call is received.
   void OnError(const ErrorCallback& error_callback,
                dbus::ErrorResponse* response) {