From 3f7bdee1fdd0b5d3e4f955a4a8be9537a716e5e6 Mon Sep 17 00:00:00 2001 From: Tarja Sundqvist Date: Tue, 7 Sep 2021 17:09:43 +0300 Subject: Bump version Change-Id: Ied1443054ec7c8b9e835fd1682c3c58ad67935be --- .qmake.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.qmake.conf b/.qmake.conf index d7055d75..6d03a03f 100644 --- a/.qmake.conf +++ b/.qmake.conf @@ -2,4 +2,4 @@ load(qt_build_config) DEFINES += QT_NO_FOREACH QT_NO_JAVA_STYLE_ITERATORS QT_NO_LINKED_LIST -MODULE_VERSION = 5.15.6 +MODULE_VERSION = 5.15.7 -- cgit v1.2.3 From 7768a384e0a4f92a82bd14ea04d76f163e4bb38f Mon Sep 17 00:00:00 2001 From: Jens Trillmann Date: Tue, 17 Aug 2021 09:25:46 +0200 Subject: Android: Call handleTargetLost directly on NFC TagLostException If in a transceive() a TagLostException occurs, subsequent calls to transceive() may lead to an app crash with "JNI DETECTED ERROR IN APPLICATION: GetMethodID received NULL jclass" on Android 11 if the timer did not clean up first. This fix calls handleTargetLost directly. This could lead to a race condition in which the targetLost signal is send twice, which should be handled correctly by the Qt code. Change-Id: I886e0bd436272ee393a92b0193fd4d40ed4e965e Reviewed-by: Ivan Solovev Reviewed-by: Alex Blasche Reviewed-by: Lars Schmertmann (cherry picked from commit 8362edb220cb77e7bcaf766dc8a9545d1c9246e5) Reviewed-by: Qt CI Bot --- src/nfc/qnearfieldtarget_android.cpp | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/src/nfc/qnearfieldtarget_android.cpp b/src/nfc/qnearfieldtarget_android.cpp index 295c603c..5cc01807 100644 --- a/src/nfc/qnearfieldtarget_android.cpp +++ b/src/nfc/qnearfieldtarget_android.cpp @@ -253,6 +253,12 @@ QNearFieldTarget::RequestId NearFieldTarget::sendCommand(const QByteArray &comma // Writing QAndroidJniObject myNewVal = m_tagTech.callObjectMethod("transceive", "([B)[B", jba); if (catchJavaExceptions()) { + // Some devices (Samsung, Huawei) throw an exception when the card is lost: + // "android.nfc.TagLostException: Tag was lost". But there seems to be a bug that + // isConnected still reports true. So we need to invalidate the target as soon as + // possible and treat the card as lost. + handleTargetLost(); + reportError(QNearFieldTarget::CommandError, requestId); return requestId; } -- cgit v1.2.3 From 4dee30dec3926c556342a0d383db3a25b5ecd90f Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 7 Sep 2021 12:27:41 +0200 Subject: Heart-rate example: add a proper Info.plist Apple (iOS) requires a "proper" Info.plist entries, explaining _why_ we want to use Bluetooth on this particular device. So, let's add it. Both the server and game apps need it. Fixes: QTBUG-93991 Change-Id: I09133c6714f30bc402049cd40969a9b9644a0b4f Reviewed-by: Alex Blasche (cherry picked from commit 0e836b5a8b6d7c659a9540e2718918f8cfc28279) Reviewed-by: Qt Cherry-pick Bot --- examples/bluetooth/heartrate-game/Info.plist | 43 ++++++++++++++++++++++ .../bluetooth/heartrate-game/heartrate-game.pro | 2 + examples/bluetooth/heartrate-server/Info.plist | 43 ++++++++++++++++++++++ .../heartrate-server/heartrate-server.pro | 2 + 4 files changed, 90 insertions(+) create mode 100644 examples/bluetooth/heartrate-game/Info.plist create mode 100644 examples/bluetooth/heartrate-server/Info.plist diff --git a/examples/bluetooth/heartrate-game/Info.plist b/examples/bluetooth/heartrate-game/Info.plist new file mode 100644 index 00000000..d3c8c281 --- /dev/null +++ b/examples/bluetooth/heartrate-game/Info.plist @@ -0,0 +1,43 @@ + + + + + CFBundleDisplayName + ${PRODUCT_NAME} + CFBundleExecutable + ${EXECUTABLE_NAME} + CFBundleIconFile + ${ASSETCATALOG_COMPILER_APPICON_NAME} + CFBundleIdentifier + ${PRODUCT_BUNDLE_IDENTIFIER} + CFBundleName + ${PRODUCT_NAME} + CFBundlePackageType + APPL + CFBundleShortVersionString + ${QMAKE_SHORT_VERSION} + CFBundleSignature + ${QMAKE_PKGINFO_TYPEINFO} + CFBundleVersion + ${QMAKE_FULL_VERSION} + LSRequiresIPhoneOS + + MinimumOSVersion + ${IPHONEOS_DEPLOYMENT_TARGET} + NOTE + This file was generated by Qt/QMake. + NSBluetoothAlwaysUsageDescription + Qt's Heartrate-game needs Bluetooth LE + NSBluetoothPeripheralUsageDescription + Qt's Heartrate-game needs Bluetooth LE + UILaunchStoryboardName + LaunchScreen + UISupportedInterfaceOrientations + + UIInterfaceOrientationPortrait + UIInterfaceOrientationPortraitUpsideDown + UIInterfaceOrientationLandscapeLeft + UIInterfaceOrientationLandscapeRight + + + diff --git a/examples/bluetooth/heartrate-game/heartrate-game.pro b/examples/bluetooth/heartrate-game/heartrate-game.pro index fcec0bd2..a0827d89 100644 --- a/examples/bluetooth/heartrate-game/heartrate-game.pro +++ b/examples/bluetooth/heartrate-game/heartrate-game.pro @@ -19,6 +19,8 @@ SOURCES += main.cpp \ devicehandler.cpp \ bluetoothbaseclass.cpp +ios: QMAKE_INFO_PLIST = Info.plist + RESOURCES += qml.qrc \ images.qrc diff --git a/examples/bluetooth/heartrate-server/Info.plist b/examples/bluetooth/heartrate-server/Info.plist new file mode 100644 index 00000000..b50431e8 --- /dev/null +++ b/examples/bluetooth/heartrate-server/Info.plist @@ -0,0 +1,43 @@ + + + + + CFBundleDisplayName + ${PRODUCT_NAME} + CFBundleExecutable + ${EXECUTABLE_NAME} + CFBundleIconFile + ${ASSETCATALOG_COMPILER_APPICON_NAME} + CFBundleIdentifier + ${PRODUCT_BUNDLE_IDENTIFIER} + CFBundleName + ${PRODUCT_NAME} + CFBundlePackageType + APPL + CFBundleShortVersionString + ${QMAKE_SHORT_VERSION} + CFBundleSignature + ${QMAKE_PKGINFO_TYPEINFO} + CFBundleVersion + ${QMAKE_FULL_VERSION} + LSRequiresIPhoneOS + + MinimumOSVersion + ${IPHONEOS_DEPLOYMENT_TARGET} + NOTE + This file was generated by Qt/QMake. + NSBluetoothAlwaysUsageDescription + This is Qt's Heartrate server example, it needs your Bluetooth radio! + NSBluetoothPeripheralUsageDescription + Well, this is Qt's Heartrate server, it needs your Bluetooth radio if you don't mind! + UILaunchStoryboardName + LaunchScreen + UISupportedInterfaceOrientations + + UIInterfaceOrientationPortrait + UIInterfaceOrientationPortraitUpsideDown + UIInterfaceOrientationLandscapeLeft + UIInterfaceOrientationLandscapeRight + + + diff --git a/examples/bluetooth/heartrate-server/heartrate-server.pro b/examples/bluetooth/heartrate-server/heartrate-server.pro index 0333ffc9..992f2d66 100644 --- a/examples/bluetooth/heartrate-server/heartrate-server.pro +++ b/examples/bluetooth/heartrate-server/heartrate-server.pro @@ -7,5 +7,7 @@ CONFIG += c++11 SOURCES += main.cpp +ios: QMAKE_INFO_PLIST = Info.plist + target.path = $$[QT_INSTALL_EXAMPLES]/bluetooth/heartrate-server INSTALLS += target -- cgit v1.2.3 From c97424d1b5ffd64dca34eb3a9da60becfcf4928d Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 7 Sep 2021 14:44:09 +0200 Subject: Bluetooth docs: mention the Apple's specific requirements Info.plist with a textual description of why and what Bluetooth is needed for by a Qt-based app is required on iOS. Change-Id: If838b57d6c13da9aeaf436dd013bd8496984a8bb Reviewed-by: Alex Blasche Reviewed-by: Andreas Buhr (cherry picked from commit 5953696dbd294918baf33b26ccad526d3332f967) Reviewed-by: Qt Cherry-pick Bot --- src/bluetooth/doc/src/bluetooth-le-overview.qdoc | 10 ++++++++++ 1 file changed, 10 insertions(+) diff --git a/src/bluetooth/doc/src/bluetooth-le-overview.qdoc b/src/bluetooth/doc/src/bluetooth-le-overview.qdoc index 7bfc0c44..b161e362 100644 --- a/src/bluetooth/doc/src/bluetooth-le-overview.qdoc +++ b/src/bluetooth/doc/src/bluetooth-le-overview.qdoc @@ -286,4 +286,14 @@ Low Energy devices. In general characteristic and descriptor value updates on the peripheral device use the same methods as connecting Bluetooth Low Energy devices. + + \note To use QtBluetooth (in both central and peripheral roles) on iOS, you have to provide an Info.plist + file containing the usage description. According to the CoreBluetooth's documentation: + + "Important + + Your app will crash if its Info.plist doesn’t include usage description keys for the types of data + it needs to access. To access Core Bluetooth APIs on apps linked on or after iOS 13, include the + NSBluetoothAlwaysUsageDescription key. In iOS 12 and earlier, include NSBluetoothPeripheralUsageDescription + to access Bluetooth peripheral data." */ -- cgit v1.2.3 From d3c6fc147bdc84ab43fb6fca07c459f018db1a23 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 7 Sep 2021 17:57:44 +0200 Subject: CoreBluetooth: provide a description (in Info.plist) of BT usage Our lowenergyscanner example can be built and deployed on iOS device. As such, it has to provide an explanation, why it needs an access to BT adapter, otherwise, it would crash with the most recent versions of iOS. Change-Id: Iebcdf8af931002532aada0c452263effd342300e Reviewed-by: Alex Blasche (cherry picked from commit 42e1fdacd51c48dd6892e32f8c0d7a2882a3ad7e) Reviewed-by: Qt Cherry-pick Bot --- examples/bluetooth/lowenergyscanner/Info.plist | 39 ++++++++++++++++++++++ .../lowenergyscanner/lowenergyscanner.pro | 2 ++ 2 files changed, 41 insertions(+) create mode 100644 examples/bluetooth/lowenergyscanner/Info.plist diff --git a/examples/bluetooth/lowenergyscanner/Info.plist b/examples/bluetooth/lowenergyscanner/Info.plist new file mode 100644 index 00000000..7f056858 --- /dev/null +++ b/examples/bluetooth/lowenergyscanner/Info.plist @@ -0,0 +1,39 @@ + + + + + CFBundleDisplayName + ${PRODUCT_NAME} + CFBundleExecutable + ${EXECUTABLE_NAME} + CFBundleIconFile + ${ASSETCATALOG_COMPILER_APPICON_NAME} + CFBundleIdentifier + ${PRODUCT_BUNDLE_IDENTIFIER} + CFBundleName + ${PRODUCT_NAME} + CFBundlePackageType + APPL + CFBundleShortVersionString + ${QMAKE_SHORT_VERSION} + CFBundleSignature + ${QMAKE_PKGINFO_TYPEINFO} + CFBundleVersion + ${QMAKE_FULL_VERSION} + LSRequiresIPhoneOS + + MinimumOSVersion + ${IPHONEOS_DEPLOYMENT_TARGET} + NSBluetoothAlwaysUsageDescription + Qt LE scanner wants to access your Bluetooth adapter + UILaunchStoryboardName + LaunchScreen + UISupportedInterfaceOrientations + + UIInterfaceOrientationPortrait + UIInterfaceOrientationPortraitUpsideDown + UIInterfaceOrientationLandscapeLeft + UIInterfaceOrientationLandscapeRight + + + diff --git a/examples/bluetooth/lowenergyscanner/lowenergyscanner.pro b/examples/bluetooth/lowenergyscanner/lowenergyscanner.pro index f3378b4c..934f6cfa 100644 --- a/examples/bluetooth/lowenergyscanner/lowenergyscanner.pro +++ b/examples/bluetooth/lowenergyscanner/lowenergyscanner.pro @@ -10,6 +10,8 @@ SOURCES += main.cpp \ serviceinfo.cpp \ characteristicinfo.cpp +ios: QMAKE_INFO_PLIST = Info.plist + OTHER_FILES += assets/*.qml HEADERS += \ -- cgit v1.2.3 From 9ce0f0459d1d55d9399801cac90db1f9924f5f1f Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 7 Sep 2021 18:31:47 +0200 Subject: QtBluetooth (examples): add Info.plist for iOS btscanner example can be built and deployed on iOS, but then it requires special entries in its Info.plist, explaining the usage of BT adapter. Change-Id: I86edfe84a19f4da8173bb5374fb10e55811a4b20 Reviewed-by: Alex Blasche (cherry picked from commit e01660fbe52963bcdef594cf34eb443c7f6acc65) Reviewed-by: Qt Cherry-pick Bot --- examples/bluetooth/btscanner/Info.plist | 41 ++++++++++++++++++++++++++++++ examples/bluetooth/btscanner/btscanner.pro | 2 ++ 2 files changed, 43 insertions(+) create mode 100644 examples/bluetooth/btscanner/Info.plist diff --git a/examples/bluetooth/btscanner/Info.plist b/examples/bluetooth/btscanner/Info.plist new file mode 100644 index 00000000..49fd2191 --- /dev/null +++ b/examples/bluetooth/btscanner/Info.plist @@ -0,0 +1,41 @@ + + + + + CFBundleDisplayName + ${PRODUCT_NAME} + CFBundleExecutable + ${EXECUTABLE_NAME} + CFBundleIconFile + ${ASSETCATALOG_COMPILER_APPICON_NAME} + CFBundleIdentifier + ${PRODUCT_BUNDLE_IDENTIFIER} + CFBundleInfoDictionaryVersion + 6.0 + CFBundleName + ${PRODUCT_NAME} + CFBundlePackageType + APPL + CFBundleShortVersionString + ${QMAKE_SHORT_VERSION} + CFBundleSignature + ${QMAKE_PKGINFO_TYPEINFO} + CFBundleVersion + ${QMAKE_FULL_VERSION} + LSRequiresIPhoneOS + + MinimumOSVersion + ${IPHONEOS_DEPLOYMENT_TARGET} + NSBluetoothAlwaysUsageDescription + Qt's BT scanner wants to access your Bluetooth adapter! + UILaunchStoryboardName + LaunchScreen + UISupportedInterfaceOrientations + + UIInterfaceOrientationPortrait + UIInterfaceOrientationPortraitUpsideDown + UIInterfaceOrientationLandscapeLeft + UIInterfaceOrientationLandscapeRight + + + diff --git a/examples/bluetooth/btscanner/btscanner.pro b/examples/bluetooth/btscanner/btscanner.pro index 904ea5a8..d5fd66ae 100644 --- a/examples/bluetooth/btscanner/btscanner.pro +++ b/examples/bluetooth/btscanner/btscanner.pro @@ -9,6 +9,8 @@ SOURCES = \ device.cpp \ service.cpp +ios: QMAKE_INFO_PLIST = Info.plist + HEADERS = \ device.h \ service.h -- cgit v1.2.3 From 4af45b1256b92880c1a7d04d16d732eef1bee2b9 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 30 Aug 2021 19:10:00 +0200 Subject: IOBluetoothDeviceInquiry: manually interrupt Classic scan If 'inquiryLength' was ignored. This change is to be revisited after the official release of macOS 12 - it could be a bug in beta versions and all this additional timer machinery is a waste. The fact I was able to observe the same problem with 10.15.7 is quite troublesome though. Fixes: QTBUG-95960 Task-number: QTBUG-95686 Change-Id: I56685e07791dd751aec81bebbeefde19d31555df Reviewed-by: Ivan Solovev Reviewed-by: Alex Blasche (cherry picked from commit cf8ade9fd809e62cb61bb897b3789c75a48f5996) --- src/bluetooth/osx/osxbtdeviceinquiry.mm | 65 +++++++++++++++++++++++---------- 1 file changed, 46 insertions(+), 19 deletions(-) diff --git a/src/bluetooth/osx/osxbtdeviceinquiry.mm b/src/bluetooth/osx/osxbtdeviceinquiry.mm index d142da4d..5f95d617 100644 --- a/src/bluetooth/osx/osxbtdeviceinquiry.mm +++ b/src/bluetooth/osx/osxbtdeviceinquiry.mm @@ -41,15 +41,22 @@ #include "osxbtutility_p.h" #include +#include #include +#include + QT_USE_NAMESPACE +const uint8_t IOBlueoothInquiryLengthS = 15; + @implementation QT_MANGLE_NAMESPACE(OSXBTDeviceInquiry) { IOBluetoothDeviceInquiry *m_inquiry; bool m_active; DarwinBluetooth::DeviceInquiryDelegate *m_delegate;//C++ "delegate" + + std::unique_ptr watchDog; } - (id)initWithDelegate:(DarwinBluetooth::DeviceInquiryDelegate *)delegate @@ -60,7 +67,13 @@ QT_USE_NAMESPACE m_inquiry = [[IOBluetoothDeviceInquiry inquiryWithDelegate:self] retain]; if (m_inquiry) { - [m_inquiry setInquiryLength:15]; + // Inquiry length is 15 seconds. Starting from macOS 10.15.7 + // (the lowest version I was able to test on, though initially + // the problem was found on macOS 11, arm64 machine and then + // confirmed on macOS 12 Beta 4), it seems to be ignored, + // thus scan never stops. See -start for how we try to prevent + // this. + [m_inquiry setInquiryLength:IOBlueoothInquiryLengthS]; [m_inquiry setUpdateNewDeviceNames:NO];//Useless, disable! m_delegate = delegate; } else { @@ -102,9 +115,21 @@ QT_USE_NAMESPACE const IOReturn result = [m_inquiry start]; if (result != kIOReturnSuccess) { // QtBluetooth will probably convert an error into UnknownError, - // loosing the actual information. + // losing the actual information. qCWarning(QT_BT_OSX) << "failed with IOKit error code:" << result; m_active = false; + } else { + // Docs say it's 10 s. by default, we set it to 15 s. (see -initWithDelegate:), + // and it may fail to finish. + watchDog.reset(new QTimer); + watchDog->connect(watchDog.get(), &QTimer::timeout, watchDog.get(), [self]{ + qCWarning(QT_BT_OSX, "Manually interrupting IOBluetoothDeviceInquiry"); + [self stop]; + }); + + watchDog->setSingleShot(true); + watchDog->setInterval(IOBlueoothInquiryLengthS * 2 * 1000); // Let's make it twice as long. + watchDog->start(); } return result; @@ -112,40 +137,36 @@ QT_USE_NAMESPACE - (IOReturn)stop { - if (m_active) { - Q_ASSERT_X(m_inquiry, Q_FUNC_INFO, "active but nil inquiry"); + if (!m_active) + return kIOReturnSuccess; - m_active = false; - const IOReturn res = [m_inquiry stop]; - if (res != kIOReturnSuccess) - m_active = true; - else - qCDebug(QT_BT_OSX) << "-stop, success (waiting for 'inquiryComplete')"; - - return res; - } + Q_ASSERT_X(m_inquiry, Q_FUNC_INFO, "active but nil inquiry"); - return kIOReturnSuccess; + return [m_inquiry stop]; } - (void)deviceInquiryComplete:(IOBluetoothDeviceInquiry *)sender error:(IOReturn)error aborted:(BOOL)aborted { - Q_UNUSED(aborted) + if (!m_active) + return; if (sender != m_inquiry) // Can never happen in the current version. return; - m_active = false; - Q_ASSERT_X(m_delegate, Q_FUNC_INFO, "invalid device inquiry delegate (null)"); - if (error != kIOReturnSuccess) { + if (error != kIOReturnSuccess && !aborted) { // QtBluetooth has not too many error codes, 'UnknownError' is not really // useful, report the actual error code here: qCWarning(QT_BT_OSX) << "IOKit error code: " << error; m_delegate->error(error); + // Let watchDog to stop it, calling -stop at timeout, otherwise, + // it looks like inquiry continues and keeps reporting new devices found. } else { + // Either a normal completion or from a timer slot. + watchDog.reset(); + m_active = false; m_delegate->inquiryFinished(); } } @@ -156,13 +177,19 @@ QT_USE_NAMESPACE if (sender != m_inquiry) // Can never happen in the current version. return; + if (!m_active) { + // We are not expecting new device(s) to be found after we reported 'finished'. + qCWarning(QT_BT_OSX, "IOBluetooth device found after inquiry complete/interrupted"); + return; + } + Q_ASSERT_X(m_delegate, Q_FUNC_INFO, "invalid device inquiry delegate (null)"); m_delegate->classicDeviceFound(device); } - (void)deviceInquiryStarted:(IOBluetoothDeviceInquiry *)sender { - Q_UNUSED(sender) + Q_UNUSED(sender); } @end -- cgit v1.2.3 From 5beb4a8d37ae72abf801d20f04870f3f0652d89c Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Tue, 14 Sep 2021 16:15:01 +0200 Subject: winrt: Protect from late AdvertisementReceived callback This commit amends 83a845aa0e5bb155fae3f1d5d27bb80801f90935. The callback can be called after the worker was deleted. Task-number: QTBUG-96057 Change-Id: Ifbd58b240667bd5ff805b4250cf98a9fd2f96e90 Reviewed-by: Alex Blasche (cherry picked from commit 3573702aa81f3f7ea20e947dbd27c5d1122cc42a) Reviewed-by: Qt CI Bot --- .../qbluetoothdevicediscoveryagent_winrt.cpp | 206 +++++++++++---------- 1 file changed, 109 insertions(+), 97 deletions(-) diff --git a/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp b/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp index a64f7f0f..e515d4e1 100644 --- a/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp +++ b/src/bluetooth/qbluetoothdevicediscoveryagent_winrt.cpp @@ -155,6 +155,7 @@ private: #if QT_CONFIG(winrt_btle_no_pairing) HRESULT onBluetoothLEDeviceFound(ComPtr device); #endif + HRESULT onBluetoothLEAdvertisementReceived(IBluetoothLEAdvertisementReceivedEventArgs *args); public slots: void finishDiscovery(); @@ -334,113 +335,74 @@ void QWinRTBluetoothDeviceDiscoveryWorker::gatherMultipleDeviceInformation(quint } } -void QWinRTBluetoothDeviceDiscoveryWorker::setupLEDeviceWatcher() +HRESULT QWinRTBluetoothDeviceDiscoveryWorker::onBluetoothLEAdvertisementReceived(IBluetoothLEAdvertisementReceivedEventArgs *args) { - HRESULT hr = RoActivateInstance(HString::MakeReference(RuntimeClass_Windows_Devices_Bluetooth_Advertisement_BluetoothLEAdvertisementWatcher).Get(), &m_leWatcher); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not create advertisment watcher", - QBluetoothDeviceDiscoveryAgent::Error::UnknownError, - return); + quint64 address; + HRESULT hr; + hr = args->get_BluetoothAddress(&address); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain bluetooth address", + QBluetoothDeviceDiscoveryAgent::Error::UnknownError, + return S_OK); + qint16 rssi; + hr = args->get_RawSignalStrengthInDBm(&rssi); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain signal strength", + QBluetoothDeviceDiscoveryAgent::Error::UnknownError, + return S_OK); + ComPtr ad; + hr = args->get_Advertisement(&ad); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could get advertisement", + QBluetoothDeviceDiscoveryAgent::Error::UnknownError, + return S_OK); + const ManufacturerData manufacturerData = extractManufacturerData(ad); + QBluetoothDeviceInfo::Fields changedFields = QBluetoothDeviceInfo::Field::None; + if (!m_foundLEManufacturerData.contains(address)) { + m_foundLEManufacturerData.insert(address, manufacturerData); + changedFields.setFlag(QBluetoothDeviceInfo::Field::ManufacturerData); + } else if (m_foundLEManufacturerData.value(address) != manufacturerData) { + m_foundLEManufacturerData[address] = manufacturerData; + changedFields.setFlag(QBluetoothDeviceInfo::Field::ManufacturerData); + } #if QT_CONFIG(winrt_btle_no_pairing) if (supportsNewLEApi()) { - hr = m_leWatcher->put_ScanningMode(BluetoothLEScanningMode_Active); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not set scanning mode", - QBluetoothDeviceDiscoveryAgent::Error::UnknownError, - return); - } -#endif // winrt_btle_no_pairing - hr = m_leWatcher->add_Received(Callback>([this](IBluetoothLEAdvertisementWatcher *, IBluetoothLEAdvertisementReceivedEventArgs *args) { - quint64 address; - HRESULT hr; - hr = args->get_BluetoothAddress(&address); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain bluetooth address", - QBluetoothDeviceDiscoveryAgent::Error::UnknownError, - return S_OK); - qint16 rssi; - hr = args->get_RawSignalStrengthInDBm(&rssi); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain signal strength", + ComPtr> guids; + hr = ad->get_ServiceUuids(&guids); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain service uuid list", QBluetoothDeviceDiscoveryAgent::Error::UnknownError, return S_OK); - ComPtr ad; - hr = args->get_Advertisement(&ad); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could get advertisement", + quint32 size; + hr = guids->get_Size(&size); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain service uuid list size", QBluetoothDeviceDiscoveryAgent::Error::UnknownError, return S_OK); - const ManufacturerData manufacturerData = extractManufacturerData(ad); - QBluetoothDeviceInfo::Fields changedFields = QBluetoothDeviceInfo::Field::None; - if (!m_foundLEManufacturerData.contains(address)) { - m_foundLEManufacturerData.insert(address, manufacturerData); - changedFields.setFlag(QBluetoothDeviceInfo::Field::ManufacturerData); - } else if (m_foundLEManufacturerData.value(address) != manufacturerData) { - m_foundLEManufacturerData[address] = manufacturerData; - changedFields.setFlag(QBluetoothDeviceInfo::Field::ManufacturerData); - } -#if QT_CONFIG(winrt_btle_no_pairing) - if (supportsNewLEApi()) { - ComPtr> guids; - hr = ad->get_ServiceUuids(&guids); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain service uuid list", - QBluetoothDeviceDiscoveryAgent::Error::UnknownError, - return S_OK); - quint32 size; - hr = guids->get_Size(&size); - QVector serviceUuids; - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain service uuid list size", + QVector serviceUuids; + for (quint32 i = 0; i < size; ++i) { + GUID guid; + hr = guids->GetAt(i, &guid); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain uuid", QBluetoothDeviceDiscoveryAgent::Error::UnknownError, return S_OK); - for (quint32 i = 0; i < size; ++i) { - GUID guid; - hr = guids->GetAt(i, &guid); - EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not obtain uuid", - QBluetoothDeviceDiscoveryAgent::Error::UnknownError, - return S_OK); - QBluetoothUuid uuid(guid); - serviceUuids.append(uuid); + QBluetoothUuid uuid(guid); + serviceUuids.append(uuid); + } + QMutexLocker locker(&m_foundDevicesMutex); + // Merge newly found services with list of currently found ones + if (m_foundLEDevicesMap.contains(address)) { + if (size == 0) + return S_OK; + const LEAdvertisingInfo adInfo = m_foundLEDevicesMap.value(address); + QVector foundServices = adInfo.services; + if (adInfo.rssi != rssi) { + m_foundLEDevicesMap[address].rssi = rssi; + changedFields.setFlag(QBluetoothDeviceInfo::Field::RSSI); } - QMutexLocker locker(&m_foundDevicesMutex); - // Merge newly found services with list of currently found ones - if (m_foundLEDevicesMap.contains(address)) { - if (size == 0) - return S_OK; - const LEAdvertisingInfo adInfo = m_foundLEDevicesMap.value(address); - QVector foundServices = adInfo.services; - if (adInfo.rssi != rssi) { - m_foundLEDevicesMap[address].rssi = rssi; - changedFields.setFlag(QBluetoothDeviceInfo::Field::RSSI); - } - bool newServiceAdded = false; - for (const QBluetoothUuid &uuid : qAsConst(serviceUuids)) { - if (!foundServices.contains(uuid)) { - foundServices.append(uuid); - newServiceAdded = true; - } + bool newServiceAdded = false; + for (const QBluetoothUuid &uuid : qAsConst(serviceUuids)) { + if (!foundServices.contains(uuid)) { + foundServices.append(uuid); + newServiceAdded = true; } - if (!newServiceAdded) { - if (!changedFields.testFlag(QBluetoothDeviceInfo::Field::None)) { - QMetaObject::invokeMethod(this, "deviceDataChanged", Qt::AutoConnection, - Q_ARG(QBluetoothAddress, QBluetoothAddress(address)), - Q_ARG(QBluetoothDeviceInfo::Fields, changedFields), - Q_ARG(qint16, rssi), - Q_ARG(ManufacturerData, manufacturerData)); - } - return S_OK; - } - m_foundLEDevicesMap[address].services = foundServices; - } else { - LEAdvertisingInfo info; - info.services = std::move(serviceUuids); - info.rssi = rssi; - m_foundLEDevicesMap.insert(address, info); } - - locker.unlock(); - } else -#endif - { - if (m_foundLEDevices.contains(address)) { - if (m_foundLEDevices.value(address) != rssi) { - m_foundLEDevices[address] = rssi; - changedFields.setFlag(QBluetoothDeviceInfo::Field::RSSI); - } + if (!newServiceAdded) { if (!changedFields.testFlag(QBluetoothDeviceInfo::Field::None)) { QMetaObject::invokeMethod(this, "deviceDataChanged", Qt::AutoConnection, Q_ARG(QBluetoothAddress, QBluetoothAddress(address)), @@ -450,9 +412,59 @@ void QWinRTBluetoothDeviceDiscoveryWorker::setupLEDeviceWatcher() } return S_OK; } - m_foundLEDevices.insert(address, rssi); + m_foundLEDevicesMap[address].services = foundServices; + } else { + LEAdvertisingInfo info; + info.services = std::move(serviceUuids); + info.rssi = rssi; + m_foundLEDevicesMap.insert(address, info); + } + + locker.unlock(); + } else +#endif // QT_CONFIG(winrt_btle_no_pairing) + { + if (m_foundLEDevices.contains(address)) { + if (m_foundLEDevices.value(address) != rssi) { + m_foundLEDevices[address] = rssi; + changedFields.setFlag(QBluetoothDeviceInfo::Field::RSSI); + } + if (!changedFields.testFlag(QBluetoothDeviceInfo::Field::None)) { + QMetaObject::invokeMethod(this, "deviceDataChanged", Qt::AutoConnection, + Q_ARG(QBluetoothAddress, QBluetoothAddress(address)), + Q_ARG(QBluetoothDeviceInfo::Fields, changedFields), + Q_ARG(qint16, rssi), + Q_ARG(ManufacturerData, manufacturerData)); + } + return S_OK; } - leBluetoothInfoFromAddressAsync(address); + m_foundLEDevices.insert(address, rssi); + } + leBluetoothInfoFromAddressAsync(address); + return S_OK; +} + +void QWinRTBluetoothDeviceDiscoveryWorker::setupLEDeviceWatcher() +{ + HRESULT hr = RoActivateInstance(HString::MakeReference(RuntimeClass_Windows_Devices_Bluetooth_Advertisement_BluetoothLEAdvertisementWatcher).Get(), &m_leWatcher); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not create advertisment watcher", + QBluetoothDeviceDiscoveryAgent::Error::UnknownError, + return); +#if QT_CONFIG(winrt_btle_no_pairing) + if (supportsNewLEApi()) { + hr = m_leWatcher->put_ScanningMode(BluetoothLEScanningMode_Active); + EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not set scanning mode", + QBluetoothDeviceDiscoveryAgent::Error::UnknownError, + return); + } +#endif // QT_CONFIG(winrt_btle_no_pairing) + QPointer thisPointer(this); + hr = m_leWatcher->add_Received( + Callback>( + [thisPointer](IBluetoothLEAdvertisementWatcher *, IBluetoothLEAdvertisementReceivedEventArgs *args) { + if (thisPointer) + return thisPointer->onBluetoothLEAdvertisementReceived(args); + return S_OK; }).Get(), &m_leDeviceAddedToken); EMIT_WORKER_ERROR_AND_RETURN_IF_FAILED("Could not add device callback", -- cgit v1.2.3 From 359721c3b4c099b3bd00ae72b51935d38a0ec640 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Mon, 13 Sep 2021 15:54:26 +0200 Subject: QLowEnergyController(WinRT): introduce timeout for connection The connection method was infinitely trying to connect to a specified bluetooth device, which does not make much sense and could lead to the application being stuck. This patch limits the connection attempts to a reasonable amount of time. If the connection was not established within this time, ConnectionError is set and controller state is reset to UnconnectedState Fixes: QTBUG-80719 Fixes: QTBUG-89149 Change-Id: Ib8efb690a8b0485c8e9d4844799c7332ab81bf97 Reviewed-by: Alex Blasche (cherry picked from commit 007dcbf074a9e4d72e2a29a4a28ac5e502830e20) Reviewed-by: Qt CI Bot --- src/bluetooth/qlowenergycontroller_winrt_new.cpp | 21 +++++++++++++++++++-- 1 file changed, 19 insertions(+), 2 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt_new.cpp b/src/bluetooth/qlowenergycontroller_winrt_new.cpp index a1f27fc3..9fa86e05 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt_new.cpp @@ -52,6 +52,7 @@ #include #include #include +#include #include #include @@ -115,6 +116,8 @@ typedef IGattReadClientCharacteristicConfigurationDescriptorResult IClientCharCo Q_DECLARE_LOGGING_CATEGORY(QT_BT_WINRT) Q_DECLARE_LOGGING_CATEGORY(QT_BT_WINRT_SERVICE_THREAD) +static constexpr qint64 kMaxConnectTimeout = 20000; // 20 sec + QLowEnergyControllerPrivate *createWinRTLowEnergyController() { if (supportsNewLEApi()) { @@ -1574,7 +1577,8 @@ void QLowEnergyControllerPrivateWinRTNew::connectToPairedDevice() HRESULT hr = mDevice.As(&device3); CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast device", return) ComPtr> deviceServicesOp; - while (!mAbortPending) { + QDeadlineTimer deadline(kMaxConnectTimeout); + while (!mAbortPending && !deadline.hasExpired()) { hr = device3->GetGattServicesAsync(&deviceServicesOp); CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain services", return) ComPtr deviceServicesResult; @@ -1683,6 +1687,12 @@ void QLowEnergyControllerPrivateWinRTNew::connectToPairedDevice() } } } + if (deadline.hasExpired()) { + qCWarning(QT_BT_WINRT) << "Connect to device failed due to timeout!"; + setError(QLowEnergyController::ConnectionError); + setState(QLowEnergyController::UnconnectedState); + unregisterFromStatusChanges(); + } } void QLowEnergyControllerPrivateWinRTNew::connectToUnpairedDevice() @@ -1696,7 +1706,8 @@ void QLowEnergyControllerPrivateWinRTNew::connectToUnpairedDevice() HRESULT hr = mDevice.As(&device3); CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast device", return) ComPtr deviceServicesResult; - while (!mAbortPending) { + QDeadlineTimer deadline(kMaxConnectTimeout); + while (!mAbortPending && !deadline.hasExpired()) { ComPtr> deviceServicesOp; hr = device3->GetGattServicesAsync(&deviceServicesOp); CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain services", return) @@ -1719,6 +1730,12 @@ void QLowEnergyControllerPrivateWinRTNew::connectToUnpairedDevice() break; } + if (deadline.hasExpired()) { + qCWarning(QT_BT_WINRT) << "Connect to device failed due to timeout!"; + setError(QLowEnergyController::ConnectionError); + setState(QLowEnergyController::UnconnectedState); + unregisterFromStatusChanges(); + } } QT_END_NAMESPACE -- cgit v1.2.3 From 8df5f6ad7056a6cbba272a1a8c8861fa9b0f4ac6 Mon Sep 17 00:00:00 2001 From: Juha Vuolle Date: Tue, 14 Sep 2021 13:35:25 +0300 Subject: Move connecting to a LE device to background on Windows The BT LE connectToDevice() method on Windows was a synchronous operation for the caller by blocking while spinning the event loop to keep rest of the application responsive. Other platforms behave differently. This commit makes the connect call to return immediately while scheduling the connection in the background. The QTBUG-83633 was originally about slightly different crash which seems not be reproducible anymore, but this crash was found while investigating that and is investigated in that item. Task-number: QTBUG-83633 Change-Id: I092a94f2437351d27da758db6746f5b24d5fa9c7 Reviewed-by: Alex Blasche (cherry picked from commit 3fc3716e0cd209cd9475d632925a9148b40b2f1d) Reviewed-by: Oliver Wolff --- src/bluetooth/qlowenergycontroller_winrt.cpp | 12 ++++++++++-- src/bluetooth/qlowenergycontroller_winrt_new.cpp | 13 +++++++++++-- src/bluetooth/qlowenergycontroller_winrt_new_p.h | 1 + src/bluetooth/qlowenergycontroller_winrt_p.h | 1 + 4 files changed, 23 insertions(+), 4 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt.cpp b/src/bluetooth/qlowenergycontroller_winrt.cpp index a9f9fd64..6a039337 100644 --- a/src/bluetooth/qlowenergycontroller_winrt.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt.cpp @@ -260,14 +260,22 @@ void QLowEnergyControllerPrivateWinRT::init() void QLowEnergyControllerPrivateWinRT::connectToDevice() { qCDebug(QT_BT_WINRT) << __FUNCTION__; - Q_Q(QLowEnergyController); if (remoteDevice.isNull()) { qWarning() << "Invalid/null remote device address"; setError(QLowEnergyController::UnknownRemoteDeviceError); return; } - setState(QLowEnergyController::ConnectingState); + // Queue the device connecting to happen in the background + QMetaObject::invokeMethod(this, + &QLowEnergyControllerPrivateWinRT::doConnectToDevice, + Qt::QueuedConnection); +} + +void QLowEnergyControllerPrivateWinRT::doConnectToDevice() +{ + qCDebug(QT_BT_WINRT) << __FUNCTION__; + Q_Q(QLowEnergyController); ComPtr deviceStatics; HRESULT hr = GetActivationFactory(HString::MakeReference(RuntimeClass_Windows_Devices_Bluetooth_BluetoothLEDevice).Get(), &deviceStatics); diff --git a/src/bluetooth/qlowenergycontroller_winrt_new.cpp b/src/bluetooth/qlowenergycontroller_winrt_new.cpp index 9fa86e05..f448fe33 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt_new.cpp @@ -459,14 +459,23 @@ void QLowEnergyControllerPrivateWinRTNew::connectToDevice() { qCDebug(QT_BT_WINRT) << __FUNCTION__; mAbortPending = false; - Q_Q(QLowEnergyController); if (remoteDevice.isNull()) { qWarning() << "Invalid/null remote device address"; setError(QLowEnergyController::UnknownRemoteDeviceError); return; } - setState(QLowEnergyController::ConnectingState); + // Queue the device connecting to happen in the background + QMetaObject::invokeMethod(this, + &QLowEnergyControllerPrivateWinRTNew::doConnectToDevice, + Qt::QueuedConnection); +} + + +void QLowEnergyControllerPrivateWinRTNew::doConnectToDevice() +{ + qCDebug(QT_BT_WINRT) << __FUNCTION__; + Q_Q(QLowEnergyController); ComPtr deviceStatics; HRESULT hr = GetActivationFactory( diff --git a/src/bluetooth/qlowenergycontroller_winrt_new_p.h b/src/bluetooth/qlowenergycontroller_winrt_new_p.h index d7d90dbd..b238f48e 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new_p.h +++ b/src/bluetooth/qlowenergycontroller_winrt_new_p.h @@ -136,6 +136,7 @@ signals: private slots: void handleCharacteristicChanged(quint16 charHandle, const QByteArray &data); void handleServiceHandlerError(const QString &error); + void doConnectToDevice(); private: void connectToPairedDevice(); diff --git a/src/bluetooth/qlowenergycontroller_winrt_p.h b/src/bluetooth/qlowenergycontroller_winrt_p.h index d87477c6..629f62fc 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_p.h +++ b/src/bluetooth/qlowenergycontroller_winrt_p.h @@ -119,6 +119,7 @@ signals: private slots: void handleCharacteristicChanged(quint16 charHandle, const QByteArray &data); + void doConnectToDevice(); private: Microsoft::WRL::ComPtr mDevice; -- cgit v1.2.3 From 571b9ee1f7ae42ccb220ae6314d4fa73dc9884cb Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Thu, 23 Sep 2021 18:07:38 +0200 Subject: QLowEnergyControllerWinRT: refactor connection to device Introduce a separate worker that will do all the system calls in a separate thread and emit a signal once it's done. This will allow to avoid the crashes caused by disconnectFromDevice() being called while connection is still in progress and spinning in the QWinRTFunctions::await() method. Basically this patch moves the connection code to a separate worker, and introduces new macros to handle the errors. It also makes use of the new early return condition of QWinRTFunctions::await() that was introduced in 1f86957f1dd14cc538e7ad9ffee4eb63001af407. As a drive-by: increased the characteristics read timeout, because the initial one was not always enough even for a turned-on device. Apart from that this patch also solves some other issues and crashes that could happen due to async nature of some calls. For example: - handle the fact that service discovery might finish after the device was disconnected. No need to notify about discovered services in this case. - add missing checks for thisPtr != nullptr in some callbacks that capture thisPtr. Task-number: QTBUG-96057 Change-Id: Ia2d044a89e3427a53d0879e045b6230d16bac3ce Reviewed-by: Oliver Wolff (cherry picked from commit a00ffdfc3569a5741bc42739570dea2ec52b7f0f) --- src/bluetooth/qlowenergycontroller_winrt_new.cpp | 576 ++++++++++++++--------- src/bluetooth/qlowenergycontroller_winrt_new_p.h | 6 +- 2 files changed, 358 insertions(+), 224 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt_new.cpp b/src/bluetooth/qlowenergycontroller_winrt_new.cpp index f448fe33..7b0a6f14 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt_new.cpp @@ -82,10 +82,16 @@ typedef ITypedEventHandler Va typedef GattReadClientCharacteristicConfigurationDescriptorResult ClientCharConfigDescriptorResult; typedef IGattReadClientCharacteristicConfigurationDescriptorResult IClientCharConfigDescriptorResult; -#define EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, ret) \ +#define EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr) \ if (FAILED(hr)) { \ emitErrorAndQuitThread(hr); \ - ret; \ + return; \ + } + +#define EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, message) \ + if (FAILED(hr)) { \ + emitErrorAndQuitThread(message); \ + return; \ } #define WARN_AND_CONTINUE_IF_FAILED(hr, msg) \ @@ -166,24 +172,24 @@ public slots: ComPtr> characteristicsOp; ComPtr characteristicsResult; HRESULT hr = mDeviceService->GetCharacteristicsAsync(&characteristicsOp); - EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, return); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr); hr = QWinRTFunctions::await(characteristicsOp, characteristicsResult.GetAddressOf(), QWinRTFunctions::ProcessMainThreadEvents, 5000); - EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, return); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr); GattCommunicationStatus status; hr = characteristicsResult->get_Status(&status); - EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, return); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr); if (status != GattCommunicationStatus_Success) { emitErrorAndQuitThread(QLatin1String("Could not obtain char list")); return; } ComPtr> characteristics; hr = characteristicsResult->get_Characteristics(&characteristics); - EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, return); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr); uint characteristicsCount; hr = characteristics->get_Size(&characteristicsCount); - EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr, return); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED(hr); mCharacteristicsCountToBeDiscovered = characteristicsCount; for (uint i = 0; i < characteristicsCount; ++i) { @@ -435,6 +441,292 @@ void QWinRTLowEnergyServiceHandlerNew::emitErrorAndQuitThread(const QString &err QThread::currentThread()->quit(); } +class QWinRTLowEnergyConnectionHandler : public QObject +{ + Q_OBJECT +public: + explicit QWinRTLowEnergyConnectionHandler(const QBluetoothAddress &address) : mAddress(address) + { + qCDebug(QT_BT_WINRT) << __FUNCTION__; + // This should be checked before the handler is created + Q_ASSERT(!mAddress.isNull()); + qRegisterMetaType>("ComPtr"); + } + ~QWinRTLowEnergyConnectionHandler() + { + qCDebug(QT_BT_WINRT) << __FUNCTION__; + // To close the COM library gracefully, each successful call to + // CoInitialize, including those that return S_FALSE, must be balanced + // by a corresponding call to CoUninitialize. + if (mInitialized == S_OK || mInitialized == S_FALSE) + CoUninitialize(); + } + +public slots: + void connectToDevice(); + void handleDeviceDisconnectRequest(); + +signals: + void deviceConnected(ComPtr device); + void errorOccurred(const QString &error); + +private: + void connectToPairedDevice(); + void connectToUnpairedDevice(); + void emitErrorAndQuitThread(const QString &error); + void emitErrorAndQuitThread(const char *error); + void emitConnectedAndQuitThread(); + + ComPtr mDevice = nullptr; + ComPtr mGattSession = nullptr; + const QBluetoothAddress mAddress; + bool mAbortConnection = false; + HRESULT mInitialized = E_UNEXPECTED; // some error code +}; + +void QWinRTLowEnergyConnectionHandler::connectToDevice() +{ + qCDebug(QT_BT_WINRT) << __FUNCTION__; + mInitialized = CoInitialize(NULL); + qCDebug(QT_BT_WINRT) << qt_error_string(mInitialized); + + auto earlyExit = [this]() { return mAbortConnection; }; + ComPtr deviceStatics; + HRESULT hr = GetActivationFactory( + HString::MakeReference(RuntimeClass_Windows_Devices_Bluetooth_BluetoothLEDevice).Get(), + &deviceStatics); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain device factory"); + ComPtr> deviceFromIdOperation; + hr = deviceStatics->FromBluetoothAddressAsync(mAddress.toUInt64(), &deviceFromIdOperation); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not find LE device from address"); + hr = QWinRTFunctions::await(deviceFromIdOperation, mDevice.GetAddressOf(), + QWinRTFunctions::ProcessMainThreadEvents, 5000, earlyExit); + if (FAILED(hr) || !mDevice) { + emitErrorAndQuitThread("Could not find LE device"); + return; + } + + // get GattSession: 1. get device id + ComPtr device4; + hr = mDevice.As(&device4); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not cast device"); + + ComPtr deviceId; + hr = device4->get_BluetoothDeviceId(&deviceId); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not get bluetooth device id"); + + // get GattSession: 2. get session statics + ComPtr sessionStatics; + hr = GetActivationFactory( + HString::MakeReference( + RuntimeClass_Windows_Devices_Bluetooth_GenericAttributeProfile_GattSession) + .Get(), + &sessionStatics); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain GattSession statics"); + + // get GattSession: 3. get session + ComPtr> gattSessionFromIdOperation; + hr = sessionStatics->FromDeviceIdAsync(deviceId.Get(), &gattSessionFromIdOperation); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not get GattSession from id"); + hr = QWinRTFunctions::await(gattSessionFromIdOperation, mGattSession.GetAddressOf(), + QWinRTFunctions::ProcessMainThreadEvents, 5000, earlyExit); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not complete Gatt session acquire"); + + BluetoothConnectionStatus status; + hr = mDevice->get_ConnectionStatus(&status); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain device's connection status"); + if (status == BluetoothConnectionStatus::BluetoothConnectionStatus_Connected) { + emitConnectedAndQuitThread(); + return; + } + + QBluetoothLocalDevice localDevice; + QBluetoothLocalDevice::Pairing pairing = localDevice.pairingStatus(mAddress); + if (pairing == QBluetoothLocalDevice::Unpaired) + connectToUnpairedDevice(); + else + connectToPairedDevice(); +} + +void QWinRTLowEnergyConnectionHandler::handleDeviceDisconnectRequest() +{ + mAbortConnection = true; + // Disconnect from the QLowEnergyControllerPrivateWinRT, so that it does + // not get notifications. It's absolutely fine to keep doing smth in + // background, as multiple connections to the same device should be handled + // correctly by OS. + disconnect(this, &QWinRTLowEnergyConnectionHandler::deviceConnected, nullptr, nullptr); + disconnect(this, &QWinRTLowEnergyConnectionHandler::errorOccurred, nullptr, nullptr); +} + +void QWinRTLowEnergyConnectionHandler::connectToPairedDevice() +{ + qCDebug(QT_BT_WINRT) << __FUNCTION__; + ComPtr device3; + HRESULT hr = mDevice.As(&device3); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not cast device"); + ComPtr> deviceServicesOp; + auto earlyExit = [this]() { return mAbortConnection; }; + QDeadlineTimer deadline(kMaxConnectTimeout); + while (!mAbortConnection && !deadline.hasExpired()) { + hr = device3->GetGattServicesAsync(&deviceServicesOp); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain services"); + ComPtr deviceServicesResult; + hr = QWinRTFunctions::await(deviceServicesOp, deviceServicesResult.GetAddressOf(), + QWinRTFunctions::ProcessThreadEvents, 5000, earlyExit); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not await services operation"); + + GattCommunicationStatus commStatus; + hr = deviceServicesResult->get_Status(&commStatus); + if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { + emitErrorAndQuitThread("Service operation failed"); + return; + } + + ComPtr> deviceServices; + hr = deviceServicesResult->get_Services(&deviceServices); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain list of services"); + uint serviceCount; + hr = deviceServices->get_Size(&serviceCount); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain service count"); + + if (serviceCount == 0) { + emitErrorAndQuitThread("Found devices without services"); + return; + } + + // Windows automatically connects to the device as soon as a service value is read/written. + // Thus we read one value in order to establish the connection. + for (uint i = 0; i < serviceCount; ++i) { + ComPtr service; + hr = deviceServices->GetAt(i, &service); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain service"); + ComPtr service3; + hr = service.As(&service3); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not cast service"); + ComPtr> characteristicsOp; + hr = service3->GetCharacteristicsAsync(&characteristicsOp); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain characteristic"); + ComPtr characteristicsResult; + hr = QWinRTFunctions::await(characteristicsOp, characteristicsResult.GetAddressOf(), + QWinRTFunctions::ProcessThreadEvents, 5000, earlyExit); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not await characteristic operation"); + GattCommunicationStatus commStatus; + hr = characteristicsResult->get_Status(&commStatus); + if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { + qCWarning(QT_BT_WINRT) << "Characteristic operation failed"; + break; + } + ComPtr> characteristics; + hr = characteristicsResult->get_Characteristics(&characteristics); + if (hr == E_ACCESSDENIED) { + // Everything will work as expected up until this point if the + // manifest capabilties for bluetooth LE are not set. + emitErrorAndQuitThread("Could not obtain characteristic list. " + "Please check your manifest capabilities"); + return; + } + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain characteristic list"); + uint characteristicsCount; + hr = characteristics->get_Size(&characteristicsCount); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, + "Could not obtain characteristic list's size"); + for (uint j = 0; j < characteristicsCount; ++j) { + ComPtr characteristic; + hr = characteristics->GetAt(j, &characteristic); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain characteristic"); + ComPtr> op; + GattCharacteristicProperties props; + hr = characteristic->get_CharacteristicProperties(&props); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2( + hr, "Could not obtain characteristic's properties"); + if (!(props & GattCharacteristicProperties_Read)) + continue; + hr = characteristic->ReadValueWithCacheModeAsync( + BluetoothCacheMode::BluetoothCacheMode_Uncached, &op); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not read characteristic value"); + ComPtr result; + // Reading characteristics can take surprisingly long at the + // first time, so we need to have a large the timeout here. + hr = QWinRTFunctions::await(op, result.GetAddressOf(), + QWinRTFunctions::ProcessThreadEvents, 5000, earlyExit); + // E_ILLEGAL_METHOD_CALL will be the result for a device, that is not reachable at + // the moment. In this case we should jump back into the outer loop and keep trying. + if (hr == E_ILLEGAL_METHOD_CALL) + break; + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not await characteristic read"); + ComPtr buffer; + hr = result->get_Value(&buffer); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain characteristic value"); + if (!buffer) { + qCDebug(QT_BT_WINRT) << "Problem reading value"; + break; + } + + emitConnectedAndQuitThread(); + return; + } + } + } + // If we got here because of mAbortConnection == true, the error message + // will not be delivered, so it does not matter. But we need to terminate + // the thread anyway! + emitErrorAndQuitThread("Connect to device failed due to timeout!"); +} + +void QWinRTLowEnergyConnectionHandler::connectToUnpairedDevice() +{ + qCDebug(QT_BT_WINRT) << __FUNCTION__; + ComPtr device3; + HRESULT hr = mDevice.As(&device3); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not cast device"); + ComPtr deviceServicesResult; + auto earlyExit = [this]() { return mAbortConnection; }; + QDeadlineTimer deadline(kMaxConnectTimeout); + while (!mAbortConnection && !deadline.hasExpired()) { + ComPtr> deviceServicesOp; + hr = device3->GetGattServicesAsync(&deviceServicesOp); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not obtain services"); + hr = QWinRTFunctions::await(deviceServicesOp, deviceServicesResult.GetAddressOf(), + QWinRTFunctions::ProcessMainThreadEvents, 0, earlyExit); + EMIT_WORKER_ERROR_AND_QUIT_IF_FAILED_2(hr, "Could not await services operation"); + + GattCommunicationStatus commStatus; + hr = deviceServicesResult->get_Status(&commStatus); + if (commStatus == GattCommunicationStatus_Unreachable) + continue; + + if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { + emitErrorAndQuitThread("Service operation failed"); + return; + } + + emitConnectedAndQuitThread(); + return; + } + // If we got here because of mAbortConnection == true, the error message + // will not be delivered, so it does not matter. But we need to terminate + // the thread anyway! + emitErrorAndQuitThread("Connect to device failed due to timeout!"); +} + +void QWinRTLowEnergyConnectionHandler::emitErrorAndQuitThread(const QString &error) +{ + emit errorOccurred(error); + QThread::currentThread()->quit(); +} + +void QWinRTLowEnergyConnectionHandler::emitErrorAndQuitThread(const char *error) +{ + emitErrorAndQuitThread(QString::fromUtf8(error)); +} + +void QWinRTLowEnergyConnectionHandler::emitConnectedAndQuitThread() +{ + emit deviceConnected(mDevice); + QThread::currentThread()->quit(); +} + QLowEnergyControllerPrivateWinRTNew::QLowEnergyControllerPrivateWinRTNew() : QLowEnergyControllerPrivate() { @@ -448,7 +740,6 @@ QLowEnergyControllerPrivateWinRTNew::~QLowEnergyControllerPrivateWinRTNew() { unregisterFromStatusChanges(); unregisterFromValueChanges(); - mAbortPending = true; } void QLowEnergyControllerPrivateWinRTNew::init() @@ -458,56 +749,39 @@ void QLowEnergyControllerPrivateWinRTNew::init() void QLowEnergyControllerPrivateWinRTNew::connectToDevice() { qCDebug(QT_BT_WINRT) << __FUNCTION__; - mAbortPending = false; if (remoteDevice.isNull()) { qWarning() << "Invalid/null remote device address"; setError(QLowEnergyController::UnknownRemoteDeviceError); return; } setState(QLowEnergyController::ConnectingState); - // Queue the device connecting to happen in the background - QMetaObject::invokeMethod(this, - &QLowEnergyControllerPrivateWinRTNew::doConnectToDevice, - Qt::QueuedConnection); -} - - -void QLowEnergyControllerPrivateWinRTNew::doConnectToDevice() -{ - qCDebug(QT_BT_WINRT) << __FUNCTION__; - Q_Q(QLowEnergyController); - ComPtr deviceStatics; - HRESULT hr = GetActivationFactory( - HString::MakeReference(RuntimeClass_Windows_Devices_Bluetooth_BluetoothLEDevice).Get(), - &deviceStatics); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain device factory", return) - ComPtr> deviceFromIdOperation; - hr = deviceStatics->FromBluetoothAddressAsync(remoteDevice.toUInt64(), &deviceFromIdOperation); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not find LE device from address", return) - hr = QWinRTFunctions::await(deviceFromIdOperation, mDevice.GetAddressOf(), - QWinRTFunctions::ProcessMainThreadEvents, 5000); - if (FAILED(hr) || !mDevice) { - qCWarning(QT_BT_WINRT) << "Could not find LE device"; - setError(QLowEnergyController::InvalidBluetoothAdapterError); - setState(QLowEnergyController::UnconnectedState); - return; - } - BluetoothConnectionStatus status; - hr = mDevice->get_ConnectionStatus(&status); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain device's connection status", return) - if (status == BluetoothConnectionStatus::BluetoothConnectionStatus_Connected) { - setState(QLowEnergyController::ConnectedState); - emit q->connected(); - return; - } - - QBluetoothLocalDevice localDevice; - QBluetoothLocalDevice::Pairing pairing = localDevice.pairingStatus(remoteDevice); - if (pairing == QBluetoothLocalDevice::Unpaired) - connectToUnpairedDevice(); - else - connectToPairedDevice(); + QWinRTLowEnergyConnectionHandler *worker = new QWinRTLowEnergyConnectionHandler(remoteDevice); + QThread *thread = new QThread; + worker->moveToThread(thread); + connect(this, &QLowEnergyControllerPrivateWinRTNew::abortConnection, worker, + &QWinRTLowEnergyConnectionHandler::handleDeviceDisconnectRequest); + connect(thread, &QThread::started, worker, &QWinRTLowEnergyConnectionHandler::connectToDevice); + connect(thread, &QThread::finished, thread, &QObject::deleteLater); + connect(thread, &QThread::finished, worker, &QObject::deleteLater); + connect(worker, &QWinRTLowEnergyConnectionHandler::errorOccurred, this, + [this](const QString &msg) { handleConnectionError(msg.toUtf8().constData()); }); + connect(worker, &QWinRTLowEnergyConnectionHandler::deviceConnected, this, + [this](ComPtr device) { + if (!device) { + handleConnectionError("Failed to get device"); + return; + } + mDevice = device; + if (!registerForStatusChanges()) { + handleConnectionError("Failed to register for changes"); + return; + } + Q_Q(QLowEnergyController); + setState(QLowEnergyController::ConnectedState); + emit q->connected(); + }); + thread->start(); } void QLowEnergyControllerPrivateWinRTNew::disconnectFromDevice() @@ -515,9 +789,9 @@ void QLowEnergyControllerPrivateWinRTNew::disconnectFromDevice() qCDebug(QT_BT_WINRT) << __FUNCTION__; Q_Q(QLowEnergyController); setState(QLowEnergyController::ClosingState); + emit abortConnection(); unregisterFromValueChanges(); unregisterFromStatusChanges(); - mAbortPending = true; mDevice = nullptr; setState(QLowEnergyController::UnconnectedState); emit q->disconnected(); @@ -701,6 +975,9 @@ void QLowEnergyControllerPrivateWinRTNew::obtainIncludedServices( ComPtr result; hr = QWinRTFunctions::await(op, result.GetAddressOf(), QWinRTFunctions::ProcessMainThreadEvents, 5000); RETURN_IF_FAILED("Could not await service operation", return); + // The device can be disconnected by the time we return from await() + if (state != QLowEnergyController::DiscoveringState) + return; GattCommunicationStatus status; hr = result->get_Status(&status); if (FAILED(hr) || status != GattCommunicationStatus_Success) { @@ -747,6 +1024,15 @@ void QLowEnergyControllerPrivateWinRTNew::obtainIncludedServices( HRESULT QLowEnergyControllerPrivateWinRTNew::onServiceDiscoveryFinished(ABI::Windows::Foundation::IAsyncOperation *op, AsyncStatus status) { + // Check if the device is in the proper state, because it can already be + // disconnected when the callback arrives. + // Also the callback can theoretically come when the connection is + // reestablisheed again (for example, if the user quickly clicks + // "Disconnect" and then "Connect" again in some UI). But we can probably + // omit such details, as we are connecting to the same device anyway. + if (state != QLowEnergyController::DiscoveringState) + return S_OK; + Q_Q(QLowEnergyController); if (status != AsyncStatus::Completed) { qCDebug(QT_BT_WINRT) << "Could not obtain services"; @@ -798,6 +1084,14 @@ HRESULT QLowEnergyControllerPrivateWinRTNew::onServiceDiscoveryFinished(ABI::Win pointer->type |= QLowEnergyService::PrimaryService; obtainIncludedServices(pointer, deviceService); + // The obtainIncludedServices method calls await(), so the device can be + // disconnected by the time we return from it. TODO - rewrite in an + // async way! + if (state != QLowEnergyController::DiscoveringState) { + emit q->discoveryFinished(); // Probably not needed when the device + // is already disconnected? + return S_OK; + } emit q->serviceDiscovered(service); } @@ -1320,7 +1614,7 @@ void QLowEnergyControllerPrivateWinRTNew::writeCharacteristic( } // only update cache when property is readable. Otherwise it remains // empty. - if (charData.properties & QLowEnergyCharacteristic::Read) + if (thisPtr && charData.properties & QLowEnergyCharacteristic::Read) thisPtr->updateValueOfCharacteristic(charHandle, newValue, false); if (writeWithResponse) emit service->characteristicWritten(QLowEnergyCharacteristic(service, charHandle), @@ -1422,7 +1716,8 @@ void QLowEnergyControllerPrivateWinRTNew::writeDescriptor( service->setError(QLowEnergyService::DescriptorWriteError); return S_OK; } - thisPtr->updateValueOfDescriptor(charHandle, descHandle, newValue, false); + if (thisPtr) + thisPtr->updateValueOfDescriptor(charHandle, descHandle, newValue, false); emit service->descriptorWritten(QLowEnergyDescriptor(service, charHandle, descHandle), newValue); return S_OK; @@ -1516,7 +1811,8 @@ void QLowEnergyControllerPrivateWinRTNew::writeDescriptor( service->setError(QLowEnergyService::DescriptorWriteError); return S_OK; } - thisPtr->updateValueOfDescriptor(charHandle, descHandle, newValue, false); + if (thisPtr) + thisPtr->updateValueOfDescriptor(charHandle, descHandle, newValue, false); emit service->descriptorWritten(QLowEnergyDescriptor(service, charHandle, descHandle), newValue); return S_OK; @@ -1579,172 +1875,12 @@ void QLowEnergyControllerPrivateWinRTNew::handleServiceHandlerError(const QStrin setError(QLowEnergyController::ConnectionError); } -void QLowEnergyControllerPrivateWinRTNew::connectToPairedDevice() +void QLowEnergyControllerPrivateWinRTNew::handleConnectionError(const char *logMessage) { - Q_Q(QLowEnergyController); - ComPtr device3; - HRESULT hr = mDevice.As(&device3); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast device", return) - ComPtr> deviceServicesOp; - QDeadlineTimer deadline(kMaxConnectTimeout); - while (!mAbortPending && !deadline.hasExpired()) { - hr = device3->GetGattServicesAsync(&deviceServicesOp); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain services", return) - ComPtr deviceServicesResult; - hr = QWinRTFunctions::await(deviceServicesOp, deviceServicesResult.GetAddressOf(), - QWinRTFunctions::ProcessThreadEvents, 5000); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not await services operation", return) - - GattCommunicationStatus commStatus; - hr = deviceServicesResult->get_Status(&commStatus); - if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { - qCWarning(QT_BT_WINRT()) << "Service operation failed"; - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - unregisterFromStatusChanges(); - return; - } - - ComPtr> deviceServices; - hr = deviceServicesResult->get_Services(&deviceServices); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain list of services", return) - uint serviceCount; - hr = deviceServices->get_Size(&serviceCount); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain service count", return) - - if (serviceCount == 0) { - qCWarning(QT_BT_WINRT()) << "Found devices without services"; - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - unregisterFromStatusChanges(); - return; - } - - // Windows automatically connects to the device as soon as a service value is read/written. - // Thus we read one value in order to establish the connection. - for (uint i = 0; i < serviceCount; ++i) { - ComPtr service; - hr = deviceServices->GetAt(i, &service); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain service", return); - ComPtr service3; - hr = service.As(&service3); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast service", return); - ComPtr> characteristicsOp; - hr = service3->GetCharacteristicsAsync(&characteristicsOp); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic", return); - ComPtr characteristicsResult; - hr = QWinRTFunctions::await(characteristicsOp, characteristicsResult.GetAddressOf(), - QWinRTFunctions::ProcessThreadEvents, 5000); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not await characteristic operation", return); - GattCommunicationStatus commStatus; - hr = characteristicsResult->get_Status(&commStatus); - if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { - qCWarning(QT_BT_WINRT) << "Characteristic operation failed"; - break; - } - ComPtr> characteristics; - hr = characteristicsResult->get_Characteristics(&characteristics); - if (hr == E_ACCESSDENIED) { - // Everything will work as expected up until this point if the manifest capabilties - // for bluetooth LE are not set. - qCWarning(QT_BT_WINRT) << "Could not obtain characteristic list. Please check your " - "manifest capabilities"; - setState(QLowEnergyController::UnconnectedState); - setError(QLowEnergyController::ConnectionError); - unregisterFromStatusChanges(); - return; - } - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic list", return); - uint characteristicsCount; - hr = characteristics->get_Size(&characteristicsCount); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic list's size", return); - for (uint j = 0; j < characteristicsCount; ++j) { - ComPtr characteristic; - hr = characteristics->GetAt(j, &characteristic); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic", return); - ComPtr> op; - GattCharacteristicProperties props; - hr = characteristic->get_CharacteristicProperties(&props); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic's properties", return); - if (!(props & GattCharacteristicProperties_Read)) - continue; - hr = characteristic->ReadValueWithCacheModeAsync(BluetoothCacheMode::BluetoothCacheMode_Uncached, &op); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not read characteristic value", return); - ComPtr result; - hr = QWinRTFunctions::await(op, result.GetAddressOf(), QWinRTFunctions::ProcessThreadEvents, 500); - // E_ILLEGAL_METHOD_CALL will be the result for a device, that is not reachable at - // the moment. In this case we should jump back into the outer loop and keep trying. - if (hr == E_ILLEGAL_METHOD_CALL) - break; - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not await characteristic read", return); - ComPtr buffer; - hr = result->get_Value(&buffer); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain characteristic value", return); - if (!buffer) { - qCDebug(QT_BT_WINRT) << "Problem reading value"; - break; - } - - setState(QLowEnergyController::ConnectedState); - emit q->connected(); - if (!registerForStatusChanges()) { - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - return; - } - return; - } - } - } - if (deadline.hasExpired()) { - qCWarning(QT_BT_WINRT) << "Connect to device failed due to timeout!"; - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - unregisterFromStatusChanges(); - } -} - -void QLowEnergyControllerPrivateWinRTNew::connectToUnpairedDevice() -{ - if (!registerForStatusChanges()) { - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - return; - } - ComPtr device3; - HRESULT hr = mDevice.As(&device3); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not cast device", return) - ComPtr deviceServicesResult; - QDeadlineTimer deadline(kMaxConnectTimeout); - while (!mAbortPending && !deadline.hasExpired()) { - ComPtr> deviceServicesOp; - hr = device3->GetGattServicesAsync(&deviceServicesOp); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not obtain services", return) - hr = QWinRTFunctions::await(deviceServicesOp, deviceServicesResult.GetAddressOf(), - QWinRTFunctions::ProcessMainThreadEvents); - CHECK_FOR_DEVICE_CONNECTION_ERROR(hr, "Could not await services operation", return) - - GattCommunicationStatus commStatus; - hr = deviceServicesResult->get_Status(&commStatus); - if (commStatus == GattCommunicationStatus_Unreachable) - continue; - - if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { - qCWarning(QT_BT_WINRT()) << "Service operation failed"; - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - unregisterFromStatusChanges(); - return; - } - - break; - } - if (deadline.hasExpired()) { - qCWarning(QT_BT_WINRT) << "Connect to device failed due to timeout!"; - setError(QLowEnergyController::ConnectionError); - setState(QLowEnergyController::UnconnectedState); - unregisterFromStatusChanges(); - } + qCWarning(QT_BT_WINRT) << logMessage; + setError(QLowEnergyController::ConnectionError); + setState(QLowEnergyController::UnconnectedState); + unregisterFromStatusChanges(); } QT_END_NAMESPACE diff --git a/src/bluetooth/qlowenergycontroller_winrt_new_p.h b/src/bluetooth/qlowenergycontroller_winrt_new_p.h index b238f48e..8f0f85bf 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new_p.h +++ b/src/bluetooth/qlowenergycontroller_winrt_new_p.h @@ -132,17 +132,15 @@ public: signals: void characteristicChanged(quint16 charHandle, const QByteArray &data); + void abortConnection(); private slots: void handleCharacteristicChanged(quint16 charHandle, const QByteArray &data); void handleServiceHandlerError(const QString &error); - void doConnectToDevice(); private: - void connectToPairedDevice(); - void connectToUnpairedDevice(); + void handleConnectionError(const char *logMessage); - bool mAbortPending = false; Microsoft::WRL::ComPtr mDevice; EventRegistrationToken mStatusChangedToken; struct ValueChangedEntry { -- cgit v1.2.3 From 6444868ff5f95e868471d16f61ca3d0696bbd8a5 Mon Sep 17 00:00:00 2001 From: Ivan Solovev Date: Fri, 8 Oct 2021 14:10:08 +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: If46c3f0afe13a56d93f2b8fc46b87a2c90555bf3 (cherry picked from commit 2a7ef291d696745887e71f8b0e27cb4c8701bdd2) Reviewed-by: Oliver Wolff --- src/bluetooth/qlowenergycontroller_winrt_new.cpp | 291 ++++++++++------------- 1 file changed, 120 insertions(+), 171 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller_winrt_new.cpp b/src/bluetooth/qlowenergycontroller_winrt_new.cpp index 7b0a6f14..ed082ba1 100644 --- a/src/bluetooth/qlowenergycontroller_winrt_new.cpp +++ b/src/bluetooth/qlowenergycontroller_winrt_new.cpp @@ -100,6 +100,13 @@ typedef IGattReadClientCharacteristicConfigurationDescriptorResult IClientCharCo continue; \ } +#define DEC_CHAR_COUNT_AND_CONTINUE_IF_FAILED(hr, msg) \ + if (FAILED(hr)) { \ + qCWarning(QT_BT_WINRT) << msg; \ + --mCharacteristicsCountToBeDiscovered; \ + continue; \ + } + #define CHECK_FOR_DEVICE_CONNECTION_ERROR_IMPL(this, hr, msg, ret) \ if (FAILED(hr)) { \ qCWarning(QT_BT_WINRT) << msg; \ @@ -162,6 +169,7 @@ public: ~QWinRTLowEnergyServiceHandlerNew() { + qCDebug(QT_BT_WINRT) << __FUNCTION__; } public slots: @@ -213,185 +221,126 @@ 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_WINRT) << "Could not obtain list of descriptors"; + 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) { + 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_WINRT) << "Characteristic read result is null"; + else + charData.value = byteArrayFromGattResult(readResult); + } + mCharacteristicList.insert(handle, charData); + + ComPtr> descriptors; + + GattCommunicationStatus commStatus; + hr = descResult->get_Status(&commStatus); + if (FAILED(hr) || commStatus != GattCommunicationStatus_Success) { + qCWarning(QT_BT_WINRT) << "Descriptor operation failed"; --mCharacteristicsCountToBeDiscovered; continue; } - hr = descAsyncResult->put_Completed( - Callback>( - [this, characteristic] - (IAsyncOperation *op, - AsyncStatus status) { - if (status != AsyncStatus::Completed) { - qCWarning(QT_BT_WINRT) << "Descriptor operation unsuccessful"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - quint16 handle; - HRESULT hr = characteristic->get_AttributeHandle(&handle); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "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_WINRT) << "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_WINRT) << "Could not obtain characteristic's properties"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - charData.properties = QLowEnergyCharacteristic::PropertyTypes(properties & 0xff); - if (charData.properties & QLowEnergyCharacteristic::Read) { - ComPtr> readOp; - hr = characteristic->ReadValueWithCacheModeAsync(BluetoothCacheMode_Uncached, - &readOp); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "Could not read characteristic"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; + 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)) { + 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()); + mIndicateChars << charData.uuid; + } else { + 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()); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "Could not obtain characteristic read result"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - if (!readResult) - qCWarning(QT_BT_WINRT) << "Characteristic read result is null"; + WARN_AND_CONTINUE_IF_FAILED(hr, "Could await descriptor read result") + if (descData.uuid == QBluetoothUuid::DescriptorType::CharacteristicUserDescription) + descData.value = byteArrayFromGattResult(readResult, true); else - charData.value = byteArrayFromGattResult(readResult); + descData.value = byteArrayFromGattResult(readResult); } - mCharacteristicList.insert(handle, charData); - - ComPtr> descriptors; - - ComPtr result; - hr = op->GetResults(&result); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "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_WINRT) << "Descriptor operation failed"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - - hr = result->get_Descriptors(&descriptors); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "Could not obtain list of descriptors"; - --mCharacteristicsCountToBeDiscovered; - checkAllCharacteristicsDiscovered(); - return S_OK; - } - - uint descriptorCount; - hr = descriptors->get_Size(&descriptorCount); - if (FAILED(hr)) { - qCWarning(QT_BT_WINRT) << "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::ClientCharacteristicConfiguration)) { - 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()); - mIndicateChars << charData.uuid; - } else { - 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::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_WINRT) << "Could not register descriptor callback"; - --mCharacteristicsCountToBeDiscovered; - continue; + charData.descriptorList.insert(descHandle, descData); } + + mCharacteristicList.insert(handle, charData); + --mCharacteristicsCountToBeDiscovered; } checkAllCharacteristicsDiscovered(); } @@ -1226,8 +1175,8 @@ void QLowEnergyControllerPrivateWinRTNew::discoverServiceDetails(const QBluetoot QLowEnergyServicePrivate::CharData> charList, QVector indicateChars, QLowEnergyHandle startHandle, QLowEnergyHandle endHandle) { if (!serviceList.contains(service)) { - qCWarning(QT_BT_WINRT) << "Discovery done of unknown service:" - << service.toString(); + qCWarning(QT_BT_WINRT) + << "Discovery complete for unknown service:" << service.toString(); return; } -- cgit v1.2.3