From 0f4f02c7f00ee8d13d04feee7046e29d77fa8cbd Mon Sep 17 00:00:00 2001 From: Guogang Li Date: Thu, 11 Jul 2024 21:23:43 -0700 Subject: [PATCH] Internal crash fix PiperOrigin-RevId: 651635619 --- .../implementation/windows/ble_gatt_client.cc | 52 ++++---- .../implementation/windows/ble_gatt_client.h | 33 +++-- .../implementation/windows/ble_gatt_server.cc | 75 ++++++++--- .../implementation/windows/ble_gatt_server.h | 24 +++- .../platform/implementation/windows/ble_v2.cc | 120 ++++++++++++++---- .../platform/implementation/windows/ble_v2.h | 52 +++++--- 6 files changed, 248 insertions(+), 108 deletions(-) diff --git a/internal/platform/implementation/windows/ble_gatt_client.cc b/internal/platform/implementation/windows/ble_gatt_client.cc index 7c553252..39b6fa29 100644 --- a/internal/platform/implementation/windows/ble_gatt_client.cc +++ b/internal/platform/implementation/windows/ble_gatt_client.cc @@ -37,6 +37,7 @@ #include "internal/platform/byte_array.h" #include "internal/platform/flags/nearby_platform_feature_flags.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/implementation/windows/bluetooth_adapter.h" #include "internal/platform/implementation/windows/utils.h" #include "internal/platform/logging.h" #include "internal/platform/uuid.h" @@ -101,28 +102,33 @@ std::string GattCommunicationStatusToString(GattCommunicationStatus status) { BleGattClient::BleGattClient(BluetoothLEDevice ble_device) : ble_device_(ble_device) { - NEARBY_LOGS(VERBOSE) << __func__ << ": GATT client is created."; + if (ble_device_ == nullptr) { + NEARBY_LOGS(WARNING) << __func__ << ": ble_device is null."; + } else { + NEARBY_LOGS(INFO) << __func__ << ": GATT client is created, address: " + << uint64_to_mac_address_string( + ble_device_.BluetoothAddress()); + } } BleGattClient::~BleGattClient() { - NEARBY_LOGS(VERBOSE) << __func__ << ": GATT client is released."; + NEARBY_LOGS(INFO) << __func__ << ": GATT client is released."; Disconnect(); } bool BleGattClient::DiscoverServiceAndCharacteristics( const Uuid& service_uuid, const std::vector& characteristic_uuids) { + absl::MutexLock lock(&mutex_); if (!NearbyFlags::GetInstance().GetBoolFlag( platform::config_package_nearby::nearby_platform_feature:: kEnableBleV2Gatt)) { - auto windows_bluetooth_adapter_ = ::winrt::Windows::Devices::Bluetooth:: - BluetoothAdapter::GetDefaultAsync() - .get(); - if (windows_bluetooth_adapter_.IsExtendedAdvertisingSupported()) { + BluetoothAdapter bluetooth_adapter; + if (bluetooth_adapter.IsExtendedAdvertisingSupported()) { NEARBY_LOGS(WARNING) << __func__ << ": GATT is disabled."; return false; } - if (!windows_bluetooth_adapter_.IsCentralRoleSupported()) { + if (!bluetooth_adapter.IsCentralRoleSupported()) { NEARBY_LOGS(ERROR) << __func__ << ": Bluetooth Hardware does not support Central " "Role, which is required to start GATT client."; @@ -192,9 +198,8 @@ bool BleGattClient::DiscoverServiceAndCharacteristics( winrt::to_string(winrt::to_hstring(service.Uuid()))); }); - NEARBY_LOGS(VERBOSE) << __func__ - << ": Found GATT services=" << flat_services - << " from BLE device."; + NEARBY_LOGS(INFO) << __func__ << ": Found GATT services=" << flat_services + << " from BLE device."; // Needs to check each service to make sure it includes all characteristic // uuids. Services may include duplicate service UUID, but each of them may @@ -269,8 +274,8 @@ bool BleGattClient::DiscoverServiceAndCharacteristics( return true; } - NEARBY_LOGS(VERBOSE) << __func__ - << ": Failed to find service and all characteristics."; + NEARBY_LOGS(ERROR) << __func__ + << ": Failed to find service and all characteristics."; } catch (std::exception exception) { NEARBY_LOGS(ERROR) << __func__ << ": Failed to get GATT services. exception: " @@ -288,10 +293,10 @@ bool BleGattClient::DiscoverServiceAndCharacteristics( absl::optional BleGattClient::GetCharacteristic(const Uuid& service_uuid, const Uuid& characteristic_uuid) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(VERBOSE) << __func__ << ": Stared to get characteristic UUID=" << std::string(characteristic_uuid) << " in service UUID=" << std::string(service_uuid); - absl::MutexLock lock(&mutex_); try { std::optional gatt_characteristic = GetNativeCharacteristic(service_uuid, characteristic_uuid); @@ -355,6 +360,7 @@ BleGattClient::GetCharacteristic(const Uuid& service_uuid, absl::optional BleGattClient::ReadCharacteristic( const api::ble_v2::GattCharacteristic& characteristic) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(VERBOSE) << __func__ << ": Read characteristic=" << std::string(characteristic.uuid); try { @@ -411,9 +417,9 @@ absl::optional BleGattClient::ReadCharacteristic( bool BleGattClient::WriteCharacteristic( const api::ble_v2::GattCharacteristic& characteristic, absl::string_view value, api::ble_v2::GattClient::WriteType write_type) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(VERBOSE) << __func__ << ": write characteristic: " << std::string(characteristic.uuid); - absl::MutexLock lock(&mutex_); try { std::optional gatt_characteristic = native_characteristic_map_[characteristic].native_characteristic; @@ -466,6 +472,7 @@ bool BleGattClient::SetCharacteristicSubscription( const api::ble_v2::GattCharacteristic& characteristic, bool enable, absl::AnyInvocable on_characteristic_changed_cb) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(VERBOSE) << __func__ << ": Started to set Characteristic Subscription."; GattClientCharacteristicConfigurationDescriptorValue gcccd_value = @@ -484,11 +491,9 @@ bool BleGattClient::SetCharacteristicSubscription( } std::optional gatt_characteristic; - { - absl::MutexLock lock(&mutex_); - gatt_characteristic = - native_characteristic_map_[characteristic].native_characteristic; - } + + gatt_characteristic = + native_characteristic_map_[characteristic].native_characteristic; if (!gatt_characteristic.has_value()) { NEARBY_LOGS(ERROR) << __func__ @@ -505,7 +510,6 @@ bool BleGattClient::SetCharacteristicSubscription( return false; } - absl::MutexLock lock(&mutex_); // Set value changed handler try { if (enable) { @@ -643,10 +647,10 @@ bool BleGattClient::WriteCharacteristicConfigurationDescriptor( "configuration descriptor"; return true; } - NEARBY_LOGS(VERBOSE) << __func__ - << ": Failed to write client characteristic " - "configuration descriptor with error: " - << GattCommunicationStatusToString(status); + NEARBY_LOGS(ERROR) << __func__ + << ": Failed to write client characteristic " + "configuration descriptor with error: " + << GattCommunicationStatusToString(status); } catch (std::exception exception) { // This usually happens when a device reports that it support notify, but // it actually doesn't. diff --git a/internal/platform/implementation/windows/ble_gatt_client.h b/internal/platform/implementation/windows/ble_gatt_client.h index 32b216db..473e5e2f 100644 --- a/internal/platform/implementation/windows/ble_gatt_client.h +++ b/internal/platform/implementation/windows/ble_gatt_client.h @@ -23,10 +23,15 @@ #include #include +#include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" +#include "absl/functional/any_invocable.h" +#include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" +#include "absl/types/optional.h" #include "internal/platform/byte_array.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/uuid.h" #include "winrt/Windows.Devices.Bluetooth.GenericAttributeProfile.h" #include "winrt/Windows.Devices.Bluetooth.h" #include "winrt/base.h" @@ -42,25 +47,29 @@ class BleGattClient : public api::ble_v2::GattClient { bool DiscoverServiceAndCharacteristics( const Uuid& service_uuid, - const std::vector& characteristic_uuids) override; + const std::vector& characteristic_uuids) override + ABSL_LOCKS_EXCLUDED(mutex_); absl::optional GetCharacteristic( - const Uuid& service_uuid, const Uuid& characteristic_uuid) override; + const Uuid& service_uuid, const Uuid& characteristic_uuid) override + ABSL_LOCKS_EXCLUDED(mutex_); absl::optional ReadCharacteristic( - const api::ble_v2::GattCharacteristic& characteristic) override; + const api::ble_v2::GattCharacteristic& characteristic) override + ABSL_LOCKS_EXCLUDED(mutex_); bool WriteCharacteristic( const api::ble_v2::GattCharacteristic& characteristic, absl::string_view value, - api::ble_v2::GattClient::WriteType write_type) override; + api::ble_v2::GattClient::WriteType write_type) override + ABSL_LOCKS_EXCLUDED(mutex_); bool SetCharacteristicSubscription( const api::ble_v2::GattCharacteristic& characteristic, bool enable, absl::AnyInvocable - on_characteristic_changed_cb) override; + on_characteristic_changed_cb) override ABSL_LOCKS_EXCLUDED(mutex_); - void Disconnect() override; + void Disconnect() override ABSL_LOCKS_EXCLUDED(mutex_); private: // Used to save native data related to the GATT characteristic. @@ -75,13 +84,15 @@ class BleGattClient : public api::ble_v2::GattClient { std::optional<::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattCharacteristic> GetNativeCharacteristic(const Uuid& service_uuid, - const Uuid& characteristic_uuid); + const Uuid& characteristic_uuid) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); bool WriteCharacteristicConfigurationDescriptor( ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattCharacteristic& characteristic, ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: - GattClientCharacteristicConfigurationDescriptorValue value); + GattClientCharacteristicConfigurationDescriptorValue value) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); void OnCharacteristicValueChanged( const api::ble_v2::GattCharacteristic& characteristic, @@ -90,9 +101,11 @@ class BleGattClient : public api::ble_v2::GattClient { absl::Mutex mutex_; - ::winrt::Windows::Devices::Bluetooth::BluetoothLEDevice ble_device_; + ::winrt::Windows::Devices::Bluetooth::BluetoothLEDevice ble_device_ + ABSL_GUARDED_BY(mutex_); ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: - GattDeviceServicesResult gatt_devices_services_result_ = nullptr; + GattDeviceServicesResult gatt_devices_services_result_ + ABSL_GUARDED_BY(mutex_) = nullptr; absl::flat_hash_map native_characteristic_map_ ABSL_GUARDED_BY(mutex_); diff --git a/internal/platform/implementation/windows/ble_gatt_server.cc b/internal/platform/implementation/windows/ble_gatt_server.cc index 6fe40e07..84c0d63f 100644 --- a/internal/platform/implementation/windows/ble_gatt_server.cc +++ b/internal/platform/implementation/windows/ble_gatt_server.cc @@ -15,6 +15,7 @@ #include "internal/platform/implementation/windows/ble_gatt_server.h" #include +#include #include #include #include @@ -29,11 +30,17 @@ #include "absl/status/status.h" #include "absl/strings/escaping.h" #include "absl/strings/str_format.h" +#include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" #include "absl/types/optional.h" #include "internal/platform/byte_array.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/implementation/bluetooth_adapter.h" +#include "internal/platform/implementation/windows/bluetooth_adapter.h" #include "internal/platform/implementation/windows/utils.h" #include "internal/platform/logging.h" +#include "internal/platform/uuid.h" #include "winrt/Windows.Foundation.Collections.h" #include "winrt/Windows.Storage.Streams.h" #include "winrt/base.h" @@ -81,6 +88,9 @@ using ::winrt::Windows::Storage::Streams::DataWriter; using Permission = api::ble_v2::GattCharacteristic::Permission; using Property = api::ble_v2::GattCharacteristic::Property; +constexpr absl::Duration kGattServerTimeout = absl::Milliseconds(500); +constexpr int kGattServerCheckIntervalInMills = 50; + std::string ConvertGattStatusToString( GattServiceProviderAdvertisementStatus status) { switch (status) { @@ -113,9 +123,10 @@ BleGattServer::CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, 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); + absl::MutexLock lock(&mutex_); + NEARBY_LOGS(INFO) << __func__ << ": create characteristic, service_uuid: " + << std::string(service_uuid) << ", characteristic_uuid: " + << std::string(characteristic_uuid); if (!service_uuid_.IsEmpty() && service_uuid_ != service_uuid) { NEARBY_LOGS(ERROR) << __func__ @@ -142,8 +153,9 @@ BleGattServer::CreateCharacteristic( bool BleGattServer::UpdateCharacteristic( const api::ble_v2::GattCharacteristic& characteristic, const nearby::ByteArray& value) { - NEARBY_LOGS(VERBOSE) << __func__ << ": update characteristic: " - << std::string(characteristic.uuid); + absl::MutexLock lock(&mutex_); + NEARBY_LOGS(INFO) << __func__ << ": update characteristic: " + << std::string(characteristic.uuid); if (characteristic.service_uuid != service_uuid_) { NEARBY_LOGS(ERROR) << __func__ << ": Cannot found the GATT service."; @@ -184,6 +196,7 @@ bool BleGattServer::UpdateCharacteristic( absl::Status BleGattServer::NotifyCharacteristicChanged( const api::ble_v2::GattCharacteristic& characteristic, bool confirm, const ByteArray& new_value) { + absl::MutexLock lock(&mutex_); // Currently, the method is not hooked up at platform layer. NEARBY_LOGS(VERBOSE) << __func__ << ": Notify characteristic=" << std::string(characteristic.uuid) << " changed."; @@ -191,6 +204,7 @@ absl::Status BleGattServer::NotifyCharacteristicChanged( } void BleGattServer::Stop() { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(VERBOSE) << __func__ << ": Start to stop GATT server."; try { if (gatt_service_provider_ == nullptr) { @@ -373,6 +387,8 @@ bool BleGattServer::InitializeGattServer() { bool BleGattServer::StartAdvertisement(const ByteArray& service_data, bool is_connectable) { + absl::MutexLock lock(&mutex_); + try { NEARBY_LOGS(VERBOSE) << __func__ << ": service_data=" << absl::BytesToHexString(service_data.AsStringView()) @@ -385,13 +401,11 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data, if (!is_gatt_server_inited_ && !InitializeGattServer()) { NEARBY_LOGS(ERROR) << ":Failed to initalize GATT service."; - is_advertising_ = false; return false; } if (gatt_service_provider_ == nullptr) { NEARBY_LOGS(WARNING) << __func__ << ": no GATT server is running."; - is_advertising_ = false; return false; } @@ -399,7 +413,6 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data, GattServiceProviderAdvertisementStatus::Started) { NEARBY_LOGS(WARNING) << __func__ << ": GATT server is already in advertising."; - is_advertising_ = true; return false; } @@ -408,12 +421,9 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data, << __func__ << ": Bluetooth Hardware does not support Peripheral Role, which is " "required to start GATT server."; - is_advertising_ = false; return false; } - is_advertising_ = true; - // Start the GATT server advertising GattServiceProviderAdvertisingParameters advertisement_parameters; advertisement_parameters.IsConnectable(is_connectable); @@ -427,6 +437,21 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data, advertisement_parameters.ServiceData(data_writer.DetachBuffer()); gatt_service_provider_.StartAdvertising(advertisement_parameters); + + // Wait for the advertising to start. + int wait_milliseconds = 0; + while (gatt_service_provider_.AdvertisementStatus() != + GattServiceProviderAdvertisementStatus::Started) { + absl::SleepFor(absl::Milliseconds(kGattServerCheckIntervalInMills)); + wait_milliseconds += kGattServerCheckIntervalInMills; + if (absl::Milliseconds(wait_milliseconds) > kGattServerTimeout) { + NEARBY_LOGS(ERROR) + << __func__ << ": Failed to start GATT advertising due to timeout."; + return false; + } + } + + is_advertising_ = true; NEARBY_LOGS(INFO) << __func__ << ": GATT server started."; return true; @@ -445,6 +470,8 @@ bool BleGattServer::StartAdvertisement(const ByteArray& service_data, } bool BleGattServer::StopAdvertisement() { + absl::MutexLock lock(&mutex_); + try { NEARBY_LOGS(INFO) << __func__ << ": stop advertisement."; @@ -467,6 +494,11 @@ bool BleGattServer::StopAdvertisement() { } gatt_service_provider_.StopAdvertising(); + + // Don't wait for the advertising to stop, because the advertisement status + // cannot back to stopped. Based on the observation, the advertisement + // status is stopped after the stop advertising is called. + is_advertising_ = false; NEARBY_LOGS(INFO) << __func__ << ": GATT server stopped."; return true; @@ -487,9 +519,9 @@ bool BleGattServer::StopAdvertisement() { GattLocalCharacteristic const& gatt_local_characteristic, ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattReadRequestedEventArgs args) { - NEARBY_LOGS(VERBOSE) << __func__ << ": Read characteristic. uuid: " - << winrt::to_string(winrt::to_hstring( - gatt_local_characteristic.Uuid())); + NEARBY_LOGS(INFO) << __func__ << ": Read characteristic. uuid: " + << winrt::to_string( + winrt::to_hstring(gatt_local_characteristic.Uuid())); auto deferral = args.GetDeferral(); @@ -542,7 +574,7 @@ bool BleGattServer::StopAdvertisement() { GattLocalCharacteristic const& gatt_local_characteristic, ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattWriteRequestedEventArgs args) { - // In Nearby Connctions, don't support write charaterisctics right now. + // In Nearby Connections, don't support write characteristics right now. throw std::logic_error("Not implemented."); } @@ -550,10 +582,10 @@ void BleGattServer::Characteristic_SubscribedClientsChanged( ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattLocalCharacteristic const& gatt_local_characteristic, ::winrt::Windows::Foundation::IInspectable const& args) { - NEARBY_LOGS(VERBOSE) << __func__ - << ": Subscribed clients changed. characteristic=" - << ::winrt::to_string(::winrt::to_hstring( - gatt_local_characteristic.Uuid())); + NEARBY_LOGS(INFO) << __func__ + << ": Subscribed clients changed. characteristic=" + << ::winrt::to_string(::winrt::to_hstring( + gatt_local_characteristic.Uuid())); try { std::vector @@ -637,8 +669,9 @@ void BleGattServer::ServiceProvider_AdvertisementStatusChanged( GattServiceProvider const& sender, ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattServiceProviderAdvertisementStatusChangedEventArgs const& args) { - NEARBY_LOGS(VERBOSE) << __func__ << ": Advertisement status changed. status=" - << ConvertGattStatusToString(args.Status()); + NEARBY_LOGS(INFO) << __func__ << ": Advertisement status changed. status=" + << ConvertGattStatusToString(args.Status()) + << ", error=" << static_cast(args.Error()); } void BleGattServer::NotifyValueChanged( diff --git a/internal/platform/implementation/windows/ble_gatt_server.h b/internal/platform/implementation/windows/ble_gatt_server.h index a6cf3b66..324fa57f 100644 --- a/internal/platform/implementation/windows/ble_gatt_server.h +++ b/internal/platform/implementation/windows/ble_gatt_server.h @@ -17,13 +17,19 @@ #include -#include +#include #include +#include "absl/base/thread_annotations.h" #include "absl/container/flat_hash_map.h" +#include "absl/status/status.h" #include "absl/strings/string_view.h" +#include "absl/synchronization/mutex.h" +#include "absl/synchronization/notification.h" +#include "absl/types/optional.h" #include "internal/platform/byte_array.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/implementation/bluetooth_adapter.h" #include "internal/platform/implementation/windows/ble_v2_peripheral.h" #include "internal/platform/implementation/windows/bluetooth_adapter.h" #include "internal/platform/uuid.h" @@ -43,20 +49,22 @@ class BleGattServer : public api::ble_v2::GattServer { absl::optional CreateCharacteristic( const Uuid& service_uuid, const Uuid& characteristic_uuid, api::ble_v2::GattCharacteristic::Permission permission, - api::ble_v2::GattCharacteristic::Property property) override; + api::ble_v2::GattCharacteristic::Property property) override + ABSL_LOCKS_EXCLUDED(mutex_); bool UpdateCharacteristic( const api::ble_v2::GattCharacteristic& characteristic, - const nearby::ByteArray& value) override; + const nearby::ByteArray& value) override ABSL_LOCKS_EXCLUDED(mutex_); absl::Status NotifyCharacteristicChanged( const api::ble_v2::GattCharacteristic& characteristic, bool confirm, - const ByteArray& new_value) override; + const ByteArray& new_value) override ABSL_LOCKS_EXCLUDED(mutex_); - void Stop() override; + void Stop() override ABSL_LOCKS_EXCLUDED(mutex_); - bool StartAdvertisement(const ByteArray& service_data, bool is_connectable); - bool StopAdvertisement(); + bool StartAdvertisement(const ByteArray& service_data, bool is_connectable) + ABSL_LOCKS_EXCLUDED(mutex_); + bool StopAdvertisement() ABSL_LOCKS_EXCLUDED(mutex_); api::ble_v2::BlePeripheral& GetBlePeripheral() override { return peripheral_; @@ -109,6 +117,8 @@ class BleGattServer : public api::ble_v2::GattServer { ::winrt::Windows::Devices::Bluetooth::GenericAttributeProfile:: GattServiceProviderAdvertisementStatusChangedEventArgs const& args); + absl::Mutex mutex_; + BluetoothAdapter* adapter_ = nullptr; BleV2Peripheral peripheral_; diff --git a/internal/platform/implementation/windows/ble_v2.cc b/internal/platform/implementation/windows/ble_v2.cc index 13e0f062..17028c0f 100644 --- a/internal/platform/implementation/windows/ble_v2.cc +++ b/internal/platform/implementation/windows/ble_v2.cc @@ -16,6 +16,7 @@ #include #include +#include #include #include #include @@ -29,18 +30,21 @@ #include "absl/strings/str_cat.h" #include "absl/strings/str_format.h" #include "absl/synchronization/mutex.h" +#include "absl/time/clock.h" #include "absl/time/time.h" #include "internal/flags/nearby_flags.h" -#include "internal/platform/bluetooth_adapter.h" #include "internal/platform/byte_array.h" #include "internal/platform/cancellation_flag.h" #include "internal/platform/cancellation_flag_listener.h" #include "internal/platform/flags/nearby_platform_feature_flags.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/implementation/bluetooth_adapter.h" #include "internal/platform/implementation/windows/ble_gatt_client.h" #include "internal/platform/implementation/windows/ble_gatt_server.h" +#include "internal/platform/implementation/windows/ble_v2_peripheral.h" #include "internal/platform/implementation/windows/ble_v2_server_socket.h" #include "internal/platform/implementation/windows/ble_v2_socket.h" +#include "internal/platform/implementation/windows/bluetooth_adapter.h" #include "internal/platform/implementation/windows/utils.h" #include "internal/platform/logging.h" #include "internal/platform/prng.h" @@ -116,6 +120,9 @@ static constexpr uint64_t kGenerateSessionIdRetryLimit = 3; // Indicating failed to generate unused session id. static constexpr uint64_t kFailedGenerateSessionId = 0; +constexpr absl::Duration kMediumTimeout = absl::Milliseconds(500); +constexpr int kMediumCheckIntervalInMills = 50; + // Remove lost/unused peripherals after a timeout. constexpr absl::Duration kPeripheralExpiryTime = absl::Minutes(15); // Prevent too frequent cleanup tasks. @@ -128,6 +135,7 @@ BleV2Medium::BleV2Medium(api::BluetoothAdapter& adapter) // Advertisement packet and populate accordingly. bool BleV2Medium::StartAdvertising(const BleAdvertisementData& advertising_data, AdvertiseParameters advertising_parameters) { + absl::MutexLock lock(&mutex_); std::string service_data_info; for (const auto& it : advertising_data.service_data) { service_data_info += "{uuid:" + std::string(it.first) + @@ -160,6 +168,7 @@ bool BleV2Medium::StartAdvertising(const BleAdvertisementData& advertising_data, } bool BleV2Medium::StopAdvertising() { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << __func__ << ": Stop advertising."; bool result; if (is_gatt_publisher_started_) { @@ -222,6 +231,7 @@ std::unique_ptr BleV2Medium::StartAdvertising( bool BleV2Medium::StartScanning(const Uuid& service_uuid, TxPowerLevel tx_power_level, ScanCallback callback) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << __func__ << ": service UUID: " << std::string(service_uuid) << ", TxPowerLevel: " << TxPowerLevelToName(tx_power_level); @@ -270,6 +280,22 @@ bool BleV2Medium::StartScanning(const Uuid& service_uuid, watcher_.AdvertisementFilter(advertisement_filter); watcher_.Start(); + // Wait for the watcher to start. + int wait_milliseconds = 0; + while (watcher_.Status() != + BluetoothLEAdvertisementWatcherStatus::Started) { + absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills)); + wait_milliseconds += kMediumCheckIntervalInMills; + if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) { + NEARBY_LOGS(ERROR) << __func__ + << ": Failed to start BLE scan due to timeout.."; + watcher_.Stopped(watcher_token_); + watcher_.Received(advertisement_received_token_); + watcher_ = nullptr; + return false; + } + } + is_watcher_started_ = true; NEARBY_LOGS(INFO) << __func__ << ": BLE scanning started."; @@ -414,6 +440,7 @@ std::unique_ptr BleV2Medium::StartScanning( std::unique_ptr BleV2Medium::StartGattServer( api::ble_v2::ServerGattConnectionCallback callback) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << __func__ << ": Start GATT server."; if (!NearbyFlags::GetInstance().GetBoolFlag( @@ -442,6 +469,7 @@ std::unique_ptr BleV2Medium::StartGattServer( std::unique_ptr BleV2Medium::ConnectToGattServer( api::ble_v2::BlePeripheral& peripheral, TxPowerLevel tx_power_level, api::ble_v2::ClientGattConnectionCallback callback) { + absl::MutexLock lock(&mutex_); NEARBY_LOGS(INFO) << "ConnectToGattServer is called, address: " << peripheral.GetAddress() << ", power:" << TxPowerLevelToName(tx_power_level); @@ -482,6 +510,8 @@ std::unique_ptr BleV2Medium::ConnectToGattServer( } bool BleV2Medium::StopScanning() { + absl::MutexLock lock(&mutex_); + NEARBY_LOGS(INFO) << __func__ << ": BLE StopScanning: service_uuid: " << std::string(service_uuid_); try { @@ -498,9 +528,26 @@ bool BleV2Medium::StopScanning() { watcher_.Stop(); - // Don't need to wait for the status becomes to `Stopped`. If application - // starts to scanning immediately, the scanning still needs to wait the - // stopping to finish. + // Wait for the watcher to stop. + int wait_milliseconds = 0; + while (watcher_.Status() != + BluetoothLEAdvertisementWatcherStatus::Stopped) { + absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills)); + wait_milliseconds += kMediumCheckIntervalInMills; + if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) { + NEARBY_LOGS(ERROR) << __func__ + << ": Failed to stop BLE scan due to timeout."; + watcher_.Stopped(watcher_token_); + watcher_.Received(advertisement_received_token_); + watcher_ = nullptr; + is_watcher_started_ = false; + return false; + } + } + + watcher_.Stopped(watcher_token_); + watcher_.Received(advertisement_received_token_); + watcher_ = nullptr; is_watcher_started_ = false; NEARBY_LOGS(ERROR) @@ -662,6 +709,22 @@ bool BleV2Medium::StartBleAdvertising( publisher_.Start(); + // Wait for the publisher to start. + int wait_milliseconds = 0; + while (publisher_.Status() != + BluetoothLEAdvertisementPublisherStatus::Started) { + absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills)); + wait_milliseconds += kMediumCheckIntervalInMills; + if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) { + NEARBY_LOGS(ERROR) + << __func__ << ": BLE advertising failed to start due to timeout."; + publisher_.StatusChanged(publisher_token_); + publisher_ = nullptr; + is_ble_publisher_started_ = false; + return false; + } + } + is_ble_publisher_started_ = true; NEARBY_LOGS(INFO) << "BLE advertising started."; return true; @@ -697,15 +760,34 @@ bool BleV2Medium::StopBleAdvertising() { } // publisher_ may be null when status changed during advertising. - if (publisher_ != nullptr && - publisher_.Status() == + if (publisher_ == nullptr || + publisher_.Status() != BluetoothLEAdvertisementPublisherStatus::Started) { - publisher_.Stop(); + NEARBY_LOGS(WARNING) << "No started publisher is running."; + return false; } - // Don't need to wait for the status becomes to `Stopped`. If application - // starts to scanning immediately, the scanning still needs to wait the - // stopping to finish. + publisher_.Stop(); + + // Wait for the publisher to stop. + int wait_milliseconds = 0; + while (publisher_.Status() != + BluetoothLEAdvertisementPublisherStatus::Stopped) { + absl::SleepFor(absl::Milliseconds(kMediumCheckIntervalInMills)); + wait_milliseconds += kMediumCheckIntervalInMills; + if (absl::Milliseconds(wait_milliseconds) > kMediumTimeout) { + NEARBY_LOGS(ERROR) + << __func__ << ": BLE advertising failed to stop due to timeout."; + publisher_.StatusChanged(publisher_token_); + publisher_ = nullptr; + is_ble_publisher_started_ = false; + return false; + } + } + + // Reset publisher. + publisher_.StatusChanged(publisher_token_); + publisher_ = nullptr; is_ble_publisher_started_ = false; return true; @@ -912,14 +994,6 @@ void BleV2Medium::PublisherHandler( default: break; } - - // The publisher is stopped. Clean up the running publisher - if (publisher_ != nullptr) { - NEARBY_LOGS(ERROR) << "Nearby BLE Medium cleaned the publisher."; - publisher_.StatusChanged(publisher_token_); - publisher_ = nullptr; - is_ble_publisher_started_ = false; - } } void BleV2Medium::WatcherHandler( @@ -972,16 +1046,6 @@ void BleV2Medium::WatcherHandler( << "Nearby BLE Medium stoped to scan due to unknown errors."; break; } - - // No matter the reason, should clean up the watcher if it is not empty. - // The BLE V1 interface doesn't have API to return the error to upper layer. - if (watcher_ != nullptr) { - NEARBY_LOGS(ERROR) << "Nearby BLE Medium cleaned the watcher."; - watcher_.Stopped(watcher_token_); - watcher_.Received(advertisement_received_token_); - watcher_ = nullptr; - is_watcher_started_ = false; - } } void BleV2Medium::AdvertisementReceivedHandler( diff --git a/internal/platform/implementation/windows/ble_v2.h b/internal/platform/implementation/windows/ble_v2.h index 5e6f8d53..dae7646d 100644 --- a/internal/platform/implementation/windows/ble_v2.h +++ b/internal/platform/implementation/windows/ble_v2.h @@ -15,18 +15,24 @@ #ifndef THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_IMPLEMENTATION_WINDOWS_BLE_V2_H_ #define THIRD_PARTY_NEARBY_INTERNAL_PLATFORM_IMPLEMENTATION_WINDOWS_BLE_V2_H_ +#include +#include #include #include +#include "absl/base/thread_annotations.h" +#include "absl/container/flat_hash_map.h" +#include "absl/strings/string_view.h" #include "absl/synchronization/mutex.h" -#include "internal/platform/byte_array.h" +#include "absl/synchronization/notification.h" +#include "absl/time/clock.h" +#include "absl/time/time.h" +#include "internal/platform/cancellation_flag.h" #include "internal/platform/implementation/ble_v2.h" +#include "internal/platform/implementation/bluetooth_adapter.h" #include "internal/platform/implementation/windows/ble_gatt_server.h" #include "internal/platform/implementation/windows/ble_v2_peripheral.h" #include "internal/platform/implementation/windows/bluetooth_adapter.h" -#include "internal/platform/implementation/windows/bluetooth_classic.h" -#include "internal/platform/input_stream.h" -#include "internal/platform/output_stream.h" #include "internal/platform/uuid.h" #include "winrt/Windows.Devices.Bluetooth.Advertisement.h" @@ -42,8 +48,9 @@ class BleV2Medium : public api::ble_v2::BleMedium { // Returns true once the Ble advertising has been initiated. bool StartAdvertising( const api::ble_v2::BleAdvertisementData& advertising_data, - api::ble_v2::AdvertiseParameters advertising_parameters) override; - bool StopAdvertising() override; + api::ble_v2::AdvertiseParameters advertising_parameters) override + ABSL_LOCKS_EXCLUDED(mutex_); + bool StopAdvertising() override ABSL_LOCKS_EXCLUDED(mutex_); std::unique_ptr StartAdvertising( const api::ble_v2::BleAdvertisementData& advertising_data, @@ -52,41 +59,48 @@ class BleV2Medium : public api::ble_v2::BleMedium { bool StartScanning(const Uuid& service_uuid, api::ble_v2::TxPowerLevel tx_power_level, - ScanCallback callback) override; - bool StopScanning() override; + ScanCallback callback) override + ABSL_LOCKS_EXCLUDED(mutex_); + bool StopScanning() override ABSL_LOCKS_EXCLUDED(mutex_); std::unique_ptr StartScanning( const Uuid& service_uuid, api::ble_v2::TxPowerLevel tx_power_level, ScanningCallback callback) override; std::unique_ptr StartGattServer( - api::ble_v2::ServerGattConnectionCallback callback) override; + api::ble_v2::ServerGattConnectionCallback callback) override + ABSL_LOCKS_EXCLUDED(mutex_); std::unique_ptr ConnectToGattServer( api::ble_v2::BlePeripheral& peripheral, api::ble_v2::TxPowerLevel tx_power_level, - api::ble_v2::ClientGattConnectionCallback callback) override; + api::ble_v2::ClientGattConnectionCallback callback) override + ABSL_LOCKS_EXCLUDED(mutex_); std::unique_ptr OpenServerSocket( - const std::string& service_id) override; + const std::string& service_id) override ABSL_LOCKS_EXCLUDED(mutex_); std::unique_ptr Connect( const std::string& service_id, api::ble_v2::TxPowerLevel tx_power_level, api::ble_v2::BlePeripheral& remote_peripheral, - CancellationFlag* cancellation_flag) override; + CancellationFlag* cancellation_flag) override ABSL_LOCKS_EXCLUDED(mutex_); bool IsExtendedAdvertisementsAvailable() override; bool GetRemotePeripheral(const std::string& mac_address, - GetRemotePeripheralCallback callback) override; + GetRemotePeripheralCallback callback) override + ABSL_LOCKS_EXCLUDED(mutex_); bool GetRemotePeripheral(api::ble_v2::BlePeripheral::UniqueId id, - GetRemotePeripheralCallback callback) override; + GetRemotePeripheralCallback callback) override + ABSL_LOCKS_EXCLUDED(mutex_); private: bool StartBleAdvertising( const api::ble_v2::BleAdvertisementData& advertising_data, - api::ble_v2::AdvertiseParameters advertising_parameters); - bool StopBleAdvertising(); + api::ble_v2::AdvertiseParameters advertising_parameters) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + bool StopBleAdvertising() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); bool StartGattAdvertising( const api::ble_v2::BleAdvertisementData& advertising_data, - api::ble_v2::AdvertiseParameters advertising_parameters); - bool StopGattAdvertising(); + api::ble_v2::AdvertiseParameters advertising_parameters) + ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); + bool StopGattAdvertising() ABSL_EXCLUSIVE_LOCKS_REQUIRED(mutex_); void PublisherHandler( winrt::Windows::Devices::Bluetooth::Advertisement:: @@ -120,6 +134,8 @@ class BleV2Medium : public api::ble_v2::BleMedium { void RemoveExpiredPeripherals() ABSL_EXCLUSIVE_LOCKS_REQUIRED(peripheral_map_mutex_); + absl::Mutex mutex_; + BluetoothAdapter* adapter_; Uuid service_uuid_; api::ble_v2::TxPowerLevel tx_power_level_;