bluetoothLowEnergy: Send onServiceAdded after all characteristics are discovered

This CL makes the following changes:

  - Add support for the new "Characteristics" and "Descriptors" properties
    exposed under org.bluez.GattService1 and org.bluez.GattCharacteristic1,
    respectively.

  - Add device::BluetoothGattService::Observer::GattDiscoveryCompleteForService
    event, which gets called after the first PropertiesChanged signal is
    received for the "Characteristics" property of a service.

  - Change BluetoothLowEnergyEventRouter, so that it sends the "onServiceAdded"
    event not in GattServiceAdded but in GattDiscoveryCompleteForService.

BUG=394537
TEST=browser_tests:BluetoothLowEnergyApiTest,
device_unittests:BluetoothGattChromeOSTest

Review URL: https://codereview.chromium.org/402303002

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@284955 0039d316-1c4b-4281-b951-d872f2087c98

chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc

@@ -330,9 +330,18 @@ IN_PROC_BROWSER_TEST_F(BluetoothLowEnergyApiTest, ServiceEvents) {
   // Cause events to be sent to the extension.
   event_router()->DeviceAdded(mock_adapter_, device0_.get());
 
+  // These will create the identifier mappings.
   event_router()->GattServiceAdded(device0_.get(), service0_.get());
   event_router()->GattServiceAdded(device0_.get(), service1_.get());
+
+  // These will send the onServiceAdded event to apps.
+  event_router()->GattDiscoveryCompleteForService(service0_.get());
+  event_router()->GattDiscoveryCompleteForService(service1_.get());
+
+  // This will send the onServiceChanged event to apps.
   event_router()->GattServiceChanged(service1_.get());
+
+  // This will send the  onServiceRemoved event to apps.
   event_router()->GattServiceRemoved(device0_.get(), service0_.get());
 
   EXPECT_TRUE(listener.WaitUntilSatisfied());
@@ -361,6 +370,7 @@ IN_PROC_BROWSER_TEST_F(BluetoothLowEnergyApiTest, GetRemovedService) {
 
   event_router()->DeviceAdded(mock_adapter_, device0_.get());
   event_router()->GattServiceAdded(device0_.get(), service0_.get());
+  event_router()->GattDiscoveryCompleteForService(service0_.get());
 
   ExtensionTestMessageListener get_service_success_listener("getServiceSuccess",
                                                             true);

chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc

@@ -855,16 +855,6 @@ void BluetoothLowEnergyEventRouter::GattServiceAdded(
   const std::string& service_id = service->GetIdentifier();
   observed_gatt_services_.insert(service_id);
   service_id_to_device_address_[service_id] = device->GetAddress();
-
-  // Signal API event.
-  apibtle::Service api_service;
-  PopulateService(service, &api_service);
-
-  scoped_ptr<base::ListValue> args =
-      apibtle::OnServiceAdded::Create(api_service);
-  scoped_ptr<Event> event(
-      new Event(apibtle::OnServiceAdded::kEventName, args.Pass()));
-  EventRouter::Get(browser_context_)->BroadcastEvent(event.Pass());
 }
 
 void BluetoothLowEnergyEventRouter::GattServiceRemoved(
@@ -896,6 +886,27 @@ void BluetoothLowEnergyEventRouter::GattServiceRemoved(
   EventRouter::Get(browser_context_)->BroadcastEvent(event.Pass());
 }
 
+void BluetoothLowEnergyEventRouter::GattDiscoveryCompleteForService(
+    BluetoothGattService* service) {
+  DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));
+  VLOG(2) << "GATT service discovery complete: " << service->GetIdentifier();
+
+  DCHECK(observed_gatt_services_.find(service->GetIdentifier()) !=
+         observed_gatt_services_.end());
+  DCHECK(service_id_to_device_address_.find(service->GetIdentifier()) !=
+         service_id_to_device_address_.end());
+
+  // Signal the service added event here.
+  apibtle::Service api_service;
+  PopulateService(service, &api_service);
+
+  scoped_ptr<base::ListValue> args =
+      apibtle::OnServiceAdded::Create(api_service);
+  scoped_ptr<Event> event(
+      new Event(apibtle::OnServiceAdded::kEventName, args.Pass()));
+  EventRouter::Get(browser_context_)->BroadcastEvent(event.Pass());
+}
+
 void BluetoothLowEnergyEventRouter::GattServiceChanged(
     BluetoothGattService* service) {
   DCHECK(BrowserThread::CurrentlyOn(BrowserThread::UI));

chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h

@@ -241,6 +241,8 @@ class BluetoothLowEnergyEventRouter
       device::BluetoothGattService* service) OVERRIDE;
 
   // device::BluetoothGattService::Observer overrides.
+  virtual void GattDiscoveryCompleteForService(
+      device::BluetoothGattService* service) OVERRIDE;
   virtual void GattServiceChanged(
       device::BluetoothGattService* service) OVERRIDE;
   virtual void GattCharacteristicAdded(

chromeos/dbus/bluetooth_gatt_characteristic_client.cc

@@ -13,15 +13,6 @@
 
 namespace chromeos {
 
-namespace {
-
-// TODO(armansito): Move these to service_constants.h later.
-const char kNotifyingProperty[] = "Notifying";
-const char kStartNotify[] = "StartNotify";
-const char kStopNotify[] = "StopNotify";
-
-}  // namespace
-
 // static
 const char BluetoothGattCharacteristicClient::kNoResponseError[] =
     "org.chromium.Error.NoResponse";
@@ -36,8 +27,11 @@ BluetoothGattCharacteristicClient::Properties::Properties(
     : dbus::PropertySet(object_proxy, interface_name, callback) {
   RegisterProperty(bluetooth_gatt_characteristic::kUUIDProperty, &uuid);
   RegisterProperty(bluetooth_gatt_characteristic::kServiceProperty, &service);
-  RegisterProperty(kNotifyingProperty, &notifying);
+  RegisterProperty(bluetooth_gatt_characteristic::kNotifyingProperty,
+                   &notifying);
   RegisterProperty(bluetooth_gatt_characteristic::kFlagsProperty, &flags);
+  RegisterProperty(bluetooth_gatt_characteristic::kDescriptorsProperty,
+                   &descriptors);
 }
 
 BluetoothGattCharacteristicClient::Properties::~Properties() {
@@ -158,7 +152,7 @@ class BluetoothGattCharacteristicClientImpl
 
     dbus::MethodCall method_call(
         bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
-        kStartNotify);
+        bluetooth_gatt_characteristic::kStartNotify);
 
     object_proxy->CallMethodWithErrorCallback(
         &method_call,
@@ -184,7 +178,7 @@ class BluetoothGattCharacteristicClientImpl
 
     dbus::MethodCall method_call(
         bluetooth_gatt_characteristic::kBluetoothGattCharacteristicInterface,
-        kStopNotify);
+        bluetooth_gatt_characteristic::kStopNotify);
 
     object_proxy->CallMethodWithErrorCallback(
         &method_call,

chromeos/dbus/bluetooth_gatt_characteristic_client.h

@@ -30,7 +30,7 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     dbus::Property<dbus::ObjectPath> service;
 
     // Whether or not this characteristic is currently sending ValueUpdated
-    // signals.
+    // signals. [read-only]
     dbus::Property<bool> notifying;
 
     // List of flags representing the GATT "Characteristic Properties bit field"
@@ -38,6 +38,10 @@ class CHROMEOS_EXPORT BluetoothGattCharacteristicClient : public DBusClient {
     // descriptor bit field. [read-only, optional]
     dbus::Property<std::vector<std::string> > flags;
 
+    // Array of object paths representing the descriptors of this
+    // characteristic. [read-only]
+    dbus::Property<std::vector<dbus::ObjectPath> > descriptors;
+
     Properties(dbus::ObjectProxy* object_proxy,
                const std::string& interface_name,
                const PropertyChangedCallback& callback);

chromeos/dbus/bluetooth_gatt_service_client.cc

@@ -22,6 +22,8 @@ BluetoothGattServiceClient::Properties::Properties(
   RegisterProperty(bluetooth_gatt_service::kIncludesProperty, &includes);
   RegisterProperty(bluetooth_gatt_service::kDeviceProperty, &device);
   RegisterProperty(bluetooth_gatt_service::kPrimaryProperty, &primary);
+  RegisterProperty(bluetooth_gatt_service::kCharacteristicsProperty,
+                   &characteristics);
 }
 
 BluetoothGattServiceClient::Properties::~Properties() {

chromeos/dbus/bluetooth_gatt_service_client.h

@@ -30,6 +30,10 @@ class CHROMEOS_EXPORT BluetoothGattServiceClient : public DBusClient {
     // Whether or not this service is a primary service.
     dbus::Property<bool> primary;
 
+    // Array of object paths representing the characteristics of this service.
+    // [read-only]
+    dbus::Property<std::vector<dbus::ObjectPath> > characteristics;
+
     // Array of object paths representing the included services of this service.
     // [read-only]
     dbus::Property<std::vector<dbus::ObjectPath> > includes;

chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc

@@ -294,6 +294,11 @@ void FakeBluetoothGattCharacteristicClient::ExposeHeartRateCharacteristics(
           kClientCharacteristicConfigurationUUID));
   DCHECK(ccc_path.IsValid());
   heart_rate_measurement_ccc_desc_path_ = ccc_path.value();
+
+  std::vector<dbus::ObjectPath> desc_paths;
+  desc_paths.push_back(ccc_path);
+
+  heart_rate_measurement_properties_->descriptors.ReplaceValue(desc_paths);
 }
 
 void FakeBluetoothGattCharacteristicClient::HideHeartRateCharacteristics() {

chromeos/dbus/fake_bluetooth_gatt_service_client.cc

@@ -182,6 +182,13 @@ void FakeBluetoothGattServiceClient::ExposeHeartRateCharacteristics() {
           DBusThreadManager::Get()->GetBluetoothGattCharacteristicClient());
   char_client->ExposeHeartRateCharacteristics(
       dbus::ObjectPath(heart_rate_service_path_));
+
+  std::vector<dbus::ObjectPath> char_paths;
+  char_paths.push_back(char_client->GetHeartRateMeasurementPath());
+  char_paths.push_back(char_client->GetBodySensorLocationPath());
+  char_paths.push_back(char_client->GetHeartRateControlPointPath());
+
+  heart_rate_service_properties_->characteristics.ReplaceValue(char_paths);
 }
 
 }  // namespace chromeos

device/bluetooth/bluetooth_device_chromeos.cc

@@ -518,7 +518,7 @@ void BluetoothDeviceChromeOS::GattServiceRemoved(
     const dbus::ObjectPath& object_path) {
   GattServiceMap::iterator iter = gatt_services_.find(object_path.value());
   if (iter == gatt_services_.end()) {
-    VLOG(2) << "Unknown GATT service removed: " << object_path.value();
+    VLOG(3) << "Unknown GATT service removed: " << object_path.value();
     return;
   }
 

device/bluetooth/bluetooth_gatt_chromeos_unittest.cc

@@ -134,6 +134,7 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
                           BluetoothDevice* device,
                           BluetoothGattService* service)
       : gatt_service_changed_count_(0),
+        gatt_discovery_complete_count_(0),
         gatt_characteristic_added_count_(0),
         gatt_characteristic_removed_count_(0),
         gatt_characteristic_value_changed_count_(0),
@@ -160,6 +161,14 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
   }
 
   // BluetoothGattService::Observer overrides.
+  virtual void GattDiscoveryCompleteForService(
+      BluetoothGattService* service) OVERRIDE {
+    ASSERT_EQ(gatt_service_id_, service->GetIdentifier());
+    ++gatt_discovery_complete_count_;
+
+    QuitMessageLoop();
+  }
+
   virtual void GattServiceChanged(BluetoothGattService* service) OVERRIDE {
     ASSERT_EQ(gatt_service_id_, service->GetIdentifier());
     ++gatt_service_changed_count_;
@@ -268,6 +277,7 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
   }
 
   int gatt_service_changed_count_;
+  int gatt_discovery_complete_count_;
   int gatt_characteristic_added_count_;
   int gatt_characteristic_removed_count_;
   int gatt_characteristic_value_changed_count_;
@@ -608,6 +618,7 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicAddedAndRemoved) {
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
@@ -619,7 +630,8 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicAddedAndRemoved) {
   // 3 characteristics should appear. Only 1 of the characteristics sends
   // value changed signals. Service changed should be fired once for
   // descriptor added.
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
@@ -627,16 +639,20 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicAddedAndRemoved) {
 
   // Hide the characteristics. 3 removed signals should be received.
   fake_bluetooth_gatt_characteristic_client_->HideHeartRateCharacteristics();
-  EXPECT_EQ(8, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
-  // Re-expose the heart rate characteristics.
+  // Re-expose the heart rate characteristics. We shouldn't get another
+  // GattDiscoveryCompleteForService call, since the service thinks that
+  // discovery is done. On the bluetoothd side, characteristics will be removed
+  // only if the service will also be subsequently removed.
   fake_bluetooth_gatt_characteristic_client_->ExposeHeartRateCharacteristics(
       fake_bluetooth_gatt_service_client_->GetHeartRateServicePath());
-  EXPECT_EQ(12, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(3, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
@@ -644,7 +660,7 @@ TEST_F(BluetoothGattChromeOSTest, GattCharacteristicAddedAndRemoved) {
 
   // Hide the service. All characteristics should disappear.
   fake_bluetooth_gatt_service_client_->HideHeartRateService();
-  EXPECT_EQ(16, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_added_count_);
   EXPECT_EQ(6, service_observer.gatt_characteristic_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_characteristic_value_changed_count_);
@@ -679,7 +695,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
 
   // Run the message loop so that the characteristics appear.
   base::MessageLoop::current()->Run();
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
 
   // Only the Heart Rate Measurement characteristic has a descriptor.
   EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
@@ -717,7 +733,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
   fake_bluetooth_gatt_descriptor_client_->HideDescriptor(
       dbus::ObjectPath(descriptor->GetIdentifier()));
   EXPECT_TRUE(characteristic->GetDescriptors().empty());
-  EXPECT_EQ(5, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
@@ -729,7 +745,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
       dbus::ObjectPath(characteristic->GetIdentifier()),
       FakeBluetoothGattDescriptorClient::
           kClientCharacteristicConfigurationUUID);
-  EXPECT_EQ(6, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1U, characteristic->GetDescriptors().size());
   EXPECT_EQ(2, service_observer.gatt_descriptor_added_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_removed_count_);
@@ -1001,12 +1017,14 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
 
   TestGattServiceObserver service_observer(adapter_, device, service);
   EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_discovery_complete_count_);
   EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);
   EXPECT_TRUE(service->GetCharacteristics().empty());
 
   // Run the message loop so that the characteristics appear.
   base::MessageLoop::current()->Run();
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(1, service_observer.gatt_discovery_complete_count_);
 
   // Only the Heart Rate Measurement characteristic has a descriptor.
   BluetoothGattCharacteristic* characteristic = service->GetCharacteristic(
@@ -1043,7 +1061,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(0, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_TRUE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 
   // Write value. Writes to this descriptor will fail.
@@ -1058,7 +1076,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);
 
   // Read new value.
@@ -1071,7 +1089,7 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
   EXPECT_EQ(1, error_callback_count_);
   EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
   EXPECT_FALSE(ValuesEqual(desc_value, descriptor->GetValue()));
-  EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
+  EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
   EXPECT_EQ(2, service_observer.gatt_descriptor_value_changed_count_);
 }
 

device/bluetooth/bluetooth_gatt_service.h

@@ -131,16 +131,18 @@ class BluetoothGattService {
   // as well as when successive changes occur during its life cycle.
   class Observer {
    public:
+    // Called when all characteristic and descriptor discovery procedures are
+    // known to be completed for the GATT service |service|. This method will be
+    // called after the initial discovery of a GATT service and will usually be
+    // preceded by calls to GattCharacteristicAdded and GattDescriptorAdded.
+    virtual void GattDiscoveryCompleteForService(
+        BluetoothGattService* service) {}
+
     // Called when properties of the remote GATT service |service| have changed.
     // This will get called for properties such as UUID, as well as for changes
     // to the list of known characteristics and included services. Observers
     // should read all GATT characteristic and descriptors objects and do any
-    // necessary set up required for a changed service. This method may be
-    // called several times, especially when the service is discovered for the
-    // first time. It will be called for each characteristic and characteristic
-    // descriptor that is discovered or removed. Hence this method should be
-    // used to check whether or not all characteristics of a service have been
-    // discovered that correspond to the profile implemented by the Observer.
+    // necessary set up required for a changed service.
     virtual void GattServiceChanged(BluetoothGattService* service) {}
 
     // Called when the remote GATT characteristic |characteristic| belonging to
@@ -153,22 +155,12 @@ class BluetoothGattService {
     // depends on the particular profile the remote device implements, hence the
     // client of a GATT based profile will usually operate on the whole set of
     // characteristics and not just one.
-    //
-    // This method will always be followed by a call to GattServiceChanged,
-    // which can be used by observers to get all the characteristics of a
-    // service and perform the necessary updates. GattCharacteristicAdded exists
-    // mostly for convenience.
     virtual void GattCharacteristicAdded(
         BluetoothGattService* service,
         BluetoothGattCharacteristic* characteristic) {}
 
     // Called when a GATT characteristic |characteristic| belonging to GATT
-    // service |service| has been removed. This method is for convenience
-    // and will be followed by a call to GattServiceChanged (except when called
-    // after the service gets removed) which should be used for bootstrapping a
-    // GATT based profile. See the documentation of GattCharacteristicAdded and
-    // GattServiceChanged for more information. Try to obtain the service from
-    // its device to see whether or not the service has been removed.
+    // service |service| has been removed.
     virtual void GattCharacteristicRemoved(
         BluetoothGattService* service,
         BluetoothGattCharacteristic* characteristic) {}
@@ -178,19 +170,12 @@ class BluetoothGattService {
     // cache the arguments as the pointers may become invalid. Instead, use the
     // specially assigned identifier to obtain a descriptor and cache that
     // identifier as necessary.
-    //
-    // This method will always be followed by a call to GattServiceChanged,
-    // which can be used by observers to get all the characteristics of a
-    // service and perform the necessary updates. GattDescriptorAdded exists
-    // mostly for convenience.
     virtual void GattDescriptorAdded(
         BluetoothGattCharacteristic* characteristic,
         BluetoothGattDescriptor* descriptor) {}
 
     // Called when a GATT characteristic descriptor |descriptor| belonging to
-    // characteristic |characteristic| has been removed. This method is for
-    // convenience and will be followed by a call to GattServiceChanged (except
-    // when called after the service gets removed).
+    // characteristic |characteristic| has been removed.
     virtual void GattDescriptorRemoved(
         BluetoothGattCharacteristic* characteristic,
         BluetoothGattDescriptor* descriptor) {}

device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc

@@ -341,7 +341,7 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
           GetProperties(object_path);
   DCHECK(properties);
   if (properties->characteristic.value() != object_path_) {
-    VLOG(2) << "Remote GATT descriptor does not belong to this characteristic.";
+    VLOG(3) << "Remote GATT descriptor does not belong to this characteristic.";
     return;
   }
 
@@ -356,7 +356,6 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
   DCHECK(service_);
 
   service_->NotifyDescriptorAddedOrRemoved(this, descriptor, true /* added */);
-  service_->NotifyServiceChanged();
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorRemoved(
@@ -374,12 +373,10 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorRemoved(
   DCHECK(descriptor->object_path() == object_path);
   descriptors_.erase(iter);
 
-  service_->NotifyDescriptorAddedOrRemoved(this, descriptor, false /* added */);
-  delete descriptor;
-
   DCHECK(service_);
+  service_->NotifyDescriptorAddedOrRemoved(this, descriptor, false /* added */);
 
-  service_->NotifyServiceChanged();
+  delete descriptor;
 }
 
 void BluetoothRemoteGattCharacteristicChromeOS::OnValueSuccess(

device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc

@@ -22,6 +22,7 @@ BluetoothRemoteGattServiceChromeOS::BluetoothRemoteGattServiceChromeOS(
     : object_path_(object_path),
       adapter_(adapter),
       device_(device),
+      discovery_complete_(false),
       weak_ptr_factory_(this) {
   VLOG(1) << "Creating remote GATT service with identifier: "
           << object_path.value() << ", UUID: " << GetUUID().canonical_value();
@@ -157,6 +158,12 @@ BluetoothRemoteGattServiceChromeOS::GetAdapter() const {
 }
 
 void BluetoothRemoteGattServiceChromeOS::NotifyServiceChanged() {
+  // Don't send service changed unless we know that all characteristics have
+  // already been discovered. This is to prevent spammy events before sending
+  // out the first Gatt
+  if (!discovery_complete_)
+    return;
+
   FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
                     GattServiceChanged(this));
 }
@@ -203,8 +210,27 @@ void BluetoothRemoteGattServiceChromeOS::GattServicePropertyChanged(
   if (object_path != object_path_)
     return;
 
-  VLOG(1) << "Service property changed: " << object_path.value();
-  NotifyServiceChanged();
+  VLOG(1) << "Service property changed: \"" << property_name << "\", "
+          << object_path.value();
+  BluetoothGattServiceClient::Properties* properties =
+      DBusThreadManager::Get()->GetBluetoothGattServiceClient()->GetProperties(
+          object_path);
+  DCHECK(properties);
+
+  if (property_name != properties->characteristics.name()) {
+    NotifyServiceChanged();
+    return;
+  }
+
+  if (discovery_complete_)
+    return;
+
+  VLOG(1) << "All characteristics were discovered for service: "
+          << object_path.value();
+  discovery_complete_ = true;
+  FOR_EACH_OBSERVER(device::BluetoothGattService::Observer,
+                    observers_,
+                    GattDiscoveryCompleteForService(this));
 }
 
 void BluetoothRemoteGattServiceChromeOS::GattCharacteristicAdded(
@@ -235,7 +261,6 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicAdded(
 
   FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
                     GattCharacteristicAdded(this, characteristic));
-  NotifyServiceChanged();
 }
 
 void BluetoothRemoteGattServiceChromeOS::GattCharacteristicRemoved(
@@ -255,7 +280,6 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicRemoved(
 
   FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
                     GattCharacteristicRemoved(this, characteristic));
-  NotifyServiceChanged();
 
   delete characteristic;
 }
@@ -264,7 +288,7 @@ void BluetoothRemoteGattServiceChromeOS::GattCharacteristicPropertyChanged(
     const dbus::ObjectPath& object_path,
     const std::string& property_name) {
   if (characteristics_.find(object_path) == characteristics_.end()) {
-    VLOG(2) << "Properties of unknown characteristic changed";
+    VLOG(3) << "Properties of unknown characteristic changed";
     return;
   }
 

device/bluetooth/bluetooth_remote_gatt_service_chromeos.h

@@ -146,6 +146,10 @@ class BluetoothRemoteGattServiceChromeOS
       CharacteristicMap;
   CharacteristicMap characteristics_;
 
+  // Indicates whether or not the characteristics of this service are known to
+  // have been discovered.
+  bool discovery_complete_;
+
   // Note: This should remain the last member so it'll be destroyed and
   // invalidate its weak pointers before any other members are destroyed.
   base::WeakPtrFactory<BluetoothRemoteGattServiceChromeOS> weak_ptr_factory_;