From b6f854c81b4edea3a627dfe1a09f49235160cdf6 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 8 Oct 2021 12:36:46 +0200 Subject: Refactor characteristics read for WinRT The characteristics read was using IAsyncOperation->put_Completed() to process the results of descriptor reading. Each of these completed callbacks is executed in its own thread. In our case each of the callbacks is also calling QWinRTFunctions::await() to perform some async operations. As a result, if the service has a lot of characteristics, we could end up spawning multiple threads, each of them doing some CPU-consuming operations. Normally it's not a problem, but looks like for some bluetooth peripherals reading the characteristics can take significant time. Connecting to such devices can result in 100% CPU load for all cores if the number of characteristics exceeds the number of logical CPUs. This patch removes the put_Completed() approach in favor of another QWinRTFunctions::async() call. This will guarantee that all the characteristics read operations are performed in the same worker thread. As a drawback, characteristics reading becomes sequential, which makes it slower. Fixes: QTBUG-97242 Change-Id: I0d641b8c65e628bb09a26380f5b12ba2582f210e Reviewed-by: Oliver Wolff (cherry picked from commit 2a7ef291d696745887e71f8b0e27cb4c8701bdd2) Reviewed-by: Qt Cherry-pick Bot --- src/bluetooth/qlowenergycontroller_winrt.cpp | 314 +++++++++++---------------- 1 file changed, 126 insertions(+), 188 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp index 19724369..32e4489a 100644 --- a/src/bluetooth/qlowenergycontroller_winrt.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt.cpp @@ -97,6 +97,13 @@ typedef IGattReadClientCharacteristicConfigurationDescriptorResult IClientCharCo continue; \ } +#define DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, msg) \ + if (FAILED(hr)) { \ + qCWarning(QT_BT_WINDOWS) << msg; \ + --mCharacteristicsCountToBeDiscovered; \ + continue; \ + } + #define CHECK_FOR_DEVICE_CONNECTION_ERROR_IMPL(this, hr, msg, ret) \ if (FAILED(hr)) { \ this->handleConnectionError(msg); \ @@ -145,6 +152,7 @@ public: ~QWinRTLowEnergyServiceHandler() { + qCDebug(QT_BT_WINDOWS) << __FUNCTION__; } public slots: @@ -196,201 +204,131 @@ public slots: // Qt API assumes that all characteristics and their descriptors are discovered in one go. // So we start 'GetDescriptorsAsync' for each discovered characteristic and finish only // when GetDescriptorsAsync for all characteristics return. - ComPtr> descAsyncResult; - hr = characteristic3->GetDescriptorsAsync(&descAsyncResult); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain list of descriptors"; - --mCharacteristicsCountToBeDiscovered; - continue; + ComPtr> descAsyncOp; + hr = characteristic3->GetDescriptorsAsync(&descAsyncOp); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not obtain list of descriptors") + + ComPtr descResult; + hr = QWinRTFunctions::await(descAsyncOp, descResult.GetAddressOf()); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor read result") + + quint16 handle; + hr = characteristic->get_AttributeHandle(&handle); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED( + hr, "Could not obtain characteristic's attribute handle") + QLowEnergyServicePrivate::CharData charData; + charData.valueHandle = handle + 1; + if (mStartHandle == 0 || mStartHandle > handle) + mStartHandle = handle; + if (mEndHandle == 0 || mEndHandle < handle) + mEndHandle = handle; + GUID guuid; + hr = characteristic->get_Uuid(&guuid); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not obtain characteristic's Uuid") + charData.uuid = QBluetoothUuid(guuid); + GattCharacteristicProperties properties; + hr = characteristic->get_CharacteristicProperties(&properties); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, + "Could not obtain characteristic's properties") + charData.properties = QLowEnergyCharacteristic::PropertyTypes(properties & 0xff); + if (charData.properties & QLowEnergyCharacteristic::Read + && mMode == QLowEnergyService::FullDiscovery) { + ComPtr> readOp; + hr = characteristic->ReadValueWithCacheModeAsync(BluetoothCacheMode_Uncached, + &readOp); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not read characteristic") + ComPtr readResult; + hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, + "Could not obtain characteristic read result") + if (!readResult) + qCWarning(QT_BT_WINDOWS) << "Characteristic read result is null"; + else + charData.value = byteArrayFromGattResult(readResult); } - hr = descAsyncResult->put_Completed( - Callback>( - [this, characteristic] - (IAsyncOperation *op, - AsyncStatus status) { - if (status != AsyncStatus::Completed) { - qCWarning(QT_BT_WINDOWS) << "Descriptor operation unsuccessful"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - quint16 handle; - - HRESULT hr = characteristic->get_AttributeHandle(&handle); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain characteristic's attribute handle"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - QLowEnergyServicePrivate::CharData charData; - charData.valueHandle = handle + 1; - if (mStartHandle == 0 || mStartHandle > handle) - mStartHandle = handle; - if (mEndHandle == 0 || mEndHandle < handle) - mEndHandle = handle; - GUID guuid; - hr = characteristic->get_Uuid(&guuid); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain characteristic's Uuid"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - charData.uuid = QBluetoothUuid(guuid); - GattCharacteristicProperties properties; - hr = characteristic->get_CharacteristicProperties(&properties); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain characteristic's properties"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - charData.properties = QLowEnergyCharacteristic::PropertyTypes(properties & 0xff); - if (charData.properties & QLowEnergyCharacteristic::Read - && mMode == QLowEnergyService::FullDiscovery) { - ComPtr> readOp; - hr = characteristic->ReadValueWithCacheModeAsync(BluetoothCacheMode_Uncached, - &readOp); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not read characteristic"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - ComPtr readResult; - hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain characteristic read result"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - if (!readResult) - qCWarning(QT_BT_WINDOWS) << "Characteristic read result is null"; - else - charData.value = byteArrayFromGattResult(readResult); - } - mCharacteristicList.insert(handle, charData); + mCharacteristicList.insert(handle, charData); - ComPtr> descriptors; + ComPtr> descriptors; - ComPtr result; - hr = op->GetResults(&result); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain descriptor read result"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - GattCommunicationStatus commStatus; - hr = result->get_Status(&commStatus); - if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { - qCWarning(QT_BT_WINDOWS) << "Descriptor operation failed"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - - hr = result->get_Descriptors(&descriptors); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain list of descriptors"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } + GattCommunicationStatus commStatus; + hr = descResult->get_Status(&commStatus); + if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { + qCWarning(QT_BT_WINDOWS) << "Descriptor operation failed"; + --mCharacteristicsCountToBeDiscovered; + continue; + } - uint descriptorCount; - hr = descriptors->get_Size(&descriptorCount); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not obtain list of descriptors' size"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - for (uint j = 0; j < descriptorCount; ++j) { - QLowEnergyServicePrivate::DescData descData; - ComPtr descriptor; - hr = descriptors->GetAt(j, &descriptor); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor") - quint16 descHandle; - hr = descriptor->get_AttributeHandle(&descHandle); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor's attribute handle") - GUID descriptorUuid; - hr = descriptor->get_Uuid(&descriptorUuid); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor's Uuid") - descData.uuid = QBluetoothUuid(descriptorUuid); - charData.descriptorList.insert(descHandle, descData); - if (descData.uuid == QBluetoothUuid(QBluetoothUuid::DescriptorType::ClientCharacteristicConfiguration)) { - if (mMode == QLowEnergyService::FullDiscovery) { - ComPtr> readOp; - hr = characteristic - ->ReadClientCharacteristicConfigurationDescriptorAsync( - &readOp); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could not read descriptor value") - ComPtr readResult; - hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); - WARN_AND_CONTINUE_IF_FAILED(hr, - "Could not await descriptor read result") - GattClientCharacteristicConfigurationDescriptorValue value; - hr = readResult->get_ClientCharacteristicConfigurationDescriptor( - &value); - WARN_AND_CONTINUE_IF_FAILED( - hr, "Could not get descriptor value from result") - quint16 result = 0; - bool correct = false; - if (value - & GattClientCharacteristicConfigurationDescriptorValue_Indicate) { - result |= - GattClientCharacteristicConfigurationDescriptorValue_Indicate; - correct = true; - } - if (value - & GattClientCharacteristicConfigurationDescriptorValue_Notify) { - result |= - GattClientCharacteristicConfigurationDescriptorValue_Notify; - correct = true; - } - if (value - == GattClientCharacteristicConfigurationDescriptorValue_None) { - correct = true; - } - if (!correct) - continue; - - descData.value = QByteArray(2, Qt::Uninitialized); - qToLittleEndian(result, descData.value.data()); + hr = descResult->get_Descriptors(&descriptors); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not obtain list of descriptors") + + uint descriptorCount; + hr = descriptors->get_Size(&descriptorCount); + DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, "Could not obtain list of descriptors' size") + for (uint j = 0; j < descriptorCount; ++j) { + QLowEnergyServicePrivate::DescData descData; + ComPtr descriptor; + hr = descriptors->GetAt(j, &descriptor); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor") + quint16 descHandle; + hr = descriptor->get_AttributeHandle(&descHandle); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor's attribute handle") + GUID descriptorUuid; + hr = descriptor->get_Uuid(&descriptorUuid); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not obtain descriptor's Uuid") + descData.uuid = QBluetoothUuid(descriptorUuid); + charData.descriptorList.insert(descHandle, descData); + if (descData.uuid == QBluetoothUuid(QBluetoothUuid::DescriptorType::ClientCharacteristicConfiguration)) { + if (mMode == QLowEnergyService::FullDiscovery) { + ComPtr> readOp; + hr = characteristic->ReadClientCharacteristicConfigurationDescriptorAsync( + &readOp); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not read descriptor value") + ComPtr readResult; + hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not await descriptor read result") + GattClientCharacteristicConfigurationDescriptorValue value; + hr = readResult->get_ClientCharacteristicConfigurationDescriptor(&value); + WARN_AND_CONTINUE_IF_FAILED(hr, + "Could not get descriptor value from result") + quint16 result = 0; + bool correct = false; + if (value & GattClientCharacteristicConfigurationDescriptorValue_Indicate) { + result |= GattClientCharacteristicConfigurationDescriptorValue_Indicate; + correct = true; } - mIndicateChars << charData.uuid; - } else { - if (mMode == QLowEnergyService::FullDiscovery) { - ComPtr> readOp; - hr = descriptor->ReadValueWithCacheModeAsync( - BluetoothCacheMode_Uncached, &readOp); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could not read descriptor value") - ComPtr readResult; - hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); - WARN_AND_CONTINUE_IF_FAILED(hr, "Could await descriptor read result") - if (descData.uuid - == QBluetoothUuid::DescriptorType::CharacteristicUserDescription) - descData.value = byteArrayFromGattResult(readResult, true); - else - descData.value = byteArrayFromGattResult(readResult); + if (value & GattClientCharacteristicConfigurationDescriptorValue_Notify) { + result |= GattClientCharacteristicConfigurationDescriptorValue_Notify; + correct = true; } + if (value == GattClientCharacteristicConfigurationDescriptorValue_None) + correct = true; + if (!correct) + continue; + + descData.value = QByteArray(2, Qt::Uninitialized); + qToLittleEndian(result, descData.value.data()); + } + mIndicateChars << charData.uuid; + } else { + if (mMode == QLowEnergyService::FullDiscovery) { + ComPtr> readOp; + hr = descriptor->ReadValueWithCacheModeAsync(BluetoothCacheMode_Uncached, + &readOp); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could not read descriptor value") + ComPtr readResult; + hr = QWinRTFunctions::await(readOp, readResult.GetAddressOf()); + WARN_AND_CONTINUE_IF_FAILED(hr, "Could await descriptor read result") + if (descData.uuid == QBluetoothUuid::DescriptorType::CharacteristicUserDescription) + descData.value = byteArrayFromGattResult(readResult, true); + else + descData.value = byteArrayFromGattResult(readResult); } - charData.descriptorList.insert(descHandle, descData); } - - mCharacteristicList.insert(handle, charData); - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - }).Get()); - if (FAILED(hr)) { - qCWarning(QT_BT_WINDOWS) << "Could not register descriptor callback"; - --mCharacteristicsCountToBeDiscovered; - continue; + charData.descriptorList.insert(descHandle, descData); } + + mCharacteristicList.insert(handle, charData); + --mCharacteristicsCountToBeDiscovered; } checkAllCharacteristicsDiscovered(); } @@ -1259,8 +1197,8 @@ void QLowEnergyControllerPrivateWinRT::discoverServiceDetails( QLowEnergyServicePrivate::CharData> charList, QList indicateChars, QLowEnergyHandle startHandle, QLowEnergyHandle endHandle) { if (!serviceList.contains(service)) { - qCWarning(QT_BT_WINDOWS) << "Discovery done of unknown service:" - << service.toString(); + qCWarning(QT_BT_WINDOWS) + << "Discovery complete for unknown service:" << service.toString(); return; } -- cgit v1.2.3