Fix bluetooth service discovery not finishing on Android

The service discovery finished signal is not emitted when
the SDP cache is empty when last device inquiry of services finishes.

This commit changes the logic so that the the inquiry is finished
independent of whether actual services were discovered on (any) of
the devices.

As a related drive-by:
- Document the role of sdpCache to ease understanding
- Change raw timeout limits into a variable

Fixes: QTBUG-104479
Change-Id: Ifc9e8587a66769a1fc7959a8154f2be72ffd7461
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>
(cherry picked from commit 7c7d860ca52dc19e994a1166b6e2d0f5fa869455)
Reviewed-by: Ivan Solovev <ivan.solovev@qt.io>

2 files changed, 23 insertions, 12 deletions

diff --git a/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp b/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
index e5024dfa..ba09282d 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
+++ b/src/bluetooth/qbluetoothservicediscoveryagent_android.cpp
@@ -57,6 +57,8 @@ QT_BEGIN_NAMESPACE
 
 Q_DECLARE_LOGGING_CATEGORY(QT_BT_ANDROID)
 
+static constexpr auto uuidFetchTimeLimit = std::chrono::seconds{4};
+
 QBluetoothServiceDiscoveryAgentPrivate::QBluetoothServiceDiscoveryAgentPrivate(
         QBluetoothServiceDiscoveryAgent *qp, const QBluetoothAddress &deviceAdapter)
     : error(QBluetoothServiceDiscoveryAgent::NoError),
@@ -285,7 +287,7 @@ void QBluetoothServiceDiscoveryAgentPrivate::_q_processFetchedUuids(
     if (address.isNull() || uuids.isEmpty()) {
         if (discoveredDevices.count() == 1) {
             Q_Q(QBluetoothServiceDiscoveryAgent);
-            QTimer::singleShot(4000, q, [this]() {
+            QTimer::singleShot(uuidFetchTimeLimit, q, [this]() {
                 this->_q_fetchUuidsTimeout();
             }); // will also call _q_serviceDiscoveryFinished()
         } else {
@@ -304,7 +306,7 @@ void QBluetoothServiceDiscoveryAgentPrivate::_q_processFetchedUuids(
         qCDebug(QT_BT_ANDROID) << result;
     }
 
-    /* In general there are two uuid events per device.
+    /* In general there may be up-to two uuid events per device.
      * We'll wait for the second event to arrive before we process the UUIDs.
      * We utilize a timeout to catch cases when the second
      * event doesn't arrive at all.
@@ -334,11 +336,10 @@ void QBluetoothServiceDiscoveryAgentPrivate::_q_processFetchedUuids(
 
         sdpCache.insert(address, pair);
 
-        //the discovery on the last device cannot immediately finish
-        //we have to grant the 2 seconds timeout delay
-        if (discoveredDevices.count() == 1) {
+        //we have to grant the timeout delay to allow potential second event to arrive
+        if (discoveredDevices.size() == 1) {
             Q_Q(QBluetoothServiceDiscoveryAgent);
-            QTimer::singleShot(4000, q, [this]() {
+            QTimer::singleShot(uuidFetchTimeLimit, q, [this]() {
                 this->_q_fetchUuidsTimeout();
             });
             return;
@@ -493,14 +494,18 @@ void QBluetoothServiceDiscoveryAgentPrivate::populateDiscoveredServices(const QB
 
 void QBluetoothServiceDiscoveryAgentPrivate::_q_fetchUuidsTimeout()
 {
-    if (sdpCache.isEmpty())
+    // In practice if device list is empty, discovery has been stopped or bluetooth is offline
+    if (discoveredDevices.isEmpty())
         return;
 
-    QPair<QBluetoothDeviceInfo,QList<QBluetoothUuid> > pair;
-    const QList<QBluetoothAddress> keys = sdpCache.keys();
-    for (const QBluetoothAddress &key : keys) {
-        pair = sdpCache.take(key);
-        populateDiscoveredServices(pair.first, pair.second);
+    // Process remaining services in the cache (these didn't get a second UUID event)
+    if (!sdpCache.isEmpty()) {
+        QPair<QBluetoothDeviceInfo,QList<QBluetoothUuid> > pair;
+        const QList<QBluetoothAddress> keys = sdpCache.keys();
+        for (const QBluetoothAddress &key : keys) {
+            pair = sdpCache.take(key);
+            populateDiscoveredServices(pair.first, pair.second);
+        }
     }
 
     Q_ASSERT(sdpCache.isEmpty());
diff --git a/src/bluetooth/qbluetoothservicediscoveryagent_p.h b/src/bluetooth/qbluetoothservicediscoveryagent_p.h
index d163a80c..7fb57c0d 100644
--- a/src/bluetooth/qbluetoothservicediscoveryagent_p.h
+++ b/src/bluetooth/qbluetoothservicediscoveryagent_p.h
@@ -223,6 +223,12 @@ private:
     LocalDeviceBroadcastReceiver *localDeviceReceiver = nullptr;
 
     QAndroidJniObject btAdapter;
+    // The sdpCache caches service discovery results while it is running, and is
+    // cleared once finished. The cache is used as we may (or may not) get more accurate
+    // results after the first result. This temporary caching allows to send the
+    // serviceDiscovered() signal once per service and with the most accurate information.
+    // Partial cache clearing may occur already during the scan if the second (more accurate)
+    // scan result is received.
     QMap<QBluetoothAddress,QPair<QBluetoothDeviceInfo,QList<QBluetoothUuid> > > sdpCache;
 #endif