Merge remote-tracking branch 'origin/tqtc/lts-5.15.4' into tqtc/lts-5.15-opensourcev5.15.4-lts-lgpl

Change-Id: Ibfaf27eedd35197b9b2ba306c0e437f15c5c9682

6 files changed, 71 insertions, 53 deletions

diff --git a/.qmake.conf b/.qmake.conf
index 3dea105..0044fcc 100644
--- a/.qmake.conf
+++ b/.qmake.conf
@@ -2,4 +2,4 @@ load(qt_build_config)
 CONFIG += warning_clean
 DEFINES += QT_NO_FOREACH
 
-MODULE_VERSION = 5.15.3
+MODULE_VERSION = 5.15.4
diff --git a/src/serialbus/qmodbusclient.cpp b/src/serialbus/qmodbusclient.cpp
index 56a1ab5..1c8fd80 100644
--- a/src/serialbus/qmodbusclient.cpp
+++ b/src/serialbus/qmodbusclient.cpp
@@ -361,7 +361,7 @@ void QModbusClientPrivate::processQueueElement(const QModbusResponse &pdu,
         return;
     }
 
-    if (element.reply->type() != QModbusReply::Common) {
+    if (element.reply->type() == QModbusReply::Broadcast) {
         element.reply->setFinished(true);
         return;
     }
diff --git a/src/serialbus/qmodbuspdu.cpp b/src/serialbus/qmodbuspdu.cpp
index 9ca9027..d97d614 100644
--- a/src/serialbus/qmodbuspdu.cpp
+++ b/src/serialbus/qmodbuspdu.cpp
@@ -48,13 +48,23 @@ Q_GLOBAL_STATIC(ReqSizeCalc, requestSizeCalculators);
 using ResSizeCalc = QHash<quint8, QModbusResponse::CalcFuncPtr>;
 Q_GLOBAL_STATIC(ResSizeCalc, responseSizeCalculators);
 
-namespace Private {
+struct QModbusPduPrivate
+{
+    QModbusPduPrivate() = delete;
+    Q_DISABLE_COPY_MOVE(QModbusPduPrivate)
 
 enum struct Type {
     Request,
     Response
 };
 
+/*!
+    \internal
+
+    Returns the minimum data size in bytes for the given \a pdu and the
+    Modbus PDU \a type. If the PDU's function code is invalid, undefined
+    or unknown, the return value will be \c {-1}.
+*/
 static int minimumDataSize(const QModbusPdu &pdu, Type type)
 {
     if (pdu.isException())
@@ -103,44 +113,53 @@ static int minimumDataSize(const QModbusPdu &pdu, Type type)
     return -1;
 }
 
-static QDataStream &pduFromStream(QDataStream &stream, QModbusPdu &pdu, Type type)
+/*!
+    \internal
+
+    Extracts a Modbus PDU from a \a stream into the given \a pdu based on \a type.
+*/
+static QDataStream &pduFromStream(QDataStream &stream, Type type, QModbusPdu *pdu)
 {
-    quint8 codeByte = 0;
-    if (stream.readRawData(reinterpret_cast<char *>(&codeByte), sizeof(quint8)) != sizeof(quint8))
+    struct RAII {
+        RAII(QModbusPdu *ptr = nullptr)
+            : tmp(ptr) {}
+        QModbusPdu *tmp{ nullptr };
+        ~RAII() { if (tmp) *tmp = {}; }
+    } raii = { pdu };
+
+    QModbusPdu::FunctionCode code = QModbusPdu::FunctionCode::Invalid;
+    if (stream.readRawData(reinterpret_cast<char *>(&code), sizeof(quint8)) != sizeof(quint8))
         return stream;
-    QModbusPdu::FunctionCode code = QModbusPdu::FunctionCode(codeByte);
-    pdu.setFunctionCode(code);
+    pdu->setFunctionCode(code);
 
-    auto needsAdditionalRead = [](QModbusPdu &pdu, int size) -> bool {
-        if (size < 0)
-            pdu.setFunctionCode(QModbusResponse::Invalid);
-        if (size <= 0)
-            return false;
-        return true;
-    };
-
-    const bool isResponse = (type == Type::Response);
-    int size = isResponse ? QModbusResponse::minimumDataSize(pdu)
-                          : QModbusRequest::minimumDataSize(pdu);
-    if (!needsAdditionalRead(pdu, size))
+    if (code == QModbusPdu::Invalid || code == QModbusPdu::UndefinedFunctionCode) // shortcut
         return stream;
 
-    QByteArray data(size, Qt::Uninitialized);
-    if (stream.device()->peek(data.data(), data.size()) != size)
-        return stream;
+    constexpr const int MaxPduDataSize = 252; // in bytes
 
-    pdu.setData(data);
-    size = isResponse ? QModbusResponse::calculateDataSize(pdu)
-                      : QModbusRequest::calculateDataSize(pdu);
-    if (!needsAdditionalRead(pdu, size))
+    // The calculateDataSize(...) function might need some data inside the
+    // given PDU argument to be able to figure out the right data size (e.g.
+    // WriteMultipleCoils contains some kind of "header"). So fake fill the PDU
+    // with the maximum available data but no more than the allowed max PDU
+    // data size.
+    QByteArray data(MaxPduDataSize, Qt::Uninitialized);
+    int read = stream.device()->peek(data.data(), MaxPduDataSize);
+    if (read < 0)
         return stream;
 
+    data.resize(read);
+    pdu->setData(data);
+
+    const bool isResponse = (type == Type::Response);
+    int size = isResponse ? QModbusResponse::calculateDataSize(*pdu)
+                          : QModbusRequest::calculateDataSize(*pdu);
+
     if (isResponse && (code == QModbusPdu::EncapsulatedInterfaceTransport)) {
         quint8 meiType;
-        pdu.decodeData(&meiType);
+        pdu->decodeData(&meiType);
         if (meiType == EncapsulatedInterfaceTransport::ReadDeviceIdentification) {
             int left = size, offset = 0;
-            while ((left > 0) && (size <= 252)) { // The maximum PDU data size is 252 bytes.
+            while ((left > 0) && (size <= MaxPduDataSize)) {
                 data.resize(size);
                 const int read = stream.readRawData(data.data() + offset, size - offset);
                 if ((read < 0) || (read != (size - offset))) {
@@ -152,35 +171,31 @@ static QDataStream &pduFromStream(QDataStream &stream, QModbusPdu &pdu, Type typ
                 left = QModbusResponse::calculateDataSize(QModbusResponse(code, data)) - offset;
                 size += left;
             }
-            if ((stream.status() == QDataStream::Ok) && (size <= 252)) {
-                pdu.setData(data);
+            if ((stream.status() == QDataStream::Ok) && (size <= MaxPduDataSize)) {
+                raii = {};
+                pdu->setData(data);
                 return stream; // early return to avoid second read
             }
         } else {
             data.resize(int(stream.device()->size() - 1)); // One byte for the function code.
         }
-    } else if (pdu.functionCode() == QModbusPdu::Diagnostics) {
+    } else if (pdu->functionCode() == QModbusPdu::Diagnostics) {
         quint16 subCode;
-        pdu.decodeData(&subCode);
+        pdu->decodeData(&subCode);
         if (subCode == Diagnostics::ReturnQueryData)
             data.resize(int(stream.device()->size() - 1)); // One byte for the function code.
     }
 
-    // reset what we have so far, next read might fail as well
-    pdu.setData(QByteArray());
-    pdu.setFunctionCode(QModbusPdu::Invalid);
-
-    if (data.size() <= 252) { // The maximum PDU data size is 252 bytes.
+    if (data.size() <= MaxPduDataSize) {
         data.resize(size);
         if (stream.readRawData(data.data(), data.size()) == size) {
-            pdu.setData(data);
-            pdu.setFunctionCode(code);
+            raii = {};
+            pdu->setData(data);
         }
     }
     return stream;
 }
-
-}   // namespace Private
+};
 
 /*!
     \class QModbusPdu
@@ -533,7 +548,7 @@ QDataStream &operator<<(QDataStream &stream, const QModbusPdu &pdu)
 */
 int QModbusRequest::minimumDataSize(const QModbusRequest &request)
 {
-    return Private::minimumDataSize(request, Private::Type::Request);
+    return QModbusPduPrivate::minimumDataSize(request, QModbusPduPrivate::Type::Request);
 }
 
 /*!
@@ -555,7 +570,7 @@ int QModbusRequest::calculateDataSize(const QModbusRequest &request)
         return 1;
 
     int size = -1;
-    int minimum = Private::minimumDataSize(request, Private::Type::Request);
+    int minimum = QModbusPduPrivate::minimumDataSize(request, QModbusPduPrivate::Type::Request);
     if (minimum < 0)
         return size;
 
@@ -620,7 +635,7 @@ void QModbusRequest::registerDataSizeCalculator(FunctionCode fc, CalcFuncPtr cal
 */
 QDataStream &operator>>(QDataStream &stream, QModbusRequest &pdu)
 {
-    return Private::pduFromStream(stream, pdu, Private::Type::Request);
+    return QModbusPduPrivate::pduFromStream(stream, QModbusPduPrivate::Type::Request, &pdu);
 }
 
 /*!
@@ -689,7 +704,7 @@ QDataStream &operator>>(QDataStream &stream, QModbusRequest &pdu)
 */
 int QModbusResponse::minimumDataSize(const QModbusResponse &response)
 {
-    return Private::minimumDataSize(response, Private::Type::Response);
+    return QModbusPduPrivate::minimumDataSize(response, QModbusPduPrivate::Type::Response);
 }
 
 /*!
@@ -711,7 +726,7 @@ int QModbusResponse::calculateDataSize(const QModbusResponse &response)
         return 1;
 
     int size = -1;
-    int minimum = Private::minimumDataSize(response, Private::Type::Response);
+    int minimum = QModbusPduPrivate::minimumDataSize(response, QModbusPduPrivate::Type::Response);
     if (minimum < 0)
         return size;
 
@@ -806,7 +821,7 @@ void QModbusResponse::registerDataSizeCalculator(FunctionCode fc, CalcFuncPtr ca
 */
 QDataStream &operator>>(QDataStream &stream, QModbusResponse &pdu)
 {
-    return Private::pduFromStream(stream, pdu, Private::Type::Response);
+    return QModbusPduPrivate::pduFromStream(stream, QModbusPduPrivate::Type::Response, &pdu);
 }
 
 /*!
diff --git a/src/serialbus/qmodbuspdu.h b/src/serialbus/qmodbuspdu.h
index e93ec4c..9372730 100644
--- a/src/serialbus/qmodbuspdu.h
+++ b/src/serialbus/qmodbuspdu.h
@@ -183,6 +183,7 @@ private:
     FunctionCode m_code = Invalid;
     QByteArray m_data;
     friend class QModbusSerialAdu;
+    friend struct QModbusPduPrivate;
 };
 Q_SERIALBUS_EXPORT QDebug operator<<(QDebug debug, const QModbusPdu &pdu);
 Q_SERIALBUS_EXPORT QDataStream &operator<<(QDataStream &stream, const QModbusPdu &pdu);
diff --git a/src/serialbus/qmodbusreply.cpp b/src/serialbus/qmodbusreply.cpp
index caa2dd9..374fb81 100644
--- a/src/serialbus/qmodbusreply.cpp
+++ b/src/serialbus/qmodbusreply.cpp
@@ -147,15 +147,17 @@ void QModbusReply::setFinished(bool isFinished)
     If the request has not finished, has failed with an error or was a write
     request then the returned \l QModbusDataUnit instance is invalid.
 
-    \note If the \l type() of the reply is \l QModbusReply::Raw, the return
-    value will always be invalid.
+    \note If the \l type() of the reply is \l QModbusReply::Broadcast, the
+    return value will always be invalid. If the l type() of the reply is
+    \l QModbusReply::Raw, the return value might be invalid depending on the
+    implementation of \l QModbusClient::processPrivateResponse().
 
-    \sa type(), rawResult()
+    \sa type(), rawResult(), QModbusClient::processPrivateResponse()
 */
 QModbusDataUnit QModbusReply::result() const
 {
     Q_D(const QModbusReply);
-    if (type() == QModbusReply::Common)
+    if (type() != QModbusReply::Broadcast)
         return d->m_unit;
     return QModbusDataUnit();
 }
diff --git a/tests/auto/qmodbusreply/tst_qmodbusreply.cpp b/tests/auto/qmodbusreply/tst_qmodbusreply.cpp
index 810fbee..3ac0395 100644
--- a/tests/auto/qmodbusreply/tst_qmodbusreply.cpp
+++ b/tests/auto/qmodbusreply/tst_qmodbusreply.cpp
@@ -213,7 +213,7 @@ void tst_QModbusReply::tst_setResult()
     replyRawTest.setRawResult(response);
     QCOMPARE(finishedSpy.count(), 0);
     QCOMPARE(errorSpyRaw.count(), 0);
-    QCOMPARE(replyRawTest.result().isValid(), false);
+    QCOMPARE(replyRawTest.result().isValid(), true);
     QCOMPARE(replyRawTest.rawResult().isValid(), true);
 
     tmp = replyRawTest.rawResult();