Bluetooth: implement BlueZ 5 backend for Chrome OS

device/bluetooth/bluetooth_adapter_experimental_chromeos.cc

Index: device/bluetooth/bluetooth_adapter_experimental_chromeos.cc
diff --git a/device/bluetooth/bluetooth_adapter_experimental_chromeos.cc b/device/bluetooth/bluetooth_adapter_experimental_chromeos.cc
index 8cdfb9618629b15a7c977c7fe4f889893e837602..eb8c1da7f084fe55c249d9853e68c1f1263a1486 100644
--- a/device/bluetooth/bluetooth_adapter_experimental_chromeos.cc
+++ b/device/bluetooth/bluetooth_adapter_experimental_chromeos.cc
@@ -6,32 +6,77 @@
 
 #include <string>
 
+#include "base/bind.h"
+#include "base/logging.h"
+#include "chromeos/dbus/dbus_thread_manager.h"
+#include "chromeos/dbus/experimental_bluetooth_adapter_client.h"
+#include "chromeos/dbus/experimental_bluetooth_device_client.h"
+#include "device/bluetooth/bluetooth_device.h"
+#include "device/bluetooth/bluetooth_device_experimental_chromeos.h"
+
 using device::BluetoothAdapter;
+using device::BluetoothDevice;
 
 namespace chromeos {
 
 BluetoothAdapterExperimentalChromeOS::BluetoothAdapterExperimentalChromeOS()
-    : BluetoothAdapter(),
-      weak_ptr_factory_(this) {
+    : weak_ptr_factory_(this) {
+  DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+      AddObserver(this);
+  DBusThreadManager::Get()->GetExperimentalBluetoothDeviceClient()->
+      AddObserver(this);
+
+  std::vector<dbus::ObjectPath> object_paths =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetAdapters();
+
+  if (!object_paths.empty()) {
+    VLOG(1) << object_paths.size() << " Bluetooth adapter(s) available.";
+    SetAdapter(object_paths[0]);
+  }
 }
 
 BluetoothAdapterExperimentalChromeOS::~BluetoothAdapterExperimentalChromeOS() {
+  DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+      RemoveObserver(this);
+  DBusThreadManager::Get()->GetExperimentalBluetoothDeviceClient()->
+      RemoveObserver(this);
 }
 
 void BluetoothAdapterExperimentalChromeOS::AddObserver(
     BluetoothAdapter::Observer* observer) {
+  DCHECK(observer);
+  observers_.AddObserver(observer);
 }
 
 void BluetoothAdapterExperimentalChromeOS::RemoveObserver(
     BluetoothAdapter::Observer* observer) {
+  DCHECK(observer);
+  observers_.RemoveObserver(observer);
 }
 
 std::string BluetoothAdapterExperimentalChromeOS::GetAddress() const {
-  return std::string();
+  if (object_path_.value().empty())
+    return std::string();
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+  DCHECK(properties);
+
+  return properties->address.value();
 }
 
 std::string BluetoothAdapterExperimentalChromeOS::GetName() const {
-  return std::string();
+  if (object_path_.value().empty())
+    return std::string();
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+  DCHECK(properties);
+
+  return properties->alias.value();
 }
 
 bool BluetoothAdapterExperimentalChromeOS::IsInitialized() const {
@@ -39,33 +84,77 @@ bool BluetoothAdapterExperimentalChromeOS::IsInitialized() const {
 }
 
 bool BluetoothAdapterExperimentalChromeOS::IsPresent() const {
-  return false;
+  return !object_path_.value().empty();
 }
 
 bool BluetoothAdapterExperimentalChromeOS::IsPowered() const {
-  return false;
+  if (object_path_.value().empty())

[youngki, 2013/04/16 15:26:56]

Would it be more elegant if we reuse IsPresent() here?

if (!IsPresent())
  return false;

[keybuk, 2013/04/16 23:19:13]

Done.

+    return false;
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+
+  return properties->powered.value();
 }
 
-void BluetoothAdapterExperimentalChromeOS::SetPowered(bool powered,
-                                          const base::Closure& callback,
-                                          const ErrorCallback& error_callback) {
-  error_callback.Run();
+void BluetoothAdapterExperimentalChromeOS::SetPowered(
+    bool powered,
+    const base::Closure& callback,
+    const ErrorCallback& error_callback) {
+  DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+      GetProperties(object_path_)->powered.Set(
+          powered,
+          base::Bind(&BluetoothAdapterExperimentalChromeOS::OnSetPowered,
+                     weak_ptr_factory_.GetWeakPtr(),
+                     callback, error_callback));

[youngki, 2013/04/16 15:26:56]

style nits: one argument per line.

[keybuk, 2013/04/16 23:19:13]

Done.

 }
 
 bool BluetoothAdapterExperimentalChromeOS::IsDiscovering() const {
-  return false;
+  if (object_path_.value().empty())

[youngki, 2013/04/16 15:26:56]

Can we reuse IsPresent()?

if (!IsPresent())
  return false;

[keybuk, 2013/04/16 23:19:13]

Done.

+    return false;
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+
+  return properties->discovering.value();
 }
 
 void BluetoothAdapterExperimentalChromeOS::StartDiscovering(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  error_callback.Run();
+  // BlueZ counts discovery sessions, and permits multiple sessions for a
+  // single connection, so issue a StartDiscovery() call for every use
+  // within Chromium for the right behavior.
+  DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+      StartDiscovery(
+          object_path_,
+          base::Bind(
+              &BluetoothAdapterExperimentalChromeOS::OnStartDiscovery,
+              weak_ptr_factory_.GetWeakPtr(),
+              callback),
+          base::Bind(
+              &BluetoothAdapterExperimentalChromeOS::OnStartDiscoveryError,
+              weak_ptr_factory_.GetWeakPtr(),
+              error_callback));
 }
 
 void BluetoothAdapterExperimentalChromeOS::StopDiscovering(
     const base::Closure& callback,
     const ErrorCallback& error_callback) {
-  error_callback.Run();
+  // Inform BlueZ to stop one of our open discovery sessions.
+  DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+      StopDiscovery(
+          object_path_,
+          base::Bind(
+              &BluetoothAdapterExperimentalChromeOS::OnStopDiscovery,
+              weak_ptr_factory_.GetWeakPtr(),
+              callback),
+          base::Bind(
+              &BluetoothAdapterExperimentalChromeOS::OnStopDiscoveryError,
+              weak_ptr_factory_.GetWeakPtr(),
+              error_callback));
 }
 
 void BluetoothAdapterExperimentalChromeOS::ReadLocalOutOfBandPairingData(
@@ -74,4 +163,228 @@ void BluetoothAdapterExperimentalChromeOS::ReadLocalOutOfBandPairingData(
   error_callback.Run();
 }
 
+void BluetoothAdapterExperimentalChromeOS::AdapterAdded(
+    const dbus::ObjectPath& object_path) {
+  if (object_path_.value().empty())

[satorux1, 2013/04/16 06:30:45]

Did you mean:

if (!object_path_.value().empty())

[keybuk, 2013/04/16 23:19:13]

No, this only sets the adapter if it *isn't* already tracking one - will change
and add comment to clarify

+    SetAdapter(object_path);
+}
+
+void BluetoothAdapterExperimentalChromeOS::AdapterRemoved(
+    const dbus::ObjectPath& object_path) {
+  if (object_path == object_path_)
+    RemoveAdapter();
+}
+
+void BluetoothAdapterExperimentalChromeOS::AdapterPropertyChanged(
+    const dbus::ObjectPath& object_path,
+    const std::string& property_name) {
+  if (object_path != object_path_)
+    return;
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+
+  if (property_name == properties->powered.name())
+    PoweredChanged(properties->powered.value());
+  else if (property_name == properties->discovering.name())
+    DiscoveringChanged(properties->discovering.value());
+}
+
+void BluetoothAdapterExperimentalChromeOS::DeviceAdded(
+  const dbus::ObjectPath& object_path) {
+  ExperimentalBluetoothDeviceClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothDeviceClient()->
+          GetProperties(object_path);
+  if (properties->adapter.value() != object_path_)
+    return;
+
+  BluetoothDeviceExperimentalChromeOS* device_chromeos =
+      new BluetoothDeviceExperimentalChromeOS(this, object_path);
+

[youngki, 2013/04/16 15:26:56]

Can we put DCHECK here that devices_[device_chromeos->GetAddress()] should not
already exist?

[keybuk, 2013/04/16 23:19:13]

Done.

+  devices_[device_chromeos->GetAddress()] = device_chromeos;
+
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                    DeviceAdded(this, device_chromeos));
+}
+
+void BluetoothAdapterExperimentalChromeOS::DeviceRemoved(
+    const dbus::ObjectPath& object_path) {
+  for (DevicesMap::iterator iter = devices_.begin();
+       iter != devices_.end(); ++iter) {
+    BluetoothDeviceExperimentalChromeOS* device_chromeos =
+        static_cast<BluetoothDeviceExperimentalChromeOS*>(iter->second);
+    if (device_chromeos->object_path() == object_path) {
+      devices_.erase(iter);
+
+      FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                        DeviceRemoved(this, device_chromeos));
+      delete device_chromeos;
+      return;
+    }
+  }
+}
+
+void BluetoothAdapterExperimentalChromeOS::DevicePropertyChanged(
+    const dbus::ObjectPath& object_path,
+    const std::string& property_name) {
+  BluetoothDeviceExperimentalChromeOS* device_chromeos =
+      GetDeviceWithPath(object_path);
+  if (!device_chromeos)
+    return;
+
+  ExperimentalBluetoothDeviceClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothDeviceClient()->
+          GetProperties(object_path);
+
+  if (property_name == properties->bluetooth_class.name() ||
+      property_name == properties->alias.name() ||
+      property_name == properties->paired.name() ||
+      property_name == properties->connected.name() ||
+      property_name == properties->uuids.name()) {
+    FOR_EACH_OBSERVER(
+        BluetoothAdapter::Observer, observers_,
+        DeviceChanged(this, static_cast<BluetoothDevice*>(device_chromeos)));

[youngki, 2013/04/16 15:26:56]

I don't think this cast is necessary since BluetoothDevicexperimentalChromOS is
a subclass of BluetoothDevice.

[keybuk, 2013/04/16 23:19:13]

Done.

+  }
+}
+
+BluetoothDeviceExperimentalChromeOS*
+BluetoothAdapterExperimentalChromeOS::GetDeviceWithPath(
+    const dbus::ObjectPath& object_path) {
+  for (DevicesMap::iterator iter = devices_.begin();
+       iter != devices_.end(); ++iter) {
+    BluetoothDeviceExperimentalChromeOS* device_chromeos =
+        static_cast<BluetoothDeviceExperimentalChromeOS*>(iter->second);
+    if (device_chromeos->object_path() == object_path)
+      return device_chromeos;
+  }
+
+  return NULL;
+}
+
+void BluetoothAdapterExperimentalChromeOS::SetAdapter(
+    const dbus::ObjectPath& object_path)
+{
+  DCHECK(object_path_.value().empty());
+  object_path_ = object_path;
+
+  VLOG(1) << object_path_.value() << ": using adapter.";
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+
+  PresentChanged(true);
+
+  if (properties->powered.value() != false)

[youngki, 2013/04/16 15:26:56]

change it to if (properties->powered.value())

[keybuk, 2013/04/16 23:19:13]

Done.

+    PoweredChanged(properties->powered.value());

[youngki, 2013/04/16 15:26:56]

Also I think we should just make it PoweredChanged(true).

[keybuk, 2013/04/16 23:19:13]

Done.

+  if (properties->discovering.value() != false)

[youngki, 2013/04/16 15:26:56]

change it to if (properties->discovering.value())

[keybuk, 2013/04/16 23:19:13]

Done.

+    DiscoveringChanged(properties->discovering.value());

[youngki, 2013/04/16 15:26:56]

Change it to DiscoveringChanged(true) also.

[keybuk, 2013/04/16 23:19:13]

Done.

+
+  std::vector<dbus::ObjectPath> device_paths =
+      DBusThreadManager::Get()->GetExperimentalBluetoothDeviceClient()->
+          GetDevicesForAdapter(object_path_);
+
+  for (std::vector<dbus::ObjectPath>::iterator iter = device_paths.begin();
+       iter != device_paths.end(); ++iter) {
+    BluetoothDeviceExperimentalChromeOS* device_chromeos =
+        new BluetoothDeviceExperimentalChromeOS(this, *iter);
+
+    devices_[device_chromeos->GetAddress()] = device_chromeos;
+
+    FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                      DeviceAdded(this, device_chromeos));
+  }
+}
+
+void BluetoothAdapterExperimentalChromeOS::RemoveAdapter()
+{
+  DCHECK(!object_path_.value().empty());

[youngki, 2013/04/16 15:26:56]

DCHECK(IsPresent());

[keybuk, 2013/04/16 23:19:13]

Done.

+  VLOG(1) << object_path_.value() << ": adapter removed.";
+
+  ExperimentalBluetoothAdapterClient::Properties* properties =
+      DBusThreadManager::Get()->GetExperimentalBluetoothAdapterClient()->
+          GetProperties(object_path_);
+
+  object_path_ = dbus::ObjectPath("");
+
+  if (properties->powered.value() != false)

[youngki, 2013/04/16 15:26:56]

change it to if (properties->powered.value())

[keybuk, 2013/04/16 23:19:13]

Done.

+    PoweredChanged(false);

[youngki, 2013/04/16 15:26:56]

Are you sure this logic is right? This looks to me that:

if |properties| is powered:
  Tell observers that power is off;

[keybuk, 2013/04/16 23:19:13]

"if powered was on, the adapter is going buh-bye, so tell everyone the property
changed to off in case all they were tracking is AdapterPoweredChanged"

+  if (properties->discovering.value() != false)

[youngki, 2013/04/16 15:26:56]

change it to if (properties->discovering.value())

[keybuk, 2013/04/16 23:19:13]

Done.

+    DiscoveringChanged(false);
+
+  // Copy the devices list here and clear the original so that when we
+  // send DeviceRemoved(), GetDevices() returns no devices.
+  DevicesMap devices = devices_;
+  devices_.clear();
+
+  for (DevicesMap::iterator iter = devices.begin();
+       iter != devices.end(); ++iter) {
+    FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                      DeviceRemoved(this, iter->second));
+    delete iter->second;
+  }
+
+  PresentChanged(false);
+}
+
+void BluetoothAdapterExperimentalChromeOS::PoweredChanged(bool powered)
+{
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                    AdapterPoweredChanged(this, powered));
+}
+
+void BluetoothAdapterExperimentalChromeOS::DiscoveringChanged(bool discovering)
+{
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                    AdapterDiscoveringChanged(this, discovering));
+}
+
+void BluetoothAdapterExperimentalChromeOS::PresentChanged(bool present)
+{
+  FOR_EACH_OBSERVER(BluetoothAdapter::Observer, observers_,
+                    AdapterPresentChanged(this, present));
+}
+
+void BluetoothAdapterExperimentalChromeOS::OnSetPowered(
+    const base::Closure& callback,
+    const ErrorCallback& error_callback,
+    bool success)
+{

[satorux1, 2013/04/16 06:30:45]

nit: move { to the previous line

[keybuk, 2013/04/16 23:19:13]

Done.

+  if (success)
+    callback.Run();
+  else
+    error_callback.Run();
+}
+
+void BluetoothAdapterExperimentalChromeOS::OnStartDiscovery(
+    const base::Closure& callback) {
+  callback.Run();
+}
+
+void BluetoothAdapterExperimentalChromeOS::OnStartDiscoveryError(
+    const ErrorCallback& error_callback,
+    const std::string& error_name,
+    const std::string& error_message)
+{

[satorux1, 2013/04/16 06:30:45]

ditto

[keybuk, 2013/04/16 23:19:13]

Done.

+  LOG(WARNING) << object_path_.value() << ": Failed to start discovery: "
+               << error_name << ": " << error_message;
+  error_callback.Run();
+}
+
+void BluetoothAdapterExperimentalChromeOS::OnStopDiscovery(
+    const base::Closure& callback) {
+  callback.Run();
+}
+
+void BluetoothAdapterExperimentalChromeOS::OnStopDiscoveryError(
+    const ErrorCallback& error_callback,
+    const std::string& error_name,
+    const std::string& error_message)
+{

[satorux1, 2013/04/16 06:30:45]

ditto

[keybuk, 2013/04/16 23:19:13]

Done.

+  LOG(WARNING) << object_path_.value() << ": Failed to stop discovery: "
+               << error_name << ": " << error_message;
+  error_callback.Run();
+}
+
 }  // namespace chromeos