Skip to content

Commit

Permalink
device/bluetooth: Improvements to GATT descriptor access API.
Browse files Browse the repository at this point in the history 
This CL introduces the following changes to the GATT API:

* New BluetoothGattService::Observer methods:
 - GattDescriptorAdded
 - GattDescriptorRemoved
 - GattDescriptorValueChanged

* New BluetoothGattCharacteristic::GetDescriptor method to obtain a
characteristic's descriptor by descriptor identifier.

BUG=265663
TEST=device_unittests

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

git-svn-id: svn://svn.chromium.org/chrome/trunk/src@269138 0039d316-1c4b-4281-b951-d872f2087c98
  • Loading branch information
armansito@chromium.org committed May 9, 2014
1 parent 8ed9e06 commit 67ba78a
Show file tree
Hide file tree
Showing 8 changed files with 204 additions and 1 deletion.
5 changes: 5 additions & 0 deletions device/bluetooth/bluetooth_gatt_characteristic.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -138,6 +138,11 @@ class BluetoothGattCharacteristic {
virtual std::vector<BluetoothGattDescriptor*>
GetDescriptors() const = 0;

// Returns the GATT characteristic descriptor with identifier |identifier| if
// it belongs to this GATT characteristic.
virtual BluetoothGattDescriptor* GetDescriptor(
const std::string& identifier) const = 0;

// Adds a characteristic descriptor to the locally hosted characteristic
// represented by this instance. This method only makes sense for local
// characteristics and won't have an effect if this instance represents a
Expand Down
92 changes: 91 additions & 1 deletion device/bluetooth/bluetooth_gatt_chromeos_unittest.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -132,6 +132,9 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
gatt_characteristic_added_count_(0),
gatt_characteristic_removed_count_(0),
gatt_characteristic_value_changed_count_(0),
gatt_descriptor_added_count_(0),
gatt_descriptor_removed_count_(0),
gatt_descriptor_value_changed_count_(0),
device_address_(device->GetAddress()),
gatt_service_id_(service->GetIdentifier()),
adapter_(adapter) {
Expand Down Expand Up @@ -200,10 +203,61 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
++gatt_characteristic_value_changed_count_;
last_gatt_characteristic_id_ = characteristic->GetIdentifier();
last_gatt_characteristic_uuid_ = characteristic->GetUUID();
last_changed_characteristic_value_ = characteristic->GetValue();
last_changed_characteristic_value_ = value;

EXPECT_EQ(service->GetCharacteristic(last_gatt_characteristic_id_),
characteristic);
EXPECT_EQ(service, characteristic->GetService());

QuitMessageLoop();
}

virtual void GattDescriptorAdded(
BluetoothGattCharacteristic* characteristic,
BluetoothGattDescriptor* descriptor) OVERRIDE {
ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());

++gatt_descriptor_added_count_;
last_gatt_descriptor_id_ = descriptor->GetIdentifier();
last_gatt_descriptor_uuid_ = descriptor->GetUUID();

EXPECT_EQ(characteristic->GetDescriptor(last_gatt_descriptor_id_),
descriptor);
EXPECT_EQ(characteristic, descriptor->GetCharacteristic());

QuitMessageLoop();
}

virtual void GattDescriptorRemoved(
BluetoothGattCharacteristic* characteristic,
BluetoothGattDescriptor* descriptor) OVERRIDE {
ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());

++gatt_descriptor_removed_count_;
last_gatt_descriptor_id_ = descriptor->GetIdentifier();
last_gatt_descriptor_uuid_ = descriptor->GetUUID();

// The characteristic should return NULL for this descriptor..
EXPECT_FALSE(characteristic->GetDescriptor(last_gatt_descriptor_id_));
EXPECT_EQ(characteristic, descriptor->GetCharacteristic());

QuitMessageLoop();
}

virtual void GattDescriptorValueChanged(
BluetoothGattCharacteristic* characteristic,
BluetoothGattDescriptor* descriptor,
const std::vector<uint8>& value) OVERRIDE {
ASSERT_EQ(gatt_service_id_, characteristic->GetService()->GetIdentifier());

++gatt_descriptor_value_changed_count_;
last_gatt_descriptor_id_ = descriptor->GetIdentifier();
last_gatt_descriptor_uuid_ = descriptor->GetUUID();
last_changed_descriptor_value_ = value;

EXPECT_EQ(characteristic->GetDescriptor(last_gatt_descriptor_id_),
descriptor);
EXPECT_EQ(characteristic, descriptor->GetCharacteristic());

QuitMessageLoop();
}
Expand All @@ -212,9 +266,15 @@ class TestGattServiceObserver : public BluetoothGattService::Observer {
int gatt_characteristic_added_count_;
int gatt_characteristic_removed_count_;
int gatt_characteristic_value_changed_count_;
int gatt_descriptor_added_count_;
int gatt_descriptor_removed_count_;
int gatt_descriptor_value_changed_count_;
std::string last_gatt_characteristic_id_;
BluetoothUUID last_gatt_characteristic_uuid_;
std::vector<uint8> last_changed_characteristic_value_;
std::string last_gatt_descriptor_id_;
BluetoothUUID last_gatt_descriptor_uuid_;
std::vector<uint8> last_changed_descriptor_value_;

private:
// Some tests use a message loop since background processing is simulated;
Expand Down Expand Up @@ -501,13 +561,21 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {

TestGattServiceObserver service_observer(adapter_, device, service);
EXPECT_EQ(0, service_observer.gatt_service_changed_count_);
EXPECT_EQ(0, service_observer.gatt_descriptor_added_count_);
EXPECT_EQ(0, service_observer.gatt_descriptor_removed_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_);

// Only the Heart Rate Measurement characteristic has a descriptor.
EXPECT_EQ(1, service_observer.gatt_descriptor_added_count_);
EXPECT_EQ(0, service_observer.gatt_descriptor_removed_count_);
EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);

BluetoothGattCharacteristic* characteristic = service->GetCharacteristic(
fake_bluetooth_gatt_characteristic_client_->
GetBodySensorLocationPath().value());
Expand All @@ -530,25 +598,40 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorAddedAndRemoved) {
EXPECT_FALSE(descriptor->IsLocal());
EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
descriptor->GetUUID());
EXPECT_EQ(descriptor->GetUUID(),
service_observer.last_gatt_descriptor_uuid_);
EXPECT_EQ(descriptor->GetIdentifier(),
service_observer.last_gatt_descriptor_id_);

// Hide the descriptor.
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(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_);

// Expose the descriptor again.
service_observer.last_gatt_descriptor_id_.clear();
service_observer.last_gatt_descriptor_uuid_ = BluetoothUUID();
fake_bluetooth_gatt_descriptor_client_->ExposeDescriptor(
dbus::ObjectPath(characteristic->GetIdentifier()),
FakeBluetoothGattDescriptorClient::
kClientCharacteristicConfigurationUUID);
EXPECT_EQ(6, 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_);
EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);

descriptor = characteristic->GetDescriptors()[0];
EXPECT_FALSE(descriptor->IsLocal());
EXPECT_EQ(BluetoothGattDescriptor::ClientCharacteristicConfigurationUuid(),
descriptor->GetUUID());
EXPECT_EQ(descriptor->GetUUID(), service_observer.last_gatt_descriptor_uuid_);
EXPECT_EQ(descriptor->GetIdentifier(),
service_observer.last_gatt_descriptor_id_);
}

TEST_F(BluetoothGattChromeOSTest, AdapterAddedAfterGattService) {
Expand Down Expand Up @@ -798,6 +881,7 @@ 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_descriptor_value_changed_count_);
EXPECT_TRUE(service->GetCharacteristics().empty());

// Run the message loop so that the characteristics appear.
Expand Down Expand Up @@ -834,6 +918,8 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
EXPECT_EQ(1, success_callback_count_);
EXPECT_EQ(0, error_callback_count_);
EXPECT_TRUE(ValuesEqual(last_read_value_, descriptor->GetValue()));
EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
EXPECT_EQ(0, service_observer.gatt_descriptor_value_changed_count_);

// Write value.
desc_value[0] = 0x03;
Expand All @@ -847,6 +933,8 @@ TEST_F(BluetoothGattChromeOSTest, GattDescriptorValue) {
EXPECT_EQ(0, error_callback_count_);
EXPECT_FALSE(ValuesEqual(last_read_value_, descriptor->GetValue()));
EXPECT_TRUE(ValuesEqual(desc_value, descriptor->GetValue()));
EXPECT_EQ(4, service_observer.gatt_service_changed_count_);
EXPECT_EQ(1, service_observer.gatt_descriptor_value_changed_count_);

// Read new value.
descriptor->ReadRemoteDescriptor(
Expand All @@ -858,6 +946,8 @@ 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(1, service_observer.gatt_descriptor_value_changed_count_);
}

} // namespace chromeos
28 changes: 28 additions & 0 deletions device/bluetooth/bluetooth_gatt_service.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -173,13 +173,41 @@ class BluetoothGattService {
BluetoothGattService* service,
BluetoothGattCharacteristic* characteristic) {}

// Called when the remote GATT characteristic descriptor |descriptor|
// belonging to characteristic |characteristic| has been discovered. Don't
// 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).
virtual void GattDescriptorRemoved(
BluetoothGattCharacteristic* characteristic,
BluetoothGattDescriptor* descriptor) {}

// Called when the value of a characteristic has changed. This might be a
// result of a read/write request to, or a notification/indication from, a
// remote GATT characteristic.
virtual void GattCharacteristicValueChanged(
BluetoothGattService* service,
BluetoothGattCharacteristic* characteristic,
const std::vector<uint8>& value) {}

// Called when the value of a characteristic descriptor has been updated.
virtual void GattDescriptorValueChanged(
BluetoothGattCharacteristic* characteristic,
BluetoothGattDescriptor* descriptor,
const std::vector<uint8>& value) {}
};

// The ErrorCallback is used by methods to asynchronously report errors.
Expand Down
28 changes: 28 additions & 0 deletions device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -114,6 +114,16 @@ BluetoothRemoteGattCharacteristicChromeOS::GetDescriptors() const {
return descriptors;
}

device::BluetoothGattDescriptor*
BluetoothRemoteGattCharacteristicChromeOS::GetDescriptor(
const std::string& identifier) const {
DescriptorMap::const_iterator iter =
descriptors_.find(dbus::ObjectPath(identifier));
if (iter == descriptors_.end())
return NULL;
return iter->second;
}

bool BluetoothRemoteGattCharacteristicChromeOS::AddDescriptor(
device::BluetoothGattDescriptor* descriptor) {
VLOG(1) << "Descriptors cannot be added to a remote GATT characteristic.";
Expand Down Expand Up @@ -194,6 +204,8 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorAdded(
DCHECK(descriptor->GetIdentifier() == object_path.value());
DCHECK(descriptor->GetUUID().IsValid());
DCHECK(service_);

service_->NotifyDescriptorAddedOrRemoved(this, descriptor, true /* added */);
service_->NotifyServiceChanged();
}

Expand All @@ -211,9 +223,12 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorRemoved(
BluetoothRemoteGattDescriptorChromeOS* descriptor = iter->second;
DCHECK(descriptor->object_path() == object_path);
descriptors_.erase(iter);

service_->NotifyDescriptorAddedOrRemoved(this, descriptor, false /* added */);
delete descriptor;

DCHECK(service_);

service_->NotifyServiceChanged();
}

Expand All @@ -224,8 +239,21 @@ void BluetoothRemoteGattCharacteristicChromeOS::GattDescriptorPropertyChanged(
if (iter == descriptors_.end())
return;

// Ignore all property changes except for "Value".
BluetoothGattDescriptorClient::Properties* properties =
DBusThreadManager::Get()->GetBluetoothGattDescriptorClient()->
GetProperties(object_path);
DCHECK(properties);
if (property_name != properties->value.name())
return;

VLOG(1) << "GATT descriptor property changed: " << object_path.value()
<< ", property: " << property_name;

DCHECK(service_);

service_->NotifyDescriptorValueChanged(
this, iter->second, properties->value.value());
}

void BluetoothRemoteGattCharacteristicChromeOS::OnGetValue(
Expand Down
2 changes: 2 additions & 0 deletions device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -44,6 +44,8 @@ class BluetoothRemoteGattCharacteristicChromeOS
virtual Permissions GetPermissions() const OVERRIDE;
virtual std::vector<device::BluetoothGattDescriptor*>
GetDescriptors() const OVERRIDE;
virtual device::BluetoothGattDescriptor* GetDescriptor(
const std::string& identifier) const OVERRIDE;
virtual bool AddDescriptor(
device::BluetoothGattDescriptor* descriptor) OVERRIDE;
virtual bool UpdateValue(const std::vector<uint8>& value) OVERRIDE;
Expand Down
27 changes: 27 additions & 0 deletions device/bluetooth/bluetooth_remote_gatt_service_chromeos.cc
View file
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
#include "chromeos/dbus/dbus_thread_manager.h"
#include "device/bluetooth/bluetooth_device_chromeos.h"
#include "device/bluetooth/bluetooth_remote_gatt_characteristic_chromeos.h"
#include "device/bluetooth/bluetooth_remote_gatt_descriptor_chromeos.h"

namespace chromeos {

Expand Down Expand Up @@ -164,6 +165,32 @@ void BluetoothRemoteGattServiceChromeOS::NotifyServiceChanged() {
GattServiceChanged(this));
}

void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorAddedOrRemoved(
BluetoothRemoteGattCharacteristicChromeOS* characteristic,
BluetoothRemoteGattDescriptorChromeOS* descriptor,
bool added) {
DCHECK(characteristic->GetService() == this);
DCHECK(descriptor->GetCharacteristic() == characteristic);
if (added) {
FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
GattDescriptorAdded(characteristic, descriptor));
return;
}
FOR_EACH_OBSERVER(device::BluetoothGattService::Observer, observers_,
GattDescriptorRemoved(characteristic, descriptor));
}

void BluetoothRemoteGattServiceChromeOS::NotifyDescriptorValueChanged(
BluetoothRemoteGattCharacteristicChromeOS* characteristic,
BluetoothRemoteGattDescriptorChromeOS* descriptor,
const std::vector<uint8>& value) {
DCHECK(characteristic->GetService() == this);
DCHECK(descriptor->GetCharacteristic() == characteristic);
FOR_EACH_OBSERVER(
device::BluetoothGattService::Observer, observers_,
GattDescriptorValueChanged(characteristic, descriptor, value));
}

void BluetoothRemoteGattServiceChromeOS::GattServicePropertyChanged(
const dbus::ObjectPath& object_path,
const std::string& property_name){
Expand Down
20 changes: 20 additions & 0 deletions device/bluetooth/bluetooth_remote_gatt_service_chromeos.h
View file
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@ namespace chromeos {

class BluetoothDeviceChromeOS;
class BluetoothRemoteGattCharacteristicChromeOS;
class BluetoothRemoteGattDescriptorChromeOS;

// The BluetoothRemoteGattServiceChromeOS class implements BluetootGattService
// for remote GATT services on the the Chrome OS platform.
Expand Down Expand Up @@ -68,6 +69,25 @@ class BluetoothRemoteGattServiceChromeOS
// service observers when characteristic descriptors get added and removed.
void NotifyServiceChanged();

// Notifies its observers that a descriptor |descriptor| belonging to
// characteristic |characteristic| has been added or removed. This is used
// by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
// observers when characteristic descriptors get added and removed. If |added|
// is true, an "Added" event will be sent. Otherwise, a "Removed" event will
// be sent.
void NotifyDescriptorAddedOrRemoved(
BluetoothRemoteGattCharacteristicChromeOS* characteristic,
BluetoothRemoteGattDescriptorChromeOS* descriptor,
bool added);

// Notifies its observers that the value of a descriptor has changed. Called
// by BluetoothRemoteGattCharacteristicChromeOS instances to notify service
// observers when the value of one of their descriptors gets updated.
void NotifyDescriptorValueChanged(
BluetoothRemoteGattCharacteristicChromeOS* characteristic,
BluetoothRemoteGattDescriptorChromeOS* descriptor,
const std::vector<uint8>& value);

private:
friend class BluetoothDeviceChromeOS;

Expand Down
Loading

0 comments on commit 67ba78a

Please sign in to comment.