Chrome OS: Add bluetooth dbus clients for device discovery

chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc

Index: chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
diff --git a/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
new file mode 100644
index 0000000000000000000000000000000000000000..89e8f84ed0ca38cda94f6157b3bfadd6e96416c4
--- /dev/null
+++ b/chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc
@@ -0,0 +1,453 @@
+// Copyright (c) 2011 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.
+
+#include "chrome/browser/chromeos/dbus/bluetooth_adapter_client.h"
+
+#include <map>
+
+#include "base/bind.h"
+#include "base/logging.h"
+#include "base/stl_util.h"
+#include "chrome/browser/chromeos/system/runtime_environment.h"
+#include "dbus/bus.h"
+#include "dbus/message.h"
+#include "dbus/object_proxy.h"
+#include "third_party/cros_system_api/dbus/service_constants.h"
+
+namespace chromeos {
+
+// The BluetoothAdapterClient implementation used in production.
+class BluetoothAdapterClientImpl: public BluetoothAdapterClient {
+ public:
+  explicit BluetoothAdapterClientImpl(dbus::Bus* bus)
+      : weak_ptr_factory_(this),
+        bus_(bus) {
+    VLOG(1) << "BluetoothAdapterClientImpl ctor";

[satorux1, 2011/10/25 02:17:10]

"ctor" -> "is created" or something, to make log messages more readable.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+  }
+
+  virtual ~BluetoothAdapterClientImpl() {
+    proxies_.clear();

[satorux1, 2011/10/25 02:17:10]

There is no harm, but it's unnecessary.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+  }
+
+  // BluetoothAdapterClient override.
+  virtual void AddObserver(Observer* observer, const std::string& path) {
+    VLOG(1) << "AddObserver: " << path;
+    CHECK(observer != NULL);

[satorux1, 2011/10/25 02:17:10]

DCHECK(observer);  We generally use DCHECK.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+    observers_.AddObserver(observer);
+    AddObjectProxyForPath(path);
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void RemoveObserver(Observer* observer, const std::string& path) {
+    VLOG(1) << "RemoveObserver: " << path;
+    CHECK(observer != NULL);
+    observers_.RemoveObserver(observer);
+    RemoveObjectProxyForPath(path);
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void StartDiscovery(const std::string& path) {
+    VLOG(1) << "StartDiscovery: " << path;
+
+    dbus::MethodCall method_call(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kStartDiscovery);
+
+    ProxyMap::iterator it = proxies_.find(path);
+    if (it == proxies_.end()) {
+      LOG(ERROR) << "Couldn't find proxy for path " << path;
+      return;
+    }
+    dbus::ObjectProxy* adapter_proxy =
+        static_cast<dbus::ObjectProxy*>(it->second);
+    CHECK(adapter_proxy != NULL);
+
+    adapter_proxy->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BluetoothAdapterClientImpl::OnStartDiscovery,
+                   weak_ptr_factory_.GetWeakPtr(), path));
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void StopDiscovery(const std::string& path) {
+    VLOG(1) << "StopDiscovery: " << path;
+
+    dbus::MethodCall method_call(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kStopDiscovery);
+
+    ProxyMap::iterator it = proxies_.find(path);
+    if (it == proxies_.end()) {
+      LOG(ERROR) << "Couldn't find proxy for path " << path;
+      return;
+    }
+    dbus::ObjectProxy* adapter_proxy =
+        static_cast<dbus::ObjectProxy*>(it->second);
+    CHECK(adapter_proxy != NULL);
+
+    adapter_proxy->CallMethod(
+        &method_call,
+        dbus::ObjectProxy::TIMEOUT_USE_DEFAULT,
+        base::Bind(&BluetoothAdapterClientImpl::OnStopDiscovery,
+                   weak_ptr_factory_.GetWeakPtr(), path));
+  }
+
+ private:
+  // Gets a dbus object proxy for an adapter with dbus path |path| and stores it
+  // in our |proxies_| map.
+  void AddObjectProxyForPath(const std::string& path) {
+    VLOG(1) << "AddObjectProxyForPath: " << path;
+
+    CHECK(bus_ != NULL);
+    dbus::ObjectProxy* adapter_proxy = bus_->GetObjectProxy(
+        bluetooth_adapter::kBluetoothAdapterServiceName, path);
+    CHECK(adapter_proxy != NULL);
+
+    proxies_[path] = adapter_proxy;
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kPropertyChangedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::PropertyChangedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), path),
+        base::Bind(&BluetoothAdapterClientImpl::PropertyChangedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceFoundSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceFoundReceived,
+                   weak_ptr_factory_.GetWeakPtr(), path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceFoundConnected,
+                   weak_ptr_factory_.GetWeakPtr(), path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceDisappearedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceDisappearedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceDisappearedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), path));
+  }
+
+  // Removes the dbus object proxy for the adapter with dbus path |path| from
+  // our |proxies_| map.
+  void RemoveObjectProxyForPath(const std::string& path) {

[satorux1, 2011/10/25 02:17:10]

path -> object_path to make it extra clearer (path may sound like a file path).

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+    VLOG(1) << "RemoveObjectProxyForPath: " << path;
+    ProxyMap::iterator it = proxies_.find(path);

[satorux1, 2011/10/25 02:17:10]

You can just do

proxies_.erase(path);

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+    if (it != proxies_.end()) {
+      proxies_.erase(it);
+    }
+  }
+
+  // Called by dbus:: when a PropertyChanged signal is received.
+  void PropertyChangedReceived(const std::string& path, dbus::Signal* signal) {
+    CHECK(signal != NULL);
+    dbus::MessageReader reader(signal);
+    std::string name;

[satorux1, 2011/10/25 02:17:10]

name of what? would be nice to make it more descriptive.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+    if (!reader.PopString(&name)) {
+      LOG(ERROR) << path
+          << ": PropertyChanged signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+
+    if (name != bluetooth_adapter::kDiscoveringProperty) {
+      VLOG(1) << path << ": PropertyChanged: " << name;
+      // We don't care.
+      return;
+    }
+
+    bool discovering;

[satorux1, 2011/10/25 02:17:10]

please initialize this with false to avoid possible non-determinism. 

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+    if (!reader.PopVariantOfBool(&discovering)) {
+      LOG(ERROR) << path
+          << ": PropertyChanged signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << path << ": PropertyChanged: Discovering = " << discovering;
+
+    FOR_EACH_OBSERVER(Observer, observers_,
+                      DiscoveringPropertyChanged(path, discovering));
+  }
+
+  // Called by dbus:: when the PropertyChanged signal is initially connected.
+  void PropertyChangedConnected(const std::string& path,
+                                const std::string& interface_name,
+                                const std::string& signal_name,
+                                bool success) {
+    LOG_IF(WARNING, !success) << path
+        << ": Failed to connect to PropertyChanged signal.";
+  }
+
+  // Called by dbus:: when a DeviceFound signal is received.
+  void DeviceFoundReceived(const std::string& path, dbus::Signal* signal) {
+    CHECK(signal != NULL);
+    dbus::MessageReader reader(signal);
+    std::string address;
+    if (!reader.PopString(&address)) {
+      LOG(ERROR) << path << ": DeviceFound signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << path << ": Device found: " << address;
+
+    DictionaryValue device_properties;
+    if (!PopArrayOfDictEntries(&reader, signal, &device_properties)) {
+      LOG(ERROR) << path << ": DeviceFound signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+
+    FOR_EACH_OBSERVER(Observer, observers_, DeviceFound(path, address,
+                                                        device_properties));
+  }
+
+  // Called by dbus:: when the DeviceFound signal is initially connected.
+  void DeviceFoundConnected(const std::string& path,
+                            const std::string& interface_name,
+                            const std::string& signal_name,
+                            bool success) {
+    LOG_IF(WARNING, !success) << path
+        << ": Failed to connect to DeviceFound signal.";
+  }
+
+  // Called by dbus:: when a DeviceDisappeared signal is received.
+  void DeviceDisappearedReceived(const std::string&  path,
+                                 dbus::Signal* signal) {
+    CHECK(signal != NULL);
+    dbus::MessageReader reader(signal);
+    std::string address;
+    if (!reader.PopString(&address)) {
+      LOG(ERROR) << path
+          << ": DeviceDisappeared signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << path << ": Device disappeared: " << address;
+    FOR_EACH_OBSERVER(Observer, observers_, DeviceDisappeared(path, address));
+  }
+
+  // Called by dbus:: when the DeviceDisappeared signal is initially connected.
+  void DeviceDisappearedConnected(const std::string& path,
+                                  const std::string& interface_name,
+                                  const std::string& signal_name,
+                                  bool success) {
+    LOG_IF(WARNING, !success) << path
+        << ": Failed to connect to DeviceDisappeared signal.";
+  }
+
+  // Called when a response for StartDiscovery() is received.
+  void OnStartDiscovery(const std::string& path, dbus::Response* response) {
+    VLOG(1) << "OnStartDiscovery: " << path;
+    CHECK(response != NULL);
+  }
+
+  // Called when a response for StopDiscovery() is received.
+  void OnStopDiscovery(const std::string&  path, dbus::Response* response) {
+    VLOG(1) << "OnStopDiscovery: " << path;
+    CHECK(response != NULL);
+  }
+
+  // Utility function to convert array of dbus dict_entry objects into a
+  // DictionaryValue object. We intend to move this to the chrome dbus library
+  // MessageReader when it's more fully baked.
+  // TODO(vlaviano):
+  // - Can we handle integral types better?
+  // - Add support for nested complex types.
+  // - Write an equivalent function to convert in the opposite direction.
+  // - Write unit tests.
+  static bool PopArrayOfDictEntries(dbus::MessageReader* reader,

[satorux1, 2011/10/25 02:17:10]

This function doesn't have to be inside the class. Would be nice to move this to
the beginning of the file and put it into anonymous namespace.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+                                    dbus::Message* message,
+                                    DictionaryValue* dictionary) {
+    CHECK(reader != NULL);
+    CHECK(message != NULL);
+    CHECK(dictionary != NULL);
+    dbus::MessageReader array_reader(message);
+    if (!reader->PopArray(&array_reader)) {
+      return false;
+    }
+    while (array_reader.HasMoreData()) {
+      dbus::MessageReader dict_entry_reader(message);
+      if (!array_reader.PopDictEntry(&dict_entry_reader)) {
+        return false;
+      }
+      std::string key;
+      if (!dict_entry_reader.PopString(&key)) {

[satorux1, 2011/10/25 02:17:10]

So the key needs to be string, right? Would be nice to document about it in the
function comment.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+        return false;
+      }
+      dbus::MessageReader variant_reader(message);

[satorux1, 2011/10/25 02:17:10]

So the value needs to be variant, right? Would be nice to document about it.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+      if (!dict_entry_reader.PopVariant(&variant_reader)) {
+        return false;
+      }
+      const dbus::Message::DataType type = variant_reader.GetDataType();
+      switch (type) {
+        case dbus::Message::BYTE: {
+          uint8 value = 0;
+          if (!variant_reader.PopByte(&value)) {
+            return false;
+          }
+          dictionary->SetInteger(key, value);
+          break;
+        }
+        case dbus::Message::BOOL: {
+          bool value = false;
+          if (!variant_reader.PopBool(&value)) {
+            return false;
+          }
+          dictionary->SetBoolean(key, value);
+          break;
+        }
+        case dbus::Message::INT16: {
+          int16 value = 0;
+          if (!variant_reader.PopInt16(&value)) {
+            return false;
+          }
+          dictionary->SetInteger(key, value);
+          break;
+        }
+        case dbus::Message::UINT16: {
+          uint16 value = 0;
+          if (!variant_reader.PopUint16(&value)) {
+            return false;
+          }
+          dictionary->SetInteger(key, value);
+          break;
+        }
+        case dbus::Message::INT32: {
+          int32 value = 0;
+          if (!variant_reader.PopInt32(&value)) {
+            return false;
+          }
+          dictionary->SetInteger(key, value);
+          break;
+        }
+        case dbus::Message::UINT32: {
+          uint32 value = 0;
+          if (!variant_reader.PopUint32(&value)) {
+            return false;
+          }
+          dictionary->SetInteger(key, value);
+          break;
+        }
+        case dbus::Message::INT64: {
+          int64 value = 0;
+          if (!variant_reader.PopInt64(&value)) {
+            return false;
+          }
+          dictionary->SetDouble(key, value);
+          break;
+        }
+        case dbus::Message::UINT64: {
+          uint64 value = 0;
+          if (!variant_reader.PopUint64(&value)) {
+            return false;
+          }
+          dictionary->SetDouble(key, value);
+          break;
+        }
+        case dbus::Message::DOUBLE: {
+          double value = 0;
+          if (!variant_reader.PopDouble(&value)) {
+            return false;
+          }
+          dictionary->SetDouble(key, value);
+          break;
+        }
+        case dbus::Message::STRING: {
+          std::string value;
+          if (!variant_reader.PopString(&value)) {
+            return false;
+          }
+          dictionary->SetString(key, value);
+          break;
+        }
+        case dbus::Message::OBJECT_PATH: {
+          std::string value;
+          if (!variant_reader.PopObjectPath(&value)) {
+            return false;
+          }
+          dictionary->SetString(key, value);
+          break;
+        }
+        case dbus::Message::ARRAY: {
+          // Not yet supported.
+          return false;
+        }
+        case dbus::Message::STRUCT: {
+          // Not yet supported.
+          return false;
+        }
+        case dbus::Message::DICT_ENTRY: {
+          // Not yet supported.
+          return false;
+        }
+        case dbus::Message::VARIANT: {
+          // Not yet supported.
+          return false;
+        }
+        default:
+          LOG(FATAL) << "Unknown type: " << type;
+      }
+    }
+    return true;
+  }
+
+  // Weak pointer factory for generating 'this' pointers that might live longer
+  // than we do.
+  base::WeakPtrFactory<BluetoothAdapterClientImpl> weak_ptr_factory_;
+
+  dbus::Bus* bus_;
+
+  // We maintain a collection of dbus object proxies, one for each adapter.
+  typedef std::map<const std::string, dbus::ObjectProxy*> ProxyMap;
+  ProxyMap proxies_;

[satorux1, 2011/10/25 02:17:10]

proxy_map_ may make it clearer that this variable is a map.

[Vince Laviano, 2011/10/25 19:18:03]

Done.

+
+  // List of observers interested in event notifications from us.
+  ObserverList<Observer> observers_;
+
+  DISALLOW_COPY_AND_ASSIGN(BluetoothAdapterClientImpl);
+};
+
+// The BluetoothAdapterClient implementation used on Linux desktop, which does
+// nothing.
+class BluetoothAdapterClientStubImpl : public BluetoothAdapterClient {
+ public:
+  // BluetoothAdapterClient override.
+  virtual void AddObserver(Observer* observer, const std::string& path) {
+    VLOG(1) << "AddObserver: " << path;
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void RemoveObserver(Observer* observer, const std::string& path) {
+    VLOG(1) << "RemoveObserver: " << path;
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void StartDiscovery(const std::string& path) {
+    VLOG(1) << "StartDiscovery: " << path;
+  }
+
+  // BluetoothAdapterClient override.
+  virtual void StopDiscovery(const std::string& path) {
+    VLOG(1) << "StopDiscovery: " << path;
+  }
+};
+
+BluetoothAdapterClient::BluetoothAdapterClient() {
+}
+
+BluetoothAdapterClient::~BluetoothAdapterClient() {
+}
+
+BluetoothAdapterClient* BluetoothAdapterClient::Create(dbus::Bus* bus) {
+  if (system::runtime_environment::IsRunningOnChromeOS()) {
+    return new BluetoothAdapterClientImpl(bus);
+  } else {
+    return new BluetoothAdapterClientStubImpl();
+  }
+}
+
+}  // namespace chromeos