diff --git a/internal/platform/ble.h b/internal/platform/ble.h index 7aafbce6..6eef673c 100644 --- a/internal/platform/ble.h +++ b/internal/platform/ble.h @@ -295,17 +295,6 @@ class GattClient final { return impl_->WriteCharacteristic(characteristic, value, write_type); } - // TODO(qinwangz): We should not need `on_characteristic_changed_cb` when - // unsubscribing. - // NOLINTNEXTLINE(google3-legacy-absl-backports) - bool SetCharacteristicSubscription( - const api::ble::GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) { - return impl_->SetCharacteristicSubscription( - characteristic, enable, std::move(on_characteristic_changed_cb)); - } - void Disconnect() { impl_->Disconnect(); } // Returns true if a client_gatt_connection is usable. If this method diff --git a/internal/platform/ble_test.cc b/internal/platform/ble_test.cc index 9b43629d..6c7ee843 100644 --- a/internal/platform/ble_test.cc +++ b/internal/platform/ble_test.cc @@ -55,7 +55,6 @@ using ::nearby::api::ble::BleAdvertisementData; using ::nearby::api::ble::GattCharacteristic; using ::nearby::api::ble::TxPowerLevel; using ::testing::Optional; -using ::testing::status::StatusIs; constexpr absl::Duration kWaitDuration = absl::Milliseconds(1000); constexpr absl::string_view kAdvertisementString = "\x0a\x0b\x0c\x0d"; @@ -771,83 +770,5 @@ TEST_F(BleMediumTest, GattClientOperatiosOnCharacteristic) { env_.Stop(); } -TEST_F(BleMediumTest, GattClientSubscribeNotificationGattServerCanNotify) { - env_.Start(); - BluetoothAdapter adapter_a; - BluetoothAdapter adapter_b; - BleMedium ble_a(adapter_a); - BleMedium ble_b(adapter_b); - Uuid service_uuid(1234, 5678); - Uuid characteristic_uuid(5678, 1234); - GattCharacteristic::Permission permissions = - GattCharacteristic::Permission::kRead; - GattCharacteristic::Property properties = - GattCharacteristic::Property::kRead | - GattCharacteristic::Property::kNotify; - - // Start GattServer - std::unique_ptr gatt_server = - ble_a.StartGattServer(/*ServerGattConnectionCallback=*/{}); - - ASSERT_NE(gatt_server, nullptr); - // Add characteristic and its value. - // NOLINTNEXTLINE(google3-legacy-absl-backports) - std::optional server_characteristic = - gatt_server->CreateCharacteristic(service_uuid, characteristic_uuid, - permissions, properties); - EXPECT_TRUE(gatt_server->UpdateCharacteristic(server_characteristic.value(), - ByteArray("any"))); - - // Start GattClient - MacAddress mac_address = adapter_a.GetAddress(); - std::unique_ptr gatt_client = ble_b.ConnectToGattServer( - BlePeripheral(ble_b, mac_address.address()), kTxPowerLevel, - /*ClientGattConnectionCallback=*/{}); - ASSERT_NE(gatt_client, nullptr); - - EXPECT_TRUE(gatt_client->DiscoverServiceAndCharacteristics( - service_uuid, {characteristic_uuid})); - - // Subscribes notification - EXPECT_TRUE(gatt_client->SetCharacteristicSubscription( - server_characteristic.value(), true, - [](absl::string_view value) { EXPECT_EQ(value, "hello"); })); - - // Sends notification - EXPECT_EQ(gatt_server->NotifyCharacteristicChanged( - server_characteristic.value(), false, ByteArray("hello")), - absl::OkStatus()); - - std::string notified_value; - CountDownLatch latch(1); - // Subscribes notification - EXPECT_TRUE(gatt_client->SetCharacteristicSubscription( - server_characteristic.value(), true, [&](absl::string_view value) { - notified_value = value; - latch.CountDown(); - })); - // Sends indication - EXPECT_EQ(gatt_server->NotifyCharacteristicChanged( - server_characteristic.value(), true, ByteArray("any")), - absl::OkStatus()); - latch.Await(); - EXPECT_EQ(notified_value, "any"); - - // Unsubscribes notification - EXPECT_TRUE(gatt_client->SetCharacteristicSubscription( - server_characteristic.value(), false, - [&](absl::string_view value) { GTEST_FAIL(); })); - EXPECT_THAT(gatt_server->NotifyCharacteristicChanged( - server_characteristic.value(), true, ByteArray("any")), - StatusIs(absl::StatusCode::kNotFound)); - - gatt_client->Disconnect(); - // Failed to subscribe characteristic notification as gatt is disconnected. - EXPECT_FALSE(gatt_client->SetCharacteristicSubscription( - server_characteristic.value(), true, [](absl::string_view value) {})); - gatt_server->Stop(); - env_.Stop(); -} - } // namespace } // namespace nearby diff --git a/internal/platform/implementation/apple/Tests/ble_gatt_client_test.mm b/internal/platform/implementation/apple/Tests/ble_gatt_client_test.mm index c5969c5f..ae1066a8 100644 --- a/internal/platform/implementation/apple/Tests/ble_gatt_client_test.mm +++ b/internal/platform/implementation/apple/Tests/ble_gatt_client_test.mm @@ -210,19 +210,6 @@ XCTAssertFalse(result); } -- (void)testSetCharacteristicSubscriptionReturnsFalse { - GNCBLEGATTCharacteristic *characteristic = - [[GNCBLEGATTCharacteristic alloc] initWithUUID:[CBUUID UUIDWithString:@"B2B4"] - serviceUUID:[CBUUID UUIDWithString:@"FEF3"] - permissions:CBAttributePermissionsReadable - properties:CBCharacteristicPropertyNotify]; - nearby::api::ble::GattCharacteristic cppCharacteristic = - nearby::apple::CPPGATTCharacteristicFromObjC(characteristic); - BOOL result = _gattClient->SetCharacteristicSubscription(cppCharacteristic, true, - [](absl::string_view value) {}); - XCTAssertFalse(result); -} - - (void)testDisconnectWhenFlagEnabled { nearby::NearbyFlags::GetInstance().OverrideBoolFlagValue( nearby::connections::config_package_nearby::nearby_connections_feature:: diff --git a/internal/platform/implementation/apple/ble_gatt_client.h b/internal/platform/implementation/apple/ble_gatt_client.h index f375586c..1be7ae1e 100644 --- a/internal/platform/implementation/apple/ble_gatt_client.h +++ b/internal/platform/implementation/apple/ble_gatt_client.h @@ -66,16 +66,6 @@ class GattClient : public api::ble::GattClient { bool WriteCharacteristic(const api::ble::GattCharacteristic &characteristic, absl::string_view value, api::ble::GattClient::WriteType type) override; - // Enable or disable notifications/indications for a given characteristic. - // - // Once notifications are enabled for a characteristic, on_characteristic_changed_cb will be - // triggered if the remote device indicates that the given characteristic has changed. - // - // Returns whether or not the subscription was successful. - bool SetCharacteristicSubscription( - const api::ble::GattCharacteristic &characteristic, bool enable, - absl::AnyInvocable on_characteristic_changed_cb) override; - // Disconnects an established connection, or cancels a connection attempt currently in progress. void Disconnect() override; diff --git a/internal/platform/implementation/apple/ble_gatt_client.mm b/internal/platform/implementation/apple/ble_gatt_client.mm index 655878cf..4b1a07f2 100644 --- a/internal/platform/implementation/apple/ble_gatt_client.mm +++ b/internal/platform/implementation/apple/ble_gatt_client.mm @@ -121,13 +121,6 @@ bool GattClient::WriteCharacteristic(const api::ble::GattCharacteristic &charact return false; } -// TODO(b/290385712): Implement. -bool GattClient::SetCharacteristicSubscription( - const api::ble::GattCharacteristic &characteristic, bool enable, - absl::AnyInvocable on_characteristic_changed_cb) { - return false; -} - void GattClient::Disconnect() { // There seems to be an issue between some iOS<>Android device pairs where the Android device will // not connect to the iOS device if the iOS device disconnects and then attempts to reconnect. diff --git a/internal/platform/implementation/ble.h b/internal/platform/implementation/ble.h index 00866e1e..beb7a13b 100644 --- a/internal/platform/implementation/ble.h +++ b/internal/platform/implementation/ble.h @@ -248,14 +248,6 @@ class GattClient { virtual bool WriteCharacteristic(const GattCharacteristic& characteristic, absl::string_view value, WriteType type) = 0; - // https://developer.android.com/reference/android/bluetooth/BluetoothGatt.html#setCharacteristicNotification(android.bluetooth.BluetoothGattCharacteristic,%20boolean) - // - // Enable or disable notifications/indications for a given characteristic. - virtual bool SetCharacteristicSubscription( - const GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) = 0; - // https://developer.android.com/reference/android/bluetooth/BluetoothGatt.html#disconnect() virtual void Disconnect() = 0; }; diff --git a/internal/platform/implementation/g3/ble.cc b/internal/platform/implementation/g3/ble.cc index 9e1282b5..573d2cf1 100644 --- a/internal/platform/implementation/g3/ble.cc +++ b/internal/platform/implementation/g3/ble.cc @@ -679,33 +679,6 @@ bool BleMedium::GattClient::WriteCharacteristic( return status.ok(); } -bool BleMedium::GattClient::SetCharacteristicSubscription( - const api::ble::GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) { - absl::MutexLock lock(mutex_); - if (!is_connection_alive_) { - return false; - } - Borrowed borrowed = gatt_server_.Borrow(); - if (!borrowed) { - return false; - } - BleMedium::GattServer* gatt_server = - static_cast(*borrowed); - LOG(INFO) << "G3 Ble SetCharacteristicSubscription, characteristic=(" - << characteristic.service_uuid.Get16BitAsString() << "," - << std::string(characteristic.uuid) << "), enable = " << enable; - if (enable) { - return gatt_server->AddCharacteristicSubscription( - peripheral_id_, characteristic, - std::move(on_characteristic_changed_cb)); - } else { - return gatt_server->RemoveCharacteristicSubscription(peripheral_id_, - characteristic); - } -} - void BleMedium::GattClient::Disconnect() { bool was_alive = is_connection_alive_.exchange(false); if (!was_alive) return; diff --git a/internal/platform/implementation/g3/ble.h b/internal/platform/implementation/g3/ble.h index 86c6daf8..e3eacca3 100644 --- a/internal/platform/implementation/g3/ble.h +++ b/internal/platform/implementation/g3/ble.h @@ -282,11 +282,6 @@ class BleMedium : public api::ble::BleMedium { absl::string_view value, api::ble::GattClient::WriteType write_type) override; - bool SetCharacteristicSubscription( - const api::ble::GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) override; - void Disconnect() override; void OnServerDisconnected(); diff --git a/internal/platform/implementation/windows/ble_gatt_client.cc b/internal/platform/implementation/windows/ble_gatt_client.cc index 83ed7695..3089d819 100644 --- a/internal/platform/implementation/windows/ble_gatt_client.cc +++ b/internal/platform/implementation/windows/ble_gatt_client.cc @@ -431,80 +431,6 @@ bool BleGattClient::WriteCharacteristic( return false; } -bool BleGattClient::SetCharacteristicSubscription( - const api::ble::GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) { - absl::MutexLock lock(mutex_); - VLOG(1) << __func__ << ": Started to set Characteristic Subscription."; - GattClientCharacteristicConfigurationDescriptorValue gcccd_value = - GattClientCharacteristicConfigurationDescriptorValue::None; - if ((characteristic.property & Property::kNotify) != Property::kNone) { - gcccd_value = GattClientCharacteristicConfigurationDescriptorValue::Notify; - } else if ((characteristic.property & Property::kIndicate) != - Property::kNone) { - gcccd_value = - GattClientCharacteristicConfigurationDescriptorValue::Indicate; - } else { - LOG(WARNING) << "Characeristic: " << std::string(characteristic.uuid) - << " supports neither notifications nor indications."; - return false; - } - - std::optional gatt_characteristic; - - gatt_characteristic = - native_characteristic_map_[characteristic].native_characteristic; - - if (!gatt_characteristic.has_value()) { - LOG(ERROR) << __func__ << ": Failed to get native GATT characteristic."; - return false; - } - - // Write characteristic configuration descriptor - if (!WriteCharacteristicConfigurationDescriptor( - gatt_characteristic.value(), - enable - ? gcccd_value - : GattClientCharacteristicConfigurationDescriptorValue::None)) { - return false; - } - - // Set value changed handler - try { - if (enable) { - native_characteristic_map_[characteristic].on_characteristic_changed_cb = - std::move(on_characteristic_changed_cb); - native_characteristic_map_[characteristic].notification_token = - gatt_characteristic->ValueChanged( - [&](GattCharacteristic const& native_characteristic, - GattValueChangedEventArgs args) { - BleGattClient::OnCharacteristicValueChanged(characteristic, - args); - }); - - if (!native_characteristic_map_[characteristic].notification_token) { - LOG(ERROR) << __func__ << ": Failed to add value change handler."; - return false; - } - } else if (native_characteristic_map_[characteristic].notification_token) { - gatt_characteristic->ValueChanged(std::exchange( - native_characteristic_map_[characteristic].notification_token, {})); - } - LOG(ERROR) << __func__ << ": Successfully set Characteristic Subscription."; - return true; - } catch (std::exception exception) { - LOG(ERROR) << __func__ << ": Failed to set Characteristic Subscription." - << exception.what(); - } catch (const winrt::hresult_error& error) { - LOG(ERROR) << __func__ - << ": Failed to set Characteristic Subscription." - " WinRT exception: " - << error.code() << ": " << winrt::to_string(error.message()); - } - return false; -} - void BleGattClient::Disconnect() { absl::MutexLock lock(mutex_); try { diff --git a/internal/platform/implementation/windows/ble_gatt_client.h b/internal/platform/implementation/windows/ble_gatt_client.h index 651001e1..958cc102 100644 --- a/internal/platform/implementation/windows/ble_gatt_client.h +++ b/internal/platform/implementation/windows/ble_gatt_client.h @@ -63,11 +63,6 @@ class BleGattClient : public api::ble::GattClient { api::ble::GattClient::WriteType write_type) override ABSL_LOCKS_EXCLUDED(mutex_); - bool SetCharacteristicSubscription( - const api::ble::GattCharacteristic& characteristic, bool enable, - absl::AnyInvocable - on_characteristic_changed_cb) override ABSL_LOCKS_EXCLUDED(mutex_); - void Disconnect() override ABSL_LOCKS_EXCLUDED(mutex_); private: