Add ability to have multiple manufacturer data per manufacturer ID

The current QBluetoothDeviceInfo API is somewhat limited as it cannot
clearly express the nature of multiple data sets per manufacturerId.
The API should really offer:

QMultiHash<quint16, QByteArray> QBluetoothDeviceInfo::manufacturerData() const
QVector<QByteArray> QBluetoothDeviceInfo::manufacturerData(quint16 manufacturerId) const

Adding overloads and usual deprecation warnings cannot be done during
Qt 5.x time though as the required overloads merely change the return types.
Overloads which only offer different return types are not permitted
by C++. The plan is to replace w/o deprecation in the Qt 6 branch.

To avoid the API change during Qt 5.x, this patch leverages QHash::insertMulti().
There is a minor API behavior change that comes along with this approach.
Previously, old manufacturer data for a given manufacturerId was replaced
by the new value. Now, the data is added. The fact that multi inserts are permitted
are a minor behavioral change too but actually desired by the nature of
the bug this patch addresses.

Note that there is some talk that QHash's insertMulti() capability may
disappear in Qt 6.

Task-number: QTBUG-76615
Change-Id: I35b737cb48d6ed83112ebe0b22853ca307f20115
Reviewed-by: Oliver Wolff <oliver.wolff@qt.io>
Reviewed-by: André de la Rocha <andre.rocha@qt.io>

3 files changed, 59 insertions, 5 deletions

diff --git a/src/bluetooth/qbluetoothdeviceinfo.cpp b/src/bluetooth/qbluetoothdeviceinfo.cpp
index de73bf56..443902dc 100644
--- a/src/bluetooth/qbluetoothdeviceinfo.cpp
+++ b/src/bluetooth/qbluetoothdeviceinfo.cpp
@@ -669,6 +669,10 @@ QVector<quint16> QBluetoothDeviceInfo::manufacturerIds() const
     The interpretation of the data octets is defined by the manufacturer
     specified by the company identifier.
 
+    \note The remote device may provide multiple data entries per manufacturerId.
+    This function only returns the first entry. If all entries are needed use
+    \l manufacturerData() which returns a multi hash.
+
     \sa manufacturerIds(), setManufacturerData()
     \since 5.12
  */
@@ -680,7 +684,10 @@ QByteArray QBluetoothDeviceInfo::manufacturerData(quint16 manufacturerId) const
 
 /*!
     Sets the advertised manufacturer \a data for the given \a manufacturerId.
-    Returns true if it was inserted or changed, false if it was already known.
+    Returns \c true if it was inserted, \c false if it was already known.
+
+    Since Qt 5.14, different values for \a data and the same \a manufacturerId no longer
+    replace each other but are accumulated for the duration of a device scan.
 
     \sa manufacturerData
     \since 5.12
@@ -688,16 +695,25 @@ QByteArray QBluetoothDeviceInfo::manufacturerData(quint16 manufacturerId) const
 bool QBluetoothDeviceInfo::setManufacturerData(quint16 manufacturerId, const QByteArray &data)
 {
     Q_D(QBluetoothDeviceInfo);
-    const auto it = d->manufacturerData.find(manufacturerId);
-    if (it != d->manufacturerData.end() && *it == data)
-        return false;
-    d->manufacturerData.insert(manufacturerId, data);
+    QHash<quint16, QByteArray>::const_iterator it = d->manufacturerData.find(manufacturerId);
+    while (it != d->manufacturerData.end() && it.key() == manufacturerId) {
+        if (*it == data)
+            return false;
+        it++;
+    }
+
+    d->manufacturerData.insertMulti(manufacturerId, data);
     return true;
 }
 
 /*!
     Returns the complete set of all manufacturer data.
 
+    Some devices may provide multiple manufacturer data entries per manufacturer ID.
+    An example might be a Bluetooth Low Energy device that sends a different manufacturer data via
+    advertisement packets and scan response packets respectively. Therefore the returned hash table
+    may have multiple entries per manufacturer ID or hash key.
+
     \sa setManufacturerData
     \since 5.12
 */
diff --git a/src/bluetooth/qbluetoothdeviceinfo.h b/src/bluetooth/qbluetoothdeviceinfo.h
index 92ba6526..c4fa01ec 100644
--- a/src/bluetooth/qbluetoothdeviceinfo.h
+++ b/src/bluetooth/qbluetoothdeviceinfo.h
@@ -262,6 +262,10 @@ public:
 #endif
     void setServiceUuids(const QVector<QBluetoothUuid> &uuids);
 
+    // TODO Qt6 manufacturerData()
+    // manufacturerData() and manufacturerData(quint16) return types should be modified to
+    // cope with multiple data entires per manufacturer ID. QHash<quint16, QByteArray>
+    // may stay though if it retains insertMulti() in Qt 6.
     QVector<quint16> manufacturerIds() const;
     QByteArray manufacturerData(quint16 manufacturerId) const;
     bool setManufacturerData(quint16 manufacturerId, const QByteArray &data);
diff --git a/tests/auto/qbluetoothdeviceinfo/tst_qbluetoothdeviceinfo.cpp b/tests/auto/qbluetoothdeviceinfo/tst_qbluetoothdeviceinfo.cpp
index d8c80291..45b429db 100644
--- a/tests/auto/qbluetoothdeviceinfo/tst_qbluetoothdeviceinfo.cpp
+++ b/tests/auto/qbluetoothdeviceinfo/tst_qbluetoothdeviceinfo.cpp
@@ -64,6 +64,8 @@ private slots:
     void tst_cached();
 
     void tst_flags();
+
+    void tst_manufacturerData();
 };
 
 tst_QBluetoothDeviceInfo::tst_QBluetoothDeviceInfo()
@@ -514,6 +516,38 @@ void tst_QBluetoothDeviceInfo::tst_flags()
     QVERIFY(serviceResult.testFlag(QBluetoothDeviceInfo::CapturingService));
 }
 
+void tst_QBluetoothDeviceInfo::tst_manufacturerData()
+{
+    const int manufacturerAVM = 0x1F;
+
+    QBluetoothDeviceInfo info;
+    QVERIFY(info.manufacturerIds().isEmpty());
+    QVERIFY(info.manufacturerData(manufacturerAVM).isNull());
+
+    QVERIFY(info.setManufacturerData(manufacturerAVM, QByteArray::fromHex("ABCD")));
+    QVERIFY(!info.setManufacturerData(manufacturerAVM, QByteArray::fromHex("ABCD")));
+    QCOMPARE(info.manufacturerData(manufacturerAVM), QByteArray::fromHex("ABCD"));
+    auto temp = info.manufacturerData();
+    QCOMPARE(temp.keys().count(), 1);
+    QCOMPARE(temp.values().count(), 1);
+    QCOMPARE(temp.values(), QList<QByteArray>() << QByteArray::fromHex("ABCD"));
+
+    QVERIFY(info.setManufacturerData(manufacturerAVM, QByteArray::fromHex("CDEF")));
+    QVERIFY(!info.setManufacturerData(manufacturerAVM, QByteArray::fromHex("ABCD")));
+    QVERIFY(!info.setManufacturerData(manufacturerAVM, QByteArray::fromHex("CDEF")));
+
+    temp = info.manufacturerData();
+    QCOMPARE(temp.keys().count(), 2);
+    QCOMPARE(temp.values().count(), 2);
+    auto list = temp.values();
+
+    QCOMPARE(QSet<QByteArray> (list.begin(), list.end()),
+             QSet<QByteArray>() << QByteArray::fromHex("ABCD") << QByteArray::fromHex("CDEF"));
+
+    // return latest entry
+    QCOMPARE(info.manufacturerData(manufacturerAVM), QByteArray::fromHex("CDEF"));
+}
+
 QTEST_MAIN(tst_QBluetoothDeviceInfo)
 
 #include "tst_qbluetoothdeviceinfo.moc"