From 3722a673ec101f9ed4d523b60ba8fa05392316e3 Mon Sep 17 00:00:00 2001 From: Fanda Vacek Date: Mon, 12 Feb 2024 17:11:25 +0100 Subject: [PATCH] Change Socket frame retrieve API to less error prone --- .../include/shv/chainpack/rpcdriver.h | 2 +- .../include/shv/chainpack/rpcmessage.h | 2 +- libshvchainpack/src/chainpack/rpcdriver.cpp | 4 ++-- libshvchainpack/src/chainpack/rpcmessage.cpp | 2 +- .../include/shv/iotqt/rpc/localsocket.h | 2 +- .../include/shv/iotqt/rpc/serialportsocket.h | 2 +- libshviotqt/include/shv/iotqt/rpc/socket.h | 19 ++++++++----------- libshviotqt/include/shv/iotqt/rpc/websocket.h | 2 +- libshviotqt/src/rpc/localsocket.cpp | 6 ++---- libshviotqt/src/rpc/serialportsocket.cpp | 6 ++---- libshviotqt/src/rpc/socket.cpp | 6 ++---- libshviotqt/src/rpc/socketrpcconnection.cpp | 15 +++------------ libshviotqt/src/rpc/websocket.cpp | 6 ++---- .../test_serialportsocket.cpp | 2 +- 14 files changed, 28 insertions(+), 48 deletions(-) diff --git a/libshvchainpack/include/shv/chainpack/rpcdriver.h b/libshvchainpack/include/shv/chainpack/rpcdriver.h index 026c9ea32..9682a5aa1 100644 --- a/libshvchainpack/include/shv/chainpack/rpcdriver.h +++ b/libshvchainpack/include/shv/chainpack/rpcdriver.h @@ -31,7 +31,7 @@ class SHVCHAINPACK_DECL_EXPORT RpcDriver virtual void writeFrameData(const std::string &frame_data) = 0; /// call it when new data arrived - virtual void onFrameDataRead(std::string &&frame_data); + virtual void onFrameDataRead(const std::string &frame_data); virtual void onRpcFrameReceived(RpcFrame &&frame); virtual void onParseDataException(const shv::chainpack::ParseException &e) = 0; diff --git a/libshvchainpack/include/shv/chainpack/rpcmessage.h b/libshvchainpack/include/shv/chainpack/rpcmessage.h index 3bae403d4..4c8f8449b 100644 --- a/libshvchainpack/include/shv/chainpack/rpcmessage.h +++ b/libshvchainpack/include/shv/chainpack/rpcmessage.h @@ -26,7 +26,7 @@ struct SHVCHAINPACK_DECL_EXPORT RpcFrame bool isValid() const { return !meta.isEmpty() && !data.empty(); } RpcMessage toRpcMessage(std::string *errmsg = nullptr) const; std::string toChainPack() const; - static RpcFrame fromChainPack(std::string &&frame_data); + static RpcFrame fromChainPack(const std::string &frame_data); }; class SHVCHAINPACK_DECL_EXPORT RpcMessage diff --git a/libshvchainpack/src/chainpack/rpcdriver.cpp b/libshvchainpack/src/chainpack/rpcdriver.cpp index dba2f0a27..322759476 100644 --- a/libshvchainpack/src/chainpack/rpcdriver.cpp +++ b/libshvchainpack/src/chainpack/rpcdriver.cpp @@ -45,12 +45,12 @@ void RpcDriver::sendRpcFrame(RpcFrame &&frame) writeFrameData(frame_data); } -void RpcDriver::onFrameDataRead(std::string &&frame_data) +void RpcDriver::onFrameDataRead(const std::string &frame_data) { logRpcData() << __PRETTY_FUNCTION__ << "+++++++++++++++++++++++++++++++++"; logRpcData().nospace() << "FRAME DATA " << frame_data.size() << " bytes of data read:\n" << shv::chainpack::utils::hexDump(frame_data); try { - auto frame = RpcFrame::fromChainPack(std::move(frame_data)); + auto frame = RpcFrame::fromChainPack(frame_data); onRpcFrameReceived(std::move(frame)); } catch (const ParseException &e) { diff --git a/libshvchainpack/src/chainpack/rpcmessage.cpp b/libshvchainpack/src/chainpack/rpcmessage.cpp index 5003c7cec..df6d3c86b 100644 --- a/libshvchainpack/src/chainpack/rpcmessage.cpp +++ b/libshvchainpack/src/chainpack/rpcmessage.cpp @@ -69,7 +69,7 @@ std::string RpcFrame::toChainPack() const return ret; } -RpcFrame RpcFrame::fromChainPack(std::string &&frame_data) +RpcFrame RpcFrame::fromChainPack(const std::string &frame_data) { std::istringstream in(frame_data); auto protocol = in.get(); diff --git a/libshviotqt/include/shv/iotqt/rpc/localsocket.h b/libshviotqt/include/shv/iotqt/rpc/localsocket.h index cdaa94398..2e03c6657 100644 --- a/libshviotqt/include/shv/iotqt/rpc/localsocket.h +++ b/libshviotqt/include/shv/iotqt/rpc/localsocket.h @@ -16,7 +16,7 @@ class SHVIOTQT_DECL_EXPORT LocalSocket : public Socket LocalSocket(QLocalSocket *socket, Protocol protocol, QObject *parent = nullptr); ~LocalSocket() override; - std::string readFrameData() override; + std::vector takeFrames() override; void writeFrameData(const std::string &frame_data) override; void connectToHost(const QUrl &url) override; diff --git a/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h b/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h index 32e2c56cf..d02749ffe 100644 --- a/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h +++ b/libshviotqt/include/shv/iotqt/rpc/serialportsocket.h @@ -57,7 +57,7 @@ class SHVIOTQT_DECL_EXPORT SerialPortSocket : public Socket public: SerialPortSocket(QSerialPort *port, QObject *parent = nullptr); - std::string readFrameData() override; + std::vector takeFrames() override; void writeFrameData(const std::string &frame_data) override; void setReceiveTimeout(int millis); diff --git a/libshviotqt/include/shv/iotqt/rpc/socket.h b/libshviotqt/include/shv/iotqt/rpc/socket.h index 891e20954..f90e1722f 100644 --- a/libshviotqt/include/shv/iotqt/rpc/socket.h +++ b/libshviotqt/include/shv/iotqt/rpc/socket.h @@ -7,7 +7,7 @@ #include #include -#include +#include class QTcpSocket; @@ -24,16 +24,13 @@ class FrameReader { virtual ~FrameReader() = default; virtual void addData(std::string_view data) = 0; bool isEmpty() const { return m_frames.empty(); } - std::string getFrame() { - if (!m_frames.empty()) { - auto frame = m_frames.front(); - m_frames.pop_front(); - return frame; - } - return {}; + std::vector takeFrames() { + auto frames = std::move(m_frames); + m_frames = {}; + return frames; } protected: - std::deque m_frames; + std::vector m_frames; }; class FrameWriter @@ -93,7 +90,7 @@ class SHVIOTQT_DECL_EXPORT Socket : public QObject virtual QHostAddress peerAddress() const = 0; virtual quint16 peerPort() const = 0; - virtual std::string readFrameData() = 0; + virtual std::vector takeFrames() = 0; virtual void writeFrameData(const std::string &frame_data) = 0; virtual void ignoreSslErrors() = 0; @@ -117,7 +114,7 @@ class SHVIOTQT_DECL_EXPORT TcpSocket : public Socket public: TcpSocket(QTcpSocket *socket, QObject *parent = nullptr); - std::string readFrameData() override; + std::vector takeFrames() override; void writeFrameData(const std::string &frame_data) override; void connectToHost(const QUrl &url) override; diff --git a/libshviotqt/include/shv/iotqt/rpc/websocket.h b/libshviotqt/include/shv/iotqt/rpc/websocket.h index 8573b5265..4b48be0f2 100644 --- a/libshviotqt/include/shv/iotqt/rpc/websocket.h +++ b/libshviotqt/include/shv/iotqt/rpc/websocket.h @@ -14,7 +14,7 @@ class SHVIOTQT_DECL_EXPORT WebSocket : public Socket public: WebSocket(QWebSocket *socket, QObject *parent = nullptr); - std::string readFrameData() override; + std::vector takeFrames() override; void writeFrameData(const std::string &frame_data) override; void connectToHost(const QUrl &url) override; diff --git a/libshviotqt/src/rpc/localsocket.cpp b/libshviotqt/src/rpc/localsocket.cpp index 98ff419d6..30c79e6ac 100644 --- a/libshviotqt/src/rpc/localsocket.cpp +++ b/libshviotqt/src/rpc/localsocket.cpp @@ -92,11 +92,9 @@ LocalSocket::~LocalSocket() delete m_frameWriter; } -std::string LocalSocket::readFrameData() +std::vector LocalSocket::takeFrames() { - if(m_frameReader->isEmpty()) - return {}; - return m_frameReader->getFrame(); + return m_frameReader->takeFrames(); } void LocalSocket::writeFrameData(const std::string &frame_data) diff --git a/libshviotqt/src/rpc/serialportsocket.cpp b/libshviotqt/src/rpc/serialportsocket.cpp index 5462b946b..fd6ec30d8 100644 --- a/libshviotqt/src/rpc/serialportsocket.cpp +++ b/libshviotqt/src/rpc/serialportsocket.cpp @@ -335,11 +335,9 @@ quint16 SerialPortSocket::peerPort() const return 0; } -std::string SerialPortSocket::readFrameData() +std::vector SerialPortSocket::takeFrames() { - if(m_frameReader.isEmpty()) - return {}; - return m_frameReader.getFrame(); + return m_frameReader.takeFrames(); } void SerialPortSocket::writeFrameData(const std::string &frame_data) diff --git a/libshviotqt/src/rpc/socket.cpp b/libshviotqt/src/rpc/socket.cpp index 6d8538452..06de2d2e8 100644 --- a/libshviotqt/src/rpc/socket.cpp +++ b/libshviotqt/src/rpc/socket.cpp @@ -204,11 +204,9 @@ quint16 TcpSocket::peerPort() const return m_socket->peerPort(); } -std::string TcpSocket::readFrameData() +std::vector TcpSocket::takeFrames() { - if(m_frameReader.isEmpty()) - return {}; - return m_frameReader.getFrame(); + return m_frameReader.takeFrames(); } void TcpSocket::writeFrameData(const std::string &frame_data) diff --git a/libshviotqt/src/rpc/socketrpcconnection.cpp b/libshviotqt/src/rpc/socketrpcconnection.cpp index 9b04811c4..3c3a9b0f4 100644 --- a/libshviotqt/src/rpc/socketrpcconnection.cpp +++ b/libshviotqt/src/rpc/socketrpcconnection.cpp @@ -98,18 +98,9 @@ void SocketRpcConnection::connectToHost(const QUrl &url) void SocketRpcConnection::onReadyRead() { - while (true) { - auto frame_data = socket()->readFrameData(); - if (frame_data.empty()) - break; - #ifdef DUMP_DATA_FILE - QFile *f = findChild("DUMP_DATA_FILE"); - if(f) { - f->write(ba.constData(), ba.length()); - f->flush(); - } - #endif - onFrameDataRead(std::move(frame_data)); + auto frames = socket()->takeFrames(); + for (const auto &frame : frames) { + onFrameDataRead(frame); }; emit socketDataReadyRead(); } diff --git a/libshviotqt/src/rpc/websocket.cpp b/libshviotqt/src/rpc/websocket.cpp index 37c2e22e4..5a91383fd 100644 --- a/libshviotqt/src/rpc/websocket.cpp +++ b/libshviotqt/src/rpc/websocket.cpp @@ -63,11 +63,9 @@ quint16 WebSocket::peerPort() const return m_socket->peerPort(); } -std::string WebSocket::readFrameData() +std::vector WebSocket::takeFrames() { - if(m_frameReader.isEmpty()) - return {}; - return m_frameReader.getFrame(); + return m_frameReader.takeFrames(); } void WebSocket::writeFrameData(const std::string &frame_data) diff --git a/libshviotqt/tests/serialportsocket/test_serialportsocket.cpp b/libshviotqt/tests/serialportsocket/test_serialportsocket.cpp index 815af9e2b..3ba17d312 100644 --- a/libshviotqt/tests/serialportsocket/test_serialportsocket.cpp +++ b/libshviotqt/tests/serialportsocket/test_serialportsocket.cpp @@ -111,7 +111,7 @@ DOCTEST_TEST_CASE("Test CRC error") data[5] = ~data[5]; serial->setDataToReceive(data); - REQUIRE(socket->readFrameData().empty()); + REQUIRE(socket->takeFrames().empty()); } /* DOCTEST_TEST_CASE("Test RESET message")