diff options
author | armansito@chromium.org <armansito@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-23 16:49:14 +0000 |
---|---|---|
committer | armansito@chromium.org <armansito@chromium.org@0039d316-1c4b-4281-b951-d872f2087c98> | 2014-07-23 16:49:14 +0000 |
commit | 522f0fc0d9f130af95d197478945cb96b7d902b5 (patch) | |
tree | efda0bdb052f1d4c3d7a5a02de15b2bee591d08d | |
parent | 3f571d0d68775728047fa40b522a5c38f4ae6a9e (diff) | |
download | chromium_src-522f0fc0d9f130af95d197478945cb96b7d902b5.zip chromium_src-522f0fc0d9f130af95d197478945cb96b7d902b5.tar.gz chromium_src-522f0fc0d9f130af95d197478945cb96b7d902b5.tar.bz2 |
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
15 files changed, 139 insertions, 72 deletions
diff --git a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc b/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc index 78fd1ff..07820c2d 100644 --- a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_apitest.cc +++ b/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); diff --git a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc b/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc index b5f5afd..4baeb27 100644 --- a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.cc +++ b/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)); diff --git a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h b/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h index 6421159..c7a169a 100644 --- a/chrome/browser/extensions/api/bluetooth_low_energy/bluetooth_low_energy_event_router.h +++ b/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( diff --git a/chromeos/dbus/bluetooth_gatt_characteristic_client.cc b/chromeos/dbus/bluetooth_gatt_characteristic_client.cc index 823a101..45cae14 100644 --- a/chromeos/dbus/bluetooth_gatt_characteristic_client.cc +++ b/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, ¬ifying); + RegisterProperty(bluetooth_gatt_characteristic::kNotifyingProperty, + ¬ifying); 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, diff --git a/chromeos/dbus/bluetooth_gatt_characteristic_client.h b/chromeos/dbus/bluetooth_gatt_characteristic_client.h index 29b339a..ab5fd70 100644 --- a/chromeos/dbus/bluetooth_gatt_characteristic_client.h +++ b/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); diff --git a/chromeos/dbus/bluetooth_gatt_service_client.cc b/chromeos/dbus/bluetooth_gatt_service_client.cc index 83ce29a..aa671e6 100644 --- a/chromeos/dbus/bluetooth_gatt_service_client.cc +++ b/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() { diff --git a/chromeos/dbus/bluetooth_gatt_service_client.h b/chromeos/dbus/bluetooth_gatt_service_client.h index d3ed19d..36d2123 100644 --- a/chromeos/dbus/bluetooth_gatt_service_client.h +++ b/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; diff --git a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc b/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc index 04745e6..3567567 100644 --- a/chromeos/dbus/fake_bluetooth_gatt_characteristic_client.cc +++ b/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() { diff --git a/chromeos/dbus/fake_bluetooth_gatt_service_client.cc b/chromeos/dbus/fake_bluetooth_gatt_service_client.cc index ae2b0b4..2bb68c2c 100644 --- a/chromeos/dbus/fake_bluetooth_gatt_service_client.cc +++ b/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 diff --git a/device/bluetooth/bluetooth_device_chromeos.cc b/device/bluetooth/bluetooth_device_chromeos.cc index de46e92..f607247 100644 --- a/device/bluetooth/bluetooth_device_chromeos.cc +++ b/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; } diff --git a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc b/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc index 673ed5f..a585f81 100644 --- a/device/bluetooth/bluetooth_gatt_chromeos_unittest.cc +++ b/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_); } diff --git a/device/bluetooth/bluetooth_gatt_service.h b/device/bluetooth/bluetooth_gatt_service.h index 16f4ae5..583c136 100644 --- a/device/bluetooth/bluetooth_gatt_service.h +++ b/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) {} diff --git a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc index 83231b9..ad15a06 100644 --- a/device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc +++ b/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( diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc index 7038270..f7e99b6 100644 --- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc +++ b/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; } diff --git a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h b/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h index ce481ec..600233d 100644 --- a/device/bluetooth/bluetooth_remote_gatt_service_chromeos.h +++ b/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_; |