bluetooth: Update the map of GATT services when services resolve

device/bluetooth/bluez/bluetooth_device_bluez.cc

 // Copyright 2013 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 "device/bluetooth/bluez/bluetooth_device_bluez.h"
 
 #include <stdio.h>
 
 #include <memory>
 #include <utility>
@@ skipping 138 matching lines @@
     scoped_refptr<device::BluetoothSocketThread> socket_thread)
     : BluetoothDevice(adapter),
       object_path_(object_path),
       num_connecting_calls_(0),
       connection_monitor_started_(false),
       ui_task_runner_(ui_task_runner),
       socket_thread_(socket_thread),
       weak_ptr_factory_(this) {
   bluez::BluezDBusManager::Get()->GetBluetoothGattServiceClient()->AddObserver(
       this);
-  bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->AddObserver(this);
 
-  InitializeGattServiceMap();
+  InitializeGattServicesMap();
 }
 
 BluetoothDeviceBlueZ::~BluetoothDeviceBlueZ() {
   bluez::BluezDBusManager::Get()
       ->GetBluetoothGattServiceClient()
       ->RemoveObserver(this);
 
-  bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->RemoveObserver(
-      this);
   // Copy the GATT services list here and clear the original so that when we
   // send GattServiceRemoved(), GetGattServices() returns no services.
   GattServiceMap gatt_services_swapped;
   gatt_services_swapped.swap(gatt_services_);
   for (const auto& iter : gatt_services_swapped) {
     DCHECK(adapter());
     adapter()->NotifyGattServiceRemoved(
         static_cast<BluetoothRemoteGattServiceBlueZ*>(iter.second));
   }
 }
@@ skipping 390 matching lines @@
 }
 
 BluetoothPairingBlueZ* BluetoothDeviceBlueZ::GetPairing() const {
   return pairing_.get();
 }
 
 BluetoothAdapterBlueZ* BluetoothDeviceBlueZ::adapter() const {
   return static_cast<BluetoothAdapterBlueZ*>(adapter_);
 }
 
-void BluetoothDeviceBlueZ::DevicePropertyChanged(
-    const dbus::ObjectPath& object_path,
-    const std::string& property_name) {
-  if (object_path != object_path_)
-    return;
-
-  bluez::BluetoothDeviceClient::Properties* properties =
-      bluez::BluezDBusManager::Get()->GetBluetoothDeviceClient()->GetProperties(
-          object_path);
-  DCHECK(properties);
-
-  if (property_name == properties->services_resolved.name() &&
-      properties->services_resolved.value()) {
-    VLOG(3) << "All services were discovered for device: "
-            << object_path.value();
-
-    for (const auto iter : newly_discovered_gatt_services_) {
-      adapter()->NotifyGattDiscoveryComplete(
-          static_cast<BluetoothRemoteGattService*>(iter));
-    }
-    newly_discovered_gatt_services_.clear();
-  }
-}
-
 void BluetoothDeviceBlueZ::GattServiceAdded(
     const dbus::ObjectPath& object_path) {
   if (GetGattService(object_path.value())) {
     VLOG(1) << "Remote GATT service already exists: " << object_path.value();
     return;
   }
 
   bluez::BluetoothGattServiceClient::Properties* properties =
       bluez::BluezDBusManager::Get()
           ->GetBluetoothGattServiceClient()
           ->GetProperties(object_path);
   DCHECK(properties);
   if (properties->device.value() != object_path_) {
     VLOG(2) << "Remote GATT service does not belong to this device.";
     return;
   }
 
   VLOG(1) << "Adding new remote GATT service for device: " << GetAddress();
 
   BluetoothRemoteGattServiceBlueZ* service =
       new BluetoothRemoteGattServiceBlueZ(adapter(), this, object_path);
 
-  newly_discovered_gatt_services_.push_back(
-      static_cast<BluetoothRemoteGattServiceBlueZ*>(service));
-
   gatt_services_.set(service->GetIdentifier(),
                      std::unique_ptr<BluetoothRemoteGattService>(service));
   DCHECK(service->object_path() == object_path);
   DCHECK(service->GetUUID().IsValid());
 
   DCHECK(adapter());
   adapter()->NotifyGattServiceAdded(service);
 }
 
 void BluetoothDeviceBlueZ::GattServiceRemoved(
@@ skipping 10 matching lines @@
 
   VLOG(1) << "Removing remote GATT service with UUID: '"
           << service->GetUUID().canonical_value()
           << "' from device: " << GetAddress();
 
   DCHECK(service->object_path() == object_path);
   std::unique_ptr<BluetoothRemoteGattService> scoped_service =
       gatt_services_.take_and_erase(iter->first);
 
   DCHECK(adapter());
+  discovery_complete_notified_.erase(service);
   adapter()->NotifyGattServiceRemoved(service);
 }
 
-void BluetoothDeviceBlueZ::InitializeGattServiceMap() {
+void BluetoothDeviceBlueZ::UpdateGattServices(
+    const dbus::ObjectPath& object_path) {
+  if (object_path != object_path_) {
+    // No need to update map if update is for a different device.
+    return;
+  }
+
+  DCHECK(IsGattServicesDiscoveryComplete());
+
+  const std::vector<dbus::ObjectPath> gatt_services =
+      bluez::BluezDBusManager::Get()
+          ->GetBluetoothGattServiceClient()
+          ->GetServices();
+  for (const auto& service_object_path : gatt_services) {
+    // Add all previously unknown GATT services associated with the device.
+    BluetoothRemoteGattService* service =
+        GetGattService(service_object_path.value());
+    if (service == nullptr) {
+      GattServiceAdded(service_object_path);
+    }
+
+    // Notify of GATT discovery complete if we haven't before.
+    service = GetGattService(service_object_path.value());
+    auto notified_pair = discovery_complete_notified_.insert(service);
+    if (notified_pair.second) {
+      adapter()->NotifyGattDiscoveryComplete(service);
+    }
+  }
+}
+
+void BluetoothDeviceBlueZ::InitializeGattServicesMap() {
   DCHECK(gatt_services_.empty());
 
   if (!IsGattServicesDiscoveryComplete()) {

[vudentz, 2016/07/01 11:16:08]

InitializeGattServicesMap is only called by the constructor so this probably
always evaluate to false.

[ortuno, 2016/07/01 15:51:48]

What if a device is connected and all it's attributes have been discovered
already? Wouldn't this be true in that case?

     VLOG(2) << "Gatt services have not been fully resolved for device "
             << object_path_.value();
     return;
   }
 
   VLOG(3) << "Initializing the list of GATT services associated with device "
           << object_path_.value();
 
   // Add all known GATT services associated with the device.
   const std::vector<dbus::ObjectPath> gatt_services =
       bluez::BluezDBusManager::Get()
           ->GetBluetoothGattServiceClient()
           ->GetServices();
   for (const auto& it : gatt_services)
     GattServiceAdded(it);
 
   // Notify on the discovery complete for each service which is found in the
   // first discovery.
   DCHECK(adapter());
-  for (const auto& iter : gatt_services_)
+  for (const auto& iter : gatt_services_) {

[vudentz, 2016/07/01 11:16:08]

What is the purpose of iterating to gatt_services twice? Can't this be done with
the first iteration?

[ortuno, 2016/07/01 15:51:48]

I combined the two for loops.

+    auto notified_pair = discovery_complete_notified_.insert(iter.second);
+    DCHECK(notified_pair.second);
     adapter()->NotifyGattDiscoveryComplete(iter.second);
+  }
 }
 
 void BluetoothDeviceBlueZ::OnGetConnInfo(const ConnectionInfoCallback& callback,
                                          int16_t rssi,
                                          int16_t transmit_power,
                                          int16_t max_transmit_power) {
   callback.Run(ConnectionInfo(rssi, transmit_power, max_transmit_power));
 }
 
 void BluetoothDeviceBlueZ::OnGetConnInfoError(
@@ skipping 171 matching lines @@
 void BluetoothDeviceBlueZ::OnForgetError(const ErrorCallback& error_callback,
                                          const std::string& error_name,
                                          const std::string& error_message) {
   LOG(WARNING) << object_path_.value()
                << ": Failed to remove device: " << error_name << ": "
                << error_message;
   error_callback.Run();
 }
 
 }  // namespace bluez