Add GetConnectionInfo function for BluetoothDevice, replacing the existing

device/bluetooth/bluetooth_device.h

 // Copyright (c) 2012 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.
 
 #ifndef DEVICE_BLUETOOTH_BLUETOOTH_DEVICE_H_
 #define DEVICE_BLUETOOTH_BLUETOOTH_DEVICE_H_
 
 #include <map>
 #include <string>
 #include <vector>
@@ skipping 50 matching lines @@
     DEVICE_GAMEPAD,
     DEVICE_KEYBOARD,
     DEVICE_MOUSE,
     DEVICE_TABLET,
     DEVICE_KEYBOARD_MOUSE_COMBO
   };
 
   // The value returned if the RSSI or transmit power cannot be read.
   static const int kUnknownPower = 127;
 
+  struct ConnectionInfo {
+    int rssi;
+    int transmit_power;
+    int max_transmit_power;
+
+    ConnectionInfo()
+        : rssi(kUnknownPower),
+          transmit_power(kUnknownPower),
+          max_transmit_power(kUnknownPower) {}
+
+    ConnectionInfo(int rssi, int transmit_power, int max_transmit_power)
+        : rssi(rssi),
+          transmit_power(transmit_power),
+          max_transmit_power(max_transmit_power) {}

[armansito, 2015/01/06 20:36:07]

Even though this is a simple struct, I would move the method definitions to the
source file. Also make sure to declare a destructor.

[Tim Song, 2015/01/06 22:30:51]

Done.

+  };
+
   // Possible errors passed back to an error callback function in case of a
   // failed call to Connect().
   enum ConnectErrorCode {
     ERROR_UNKNOWN,
     ERROR_INPROGRESS,
     ERROR_FAILED,
     ERROR_AUTH_FAILED,
     ERROR_AUTH_CANCELED,
     ERROR_AUTH_REJECTED,
     ERROR_AUTH_TIMEOUT,
@@ skipping 115 matching lines @@
   // be a synthesized string containing the address and localized type name
   // if the device has no obtained name.
   virtual base::string16 GetName() const;
 
   // Returns the type of the device, limited to those we support or are
   // aware of, by decoding the bluetooth class information. The returned
   // values are unique, and do not overlap, so DEVICE_KEYBOARD is not also
   // DEVICE_PERIPHERAL.
   DeviceType GetDeviceType() const;
 
-  // Gets the "received signal strength indication" (RSSI) of the current
-  // connection to the device. The RSSI indicates the power present in the
-  // received radio signal, measured in dBm, to a resolution of 1dBm. Larger
-  // (typically, less negative) values indicate a stronger signal.
-  // If the device is not currently connected, then returns the RSSI read from
-  // the last inquiry that returned the device, where available. In case of an
-  // error, returns |kUnknownPower|. Otherwise, returns the connection's RSSI.
-  virtual int GetRSSI() const = 0;
-
-  // These two methods are used to read the current or maximum transmit power
-  // ("Tx power") of the current connection to the device. The transmit power
-  // indicates the strength of the signal broadcast from the host's Bluetooth
-  // antenna when communicating with the device, measured in dBm, to a
-  // resolution of 1dBm. Larger (typically, less negative) values
-  // indicate a stronger signal.
-  // It is only meaningful to call this method when there is a connection
-  // established to the device. If there is no connection, or in case of an
-  // error, returns |kUnknownPower|. Otherwise, returns the connection's
-  // transmit power.
-  virtual int GetCurrentHostTransmitPower() const = 0;
-  virtual int GetMaximumHostTransmitPower() const = 0;
-
   // Indicates whether the device is known to support pairing based on its
   // device class and address.
   bool IsPairable() const;
 
   // Indicates whether the device is paired with the adapter.
   virtual bool IsPaired() const = 0;
 
   // Indicates whether the device is currently connected to the adapter.
   // Note that if IsConnected() is true, does not imply that the device is
   // connected to any application or service. If the device is not paired, it
@@ skipping 24 matching lines @@
 
   // The ErrorCallback is used for methods that can fail in which case it
   // is called, in the success case the callback is simply not called.
   typedef base::Callback<void()> ErrorCallback;
 
   // The ConnectErrorCallback is used for methods that can fail with an error,
   // passed back as an error code argument to this callback.
   // In the success case this callback is not called.
   typedef base::Callback<void(enum ConnectErrorCode)> ConnectErrorCallback;
 
+  typedef base::Callback<void(const ConnectionInfo&)> ConnectionInfoCallback;
+
   // Indicates whether the device is currently pairing and expecting a
   // PIN Code to be returned.
   virtual bool ExpectingPinCode() const = 0;
 
   // Indicates whether the device is currently pairing and expecting a
   // Passkey to be returned.
   virtual bool ExpectingPasskey() const = 0;
 
   // Indicates whether the device is currently pairing and expecting
   // confirmation of a displayed passkey.
   virtual bool ExpectingConfirmation() const = 0;
 
+  // Returns the RSSI and TX power of the active connection to the device:
+  //
+  // The RSSI indicates the power present in the received radio signal, measured
+  // in dBm, to a resolution of 1dBm. Larger (typically, less negative) values
+  // indicate a stronger signal.
+  //
+  // The transmit power indicates the strength of the signal broadcast from the
+  // host's Bluetooth antenna when communicating with the device, measured in
+  // dBm, to a resolution of 1dBm. Larger (typically, less negative) values
+  // indicate a stronger signal.
+  //
+  // If the device isn't connected, then the ConnectionInfo struct passed into
+  // the callback will filled by |kUnknownPower|.

[armansito, 2015/01/06 20:36:07]

nit: s/will filled by/will be populated with/

[Tim Song, 2015/01/06 22:30:51]

Done.

+  virtual void GetConnectionInfo(const ConnectionInfoCallback& callback) = 0;

[armansito, 2015/01/06 20:36:08]

Maybe it's better to have an ErrorCallback instead of returning an invalid
ConnectionInfo struct in the case of failure? Up to you.

[Tim Song, 2015/01/06 22:30:51]

The underlying dbus/mac apis might return invalid values, so I think we should
try to have one consistent error state.

[armansito, 2015/01/06 22:39:29]

That's what I figured why you chose to do it this way :) sgtm

+
   // Initiates a connection to the device, pairing first if necessary.
   //
   // Method calls will be made on the supplied object |pairing_delegate|
   // to indicate what display, and in response should make method calls
   // back to the device object. Not all devices require user responses
   // during pairing, so it is normal for |pairing_delegate| to receive no
   // calls. To explicitly force a low-security connection without bonding,
   // pass NULL, though this is ignored if the device is already paired.
   //
   // If the request fails, |error_callback| will be called; otherwise,
@@ skipping 83 matching lines @@
   // BluetoothDevice::Disconnect or other unexpected circumstances, the
   // returned BluetoothGattConnection will be automatically marked as inactive.
   // To monitor the state of the connection, observe the
   // BluetoothAdapter::Observer::DeviceChanged method.
   typedef base::Callback<void(scoped_ptr<BluetoothGattConnection>)>
       GattConnectionCallback;
   virtual void CreateGattConnection(
       const GattConnectionCallback& callback,
       const ConnectErrorCallback& error_callback) = 0;
 
-  // Starts monitoring the connection properties, RSSI and TX power. These
-  // properties will be tracked, and updated when their values change. Exactly
-  // one of |callback| or |error_callback| will be run.
-  virtual void StartConnectionMonitor(const base::Closure& callback,
-                                      const ErrorCallback& error_callback) = 0;
-
   // Returns the list of discovered GATT services.
   virtual std::vector<BluetoothGattService*> GetGattServices() const;
 
   // Returns the GATT service with device-specific identifier |identifier|.
   // Returns NULL, if no such service exists.
   virtual BluetoothGattService* GetGattService(
       const std::string& identifier) const;
 
   // Returns the |address| in the canonical format: XX:XX:XX:XX:XX:XX, where
   // each 'X' is a hex digit.  If the input |address| is invalid, returns an
@@ skipping 13 matching lines @@
 
  private:
   // Returns a localized string containing the device's bluetooth address and
   // a device type for display when |name_| is empty.
   base::string16 GetAddressWithLocalizedDeviceTypeName() const;
 };
 
 }  // namespace device
 
 #endif  // DEVICE_BLUETOOTH_BLUETOOTH_DEVICE_H_