chrome: bluetooth: hook up DeviceCreated/Removed signals

chrome/browser/chromeos/dbus/bluetooth_adapter_client.cc

 // 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"
@@ skipping 244 matching lines @@
     VLOG(1) << "AddObjectProxyForPath: " << object_path;
 
     DCHECK(bus_);
     dbus::ObjectProxy* adapter_proxy = bus_->GetObjectProxy(
         bluetooth_adapter::kBluetoothAdapterServiceName, object_path);
 
     proxy_map_[object_path] = adapter_proxy;
 
     adapter_proxy->ConnectToSignal(
         bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceCreatedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceCreatedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), object_path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceCreatedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
+        bluetooth_adapter::kDeviceRemovedSignal,
+        base::Bind(&BluetoothAdapterClientImpl::DeviceRemovedReceived,
+                   weak_ptr_factory_.GetWeakPtr(), object_path),
+        base::Bind(&BluetoothAdapterClientImpl::DeviceRemovedConnected,
+                   weak_ptr_factory_.GetWeakPtr(), object_path));
+
+    adapter_proxy->ConnectToSignal(
+        bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kPropertyChangedSignal,
         base::Bind(&BluetoothAdapterClientImpl::PropertyChangedReceived,
                    weak_ptr_factory_.GetWeakPtr(), object_path),
         base::Bind(&BluetoothAdapterClientImpl::PropertyChangedConnected,
                    weak_ptr_factory_.GetWeakPtr(), object_path));
 
     adapter_proxy->ConnectToSignal(
         bluetooth_adapter::kBluetoothAdapterInterface,
         bluetooth_adapter::kDeviceFoundSignal,
         base::Bind(&BluetoothAdapterClientImpl::DeviceFoundReceived,
@@ skipping 10 matching lines @@
                    weak_ptr_factory_.GetWeakPtr(), object_path));
   }
 
   // Removes the dbus object proxy for the adapter with dbus object path
   // |object_path| from our |proxy_map_| map.
   void RemoveObjectProxyForPath(const std::string& object_path) {
     VLOG(1) << "RemoveObjectProxyForPath: " << object_path;
     proxy_map_.erase(object_path);
   }
 
+  // Called by dbus:: when a DeviceCreated signal is received.

[mukesh agrawal, 2012/02/01 00:48:42]

why "dbus::" (trailing colons)?
(same for other functions)

[keybuk, 2012/02/04 00:35:47]

referring to "code in the dbus namespace", ie the generic dbus::ObjectProxy,
dbus::Bus, etc. code

+  void DeviceCreatedReceived(const std::string& object_path,

[mukesh agrawal, 2012/02/01 00:48:42]

you might consider "using std::string" and shortening the parameter types.
(OTOH, there's a bunch of existing code that would need to be change. so i'm
also ok leaving as-is.)

[satorux1, 2012/02/01 17:45:18]

Please don't do this in Chrome's code for consistency. std::whatever is not
shortened in Chrome's code.

+                             dbus::Signal* signal) {
+    DCHECK(signal);
+    dbus::MessageReader reader(signal);
+    std::string device_path;
+    if (!reader.PopString(&device_path)) {
+      LOG(ERROR) << object_path
+          << ": DeviceCreated signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << object_path << ": Device created: " << device_path;
+
+    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
+                      DeviceCreated(object_path, device_path));

[mukesh agrawal, 2012/02/01 00:48:42]

are the arguments in the correct order? prototype is

  void DeviceCreated(const std::string& adapter_path,
                     const std::string& object_path)

[keybuk, 2012/02/04 00:35:47]

yes, the "object_path" here refers to the adapter (this is adapter code), but
from the observer point of view, the object_path is the device being created so
that means we put our object_path into the adapter_path argument and device_path
into the object_path argument

I'm slowly converting the code to use adapter_path, device_path, etc. rather
than the generic object_path, but have to do it one step at a time

+  }
+
+  // Called by dbus:: when the DeviceCreated signal is initially connected.
+  void DeviceCreatedConnected(const std::string& object_path,
+                              const std::string& interface_name,

[mukesh agrawal, 2012/02/01 00:48:42]

google style comments out unused parameters. |interface_name| and |signal_name|
should be commented out here.

[keybuk, 2012/02/04 00:35:47]

Chrome style seems to not do that afaict?

+                              const std::string& signal_name,
+                              bool success) {
+    LOG_IF(WARNING, !success) << object_path
+        << ": Failed to connect to DeviceCreated signal.";
+  }
+
+  // Called by dbus:: when a DeviceRemoved signal is received.
+  void DeviceRemovedReceived(const std::string& object_path,
+                             dbus::Signal* signal) {
+    DCHECK(signal);
+    dbus::MessageReader reader(signal);
+    std::string device_path;
+    if (!reader.PopString(&device_path)) {
+      LOG(ERROR) << object_path
+          << ": DeviceRemoved signal has incorrect parameters: "
+          << signal->ToString();
+      return;
+    }
+    VLOG(1) << object_path << ": Device created: " << device_path;
+
+    FOR_EACH_OBSERVER(BluetoothAdapterClient::Observer, observers_,
+                      DeviceRemoved(object_path, device_path));

[mukesh agrawal, 2012/02/01 00:48:42]

as in DeviceAddedReceived: check callback parameter order

+  }
+
+  // Called by dbus:: when the DeviceRemoved signal is initially connected.
+  void DeviceRemovedConnected(const std::string& object_path,
+                              const std::string& interface_name,

[mukesh agrawal, 2012/02/01 00:48:42]

as above (comment out unused parameters).

+                              const std::string& signal_name,
+                              bool success) {
+    LOG_IF(WARNING, !success) << object_path
+        << ": Failed to connect to DeviceRemoved signal.";
+  }
+
   // Called by dbus:: when a PropertyChanged signal is received.
   void PropertyChangedReceived(const std::string& object_path,
                                dbus::Signal* signal) {
     DCHECK(signal);
     dbus::MessageReader reader(signal);
     std::string property_name;
     if (!reader.PopString(&property_name)) {
       LOG(ERROR) << object_path
           << ": PropertyChanged signal has incorrect parameters: "
           << signal->ToString();
@@ skipping 154 matching lines @@
 
 BluetoothAdapterClient* BluetoothAdapterClient::Create(dbus::Bus* bus) {
   if (system::runtime_environment::IsRunningOnChromeOS()) {
     return new BluetoothAdapterClientImpl(bus);
   } else {
     return new BluetoothAdapterClientStubImpl();
   }
 }
 
 }  // namespace chromeos