Bluetooth: clean up BluetoothDevice

device/bluetooth/bluetooth_device.h

Index: device/bluetooth/bluetooth_device.h
diff --git a/device/bluetooth/bluetooth_device.h b/device/bluetooth/bluetooth_device.h
index b7dc9e2a2da713108c88acb595c6d5a7909175a2..dce8425ec6202c9040c9d9e1805c502ce2cb82ca 100644
--- a/device/bluetooth/bluetooth_device.h
+++ b/device/bluetooth/bluetooth_device.h
@@ -6,7 +6,6 @@
 #define DEVICE_BLUETOOTH_BLUETOOTH_DEVICE_H_
 
 #include <string>
-#include <vector>
 
 #include "base/callback.h"
 #include "base/memory/ref_counted.h"
@@ -151,9 +150,17 @@ class BluetoothDevice {
 
   virtual ~BluetoothDevice();
 
+  // Returns the Bluetooth class of the device. GetDeviceType() is preferred
+  // since that already decodes this bit field.
+  virtual uint32 bluetooth_class() const = 0;

[youngki, 2013/04/03 22:17:17]

Should we make bluetooth_class() and name() (probably address() too) protected
so that we can enforce the usage of GetName() and GetDeviceType()?

[keybuk, 2013/04/03 22:18:56]

I'm happy with that

[stevenjb, 2013/04/03 23:04:57]

Do we even need these in the public interface, or can they be moved to the
implementation?

[stevenjb, 2013/04/04 00:47:31]

If we need these as pure virtuals then we should rename them, e.g.
GetBluetoothClass, GetSystemName, GetAddress().

[keybuk, 2013/04/04 01:18:07]

Done.

+
+  // Returns the system name of the Bluetooth device. When being used for
+  // display, GetName() should be preferred.
+  virtual std::string name() const = 0;
+
   // Returns the Bluetooth of address the device. This should be used as
   // a unique key to identify the device and copied where needed.
-  virtual const std::string& address() const;
+  virtual std::string address() const = 0;
 
   // Returns the name of the device suitable for displaying, this may
   // be a synthesied string containing the address and localized type name
@@ -166,35 +173,24 @@ class BluetoothDevice {
   // DEVICE_PERIPHERAL.
   DeviceType GetDeviceType() const;
 
-  // Indicates whether the device is paired to the adapter, whether or not
-  // that pairing is permanent or temporary.
+  // Indicates whether the device is paired with the adapter.
   virtual bool IsPaired() const = 0;
 
-  // Indicates whether the device is visible to the adapter, this is not
-  // mutually exclusive to being paired.
-  virtual bool IsVisible() const;
-
-  // Indicates whether the device is bonded to the adapter, bonding is
-  // formed by pairing and exchanging high-security link keys so that
-  // connections may be encrypted.
-  virtual bool IsBonded() const;
+  // Indicates whether the device is currently connected to the adapter.
+  virtual bool IsConnected() const = 0;
 
-  // Indicates whether the device is currently connected to the adapter
-  // and at least one service available for use.
-  virtual bool IsConnected() const;
-
-  // Indicates whether the bonded device accepts connections initiated from the
-  // adapter. This value is undefined for unbonded devices.
-  virtual bool IsConnectable() const;
+  // Indicates whether the paired device accepts connections initiated from the
+  // adapter. This value is undefined for unpaired devices.
+  virtual bool IsConnectable() const = 0;
 
   // Indicates whether there is a call to Connect() ongoing. For this attribute,
   // we consider a call is ongoing if none of the callbacks passed to Connect()
   // were called after the corresponding call to Connect().
-  virtual bool IsConnecting() const;
+  virtual bool IsConnecting() const = 0;
 
   // Returns the services (as UUID strings) that this device provides.
   typedef std::vector<std::string> ServiceList;
-  virtual const ServiceList& GetServices() const = 0;
+  virtual ServiceList GetServices() const = 0;

[youngki, 2013/04/03 22:17:17]

It's just my thinking, but do you think we should just make the caller pass the
mutable vector pointers and let this function fill in that vector? That way we
could avoid copying the entire vector every time this method is called:

virtual void GetServices(ServiceList* services) const = 0;

[keybuk, 2013/04/03 22:18:56]

If you think that would be better - I was kinda assuming we'd get "move
constructors" soon ;-)

[youngki, 2013/04/04 12:59:08]

actually this might not be worth passing a mutable pointer given we are dealing
with std objects;

 
   // 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.
@@ -326,32 +322,6 @@ class BluetoothDevice {
  protected:
   BluetoothDevice();
 
-  // The Bluetooth class of the device, a bitmask that may be decoded using
-  // https://www.bluetooth.org/Technical/AssignedNumbers/baseband.htm
-  uint32 bluetooth_class_;
-
-  // The name of the device, as supplied by the remote device.
-  std::string name_;
-
-  // The Bluetooth address of the device.
-  std::string address_;
-
-  // Tracked device state, updated by the adapter managing the lifecyle of
-  // the device.
-  bool visible_;
-  bool bonded_;
-  bool connected_;
-
-  // Indicates whether the device normally accepts connections initiated from
-  // the adapter once paired.
-  bool connectable_;
-
-  // Indicated whether the device is in a connecting status.
-  bool connecting_;
-
-  // The services (identified by UUIDs) that this device provides.
-  ServiceList service_uuids_;
-
  private:
   // Returns a localized string containing the device's bluetooth address and
   // a device type for display when |name_| is empty.