Fix broken implementation of KNX SRP block structures

* adapt auto test
* fix wrong size values
* fix wrong header types

Change-Id: I669a39e051365ae4a719b2ede30fae595797830d
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

2 files changed, 107 insertions, 50 deletions

diff --git a/src/knx/netip/qknxnetipsrp.cpp b/src/knx/netip/qknxnetipsrp.cpp
index 5a9babf..7b5cc5d 100644
--- a/src/knx/netip/qknxnetipsrp.cpp
+++ b/src/knx/netip/qknxnetipsrp.cpp
@@ -118,16 +118,18 @@ QKnxNetIpSrpProxy::QKnxNetIpSrpProxy(const QKnxNetIpSrp &srp)
 bool QKnxNetIpSrpProxy::isValid() const
 {
     if (m_srp.code() == QKnxNetIp::SearchParameterType::SelectByProgrammingMode)
-        return m_srp.isValid() && m_srp.size() == 8;
+        return m_srp.isValid() && m_srp.size() == 2;
 
     if (m_srp.code() == QKnxNetIp::SearchParameterType::SelectByMACAddress)
-        return m_srp.isValid() && m_srp.size() == 14;
+        return m_srp.isValid() && m_srp.size() == 8;
+
+    if (m_srp.code() == QKnxNetIp::SearchParameterType::SelectByService)
+        return m_srp.isValid() && m_srp.size() == 4;
 
-    if (m_srp.code() == QKnxNetIp::SearchParameterType::SelectByServiceSRP)
-        return m_srp.isValid() && m_srp.size() == 10;
+    if (m_srp.code() == QKnxNetIp::SearchParameterType::RequestDIBs) // must be even sized
+        return m_srp.isValid() && (m_srp.size() >= 4) && (m_srp.size() % 2 == 0);
 
-    return (m_srp.code() != QKnxNetIp::SearchParameterType::RequestDIBs) && m_srp.isValid()
-        && (m_srp.size() >= 10) && (m_srp.size() % 2 == 0); // must be even sized
+    return false;
 }
 
 /*!
@@ -177,7 +179,7 @@ QKnxByteArray QKnxNetIpSrpProxy::macAddress() const
 */
 QKnxServiceInfo QKnxNetIpSrpProxy::serviceInfo() const
 {
-    if (isValid() && m_srp.code() == QKnxNetIp::SearchParameterType::SelectByMACAddress) {
+    if (isValid() && m_srp.code() == QKnxNetIp::SearchParameterType::SelectByService) {
         const auto &data = m_srp.constData();
         return { QKnxNetIp::ServiceFamily(data.value(0)), data.value(1) };
     }
@@ -476,7 +478,7 @@ QKnxNetIpSrpProxy::SupportedFamily &QKnxNetIpSrpProxy::SupportedFamily::setManda
 */
 QKnxNetIpSrp QKnxNetIpSrpProxy::SupportedFamily::create() const
 {
-    return { { QKnxNetIp::SearchParameterType::SelectByServiceSRP, 2, d_ptr->m_mandatory },
+    return { { QKnxNetIp::SearchParameterType::SelectByService, 2, d_ptr->m_mandatory },
         { quint8(d_ptr->m_info.ServiceFamily), d_ptr->m_info.ServiceFamilyVersion } };
 }
 
diff --git a/tests/auto/qknxnetipsearchrequest/tst_qknxnetipsearchrequest.cpp b/tests/auto/qknxnetipsearchrequest/tst_qknxnetipsearchrequest.cpp
index b33a280..2abb15b 100644
--- a/tests/auto/qknxnetipsearchrequest/tst_qknxnetipsearchrequest.cpp
+++ b/tests/auto/qknxnetipsearchrequest/tst_qknxnetipsearchrequest.cpp
@@ -32,6 +32,22 @@
 #include <QtKnx/qknxnetipsrp.h>
 #include <QtKnx/QKnxServiceInfo>
 
+#if QT_VERSION < QT_VERSION_CHECK(5,13,0)
+ QT_BEGIN_NAMESPACE
+ bool operator==(const QKnxServiceInfo &lhs, const QKnxServiceInfo &rhs)
+ {
+     return (lhs.ServiceFamily == rhs.ServiceFamily)
+         && (lhs.ServiceFamilyVersion == rhs.ServiceFamilyVersion);
+ }
+ QT_END_NAMESPACE
+#endif
+
+char *toString(const QKnxByteArray &ba)
+{
+    using QTest::toString;
+    return toString("QKnxByteArray(" + ba.toByteArray() + ')');
+}
+
 static QString s_msg;
 static void myMessageHandler(QtMsgType, const QMessageLogContext &, const QString &msg)
 {
@@ -116,53 +132,92 @@ void tst_QKnxNetIpSearchRequest::testDebugStream()
 
 void tst_QKnxNetIpSearchRequest::tst_srpBuilders()
 {
-    auto srpMode = QKnxNetIpSrpProxy::programmingModeBuilder()
-                   .create();
-    QCOMPARE(srpMode.code(),
-             QKnxNetIp::SearchParameterType::SelectByProgrammingMode);
-    QVERIFY(srpMode.header().isMandatory());
-    QVERIFY(srpMode.header().isValid());
-    QVERIFY(srpMode.isValid());
+    auto srp = QKnxNetIpSrpProxy::programmingModeBuilder().create();
+    QCOMPARE(srp.code(), QKnxNetIp::SearchParameterType::SelectByProgrammingMode);
+    QVERIFY(srp.header().isMandatory());
+    QVERIFY(srp.header().isValid());
+    QVERIFY(srp.isValid());
+
+    {
+        QKnxNetIpSrpProxy proxy(srp);
+        QVERIFY(proxy.isValid());
+        QVERIFY(proxy.isMandatory());
+        QVERIFY(proxy.programmingModeOnly());
+        QCOMPARE(proxy.searchParameterType(), QKnxNetIp::SearchParameterType::SelectByProgrammingMode);
+
+        QCOMPARE(proxy.macAddress(), QKnxByteArray());
+        QCOMPARE(proxy.serviceInfo(), QKnxServiceInfo());
+        QCOMPARE(proxy.descriptionTypes(), QVector<QKnxNetIp::DescriptionType>());
+    }
 
     auto macAddress = QKnxByteArray::fromHex("4CCC6AE40000");
-    auto srpMac = QKnxNetIpSrpProxy::macAddressBuilder()
-                  .setMac(macAddress)
-                  .create();
-    QCOMPARE(srpMac.code(),
-             QKnxNetIp::SearchParameterType::SelectByMACAddress);
-    QVERIFY(srpMac.header().isMandatory());
-    QCOMPARE(srpMac.constData().mid(0,6), macAddress);
-    QVERIFY(srpMac.isValid());
-
-    auto serviceFamilyId = QKnxNetIp::ServiceFamily::ObjectServer;
-    quint8 minVersion = 4;
-    auto srpSupportedFamily = QKnxNetIpSrpProxy::supportedFamilyBuilder()
-        .setServiceInfo({ serviceFamilyId, minVersion })
+    srp = QKnxNetIpSrpProxy::macAddressBuilder()
+        .setMac(macAddress)
+        .create();
+    QCOMPARE(srp.code(), QKnxNetIp::SearchParameterType::SelectByMACAddress);
+    QVERIFY(srp.header().isMandatory());
+    QVERIFY(srp.header().isValid());
+    QVERIFY(srp.isValid());
+
+    {
+        QKnxNetIpSrpProxy proxy(srp);
+        QVERIFY(proxy.isValid());
+        QVERIFY(proxy.isMandatory());
+        QCOMPARE(proxy.macAddress(), macAddress);
+        QCOMPARE(proxy.searchParameterType(), QKnxNetIp::SearchParameterType::SelectByMACAddress);
+
+        QVERIFY(!proxy.programmingModeOnly());
+        QCOMPARE(proxy.serviceInfo(), QKnxServiceInfo());
+        QCOMPARE(proxy.descriptionTypes(), QVector<QKnxNetIp::DescriptionType>());
+    }
+
+    QKnxServiceInfo serviceInfo { QKnxNetIp::ServiceFamily::ObjectServer, 0x04 };
+    srp = QKnxNetIpSrpProxy::supportedFamilyBuilder()
+        .setServiceInfo(serviceInfo)
+        .create();
+    QCOMPARE(srp.code(), QKnxNetIp::SearchParameterType::SelectByService);
+    QVERIFY(srp.header().isMandatory());
+    QVERIFY(srp.header().isValid());
+    QVERIFY(srp.isValid());
+
+    {
+        QKnxNetIpSrpProxy proxy(srp);
+        QVERIFY(proxy.isValid());
+        QVERIFY(proxy.isMandatory());
+        QCOMPARE(proxy.serviceInfo(), serviceInfo);
+        QCOMPARE(proxy.searchParameterType(), QKnxNetIp::SearchParameterType::SelectByService);
+
+        QVERIFY(!proxy.programmingModeOnly());
+        QCOMPARE(proxy.macAddress(), QKnxByteArray());
+        QCOMPARE(proxy.descriptionTypes(), QVector<QKnxNetIp::DescriptionType>());
+    }
+
+    QVector<QKnxNetIp::DescriptionType> types {
+        QKnxNetIp::DescriptionType::DeviceInfo,
+        QKnxNetIp::DescriptionType::SupportedServiceFamilies,
+        QKnxNetIp::DescriptionType::ExtendedDeviceInfo
+    };
+
+    srp = QKnxNetIpSrpProxy::requestDibsBuilder()
+        .setDescriptionTypes(types)
         .create();
-    QCOMPARE(srpSupportedFamily.code(),
-             QKnxNetIp::SearchParameterType::SelectByServiceSRP);
-    QVERIFY(srpSupportedFamily.header().isMandatory());
-    QCOMPARE(srpSupportedFamily.constData().at(0), quint8(serviceFamilyId));
-    QCOMPARE(srpSupportedFamily.constData().at(1), minVersion);
-    QVERIFY(srpSupportedFamily.isValid());
-
-    QVector<QKnxNetIp::DescriptionType> types;
-    types.append(QKnxNetIp::DescriptionType::DeviceInfo);
-    types.append(QKnxNetIp::DescriptionType::SupportedServiceFamilies);
-    types.append(QKnxNetIp::DescriptionType::ExtendedDeviceInfo);
-    QKnxNetIpSrp srpDibs = QKnxNetIpSrpProxy::requestDibsBuilder()
-                   .setDescriptionTypes(types)
-                   .create();
-    QCOMPARE(srpDibs.code(), QKnxNetIp::SearchParameterType::RequestDIBs);
-    QVERIFY(srpDibs.header().isMandatory());
-    QVERIFY(srpDibs.isValid());
-
-    auto data = srpDibs.constData();
-    QCOMPARE(data.size(), 4);
+    QCOMPARE(srp.code(), QKnxNetIp::SearchParameterType::RequestDIBs);
+    QVERIFY(srp.header().isMandatory());
+    QVERIFY(srp.header().isValid());
+    QVERIFY(srp.isValid());
 
     types.append(QKnxNetIp::DescriptionType::Unknown);
-    for (int i = 0; i < data.size(); ++i)
-        QCOMPARE(QKnxNetIp::DescriptionType(data.at(i)), types[i]);
+    {
+        QKnxNetIpSrpProxy proxy(srp);
+        QVERIFY(proxy.isValid());
+        QVERIFY(proxy.isMandatory());
+        QCOMPARE(proxy.descriptionTypes(), types);
+        QCOMPARE(proxy.searchParameterType(), QKnxNetIp::SearchParameterType::RequestDIBs);
+
+        QVERIFY(!proxy.programmingModeOnly());
+        QCOMPARE(proxy.macAddress(), QKnxByteArray());
+        QCOMPARE(proxy.serviceInfo(), QKnxServiceInfo());
+    }
 }
 
 void tst_QKnxNetIpSearchRequest::tst_createSrpFromBytes()