From aad6e26574ff069e080bf6461c0fb7d0fee9e403 Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Thu, 17 May 2018 16:15:04 +0200 Subject: Properly invalidate services when calling disconnectService() on BlueZ While the central role implementation properly invalidates all the LowEnergyServicePrivate instances, peripheral mode was leaking service instances. This is triggered when the peripheral disconnects from a client or when the user calls disconnectService(). On the other hand stopAdvertising() does not do that. This patch fixes the service instance leak and ensures that the class docs specifically state the behavior difference between stopAdvertising() and disconnectService(). Change-Id: Ia52b141096dc1db3d0cefe3ed18c230eecccd9c0 Reviewed-by: Christian Kandeler Reviewed-by: Timur Pocheptsov Reviewed-by: Oliver Wolff --- src/bluetooth/qlowenergycontroller.cpp | 19 +++++++++++++++---- src/bluetooth/qlowenergycontroller_bluez.cpp | 9 ++++++--- src/bluetooth/qlowenergycontrollerbase.cpp | 7 +++++++ 3 files changed, 28 insertions(+), 7 deletions(-) diff --git a/src/bluetooth/qlowenergycontroller.cpp b/src/bluetooth/qlowenergycontroller.cpp index a23955ff..6b4c17d6 100644 --- a/src/bluetooth/qlowenergycontroller.cpp +++ b/src/bluetooth/qlowenergycontroller.cpp @@ -603,9 +603,10 @@ void QLowEnergyController::connectToDevice() This function does nothing if the controller is in the \l UnconnectedState. - If the controller is in the peripheral role, it stops advertising too. - The application must restart the advertising mode by calling - \l startAdvertising(). + If the controller is in the peripheral role, it stops advertising and removes + all services which have previously been added via \l addService(). + To reuse the QLowEnergyController instance the application must re-add services + and restart the advertising mode by calling \l startAdvertising(). \sa connectToDevice() */ @@ -742,6 +743,9 @@ void QLowEnergyController::startAdvertising(const QLowEnergyAdvertisingParameter /*! Stops advertising, if this object is currently in the advertising state. + The controller has to be in the \l PeripheralRole for this function to work. + It does not invalidate services which have previously been added via \l addService(). + \since 5.7 \sa startAdvertising() */ @@ -760,8 +764,15 @@ void QLowEnergyController::stopAdvertising() The controller must be in the \l PeripheralRole and in the \l UnconnectedState. The \a service object must be valid. + \note Once the peripheral instance is disconnected from the remote central device or + if \l disconnectFromDevice() is manually called, every service definition that was + previously added via this function is removed from the peripheral. Therefore this function + must be called again before re-advertising this peripheral controller instance. The described + behavior is connection specific and therefore not dependent on whether \l stopAdvertising() + was called. + \since 5.7 - \sa QLowEnergyServiceData::addIncludedService + \sa stopAdvertising(), disconnectFromDevice(), QLowEnergyServiceData::addIncludedService */ QLowEnergyService *QLowEnergyController::addService(const QLowEnergyServiceData &service, QObject *parent) diff --git a/src/bluetooth/qlowenergycontroller_bluez.cpp b/src/bluetooth/qlowenergycontroller_bluez.cpp index d58e0ee8..d288f3ad 100644 --- a/src/bluetooth/qlowenergycontroller_bluez.cpp +++ b/src/bluetooth/qlowenergycontroller_bluez.cpp @@ -809,9 +809,12 @@ void QLowEnergyControllerPrivateBluez::resetController() securityLevelValue = -1; connectionHandle = 0; - // public API behavior requires stop of advertisement - if (role == QLowEnergyController::PeripheralRole && advertiser) - advertiser->stopAdvertising(); + if (role == QLowEnergyController::PeripheralRole) { + // public API behavior requires stop of advertisement + if (advertiser) + advertiser->stopAdvertising(); + localAttributes.clear(); + } } void QLowEnergyControllerPrivateBluez::restartRequestTimer() diff --git a/src/bluetooth/qlowenergycontrollerbase.cpp b/src/bluetooth/qlowenergycontrollerbase.cpp index 687f5b9e..c7b9ec0b 100644 --- a/src/bluetooth/qlowenergycontrollerbase.cpp +++ b/src/bluetooth/qlowenergycontrollerbase.cpp @@ -261,6 +261,8 @@ void QLowEnergyControllerPrivate::invalidateServices() } serviceList.clear(); + localServices.clear(); + lastLocalHandle = {}; } QLowEnergyService *QLowEnergyControllerPrivate::addServiceHelper( @@ -308,7 +310,12 @@ QLowEnergyService *QLowEnergyControllerPrivate::addServiceHelper( return nullptr; } + if (localServices.contains(servicePrivate->uuid)) { + qWarning() << "Overriding existing local service with uuid" + << servicePrivate->uuid; + } this->localServices.insert(servicePrivate->uuid, servicePrivate); + this->addToGenericAttributeList(service, servicePrivate->startHandle); return new QLowEnergyService(servicePrivate); } -- cgit v1.2.3 From 370f751edd826f483dec0115000370811ff1202a Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 28 May 2018 13:23:51 +0200 Subject: Add G(rand)C(entral)D(ispatch) timeout handler QtBluetooth is using its own dispatch queue in CoreBluetooth back-end - this is where CoreBluetooth is executing all callbacks we're providing in delegate classes. Some operations like service discovery/characteristic or descriptor read(s) amd write(s) e.t.c. may sometimes fail to finish - no value read, no error reported (so delegate's method - callback - is never called). To deal with this we introduce the class OSXBTGCDTimer and GCDTimerDelegate protocol; GCDTimer periodically inserts blocks into the serial LE queue and checks for timeouts upon their execution. Task-number: QTBUG-68422 Change-Id: Ic17bf91d4223ad1ffc7b9808da36c902a4158227 Reviewed-by: Alex Blasche --- src/bluetooth/osx/osxbt.pri | 5 +- src/bluetooth/osx/osxbtgcdtimer.mm | 110 ++++++++++++++++++++++++++++++++++++ src/bluetooth/osx/osxbtgcdtimer_p.h | 94 ++++++++++++++++++++++++++++++ 3 files changed, 208 insertions(+), 1 deletion(-) create mode 100644 src/bluetooth/osx/osxbtgcdtimer.mm create mode 100644 src/bluetooth/osx/osxbtgcdtimer_p.h diff --git a/src/bluetooth/osx/osxbt.pri b/src/bluetooth/osx/osxbt.pri index 0f293107..b7ac0535 100644 --- a/src/bluetooth/osx/osxbt.pri +++ b/src/bluetooth/osx/osxbt.pri @@ -1,5 +1,8 @@ SOURCES += osx/uistrings.cpp osx/osxbtnotifier.cpp -PRIVATE_HEADERS += osx/uistrings_p.h +PRIVATE_HEADERS += osx/uistrings_p.h \ + osx/osxbtgcdtimer_p.h + +OBJECTIVE_SOURCES += osx/osxbtgcdtimer.mm #QMAKE_CXXFLAGS_WARN_ON += -Wno-nullability-completeness CONFIG(osx) { diff --git a/src/bluetooth/osx/osxbtgcdtimer.mm b/src/bluetooth/osx/osxbtgcdtimer.mm new file mode 100644 index 00000000..0a49c25c --- /dev/null +++ b/src/bluetooth/osx/osxbtgcdtimer.mm @@ -0,0 +1,110 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtBluetooth module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#include "osxbtgcdtimer_p.h" +#include "osxbtutility_p.h" + +#include + +#include + +@implementation QT_MANGLE_NAMESPACE(OSXBTGCDTimer) + +- (instancetype)initWithDelegate:(id)delegate +{ + if (self = [super init]) { + timeoutHandler = delegate; + timeoutMS = 0; + timeoutStepMS = 0; + cancelled = false; + } + return self; +} + +- (void)startWithTimeout:(qint64)ms step:(qint64)stepMS +{ + Q_ASSERT(!timeoutMS && !timeoutStepMS); + Q_ASSERT(!cancelled); + + if (!timeoutHandler) { + // Nobody to report timeout to, no need to start any task then. + return; + } + + if (ms <= 0 || stepMS <= 0) { + qCWarning(QT_BT_OSX, "Invalid timeout/step parameters"); + return; + } + + timeoutMS = ms; + timeoutStepMS = stepMS; + timer.start(); + + [self handleTimeout]; +} + +- (void)handleTimeout +{ + if (cancelled) + return; + + const qint64 elapsed = timer.elapsed(); + if (elapsed >= timeoutMS) { + [timeoutHandler timeout]; + } else { + using namespace QT_PREPEND_NAMESPACE(OSXBluetooth); + // Re-schedule: + dispatch_queue_t leQueue(qt_LE_queue()); + Q_ASSERT(leQueue); + const qint64 timeChunkMS = std::min(timeoutMS - elapsed, timeoutStepMS); + dispatch_after(dispatch_time(DISPATCH_TIME_NOW, + int64_t(timeChunkMS / 1000. * NSEC_PER_SEC)), + leQueue, + ^{ + [self handleTimeout]; + }); + } +} + +- (void)cancelTimer +{ + cancelled = true; +} + +@end diff --git a/src/bluetooth/osx/osxbtgcdtimer_p.h b/src/bluetooth/osx/osxbtgcdtimer_p.h new file mode 100644 index 00000000..007a004b --- /dev/null +++ b/src/bluetooth/osx/osxbtgcdtimer_p.h @@ -0,0 +1,94 @@ +/**************************************************************************** +** +** Copyright (C) 2018 The Qt Company Ltd. +** Contact: https://www.qt.io/licensing/ +** +** This file is part of the QtBluetooth module of the Qt Toolkit. +** +** $QT_BEGIN_LICENSE:LGPL$ +** Commercial License Usage +** Licensees holding valid commercial Qt licenses may use this file in +** accordance with the commercial license agreement provided with the +** Software or, alternatively, in accordance with the terms contained in +** a written agreement between you and The Qt Company. For licensing terms +** and conditions see https://www.qt.io/terms-conditions. For further +** information use the contact form at https://www.qt.io/contact-us. +** +** GNU Lesser General Public License Usage +** Alternatively, this file may be used under the terms of the GNU Lesser +** General Public License version 3 as published by the Free Software +** Foundation and appearing in the file LICENSE.LGPL3 included in the +** packaging of this file. Please review the following information to +** ensure the GNU Lesser General Public License version 3 requirements +** will be met: https://www.gnu.org/licenses/lgpl-3.0.html. +** +** GNU General Public License Usage +** Alternatively, this file may be used under the terms of the GNU +** General Public License version 2.0 or (at your option) the GNU General +** Public license version 3 or any later version approved by the KDE Free +** Qt Foundation. The licenses are as published by the Free Software +** Foundation and appearing in the file LICENSE.GPL2 and LICENSE.GPL3 +** included in the packaging of this file. Please review the following +** information to ensure the GNU General Public License requirements will +** be met: https://www.gnu.org/licenses/gpl-2.0.html and +** https://www.gnu.org/licenses/gpl-3.0.html. +** +** $QT_END_LICENSE$ +** +****************************************************************************/ + +#ifndef OSXBTGCDTIMER_P_H +#define OSXBTGCDTIMER_P_H + +// +// W A R N I N G +// ------------- +// +// This file is not part of the Qt API. It exists purely as an +// implementation detail. This header file may change from version to +// version without notice, or even be removed. +// +// We mean it. +// + +#include "osxbtutility_p.h" + +#include +#include + +#include + +@protocol QT_MANGLE_NAMESPACE(GCDTimerDelegate) +@required +- (void)timeout; +@end + +@interface QT_MANGLE_NAMESPACE(OSXBTGCDTimer) : NSObject { +@private + qint64 timeoutMS; + qint64 timeoutStepMS; + QT_PREPEND_NAMESPACE(QElapsedTimer) timer; + id timeoutHandler; + bool cancelled; +} + +- (instancetype)initWithDelegate:(id)delegate; +- (void)startWithTimeout:(qint64)ms step:(qint64)stepMS; +- (void)handleTimeout; +- (void)cancelTimer; + +@end + +QT_BEGIN_NAMESPACE + +namespace OSXBluetooth { + +using GCDTimerObjC = QT_MANGLE_NAMESPACE(OSXBTGCDTimer); +using GCDTimer = ObjCScopedPointer; + +} + +QT_END_NAMESPACE + +#endif // OSXBTGCDTIMER_P_H + -- cgit v1.2.3 From c2513dce68f2643b0c62e13d16d376115d9c8476 Mon Sep 17 00:00:00 2001 From: Oswald Buddenhagen Date: Thu, 31 May 2018 14:03:58 +0200 Subject: Bump version Change-Id: Ie059fb032d764d744a3e9dad9edf04bce161223d --- .qmake.conf | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.qmake.conf b/.qmake.conf index dc68d388..67c54ee6 100644 --- a/.qmake.conf +++ b/.qmake.conf @@ -1,3 +1,3 @@ load(qt_build_config) -MODULE_VERSION = 5.11.0 +MODULE_VERSION = 5.11.1 -- cgit v1.2.3 From 5bdc4e8b0b1e6664b7c289cfc9c1a9d12da6e87e Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Mon, 28 May 2018 14:35:17 +0200 Subject: Reimplement scan/manager state timeouts using GCD timer And remove some essentially duplicated code. Task-number: QTBUG-68422 Change-Id: I677581ebb0998d64a0081f568479efb7e8156474 Reviewed-by: Alex Blasche --- src/bluetooth/osx/osxbtgcdtimer.mm | 1 + src/bluetooth/osx/osxbtledeviceinquiry.mm | 114 +++++++---------------------- src/bluetooth/osx/osxbtledeviceinquiry_p.h | 18 ++--- 3 files changed, 34 insertions(+), 99 deletions(-) diff --git a/src/bluetooth/osx/osxbtgcdtimer.mm b/src/bluetooth/osx/osxbtgcdtimer.mm index 0a49c25c..095f8680 100644 --- a/src/bluetooth/osx/osxbtgcdtimer.mm +++ b/src/bluetooth/osx/osxbtgcdtimer.mm @@ -105,6 +105,7 @@ - (void)cancelTimer { cancelled = true; + timeoutHandler = nil; } @end diff --git a/src/bluetooth/osx/osxbtledeviceinquiry.mm b/src/bluetooth/osx/osxbtledeviceinquiry.mm index 60222370..2cece15b 100644 --- a/src/bluetooth/osx/osxbtledeviceinquiry.mm +++ b/src/bluetooth/osx/osxbtledeviceinquiry.mm @@ -65,9 +65,7 @@ QBluetoothUuid qt_uuid(NSUUID *nsUuid) } const int timeStepMS = 100; - const int powerOffTimeoutMS = 30000; -const qreal powerOffTimeStepS = 30. / 100.; struct AdvertisementData { // That's what CoreBluetooth has: @@ -115,14 +113,6 @@ QT_END_NAMESPACE QT_USE_NAMESPACE -@interface QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) (PrivateAPI) -// These two methods are scheduled with a small time step -// within a given timeout, they either re-schedule -// themselves or emit a signal/stop some operation. -- (void)stopScan; -- (void)handlePoweredOff; -@end - @implementation QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) -(id)initWithNotifier:(LECBManagerNotifier *)aNotifier @@ -153,60 +143,22 @@ QT_USE_NAMESPACE [super dealloc]; } -- (void)stopScan +- (void)timeout { - using namespace OSXBluetooth; - - // We never schedule stopScan if there is no timeout: - Q_ASSERT(inquiryTimeoutMS > 0); - if (internalState == InquiryActive) { - const int elapsed = scanTimer.elapsed(); - if (elapsed >= inquiryTimeoutMS) { - [manager stopScan]; - [manager setDelegate:nil]; - internalState = InquiryFinished; - Q_ASSERT(notifier); - emit notifier->discoveryFinished(); - } else { - // Re-schedule 'stopScan': - dispatch_queue_t leQueue(qt_LE_queue()); - Q_ASSERT(leQueue); - const int timeChunkMS = std::min(inquiryTimeoutMS - elapsed, timeStepMS); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, - int64_t(timeChunkMS / 1000. * NSEC_PER_SEC)), - leQueue, - ^{ - [self stopScan]; - }); - } - } -} - -- (void)handlePoweredOff -{ - // This is interesting on iOS only, where - // the system shows an alert asking to enable - // Bluetooth in the 'Settings' app. If not done yet (after 30 - // seconds) - we consider it an error. - using namespace OSXBluetooth; - - if (internalState == InquiryStarting) { - if (errorTimer.elapsed() >= powerOffTimeoutMS) { - [manager setDelegate:nil]; - internalState = ErrorPoweredOff; - Q_ASSERT(notifier); - emit notifier->CBManagerError(QBluetoothDeviceDiscoveryAgent::PoweredOffError); - } else { - dispatch_queue_t leQueue(qt_LE_queue()); - Q_ASSERT(leQueue); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, - (int64_t)(powerOffTimeStepS * NSEC_PER_SEC)), - leQueue, - ^{ - [self handlePoweredOff]; - }); - } + [manager stopScan]; + [manager setDelegate:nil]; + internalState = InquiryFinished; + Q_ASSERT(notifier); + emit notifier->discoveryFinished(); + } else if (internalState == InquiryStarting) { + // This is interesting on iOS only, where the system shows an alert + // asking to enable Bluetooth in the 'Settings' app. If not done yet + // (after 30 seconds) - we consider this as an error. + [manager setDelegate:nil]; + internalState = ErrorPoweredOff; + Q_ASSERT(notifier); + emit notifier->CBManagerError(QBluetoothDeviceDiscoveryAgent::PoweredOffError); } } @@ -233,9 +185,6 @@ QT_USE_NAMESPACE using namespace OSXBluetooth; - dispatch_queue_t leQueue(qt_LE_queue()); - Q_ASSERT(leQueue); - const auto state = central.state; #if QT_IOS_PLATFORM_SDK_EQUAL_OR_ABOVE(__IPHONE_10_0) || QT_OSX_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_13) if (state == CBManagerStatePoweredOn) { @@ -246,18 +195,9 @@ QT_USE_NAMESPACE internalState = InquiryActive; if (inquiryTimeoutMS > 0) { - // We have a finite-length discovery, schedule stopScan, - // with a smaller time step, otherwise it can prevent - // 'self' from being deleted in time, which is not good - // (the block will retain 'self', waiting for timeout). - scanTimer.start(); - const int timeChunkMS = std::min(timeStepMS, inquiryTimeoutMS); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, - int64_t(timeChunkMS / 1000. * NSEC_PER_SEC)), - leQueue, - ^{ - [self stopScan]; - }); + [elapsedTimer cancelTimer]; + elapsedTimer.reset([[GCDTimerObjC alloc] initWithDelegate:self]); + [elapsedTimer startWithTimeout:inquiryTimeoutMS step:timeStepMS]; } [manager scanForPeripheralsWithServices:nil options:nil]; @@ -287,19 +227,15 @@ QT_USE_NAMESPACE if (internalState == InquiryStarting) { #ifndef Q_OS_OSX // On iOS a user can see at this point an alert asking to - // enable Bluetooth in the "Settings" app. If a user does, + // enable Bluetooth in the "Settings" app. If a user does so, // we'll receive 'PoweredOn' state update later. - // No change in internalState. Wait for 30 seconds - // (we split it into smaller steps not to retain 'self' for - // too long ) ... - errorTimer.start(); - dispatch_after(dispatch_time(DISPATCH_TIME_NOW, - (int64_t)(powerOffTimeStepS * NSEC_PER_SEC)), - leQueue, - ^{ - [self handlePoweredOff]; - }); + // No change in internalState. Wait for 30 seconds. + [elapsedTimer cancelTimer]; + elapsedTimer.reset([[GCDTimerObjC alloc] initWithDelegate:self]); + [elapsedTimer startWithTimeout:powerOffTimeoutMS step:300]; return; +#else + Q_UNUSED(powerOffTimeoutMS) #endif internalState = ErrorPoweredOff; emit notifier->CBManagerError(QBluetoothDeviceDiscoveryAgent::PoweredOffError); @@ -330,6 +266,8 @@ QT_USE_NAMESPACE if (internalState == InquiryActive) [manager stopScan]; + [elapsedTimer cancelTimer]; + [manager setDelegate:nil]; internalState = InquiryCancelled; diff --git a/src/bluetooth/osx/osxbtledeviceinquiry_p.h b/src/bluetooth/osx/osxbtledeviceinquiry_p.h index fc787a6d..a19055ab 100644 --- a/src/bluetooth/osx/osxbtledeviceinquiry_p.h +++ b/src/bluetooth/osx/osxbtledeviceinquiry_p.h @@ -53,10 +53,10 @@ #include "qbluetoothdevicediscoveryagent.h" #include "qbluetoothdeviceinfo.h" +#include "osxbtgcdtimer_p.h" #include "osxbtutility_p.h" #include "osxbluetooth_p.h" -#include #include #include @@ -75,10 +75,8 @@ class LECBManagerNotifier; QT_END_NAMESPACE -// Ugly but all these QT_PREPEND_NAMESPACE etc. are even worse ... -using OSXBluetooth::LECBManagerNotifier; -using OSXBluetooth::ObjCScopedPointer; -using QT_PREPEND_NAMESPACE(QElapsedTimer); +using QT_PREPEND_NAMESPACE(OSXBluetooth)::LECBManagerNotifier; +using QT_PREPEND_NAMESPACE(OSXBluetooth)::ObjCScopedPointer; enum LEInquiryState { @@ -90,7 +88,7 @@ enum LEInquiryState ErrorLENotSupported }; -@interface QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) : NSObject +@interface QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) : NSObject { LECBManagerNotifier *notifier; ObjCScopedPointer manager; @@ -99,16 +97,14 @@ enum LEInquiryState LEInquiryState internalState; int inquiryTimeoutMS; - // Timers to check if we can execute delayed callbacks: - QT_PREPEND_NAMESPACE(QElapsedTimer) errorTimer; - QT_PREPEND_NAMESPACE(QElapsedTimer) scanTimer; + QT_PREPEND_NAMESPACE(OSXBluetooth)::GCDTimer elapsedTimer; } - (id)initWithNotifier:(LECBManagerNotifier *)aNotifier; - (void)dealloc; -// IMPORTANT: both 'startWithTimeout' and 'stop' -// can be executed only on the "Qt's LE queue". +// IMPORTANT: both 'startWithTimeout' and 'stop' MUST be executed on the "Qt's +// LE queue". - (void)startWithTimeout:(int)timeout; - (void)stop; -- cgit v1.2.3 From 82c9845fdb25f28aca6d0c8017c3fec79425941d Mon Sep 17 00:00:00 2001 From: Alex Blasche Date: Fri, 16 Mar 2018 13:16:00 +0200 Subject: Add changes file for Qt 5.9.5 Change-Id: Ie522ad947cf4a5657cfc95f82dc1b467eae954e1 Reviewed-by: Oliver Wolff (cherry picked from commit 6d2e1761ebd62ef60c39af83b7ef88ac986b2d1e) Reviewed-by: Alex Blasche --- dist/changes-5.9.5 | 29 +++++++++++++++++++++++++++++ 1 file changed, 29 insertions(+) create mode 100644 dist/changes-5.9.5 diff --git a/dist/changes-5.9.5 b/dist/changes-5.9.5 new file mode 100644 index 00000000..8a69c52f --- /dev/null +++ b/dist/changes-5.9.5 @@ -0,0 +1,29 @@ +Qt 5.9.5 is a bug-fix release. It maintains both forward and backward +compatibility (source and binary) with Qt 5.9.0. + +For more details, refer to the online documentation included in this +distribution. The documentation is also available online: + +http://doc.qt.io/qt-5/index.html + +The Qt version 5.9 series is binary compatible with the 5.8.x series. +Applications compiled for 5.9 will continue to run with 5.9. + +Some of the changes listed in this file include issue tracking numbers +corresponding to tasks in the Qt Bug Tracker: + +https://bugreports.qt.io/ + +Each of these identifiers can be entered in the bug tracker to obtain more +information about a particular change. + +**************************************************************************** +* Qt 5.9.5 Changes * +**************************************************************************** + +QtBluetooth +----------- + + - [QTBUG-65801] Fixed never ending QBLuetoothDeviceDiscoveryAhgent::start(ClassMethod) + on WinRT/UWP. + - Fixed internal cleanup issue in QBluetoothSocket on WinRT/UWP. -- cgit v1.2.3 From 5e106754314ca0f58f7f8c3ae86d5ef298894742 Mon Sep 17 00:00:00 2001 From: Antti Kokko Date: Mon, 28 May 2018 09:04:26 +0300 Subject: Add changes file for Qt 5.9.6 Change-Id: Ib91829d9b96aaefa06b0fae5730ce9d99699fbf1 Reviewed-by: Alex Blasche (cherry picked from commit 513e69bd6aca390d7e6ef28d3527c24dee99ee2b) Reviewed-by: Antti Kokko --- dist/changes-5.9.6 | 24 ++++++++++++++++++++++++ 1 file changed, 24 insertions(+) create mode 100644 dist/changes-5.9.6 diff --git a/dist/changes-5.9.6 b/dist/changes-5.9.6 new file mode 100644 index 00000000..f0179a5a --- /dev/null +++ b/dist/changes-5.9.6 @@ -0,0 +1,24 @@ +Qt 5.9.6 is a bug-fix release. It maintains both forward and backward +compatibility (source and binary) with Qt 5.9.0 through 5.9.5. + +For more details, refer to the online documentation included in this +distribution. The documentation is also available online: + +http://doc.qt.io/qt-5/index.html + +The Qt version 5.9 series is binary compatible with the 5.8.x series. +Applications compiled for 5.8 will continue to run with 5.9. + +Some of the changes listed in this file include issue tracking numbers +corresponding to tasks in the Qt Bug Tracker: + +https://bugreports.qt.io/ + +Each of these identifiers can be entered in the bug tracker to obtain more +information about a particular change. + +**************************************************************************** +* Qt 5.9.6 Changes * +**************************************************************************** + + - This release contains only minor code improvements. -- cgit v1.2.3 From 6e111b1631ca43c6c78edf0cf28b943e62cb2804 Mon Sep 17 00:00:00 2001 From: Timur Pocheptsov Date: Tue, 29 May 2018 10:21:27 +0200 Subject: LE controller (CoreBluetooth) - handle timeouts While we're discovering service's details (included services, characteristics, descriptors, etc.) some operations can result in timeout, without CoreBluetooth reporting any error back. To cope with this we need an additional logic - all such operations must be guarded with a 'watchdog' timer (OSXBTGCDTimer). Fortunately enough, we can re-use CBPeripheralDelegate's callbacks to handle these timeouts. As a micro-bonus - a couple of bugs in callbacks: whenever we are in service details discovery, after having some error we have to finish with discovery anyway, not to stick in 'discovering' state forever. Task-number: QTBUG-68422 Change-Id: I13a377ebec3983ac48a33d6b89b67223d5ec412f Reviewed-by: Alex Blasche --- src/bluetooth/osx/osxbtcentralmanager.mm | 213 +++++++++++++++++++++++++----- src/bluetooth/osx/osxbtcentralmanager_p.h | 22 ++- 2 files changed, 199 insertions(+), 36 deletions(-) diff --git a/src/bluetooth/osx/osxbtcentralmanager.mm b/src/bluetooth/osx/osxbtcentralmanager.mm index 78e6b832..b56dc0d5 100644 --- a/src/bluetooth/osx/osxbtcentralmanager.mm +++ b/src/bluetooth/osx/osxbtcentralmanager.mm @@ -79,8 +79,21 @@ NSUInteger qt_countGATTEntries(CBService *service) return n; } +ObjCStrongReference qt_timeoutNSError(OperationTimeout type) +{ + // For now we do not provide details, since nobody is using this NSError + // after all (except callbacks checking if an operation was successful + // or not). + Q_ASSERT(type != OperationTimeout::none); + Q_UNUSED(type) + NSError *nsError = [[NSError alloc] initWithDomain:CBErrorDomain + code:CBErrorOperationCancelled + userInfo:nil]; + return ObjCStrongReference(nsError, false /*do not retain, done already*/); } +} // namespace OSXBluetooth + QT_END_NAMESPACE @@ -115,9 +128,11 @@ QT_END_NAMESPACE - (id)initWith:(OSXBluetooth::LECBManagerNotifier *)aNotifier { + using namespace OSXBluetooth; + if (self = [super init]) { manager = nil; - managerState = OSXBluetooth::CentralManagerIdle; + managerState = CentralManagerIdle; disconnectPending = false; peripheral = nil; notifier = aNotifier; @@ -125,6 +140,17 @@ QT_END_NAMESPACE lastValidHandle = 0; requestPending = false; currentReadHandle = 0; + timeoutType = OperationTimeout::none; + + if (Q_UNLIKELY(!qEnvironmentVariableIsEmpty("BLUETOOTH_GATT_TIMEOUT"))) { + bool ok = false; + const int value = qEnvironmentVariableIntValue("BLUETOOTH_GATT_TIMEOUT", &ok); + if (ok && value >= 0) + timeoutMS = value; + } + + if (!timeoutMS) + timeoutMS = 20000; } return self; @@ -153,6 +179,60 @@ QT_END_NAMESPACE [super dealloc]; } +- (void)watchAfter:(id)object timeout:(OSXBluetooth::OperationTimeout)type +{ + using namespace OSXBluetooth; + + objectUnderWatch = object; + timeoutType = type; + [timeoutWatchdog cancelTimer]; + timeoutWatchdog.reset([[GCDTimerObjC alloc] initWithDelegate:self]); + [timeoutWatchdog startWithTimeout:timeoutMS step:200]; +} + +- (void)stopWatchdog +{ + [timeoutWatchdog cancelTimer]; + objectUnderWatch = nil; + timeoutType = OSXBluetooth::OperationTimeout::none; +} + +- (void)timeout +{ + using namespace OSXBluetooth; + + Q_ASSERT(objectUnderWatch); + NSLog(@"Timeout caused by: %@", objectUnderWatch); + const ObjCStrongReference nsError(qt_timeoutNSError(timeoutType)); + switch (timeoutType) { + case OperationTimeout::serviceDiscovery: + qCWarning(QT_BT_OSX, "Timeout in services discovery"); + [self peripheral:peripheral didDiscoverServices:nsError]; + break; + case OperationTimeout::includedServicesDiscovery: + qCWarning(QT_BT_OSX, "Timeout in included services discovery"); + [self peripheral:peripheral didDiscoverIncludedServicesForService:objectUnderWatch error:nsError]; + break; + case OperationTimeout::characteristicsDiscovery: + qCWarning(QT_BT_OSX, "Timeout in characteristics discovery"); + [self peripheral:peripheral didDiscoverCharacteristicsForService:objectUnderWatch error:nsError]; + break; + case OperationTimeout::characteristicRead: + qCWarning(QT_BT_OSX, "Timeout while reading a characteristic"); + [self peripheral:peripheral didUpdateValueForCharacteristic:objectUnderWatch error:nsError]; + break; + case OperationTimeout::descriptorsDiscovery: + qCWarning(QT_BT_OSX, "Timeout in descriptors discovery"); + [self peripheral:peripheral didDiscoverDescriptorsForCharacteristic:objectUnderWatch error:nsError]; + break; + case OperationTimeout::descriptorRead: + qCWarning(QT_BT_OSX, "Timeout while reading a descriptor"); + [self peripheral:peripheral didUpdateValueForDescriptor:objectUnderWatch error:nsError]; + break; + default:; + } +} + - (void)connectToDevice:(const QBluetoothUuid &)aDeviceUuid { disconnectPending = false; // Cancel the previous disconnect if any. @@ -235,10 +315,11 @@ QT_END_NAMESPACE - (void)connectToPeripheral { + using namespace OSXBluetooth; + Q_ASSERT_X(manager, Q_FUNC_INFO, "invalid central manager (nil)"); Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)"); - Q_ASSERT_X(managerState == OSXBluetooth::CentralManagerIdle, - Q_FUNC_INFO, "invalid state"); + Q_ASSERT_X(managerState == CentralManagerIdle, Q_FUNC_INFO, "invalid state"); // The state is still the same - connecting. if ([self isConnected]) { @@ -247,7 +328,7 @@ QT_END_NAMESPACE emit notifier->connected(); } else { qCDebug(QT_BT_OSX) << "trying to connect"; - managerState = OSXBluetooth::CentralManagerConnecting; + managerState = CentralManagerConnecting; [manager connectPeripheral:peripheral options:nil]; } } @@ -292,9 +373,10 @@ QT_END_NAMESPACE - (void)discoverServices { + using namespace OSXBluetooth; + Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)"); - Q_ASSERT_X(managerState == OSXBluetooth::CentralManagerIdle, - Q_FUNC_INFO, "invalid state"); + Q_ASSERT_X(managerState == CentralManagerIdle, Q_FUNC_INFO, "invalid state"); // From Apple's docs: // @@ -304,7 +386,8 @@ QT_END_NAMESPACE // // ... but we'd like to have them all: [peripheral setDelegate:self]; - managerState = OSXBluetooth::CentralManagerDiscovering; + managerState = CentralManagerDiscovering; + [self watchAfter:peripheral timeout:OperationTimeout::serviceDiscovery]; [peripheral discoverServices:nil]; } @@ -333,6 +416,7 @@ QT_END_NAMESPACE CBService *const s = [services objectAtIndex:currentService]; [visitedServices addObject:s]; managerState = CentralManagerDiscovering; + [self watchAfter:s timeout:OperationTimeout::includedServicesDiscovery]; [peripheral discoverIncludedServices:nil forService:s]; } } @@ -359,6 +443,7 @@ QT_END_NAMESPACE if (CBService *const service = [self serviceForUUID:serviceUuid]) { servicesToDiscoverDetails.append(serviceUuid); + [self watchAfter:service timeout:OperationTimeout::characteristicsDiscovery]; [peripheral discoverCharacteristics:nil forService:service]; return; } @@ -366,10 +451,8 @@ QT_END_NAMESPACE qCWarning(QT_BT_OSX) << "unknown service uuid" << serviceUuid; - if (notifier) { - emit notifier->CBManagerError(serviceUuid, - QLowEnergyService::UnknownError); - } + if (notifier) + emit notifier->CBManagerError(serviceUuid, QLowEnergyService::UnknownError); } - (void)readCharacteristics:(CBService *)service @@ -391,8 +474,10 @@ QT_END_NAMESPACE NSArray *const cs = service.characteristics; for (CBCharacteristic *c in cs) { - if (c.properties & CBCharacteristicPropertyRead) + if (c.properties & CBCharacteristicPropertyRead) { + [self watchAfter:c timeout:OperationTimeout::characteristicRead]; return [peripheral readValueForCharacteristic:c]; + } } // No readable properties? Discover descriptors then: @@ -418,15 +503,18 @@ QT_END_NAMESPACE [self serviceDetailsDiscoveryFinished:service]; } else { // Start from 0 and continue in the callback. - [peripheral discoverDescriptorsForCharacteristic:[service.characteristics objectAtIndex:0]]; + CBCharacteristic *ch = [service.characteristics objectAtIndex:0]; + [self watchAfter:ch timeout:OperationTimeout::descriptorsDiscovery]; + [peripheral discoverDescriptorsForCharacteristic:ch]; } } - (void)readDescriptors:(CBService *)service { + using namespace OSXBluetooth; + Q_ASSERT_X(service, Q_FUNC_INFO, "invalid service (nil)"); - Q_ASSERT_X(managerState != OSXBluetooth::CentralManagerUpdating, - Q_FUNC_INFO, "invalid state"); + Q_ASSERT_X(managerState != CentralManagerUpdating, Q_FUNC_INFO, "invalid state"); Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)"); QT_BT_MAC_AUTORELEASEPOOL; @@ -435,8 +523,11 @@ QT_END_NAMESPACE // We can never be here if we have no characteristics. Q_ASSERT_X(cs && cs.count, Q_FUNC_INFO, "invalid service"); for (CBCharacteristic *c in cs) { - if (c.descriptors && c.descriptors.count) - return [peripheral readValueForDescriptor:[c.descriptors objectAtIndex:0]]; + if (c.descriptors && c.descriptors.count) { + CBDescriptor *desc = [c.descriptors objectAtIndex:0]; + [self watchAfter:desc timeout:OperationTimeout::descriptorRead]; + return [peripheral readValueForDescriptor:desc]; + } } // No descriptors to read, done. @@ -571,6 +662,8 @@ QT_END_NAMESPACE requestPending = true; currentReadHandle = request.handle; + // Timeouts: for now, we do not alert timeoutWatchdog - never had such + // bug reports and after all a read timeout can be handled externally. [peripheral readValueForCharacteristic:charMap[request.handle]]; } else { if (!descMap.contains(request.handle)) { @@ -581,6 +674,7 @@ QT_END_NAMESPACE requestPending = true; currentReadHandle = request.handle; + // Timeouts: see the comment above (CharRead). [peripheral readValueForDescriptor:descMap[request.handle]]; } } @@ -1021,6 +1115,7 @@ QT_END_NAMESPACE charMap.clear(); descMap.clear(); currentReadHandle = 0; + [self stopWatchdog]; // TODO: also serviceToVisit/VisitNext and visitedServices ? } @@ -1075,12 +1170,14 @@ QT_END_NAMESPACE #else if (state == CBCentralManagerStatePoweredOff) { #endif - managerState = CentralManagerIdle; + if (managerState == CentralManagerUpdating) { + managerState = CentralManagerIdle; // I've seen this instead of Unsupported on OS X. if (notifier) emit notifier->LEnotSupported(); } else { + managerState = CentralManagerIdle; // TODO: we need a better error + // what will happen if later the state changes to PoweredOn??? if (notifier) @@ -1167,10 +1264,16 @@ QT_END_NAMESPACE Q_UNUSED(aPeripheral) if (managerState != OSXBluetooth::CentralManagerDiscovering) { - // Canceled by -disconnectFromDevice. + // Canceled by -disconnectFromDevice, or as a result of a timeout. return; } + if (objectUnderWatch != aPeripheral) { + // Timed out. + return; + } + + [self stopWatchdog]; managerState = OSXBluetooth::CentralManagerIdle; if (error) { @@ -1178,9 +1281,9 @@ QT_END_NAMESPACE // TODO: better error mapping required. if (notifier) emit notifier->CBManagerError(QLowEnergyController::UnknownError); - } else { - [self discoverIncludedServices]; } + + [self discoverIncludedServices]; } @@ -1196,10 +1299,16 @@ QT_END_NAMESPACE return; } + if (service != objectUnderWatch) { + // Timed out. + return; + } + QT_BT_MAC_AUTORELEASEPOOL; Q_ASSERT_X(peripheral, Q_FUNC_INFO, "invalid peripheral (nil)"); + [self stopWatchdog]; managerState = CentralManagerIdle; if (error) { @@ -1222,6 +1331,7 @@ QT_END_NAMESPACE // Continue with discovery ... [visitedServices addObject:s]; managerState = CentralManagerDiscovering; + [self watchAfter:s timeout:OperationTimeout::includedServicesDiscovery]; return [peripheral discoverIncludedServices:nil forService:s]; } } @@ -1237,6 +1347,7 @@ QT_END_NAMESPACE if (![visitedServices containsObject:s]) { [visitedServices addObject:s]; managerState = CentralManagerDiscovering; + [self watchAfter:s timeout:OperationTimeout::includedServicesDiscovery]; return [peripheral discoverIncludedServices:nil forService:s]; } } @@ -1265,6 +1376,13 @@ QT_END_NAMESPACE return; } + if (service != objectUnderWatch) { + // Timed out already? + return; + } + + [self stopWatchdog]; + using namespace OSXBluetooth; Q_ASSERT_X(managerState != CentralManagerUpdating, Q_FUNC_INFO, "invalid state"); @@ -1274,9 +1392,9 @@ QT_END_NAMESPACE // We did not discover any characteristics and can not discover descriptors, // inform our delegate (it will set a service state also). emit notifier->CBManagerError(qt_uuid(service.UUID), QLowEnergyController::UnknownError); - } else { - [self readCharacteristics:service]; } + + [self readCharacteristics:service]; } - (void)peripheral:(CBPeripheral *)aPeripheral @@ -1290,6 +1408,10 @@ QT_END_NAMESPACE return; } + const bool readMatch = characteristic == objectUnderWatch; + if (readMatch) + [self stopWatchdog]; + using namespace OSXBluetooth; Q_ASSERT_X(managerState != CentralManagerUpdating, Q_FUNC_INFO, "invalid state"); @@ -1316,13 +1438,17 @@ QT_END_NAMESPACE } if (isDetailsDiscovery) { - // Test if we have any other characteristic to read yet. - CBCharacteristic *const next = [self nextCharacteristicForService:characteristic.service - startingFrom:characteristic properties:CBCharacteristicPropertyRead]; - if (next) - [peripheral readValueForCharacteristic:next]; - else - [self discoverDescriptors:characteristic.service]; + if (readMatch) { + // Test if we have any other characteristic to read yet. + CBCharacteristic *const next = [self nextCharacteristicForService:characteristic.service + startingFrom:characteristic properties:CBCharacteristicPropertyRead]; + if (next) { + [self watchAfter:next timeout:OperationTimeout::characteristicRead]; + [peripheral readValueForCharacteristic:next]; + } else { + [self discoverDescriptors:characteristic.service]; + } + } } else { // This is (probably) the result of update notification. // It's very possible we can have an invalid handle here (0) - @@ -1365,6 +1491,11 @@ QT_END_NAMESPACE QT_BT_MAC_AUTORELEASEPOOL; + if (characteristic != objectUnderWatch) + return; + + [self stopWatchdog]; + using namespace OSXBluetooth; if (error) { @@ -1375,10 +1506,12 @@ QT_END_NAMESPACE // Do we have more characteristics on this service to discover descriptors? CBCharacteristic *const next = [self nextCharacteristicForService:characteristic.service startingFrom:characteristic]; - if (next) + if (next) { + [self watchAfter:next timeout:OperationTimeout::descriptorsDiscovery]; [peripheral discoverDescriptorsForCharacteristic:next]; - else + } else { [self readDescriptors:characteristic.service]; + } } - (void)peripheral:(CBPeripheral *)aPeripheral @@ -1396,6 +1529,11 @@ QT_END_NAMESPACE QT_BT_MAC_AUTORELEASEPOOL; + if (descriptor != objectUnderWatch) + return; + + [self stopWatchdog]; + using namespace OSXBluetooth; CBService *const service = descriptor.characteristic.service; @@ -1422,6 +1560,7 @@ QT_END_NAMESPACE CBDescriptor *const next = [self nextDescriptorForCharacteristic:descriptor.characteristic startingFrom:descriptor]; if (next) { + [self watchAfter:next timeout:OperationTimeout::descriptorRead]; [peripheral readValueForDescriptor:next]; } else { // We either have to read a value for a next descriptor @@ -1431,8 +1570,11 @@ QT_END_NAMESPACE CBCharacteristic *nextCh = [self nextCharacteristicForService:ch.service startingFrom:ch]; while (nextCh) { - if (nextCh.descriptors && nextCh.descriptors.count) - return [peripheral readValueForDescriptor:[nextCh.descriptors objectAtIndex:0]]; + if (nextCh.descriptors && nextCh.descriptors.count) { + CBDescriptor *desc = [nextCh.descriptors objectAtIndex:0]; + [self watchAfter:desc timeout:OperationTimeout::descriptorRead]; + return [peripheral readValueForDescriptor:desc]; + } nextCh = [self nextCharacteristicForService:ch.service startingFrom:nextCh]; @@ -1574,9 +1716,10 @@ QT_END_NAMESPACE if (notifier) { notifier->disconnect(); notifier->deleteLater(); - notifier = 0; + notifier = nullptr; } + [self stopWatchdog]; [self disconnectFromDevice]; } diff --git a/src/bluetooth/osx/osxbtcentralmanager_p.h b/src/bluetooth/osx/osxbtcentralmanager_p.h index 697d922c..c0f278b2 100644 --- a/src/bluetooth/osx/osxbtcentralmanager_p.h +++ b/src/bluetooth/osx/osxbtcentralmanager_p.h @@ -53,6 +53,7 @@ #include "qlowenergycontroller.h" #include "qlowenergyservice.h" +#include "osxbtgcdtimer_p.h" #include "qbluetoothuuid.h" #include "osxbtutility_p.h" #include "osxbluetooth_p.h" @@ -86,6 +87,17 @@ enum CentralManagerState CentralManagerDisconnecting }; +enum class OperationTimeout +{ + none, + serviceDiscovery, + includedServicesDiscovery, + characteristicsDiscovery, + characteristicRead, + descriptorsDiscovery, + descriptorRead +}; + // In Qt we work with handles and UUIDs. Core Bluetooth // has NSArrays (and nested NSArrays inside servces/characteristics). // To simplify a navigation, I need a simple way to map from a handle @@ -132,7 +144,9 @@ typedef QHash ValueHash; QT_END_NAMESPACE -@interface QT_MANGLE_NAMESPACE(OSXBTCentralManager) : NSObject +@interface QT_MANGLE_NAMESPACE(OSXBTCentralManager) : NSObject { @private CBCentralManager *manager; @@ -166,6 +180,12 @@ QT_END_NAMESPACE QT_PREPEND_NAMESPACE(QLowEnergyHandle) currentReadHandle; QT_PREPEND_NAMESPACE(OSXBluetooth)::ValueHash valuesToWrite; + + qint64 timeoutMS; + id objectUnderWatch; + QT_PREPEND_NAMESPACE(OSXBluetooth)::OperationTimeout timeoutType; + QT_PREPEND_NAMESPACE(OSXBluetooth)::GCDTimer timeoutWatchdog; + @public CBPeripheral *peripheral; } -- cgit v1.2.3