Bluetooth - LE device discovery agent, timer bug (OS X)

CBCentralManager does not have a timeout while scanning and we had
to use a delayed selector (via performSelector) to interrupt the scan. This is a bug in
the case we're in NSModalPanelRunLoopMode mode (for example) - selector
never gets performed (it waits for the default mode) or performed too late.
Found in our btchat example. Now we use external QTimer instead to
check if it's time to stop a scan.

Change-Id: I159bf5821398f3aa76f03a52a8334dea97cbf688
Reviewed-by: Alex Blasche <alexander.blasche@theqtcompany.com>

6 files changed, 170 insertions, 14 deletions

diff --git a/src/bluetooth/bluetooth.pro b/src/bluetooth/bluetooth.pro
index 9cec0a3d..be953d04 100644
--- a/src/bluetooth/bluetooth.pro
+++ b/src/bluetooth/bluetooth.pro
@@ -165,6 +165,7 @@ config_bluez:qtHaveModule(dbus) {
                        qbluetoothserver_osx_p.h \
                        qbluetoothtransferreply_osx_p.h \
                        qbluetoothtransferreply_osx_p.h \
+                       qbluetoothdevicediscoverytimer_osx_p.h \
                        qlowenergycontroller_osx_p.h
 
     SOURCES -= qbluetoothdevicediscoveryagent.cpp
diff --git a/src/bluetooth/osx/osxbtcentralmanager.mm b/src/bluetooth/osx/osxbtcentralmanager.mm
index e539788d..bacea96b 100644
--- a/src/bluetooth/osx/osxbtcentralmanager.mm
+++ b/src/bluetooth/osx/osxbtcentralmanager.mm
@@ -680,9 +680,7 @@ using namespace QT_NAMESPACE;
 
     writeQueue.enqueue(request);
     [self performNextWriteRequest];
-    // TODO: this is quite ugly: true value can be returned after
-    // write actually. If I have any problems with the order later,
-    // I'll use performSelector afterDelay with some delay.
+
     return true;
 }
 
@@ -705,9 +703,7 @@ using namespace QT_NAMESPACE;
 
     writeQueue.enqueue(request);
     [self performNextWriteRequest];
-    // TODO: this is quite ugly: true value can be returned after
-    // write actually. If I have any problems with the order later,
-    // I'll use performSelector afterDelay with some delay.
+
     return true;
 }
 
diff --git a/src/bluetooth/osx/osxbtledeviceinquiry.mm b/src/bluetooth/osx/osxbtledeviceinquiry.mm
index 4e9235af..1dbe63a6 100644
--- a/src/bluetooth/osx/osxbtledeviceinquiry.mm
+++ b/src/bluetooth/osx/osxbtledeviceinquiry.mm
@@ -115,11 +115,12 @@ using namespace QT_NAMESPACE;
 
 @implementation QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry)
 
-+ (NSTimeInterval)inquiryLength
++ (int)inquiryLength
 {
     // There is no default timeout,
     // scan does not stop if not asked.
-    return 10;
+    // Return in milliseconds
+    return 10 * 1000;
 }
 
 - (id)initWithDelegate:(OSXBluetooth::LEDeviceInquiryDelegate *)aDelegate
@@ -188,6 +189,7 @@ using namespace QT_NAMESPACE;
         [manager release];
     }
 
+    startTime = QTime();
     pendingStart = true;
     manager = [CBCentralManager alloc];
     manager = [manager initWithDelegate:self queue:nil];
@@ -215,8 +217,12 @@ using namespace QT_NAMESPACE;
         if (pendingStart) {
             pendingStart = false;
             isActive = true;
-            [self performSelector:@selector(stopScan) withObject:nil
-                  afterDelay:[QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) inquiryLength]];
+#ifndef Q_OS_OSX
+            const NSTimeInterval timeout([QT_MANGLE_NAMESPACE(OSXBTLEDeviceInquiry) inquiryLength] / 1000);
+            Q_ASSERT_X(timeout > 0., Q_FUNC_INFO, "invalid scan timeout");
+            [self performSelector:@selector(stopScan) withObject:nil afterDelay:timeout];
+#endif
+            startTime = QTime::currentTime();
             [manager scanForPeripheralsWithServices:nil options:nil];
         } // Else we ignore.
     } else if (state == CBCentralManagerStateUnsupported || state == CBCentralManagerStateUnauthorized) {
@@ -285,10 +291,10 @@ using namespace QT_NAMESPACE;
 
     using namespace OSXBluetooth;
 
-    Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
-    Q_ASSERT_X(isActive,Q_FUNC_INFO, "called while there is no active scan");
-    Q_ASSERT_X(!pendingStart, Q_FUNC_INFO, "both pendingStart and isActive are true");
+    if (!isActive)
+        return;
 
+    Q_ASSERT_X(delegate, Q_FUNC_INFO, "invalid delegate (null)");
 
 #if QT_MAC_PLATFORM_SDK_EQUAL_OR_ABOVE(__MAC_10_9, __IPHONE_6_0)
     if (QSysInfo::MacintoshVersion >= qt_OS_limit(QSysInfo::MV_10_9, QSysInfo::MV_IOS_6_0)) {
@@ -323,4 +329,9 @@ using namespace QT_NAMESPACE;
     return pendingStart || isActive;
 }
 
+- (const QTime&)startTime
+{
+    return startTime;
+}
+
 @end
diff --git a/src/bluetooth/osx/osxbtledeviceinquiry_p.h b/src/bluetooth/osx/osxbtledeviceinquiry_p.h
index 65005894..7e6d0c8f 100644
--- a/src/bluetooth/osx/osxbtledeviceinquiry_p.h
+++ b/src/bluetooth/osx/osxbtledeviceinquiry_p.h
@@ -36,6 +36,7 @@
 
 #include "qbluetoothdevicediscoveryagent.h"
 
+#include <QtCore/qdatetime.h>
 #include <QtCore/qglobal.h>
 #include <QtCore/qlist.h>
 
@@ -88,8 +89,12 @@ QT_END_NAMESPACE
     bool cancelled;
     // scan actually started.
     bool isActive;
+    QTime startTime;
 }
 
+// Inquiry length in milliseconds.
++ (int)inquiryLength;
+
 - (id)initWithDelegate:(QT_PREPEND_NAMESPACE(OSXBluetooth)::LEDeviceInquiryDelegate *)aDelegate;
 - (void)dealloc;
 
@@ -99,6 +104,7 @@ QT_END_NAMESPACE
 - (void)stop;
 
 - (bool)isActive;
+- (const QTime &)startTime;
 
 @end
 
diff --git a/src/bluetooth/qbluetoothdevicediscoveryagent_osx.mm b/src/bluetooth/qbluetoothdevicediscoveryagent_osx.mm
index 194eba07..ecd21239 100644
--- a/src/bluetooth/qbluetoothdevicediscoveryagent_osx.mm
+++ b/src/bluetooth/qbluetoothdevicediscoveryagent_osx.mm
@@ -31,6 +31,7 @@
 **
 ****************************************************************************/
 
+#include "qbluetoothdevicediscoverytimer_osx_p.h"
 #include "qbluetoothdevicediscoveryagent.h"
 #include "osx/osxbtledeviceinquiry_p.h"
 #include "osx/osxbtdeviceinquiry_p.h"
@@ -43,6 +44,7 @@
 #include "qbluetoothuuid.h"
 
 #include <QtCore/qloggingcategory.h>
+#include <QtCore/qdatetime.h>
 #include <QtCore/qglobal.h>
 #include <QtCore/qstring.h>
 #include <QtCore/qdebug.h>
@@ -60,6 +62,7 @@ class QBluetoothDeviceDiscoveryAgentPrivate : public OSXBluetooth::DeviceInquiry
                                               public OSXBluetooth::LEDeviceInquiryDelegate
 {
     friend class QBluetoothDeviceDiscoveryAgent;
+    friend class OSXBluetooth::DDATimerHandler;
 public:
     QBluetoothDeviceDiscoveryAgentPrivate(const QBluetoothAddress & address,
                                           QBluetoothDeviceDiscoveryAgent *q);
@@ -97,6 +100,8 @@ private:
     void setError(IOReturn error, const QString &text = QString());
     void setError(QBluetoothDeviceDiscoveryAgent::Error, const QString &text = QString());
 
+    void checkLETimeout();
+
     QBluetoothDeviceDiscoveryAgent *q_ptr;
     AgentState agentState;
 
@@ -121,8 +126,45 @@ private:
 
     typedef QList<QBluetoothDeviceInfo> DevicesList;
     DevicesList discoveredDevices;
+
+    OSXBluetooth::DDATimerHandler timer;
 };
 
+namespace OSXBluetooth {
+
+DDATimerHandler::DDATimerHandler(QBluetoothDeviceDiscoveryAgentPrivate *d)
+    : owner(d)
+{
+    Q_ASSERT_X(owner, Q_FUNC_INFO, "invalid pointer");
+
+    timer.setSingleShot(true);
+    connect(&timer, &QTimer::timeout, this, &DDATimerHandler::onTimer);
+}
+
+void DDATimerHandler::start(int msec)
+{
+    Q_ASSERT_X(msec > 0, Q_FUNC_INFO, "invalid time interval");
+    if (timer.isActive()) {
+        qCWarning(QT_BT_OSX) << Q_FUNC_INFO << "timer is active";
+        return;
+    }
+
+    timer.start(msec);
+}
+
+void DDATimerHandler::stop()
+{
+    timer.stop();
+}
+
+void DDATimerHandler::onTimer()
+{
+    Q_ASSERT(owner);
+    owner->checkLETimeout();
+}
+
+}
+
 QBluetoothDeviceDiscoveryAgentPrivate::QBluetoothDeviceDiscoveryAgentPrivate(const QBluetoothAddress &adapter,
                                                                              QBluetoothDeviceDiscoveryAgent *q) :
     q_ptr(q),
@@ -131,7 +173,8 @@ QBluetoothDeviceDiscoveryAgentPrivate::QBluetoothDeviceDiscoveryAgentPrivate(con
     startPending(false),
     stopPending(false),
     lastError(QBluetoothDeviceDiscoveryAgent::NoError),
-    inquiryType(QBluetoothDeviceDiscoveryAgent::GeneralUnlimitedInquiry)
+    inquiryType(QBluetoothDeviceDiscoveryAgent::GeneralUnlimitedInquiry),
+    timer(this)
 {
     Q_ASSERT_X(q != Q_NULLPTR, Q_FUNC_INFO, "invalid q_ptr (null)");
 
@@ -221,9 +264,14 @@ void QBluetoothDeviceDiscoveryAgentPrivate::startLE()
 
     agentState = LEScan;
 
+    // CoreBluetooth does not have a timeout. We start a timer here
+    // and check if scan really started and if yes if we have a timeout.
+    timer.start([LEDeviceInquiryObjC inquiryLength]);
+
     if (![inquiryLE start]) {
         // We can be here only if we have some kind of resource allocation error, so we
         // do not emit finished, we emit error.
+        timer.stop();
         setError(QBluetoothDeviceDiscoveryAgent::UnknownError,
                  QCoreApplication::translate(DEV_DISCOVERY, DD_NOT_STARTED_LE));
         agentState = NonActive;
@@ -375,6 +423,27 @@ void QBluetoothDeviceDiscoveryAgentPrivate::setError(QBluetoothDeviceDiscoveryAg
         qCDebug(QT_BT_OSX) << "error set: "<<errorString;
 }
 
+void QBluetoothDeviceDiscoveryAgentPrivate::checkLETimeout()
+{
+    Q_ASSERT_X(agentState == LEScan, Q_FUNC_INFO, "invalid agent state");
+    Q_ASSERT_X(inquiryLE, Q_FUNC_INFO, "LE device inquiry is nil");
+
+    const int timeout = [LEDeviceInquiryObjC inquiryLength];
+    Q_ASSERT(timeout > 0);
+    const QTime scanStartTime([inquiryLE startTime]);
+    if (scanStartTime.isValid()) {
+        const int elapsed = scanStartTime.msecsTo(QTime::currentTime());
+        Q_ASSERT(elapsed >= 0);
+        if (elapsed >= timeout)
+            [inquiryLE stop];
+        else
+            timer.start(timeout - elapsed);
+    } else {
+        // Scan not started yet. Wait 5 seconds more.
+        timer.start(timeout / 2);
+    }
+}
+
 void QBluetoothDeviceDiscoveryAgentPrivate::LEdeviceInquiryError(QBluetoothDeviceDiscoveryAgent::Error error)
 {
     // At the moment the only error reported can be 'powered off' error, it happens
@@ -382,6 +451,8 @@ void QBluetoothDeviceDiscoveryAgentPrivate::LEdeviceInquiryError(QBluetoothDevic
     Q_ASSERT_X(error == QBluetoothDeviceDiscoveryAgent::PoweredOffError,
                Q_FUNC_INFO, "unexpected error code");
 
+    timer.stop();
+
     agentState = NonActive;
     setError(error);
     emit q_ptr->error(lastError);
@@ -394,6 +465,7 @@ void QBluetoothDeviceDiscoveryAgentPrivate::LEnotSupported()
     // After we call startLE and before receive NotSupported,
     // the user can call stop (setting a pending stop).
     // So the same rule apply:
+    timer.stop();
 
     LEdeviceInquiryFinished();
 }
@@ -427,6 +499,8 @@ void QBluetoothDeviceDiscoveryAgentPrivate::LEdeviceInquiryFinished()
     // The same logic as in inquiryFinished, but does not start LE scan.
     agentState = NonActive;
 
+    timer.stop();
+
     if (stopPending && !startPending) {
         stopPending = false;
         emit q_ptr->canceled();
diff --git a/src/bluetooth/qbluetoothdevicediscoverytimer_osx_p.h b/src/bluetooth/qbluetoothdevicediscoverytimer_osx_p.h
new file mode 100644
index 00000000..1537d82e
--- /dev/null
+++ b/src/bluetooth/qbluetoothdevicediscoverytimer_osx_p.h
@@ -0,0 +1,68 @@
+/****************************************************************************
+**
+** Copyright (C) 2015 The Qt Company Ltd.
+** Contact: http://www.qt.io/licensing/
+**
+** This file is part of the QtBluetooth module of the Qt Toolkit.
+**
+** $QT_BEGIN_LICENSE:LGPL21$
+** 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 http://www.qt.io/terms-conditions. For further
+** information use the contact form at http://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 2.1 or version 3 as published by the Free
+** Software Foundation and appearing in the file LICENSE.LGPLv21 and
+** LICENSE.LGPLv3 included in the packaging of this file. Please review the
+** following information to ensure the GNU Lesser General Public License
+** requirements will be met: https://www.gnu.org/licenses/lgpl.html and
+** http://www.gnu.org/licenses/old-licenses/lgpl-2.1.html.
+**
+** As a special exception, The Qt Company gives you certain additional
+** rights. These rights are described in The Qt Company LGPL Exception
+** version 1.1, included in the file LGPL_EXCEPTION.txt in this package.
+**
+** $QT_END_LICENSE$
+**
+****************************************************************************/
+
+#ifndef QBLUETOOTHDEVICEDISCOVERYTIMER_OSX_P_H
+#define QBLUETOOTHDEVICEDISCOVERYTIMER_OSX_P_H
+
+#include <QtCore/qglobal.h>
+#include <QtCore/qtimer.h>
+
+QT_BEGIN_NAMESPACE
+
+class QBluetoothDeviceDiscoveryAgentPrivate;
+
+namespace OSXBluetooth {
+
+class DDATimerHandler : public QObject
+{
+    Q_OBJECT
+
+public:
+    DDATimerHandler(QBluetoothDeviceDiscoveryAgentPrivate *d);
+
+    void start(int msec);
+    void stop();
+
+private slots:
+    void onTimer();
+
+private:
+    QTimer timer;
+    QBluetoothDeviceDiscoveryAgentPrivate *owner;
+};
+
+}
+
+QT_END_NAMESPACE
+
+#endif // QBLUETOOTHDEVICEDISCOVERYTIMER_OSX_P_H