From 4727861f5dc87053ecee95aeabf0c082cf96c73b Mon Sep 17 00:00:00 2001 From: Qin Wang Date: Wed, 1 Mar 2023 14:19:06 -0800 Subject: [PATCH] Refactor GattCharactristic to support bitwise combination of Permission and Property PiperOrigin-RevId: 513339751 --- connections/implementation/mediums/ble_v2.cc | 10 +-- internal/platform/ble_v2.h | 9 ++- internal/platform/ble_v2_test.cc | 23 +++---- .../implementation/apple/Tests/GNCBleTest.mm | 6 +- internal/platform/implementation/apple/ble.h | 4 +- internal/platform/implementation/apple/ble.mm | 64 ++++++++---------- internal/platform/implementation/ble_v2.h | 66 +++++++++++++------ internal/platform/implementation/g3/ble_v2.cc | 4 +- internal/platform/implementation/g3/ble_v2.h | 6 +- .../implementation/windows/ble_gatt_client.cc | 40 ++++++----- .../implementation/windows/ble_gatt_server.cc | 56 +++++++++------- .../implementation/windows/ble_gatt_server.h | 6 +- 12 files changed, 157 insertions(+), 137 deletions(-) diff --git a/connections/implementation/mediums/ble_v2.cc b/connections/implementation/mediums/ble_v2.cc index 40a8809a..dd514c41 100644 --- a/connections/implementation/mediums/ble_v2.cc +++ b/connections/implementation/mediums/ble_v2.cc @@ -18,6 +18,7 @@ #include #include #include +#include #include "absl/strings/escaping.h" #include "absl/types/optional.h" @@ -543,10 +544,9 @@ bool BleV2::StartAdvertisementGattServerLocked( bool BleV2::GenerateAdvertisementCharacteristic( int slot, const ByteArray& gatt_advertisement, GattServer& gatt_server) { - std::vector permissions{ - GattCharacteristic::Permission::kRead}; - std::vector properties{ - GattCharacteristic::Property::kRead}; + GattCharacteristic::Permission permission = + GattCharacteristic::Permission::kRead; + GattCharacteristic::Property property = GattCharacteristic::Property::kRead; // NOLINTNEXTLINE(google3-legacy-absl-backports) absl::optional advertiement_uuid = @@ -559,7 +559,7 @@ bool BleV2::GenerateAdvertisementCharacteristic( absl::optional gatt_characteristic = gatt_server.CreateCharacteristic( mediums::bleutils::kCopresenceServiceUuid, *advertiement_uuid, - permissions, properties); + permission, property); if (!gatt_characteristic.has_value()) { NEARBY_LOGS(INFO) << "Unable to create and add a characterstic to the gatt " "server for the advertisement."; diff --git a/internal/platform/ble_v2.h b/internal/platform/ble_v2.h index 5dfb7f70..b6798709 100644 --- a/internal/platform/ble_v2.h +++ b/internal/platform/ble_v2.h @@ -18,6 +18,7 @@ #include #include #include +#include #include "absl/functional/any_invocable.h" #include "absl/types/optional.h" @@ -158,12 +159,10 @@ class GattServer final { // NOLINTNEXTLINE(google3-legacy-absl-backports) absl::optional CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& - permissions, - const std::vector& - properties) { + const api::ble_v2::GattCharacteristic::Permission permission, + const api::ble_v2::GattCharacteristic::Property property) { return impl_->CreateCharacteristic(service_uuid, characteristic_uuid, - permissions, properties); + permission, property); } bool UpdateCharacteristic( diff --git a/internal/platform/ble_v2_test.cc b/internal/platform/ble_v2_test.cc index 9db810b1..ba0d16ee 100644 --- a/internal/platform/ble_v2_test.cc +++ b/internal/platform/ble_v2_test.cc @@ -455,14 +455,13 @@ TEST_F(BleV2MediumTest, CanStartGattServer) { ASSERT_NE(gatt_server, nullptr); - std::vector permissions = { - GattCharacteristic::Permission::kRead}; - std::vector properties = { - GattCharacteristic::Property::kRead}; + GattCharacteristic::Permission permission = + GattCharacteristic::Permission::kRead; + GattCharacteristic::Property property = GattCharacteristic::Property::kRead; // NOLINTNEXTLINE(google3-legacy-absl-backports) absl::optional gatt_characteristic = gatt_server->CreateCharacteristic(service_uuid, characteristic_uuid, - permissions, properties); + permission, property); ASSERT_TRUE(gatt_characteristic.has_value()); @@ -491,10 +490,9 @@ TEST_F(BleV2MediumTest, GattClientConnectToGattServerWorks) { ASSERT_NE(gatt_server, nullptr); - std::vector permissions = { - GattCharacteristic::Permission::kRead}; - std::vector properties = { - GattCharacteristic::Property::kRead}; + GattCharacteristic::Permission permissions = + GattCharacteristic::Permission::kRead; + GattCharacteristic::Property properties = GattCharacteristic::Property::kRead; // Add characteristic and its value. // NOLINTNEXTLINE(google3-legacy-absl-backports) absl::optional server_characteristic = @@ -538,10 +536,9 @@ TEST_F(BleV2MediumTest, GattServerCanNotifyChange) { BleV2Medium ble_a(adapter_a); Uuid service_uuid(1234, 5678); Uuid characteristic_uuid(5678, 1234); - std::vector permissions = { - GattCharacteristic::Permission::kRead}; - std::vector properties = { - GattCharacteristic::Property::kRead}; + GattCharacteristic::Permission permissions = + GattCharacteristic::Permission::kRead; + GattCharacteristic::Property properties = GattCharacteristic::Property::kRead; // Start GattServer std::unique_ptr gatt_server = diff --git a/internal/platform/implementation/apple/Tests/GNCBleTest.mm b/internal/platform/implementation/apple/Tests/GNCBleTest.mm index dfece3c1..d8679575 100644 --- a/internal/platform/implementation/apple/Tests/GNCBleTest.mm +++ b/internal/platform/implementation/apple/Tests/GNCBleTest.mm @@ -87,12 +87,12 @@ static const TxPowerLevel kTxPowerLevel = TxPowerLevel::kHigh; // Test creating characteristic. Uuid service_uuid(1234, 5678); Uuid characteristic_uuid(5678, 1234); - std::vector permissions = {GattCharacteristic::Permission::kRead}; - std::vector properties = {GattCharacteristic::Property::kRead}; + GattCharacteristic::Permission permission = GattCharacteristic::Permission::kRead; + GattCharacteristic::Property property = GattCharacteristic::Property::kRead; // NOLINTNEXTLINE absl::optional gatt_characteristic = - gatt_server->CreateCharacteristic(service_uuid, characteristic_uuid, permissions, properties); + gatt_server->CreateCharacteristic(service_uuid, characteristic_uuid, permission, property); XCTAssertTrue(gatt_characteristic.has_value()); // Test updating characteristic. diff --git a/internal/platform/implementation/apple/ble.h b/internal/platform/implementation/apple/ble.h index 919522e1..ffc009f9 100644 --- a/internal/platform/implementation/apple/ble.h +++ b/internal/platform/implementation/apple/ble.h @@ -152,8 +152,8 @@ class BleMedium : public api::ble_v2::BleMedium { absl::optional CreateCharacteristic( const Uuid &service_uuid, const Uuid &characteristic_uuid, - const std::vector &permissions, - const std::vector &properties) override; + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) override; bool UpdateCharacteristic(const api::ble_v2::GattCharacteristic &characteristic, const nearby::ByteArray &value) override; diff --git a/internal/platform/implementation/apple/ble.mm b/internal/platform/implementation/apple/ble.mm index a4d16f64..baa007f5 100644 --- a/internal/platform/implementation/apple/ble.mm +++ b/internal/platform/implementation/apple/ble.mm @@ -29,45 +29,35 @@ namespace nearby { namespace apple { +using Permission = api::ble_v2::GattCharacteristic::Permission; +using Property = api::ble_v2::GattCharacteristic::Property; + namespace { -CBAttributePermissions PermissionToCBPermissions( - const std::vector& permissions) { +CBAttributePermissions PermissionToCBPermissions(Permission permission) { CBAttributePermissions characteristPermissions = 0; - for (const auto& permission : permissions) { - switch (permission) { - case api::ble_v2::GattCharacteristic::Permission::kRead: - characteristPermissions |= CBAttributePermissionsReadable; - break; - case api::ble_v2::GattCharacteristic::Permission::kWrite: - characteristPermissions |= CBAttributePermissionsWriteable; - break; - case api::ble_v2::GattCharacteristic::Permission::kLast: - case api::ble_v2::GattCharacteristic::Permission::kUnknown: - default:; // fall through - } + if ((permission & Permission::kRead) != Permission::kNone) { + characteristPermissions |= CBAttributePermissionsReadable; + } + if ((permission & Permission::kWrite) != Permission::kNone) { + characteristPermissions |= CBAttributePermissionsWriteable; } return characteristPermissions; } -CBCharacteristicProperties PropertiesToCBProperties( - const std::vector& properties) { +CBCharacteristicProperties PropertiesToCBProperties(Property property) { CBCharacteristicProperties characteristicProperties = 0; - for (const auto& property : properties) { - switch (property) { - case api::ble_v2::GattCharacteristic::Property::kRead: - characteristicProperties |= CBCharacteristicPropertyRead; - break; - case api::ble_v2::GattCharacteristic::Property::kWrite: - characteristicProperties |= CBCharacteristicPropertyWrite; - break; - case api::ble_v2::GattCharacteristic::Property::kIndicate: - characteristicProperties |= CBCharacteristicPropertyIndicate; - break; - case api::ble_v2::GattCharacteristic::Property::kLast: - case api::ble_v2::GattCharacteristic::Property::kUnknown: - default:; // fall through - } + if ((property & Property::kRead) != Property::kNone) { + characteristicProperties |= CBCharacteristicPropertyRead; + } + if ((property & Property::kWrite) != Property::kNone) { + characteristicProperties |= CBCharacteristicPropertyWrite; + } + if ((property & Property::kIndicate) != Property::kNone) { + characteristicProperties |= CBCharacteristicPropertyIndicate; + } + if ((property & Property::kNotify) != Property::kNone) { + characteristicProperties |= CBCharacteristicPropertyNotify; } return characteristicProperties; } @@ -483,12 +473,12 @@ bool BleMedium::IsExtendedAdvertisementsAvailable() { return false; } // NOLINTNEXTLINE absl::optional BleMedium::GattServer::CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& permissions, - const std::vector& properties) { + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) { api::ble_v2::GattCharacteristic characteristic = {.uuid = characteristic_uuid, .service_uuid = service_uuid, - .permissions = permissions, - .properties = properties}; + .permission = permission, + .property = property}; [peripheral_ addCBServiceWithUUID:[CBUUID UUIDWithString:ObjCStringFromCppString( @@ -497,9 +487,9 @@ absl::optional BleMedium::GattServer::CreateCha addCharacteristic:[[CBMutableCharacteristic alloc] initWithType:[CBUUID UUIDWithString:ObjCStringFromCppString(std::string( characteristic.uuid))] - properties:PropertiesToCBProperties(characteristic.properties) + properties:PropertiesToCBProperties(characteristic.property) value:nil - permissions:PermissionToCBPermissions(characteristic.permissions)]]; + permissions:PermissionToCBPermissions(characteristic.permission)]]; return characteristic; } diff --git a/internal/platform/implementation/ble_v2.h b/internal/platform/implementation/ble_v2.h index 9c87b77e..4fce85ab 100644 --- a/internal/platform/implementation/ble_v2.h +++ b/internal/platform/implementation/ble_v2.h @@ -98,41 +98,67 @@ class BlePeripheral { // // Representation of a GATT characteristic. struct GattCharacteristic { + // Represents the GATT characteristic permissions + // This enumeration supports a bitwise combination of its member values. + // |, &, |= of the values are legal. enum class Permission { - kUnknown = 0, - kRead = 1, - kWrite = 2, + kNone = 0, + kRead = 1 << 0, + kWrite = 1 << 1, kLast, }; + // Represents the GATT characteristic properties + // This enumeration supports a bitwise combination of its member values. + // |, &, |= of the values are legal. enum class Property { - kUnknown = 0, - kRead = 1, - kWrite = 2, - kIndicate = 3, + kNone = 0, + kRead = 1 << 0, + kWrite = 1 << 1, + kIndicate = 1 << 2, + kNotify = 1 << 3, kLast, }; Uuid uuid; Uuid service_uuid; - std::vector permissions; - std::vector properties; + Permission permission; + Property property; + + // overloading operator for enum class Permission and Property + friend inline Permission operator|(Permission a, Permission b) { + return static_cast(static_cast(a) | static_cast(b)); + } + + friend inline Permission operator&(Permission a, Permission b) { + return static_cast(static_cast(a) & static_cast(b)); + } + friend inline Permission& operator|=(Permission& a, Permission b) { + a = a | b; + return a; + } + + friend inline Property operator|(Property a, Property b) { + return static_cast(static_cast(a) | static_cast(b)); + } + + friend inline Property operator&(Property a, Property b) { + return static_cast(static_cast(a) & static_cast(b)); + } + friend inline Property& operator|=(Property& a, Property b) { + a = a | b; + return a; + } // Hashable template friend H AbslHashValue(H h, const GattCharacteristic& s) { - return H::combine(std::move(h), s.uuid, s.service_uuid, s.permissions, - s.properties); + return H::combine(std::move(h), s.uuid, s.service_uuid, s.permission, + s.property); } bool operator==(const GattCharacteristic& rhs) const { - bool has_equal_permissions = - std::is_permutation(this->permissions.begin(), this->permissions.end(), - rhs.permissions.begin(), rhs.permissions.end()); - bool has_equal_properties = - std::is_permutation(this->properties.begin(), this->properties.end(), - rhs.properties.begin(), rhs.properties.end()); return this->uuid == rhs.uuid && this->service_uuid == rhs.service_uuid && - has_equal_permissions && has_equal_properties; + this->permission == rhs.permission && this->property == rhs.property; } }; @@ -214,8 +240,8 @@ class GattServer { // NOLINTNEXTLINE(google3-legacy-absl-backports) virtual absl::optional CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& permissions, - const std::vector& properties) = 0; + GattCharacteristic::Permission permission, + GattCharacteristic::Property property) = 0; // https://developer.android.com/reference/android/bluetooth/BluetoothGattCharacteristic.html#setValue(byte[]) // diff --git a/internal/platform/implementation/g3/ble_v2.cc b/internal/platform/implementation/g3/ble_v2.cc index 5e60986a..c420e9cf 100644 --- a/internal/platform/implementation/g3/ble_v2.cc +++ b/internal/platform/implementation/g3/ble_v2.cc @@ -355,8 +355,8 @@ bool BleV2Medium::IsExtendedAdvertisementsAvailable() { std::optional BleV2Medium::GattServer::CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& permissions, - const std::vector& properties) { + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) { api::ble_v2::GattCharacteristic characteristic = { .uuid = characteristic_uuid, .service_uuid = service_uuid}; return characteristic; diff --git a/internal/platform/implementation/g3/ble_v2.h b/internal/platform/implementation/g3/ble_v2.h index 43223545..c8996cfd 100644 --- a/internal/platform/implementation/g3/ble_v2.h +++ b/internal/platform/implementation/g3/ble_v2.h @@ -211,10 +211,8 @@ class BleV2Medium : public api::ble_v2::BleMedium { public: std::optional CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& - permissions, - const std::vector& - properties) override; + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) override; bool UpdateCharacteristic( const api::ble_v2::GattCharacteristic& characteristic, diff --git a/internal/platform/implementation/windows/ble_gatt_client.cc b/internal/platform/implementation/windows/ble_gatt_client.cc index f7fbff19..ac610af2 100644 --- a/internal/platform/implementation/windows/ble_gatt_client.cc +++ b/internal/platform/implementation/windows/ble_gatt_client.cc @@ -61,7 +61,8 @@ using ::winrt::Windows::Foundation::Collections::IVectorView; using ::winrt::Windows::Storage::Streams::Buffer; using ::winrt::Windows::Storage::Streams::DataReader; using ::winrt::Windows::Storage::Streams::IBuffer; - +using Property = api::ble_v2::GattCharacteristic::Property; +using Permission = api::ble_v2::GattCharacteristic::Permission; } // namespace BleGattClient::BleGattClient(BluetoothLEDevice ble_device) @@ -223,22 +224,27 @@ BleGattClient::GetCharacteristic(const Uuid& service_uuid, // find a way to map it to api::ble_v2::GattCharacteristic. GattCharacteristicProperties properties = gatt_characteristic->CharacteristicProperties(); - - if (properties == GattCharacteristicProperties::Read) { - result.permissions.push_back( - api::ble_v2::GattCharacteristic::Permission::kRead); - result.properties.push_back( - api::ble_v2::GattCharacteristic::Property::kRead); - } else if (properties == GattCharacteristicProperties::Write) { - result.permissions.push_back( - api::ble_v2::GattCharacteristic::Permission::kWrite); - result.properties.push_back( - api::ble_v2::GattCharacteristic::Property::kWrite); - } else if (properties == GattCharacteristicProperties::Indicate) { - result.permissions.push_back( - api::ble_v2::GattCharacteristic::Permission::kRead); - result.properties.push_back( - api::ble_v2::GattCharacteristic::Property::kIndicate); + result.permission = Permission::kNone; + result.property = Property::kNone; + if ((properties & GattCharacteristicProperties::Read) != + GattCharacteristicProperties::None) { + result.permission |= Permission::kRead; + result.property |= Property::kRead; + } + if ((properties & GattCharacteristicProperties::Write) != + GattCharacteristicProperties::None) { + result.permission |= Permission::kWrite; + result.property |= Property::kWrite; + } + if ((properties & GattCharacteristicProperties::Indicate) != + GattCharacteristicProperties::None) { + result.permission |= Permission::kRead; + result.property |= Property::kIndicate; + } + if ((properties & GattCharacteristicProperties::Notify) != + GattCharacteristicProperties::None) { + result.permission |= Permission::kRead; + result.property |= Property::kNotify; } NEARBY_LOGS(VERBOSE) << __func__ << ": Return Characteristic. uuid=" diff --git a/internal/platform/implementation/windows/ble_gatt_server.cc b/internal/platform/implementation/windows/ble_gatt_server.cc index f1434900..d7daa3ec 100644 --- a/internal/platform/implementation/windows/ble_gatt_server.cc +++ b/internal/platform/implementation/windows/ble_gatt_server.cc @@ -77,6 +77,8 @@ using ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: using ::winrt::Windows::Foundation::Collections::IVectorView; using ::winrt::Windows::Storage::Streams::Buffer; using ::winrt::Windows::Storage::Streams::DataWriter; +using Permission = api::ble_v2::GattCharacteristic::Permission; +using Property = api::ble_v2::GattCharacteristic::Property; std::string ConvertGattStatusToString( GattServiceProviderAdvertisementStatus status) { @@ -109,8 +111,8 @@ BleGattServer::BleGattServer(api::BluetoothAdapter* adapter, absl::optional BleGattServer::CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& permissions, - const std::vector& properties) { + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) { NEARBY_LOGS(VERBOSE) << __func__ << ": create characteristic, service_uuid: " << std::string(service_uuid) << ", characteristic_uuid: " << std::string(characteristic_uuid); @@ -126,8 +128,8 @@ BleGattServer::CreateCharacteristic( api::ble_v2::GattCharacteristic gatt_characteristic; gatt_characteristic.uuid = characteristic_uuid; gatt_characteristic.service_uuid = service_uuid; - gatt_characteristic.permissions = permissions; - gatt_characteristic.properties = properties; + gatt_characteristic.permission = permission; + gatt_characteristic.property = property; GattCharacteristicData gatt_characteristic_data; gatt_characteristic_data.gatt_characteristic = gatt_characteristic; @@ -158,12 +160,9 @@ bool BleGattServer::UpdateCharacteristic( if (is_advertising_) { // Make sure the character has indication property. bool is_indicate_characteristic = false; - for (const auto property : it.gatt_characteristic.properties) { - if (property == - api::ble_v2::GattCharacteristic::Property::kIndicate) { - is_indicate_characteristic = true; - break; - } + if ((it.gatt_characteristic.property & Property::kIndicate) != + Property::kNone) { + is_indicate_characteristic = true; } NEARBY_LOGS(INFO) << __func__ @@ -236,32 +235,39 @@ bool BleGattServer::InitializeGattServer() { bool is_read_supported = false; bool is_write_supported = false; bool is_indicate_supported = false; + bool is_notify_supported = false; GattLocalCharacteristicParameters gatt_characteristic_parameters; // Set GATT properties. GattCharacteristicProperties properties = GattCharacteristicProperties::None; - for (const auto& property : - characteristic_data.gatt_characteristic.properties) { - if (property == api::ble_v2::GattCharacteristic::Property::kRead) { - properties |= GattCharacteristicProperties::Read; - is_read_supported = true; - } else if (property == - api::ble_v2::GattCharacteristic::Property::kWrite) { - properties |= GattCharacteristicProperties::Write; - is_write_supported = true; - } else if (property == - api::ble_v2::GattCharacteristic::Property::kIndicate) { - properties |= GattCharacteristicProperties::Indicate; - is_indicate_supported = true; - } + if ((characteristic_data.gatt_characteristic.property & + Property::kRead) != Property::kNone) { + properties |= GattCharacteristicProperties::Read; + is_read_supported = true; + } + if ((characteristic_data.gatt_characteristic.property & + Property::kWrite) != Property::kNone) { + properties |= GattCharacteristicProperties::Write; + is_write_supported = true; + } + if ((characteristic_data.gatt_characteristic.property & + Property::kIndicate) != Property::kNone) { + properties |= GattCharacteristicProperties::Indicate; + is_indicate_supported = true; + } + if ((characteristic_data.gatt_characteristic.property & + Property::kNotify) != Property::kNone) { + properties |= GattCharacteristicProperties::Notify; + is_notify_supported = true; } NEARBY_LOGS(VERBOSE) << __func__ << ": GATT characteristic properties: read=" << is_read_supported << ",write=" << is_write_supported - << ",indicate=" << is_indicate_supported; + << ",indicate=" << is_indicate_supported + << ",notify=" << is_notify_supported; gatt_characteristic_parameters.CharacteristicProperties(properties); gatt_characteristic_parameters.WriteProtectionLevel( diff --git a/internal/platform/implementation/windows/ble_gatt_server.h b/internal/platform/implementation/windows/ble_gatt_server.h index ab98f448..37494207 100644 --- a/internal/platform/implementation/windows/ble_gatt_server.h +++ b/internal/platform/implementation/windows/ble_gatt_server.h @@ -41,10 +41,8 @@ class BleGattServer : public api::ble_v2::GattServer { ~BleGattServer() override = default; absl::optional CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, - const std::vector& - permissions, - const std::vector& properties) - override; + api::ble_v2::GattCharacteristic::Permission permission, + api::ble_v2::GattCharacteristic::Property property) override; bool UpdateCharacteristic( const api::ble_v2::GattCharacteristic& characteristic,