From a8559d6e3adabddb63386e38676cc0fb38951faf Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sat, 14 Jun 2014 16:49:22 -0700 Subject: [PATCH 1/7] Add back TCPLinkTest after threading change fixes --- qgroundcontrol.pro | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/qgroundcontrol.pro b/qgroundcontrol.pro index 6475dc5..be0b52b 100644 --- a/qgroundcontrol.pro +++ b/qgroundcontrol.pro @@ -187,7 +187,8 @@ DebugBuild { src/qgcunittest/MockUAS.h \ src/qgcunittest/MockQGCUASParamManager.h \ src/qgcunittest/MultiSignalSpy.h \ - src/qgcunittest/FlightModeConfigTest.h + src/qgcunittest/FlightModeConfigTest.h \ + src/qgcunittest/TCPLinkTest.h SOURCES += \ src/qgcunittest/UASUnitTest.cc \ @@ -195,7 +196,8 @@ DebugBuild { src/qgcunittest/MockUAS.cc \ src/qgcunittest/MockQGCUASParamManager.cc \ src/qgcunittest/MultiSignalSpy.cc \ - src/qgcunittest/FlightModeConfigTest.cc + src/qgcunittest/FlightModeConfigTest.cc \ + src/qgcunittest/TCPLinkTest.cc } # From 74690d82424d309cad96fcd03b0037e04a343328 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sat, 14 Jun 2014 16:49:38 -0700 Subject: [PATCH 2/7] Updated for threading changes --- src/qgcunittest/TCPLinkTest.cc | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/qgcunittest/TCPLinkTest.cc b/src/qgcunittest/TCPLinkTest.cc index ac57772..045c4e5 100644 --- a/src/qgcunittest/TCPLinkTest.cc +++ b/src/qgcunittest/TCPLinkTest.cc @@ -111,7 +111,9 @@ void TCPLinkUnitTest::_connectFail_test(void) Q_ASSERT(_multiSpy); Q_ASSERT(_multiSpy->checkNoSignals() == true); - QCOMPARE(_link->connect(), false); + // With the new threading model connect will always succeed. We only get an error signal + // for a failed connected. + QCOMPARE(_link->connect(), true); // Make sure we get a linkError signal with the right link name QCOMPARE(_multiSpy->waitForSignalByIndex(communicationErrorSignalIndex, 1000), true); @@ -119,10 +121,12 @@ void TCPLinkUnitTest::_connectFail_test(void) QList arguments = _multiSpy->getSpyByIndex(communicationErrorSignalIndex)->takeFirst(); QCOMPARE(arguments.at(0).toString(), _link->getName()); _multiSpy->clearSignalByIndex(communicationErrorSignalIndex); + + _link->disconnect(); // Try to connect again to make sure everything was cleaned up correctly from previous failed connection - QCOMPARE(_link->connect(), false); + QCOMPARE(_link->connect(), true); // Make sure we get a linkError signal with the right link name QCOMPARE(_multiSpy->waitForSignalByIndex(communicationErrorSignalIndex, 1000), true); From 780606e804c7abbef2c3438ec34f47960803d795 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sat, 14 Jun 2014 16:50:18 -0700 Subject: [PATCH 3/7] Limit time in processEvents This allows other threads to signal --- src/qgcunittest/MultiSignalSpy.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/qgcunittest/MultiSignalSpy.cc b/src/qgcunittest/MultiSignalSpy.cc index 1699dad..4cebd83 100644 --- a/src/qgcunittest/MultiSignalSpy.cc +++ b/src/qgcunittest/MultiSignalSpy.cc @@ -218,7 +218,7 @@ bool MultiSignalSpy::waitForSignalByIndex( Q_ASSERT(spy); while (spy->count() == 0 && !_timeout) { - QCoreApplication::processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents); + QCoreApplication::processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents, 500); } // Clean up and return status From be2780d72870213f247b885027152e7b0cf4d889 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sun, 15 Jun 2014 14:45:43 -0700 Subject: [PATCH 4/7] Modify TCPLinkTest for new threading model --- qgroundcontrol.pro | 6 ++-- src/comm/TCPLink.cc | 12 +++++++ src/comm/TCPLink.h | 7 +++- src/qgcunittest/MultiSignalSpy.cc | 6 +++- src/qgcunittest/TCPLinkTest.cc | 67 +++++++++++++++--------------------- src/qgcunittest/TCPLinkTest.h | 4 +++ src/qgcunittest/TCPLoopBackServer.cc | 65 ++++++++++++++++++++++++++++++++++ src/qgcunittest/TCPLoopBackServer.h | 55 +++++++++++++++++++++++++++++ 8 files changed, 179 insertions(+), 43 deletions(-) create mode 100644 src/qgcunittest/TCPLoopBackServer.cc create mode 100644 src/qgcunittest/TCPLoopBackServer.h diff --git a/qgroundcontrol.pro b/qgroundcontrol.pro index be0b52b..e8f3512 100644 --- a/qgroundcontrol.pro +++ b/qgroundcontrol.pro @@ -188,7 +188,8 @@ DebugBuild { src/qgcunittest/MockQGCUASParamManager.h \ src/qgcunittest/MultiSignalSpy.h \ src/qgcunittest/FlightModeConfigTest.h \ - src/qgcunittest/TCPLinkTest.h + src/qgcunittest/TCPLinkTest.h \ + src/qgcunittest/TCPLoopBackServer.h SOURCES += \ src/qgcunittest/UASUnitTest.cc \ @@ -197,7 +198,8 @@ DebugBuild { src/qgcunittest/MockQGCUASParamManager.cc \ src/qgcunittest/MultiSignalSpy.cc \ src/qgcunittest/FlightModeConfigTest.cc \ - src/qgcunittest/TCPLinkTest.cc + src/qgcunittest/TCPLinkTest.cc \ + src/qgcunittest/TCPLoopBackServer.cc } # diff --git a/src/comm/TCPLink.cc b/src/comm/TCPLink.cc index 146e7a3..7eee21c 100644 --- a/src/comm/TCPLink.cc +++ b/src/comm/TCPLink.cc @@ -306,3 +306,15 @@ void TCPLink::_resetName(void) _name = QString("TCP Link (host:%1 port:%2)").arg(_hostAddress.toString()).arg(_port); emit nameChanged(_name); } + +void TCPLink::waitForBytesWritten(int msecs) +{ + Q_ASSERT(_socket); + _socket->waitForBytesWritten(msecs); +} + +void TCPLink::waitForReadyRead(int msecs) +{ + Q_ASSERT(_socket); + _socket->waitForReadyRead(msecs); +} diff --git a/src/comm/TCPLink.h b/src/comm/TCPLink.h index b7e7fe7..38da51e 100644 --- a/src/comm/TCPLink.h +++ b/src/comm/TCPLink.h @@ -60,6 +60,8 @@ public: quint16 getPort(void) const { return _port; } QTcpSocket* getSocket(void) { return _socket; } + void signalBytesWritten(void); + // LinkInterface methods virtual int getId(void) const; virtual QString getName(void) const; @@ -80,6 +82,9 @@ public slots: // From LinkInterface virtual void writeBytes(const char* data, qint64 length); + + void waitForBytesWritten(int msecs); + void waitForReadyRead(int msecs); protected slots: void _socketError(QAbstractSocket::SocketError socketError); @@ -90,7 +95,7 @@ protected slots: protected: // From LinkInterface->QThread virtual void run(void); - + private: void _resetName(void); bool _hardwareConnect(void); diff --git a/src/qgcunittest/MultiSignalSpy.cc b/src/qgcunittest/MultiSignalSpy.cc index 4cebd83..5d8d87c 100644 --- a/src/qgcunittest/MultiSignalSpy.cc +++ b/src/qgcunittest/MultiSignalSpy.cc @@ -25,6 +25,7 @@ #include #include #include +#include /// @file /// @brief This class allows you to keep track of signal counts on a set of signals associated with an object. @@ -218,7 +219,10 @@ bool MultiSignalSpy::waitForSignalByIndex( Q_ASSERT(spy); while (spy->count() == 0 && !_timeout) { - QCoreApplication::processEvents(QEventLoop::AllEvents | QEventLoop::WaitForMoreEvents, 500); + QCoreApplication::sendPostedEvents(); + QCoreApplication::processEvents(); + QCoreApplication::flush(); + QTest::qSleep(100); } // Clean up and return status diff --git a/src/qgcunittest/TCPLinkTest.cc b/src/qgcunittest/TCPLinkTest.cc index 045c4e5..5047125 100644 --- a/src/qgcunittest/TCPLinkTest.cc +++ b/src/qgcunittest/TCPLinkTest.cc @@ -22,7 +22,7 @@ ======================================================================*/ #include "TCPLinkTest.h" -#include +#include "TCPLoopBackServer.h" /// @file /// @brief TCPLink class unit test @@ -143,51 +143,46 @@ void TCPLinkUnitTest::_connectSucceed_test(void) Q_ASSERT(_multiSpy->checkNoSignals() == true); // Start the server side - QTcpServer* server = new QTcpServer(this); - QCOMPARE(server->listen(_hostAddress, _port), true); + TCPLoopBackServer* server = new TCPLoopBackServer(_hostAddress, _port); + Q_CHECK_PTR(server); // Connect to the server QCOMPARE(_link->connect(), true); - // Wait for the connection to come through on server side - QCOMPARE(server->waitForNewConnection(1000), true); - QTcpSocket* serverSocket = server->nextPendingConnection(); - Q_ASSERT(serverSocket); - // Make sure we get the two different connected signals QCOMPARE(_multiSpy->waitForSignalByIndex(connectedSignalIndex, 1000), true); QCOMPARE(_multiSpy->checkOnlySignalByMask(connectedSignalMask | connected2SignalMask), true); QList arguments = _multiSpy->getSpyByIndex(connected2SignalIndex)->takeFirst(); QCOMPARE(arguments.at(0).toBool(), true); - _multiSpy->clearSignalsByMask(connectedSignalMask); - - // Test server->link data path - - QByteArray bytesOut("test"); - - // Write data from server to link - serverSocket->write(bytesOut); - - // Make sure we get the bytesReceived signal, with the correct data - QCOMPARE(_multiSpy->waitForSignalByIndex(bytesReceivedSignalIndex, 1000), true); - QCOMPARE(_multiSpy->checkOnlySignalByMask(bytesReceivedSignalMask), true); - arguments = _multiSpy->getSpyByIndex(bytesReceivedSignalIndex)->takeFirst(); - QCOMPARE(arguments.at(1), QVariant(bytesOut)); - _multiSpy->clearSignalByIndex(bytesReceivedSignalIndex); + _multiSpy->clearAllSignals(); // Test link->server data path + QByteArray bytesOut("test"); + // Write data from link to server + const char* bytesWrittenSignal = SIGNAL(bytesWritten(qint64)); + MultiSignalSpy bytesWrittenSpy; + QCOMPARE(bytesWrittenSpy.init(_link->getSocket(), &bytesWrittenSignal, 1), true); _link->writeBytes(bytesOut.data(), bytesOut.size()); - QCOMPARE(_link->getSocket()->waitForBytesWritten(1000), true); + _multiSpy->clearAllSignals(); + + // We emit this signal such that it will be queued and run on the TCPLink thread. This in turn + // allows the TCPLink object to pump the bytes through. + connect(this, SIGNAL(waitForBytesWritten(int)), _link, SLOT(waitForBytesWritten(int))); + emit waitForBytesWritten(1000); - // Make sure we get readyRead on server - QCOMPARE(serverSocket->waitForReadyRead(1000), true); + // Check for loopback, both from signal received and actual bytes returned + QCOMPARE(_multiSpy->waitForSignalByIndex(bytesReceivedSignalIndex, 1000), true); + QCOMPARE(_multiSpy->checkOnlySignalByMask(bytesReceivedSignalMask), true); + // Read the data and make sure it matches - QByteArray bytesIn = serverSocket->read(bytesOut.size() + 100); - QCOMPARE(bytesIn, bytesOut); + arguments = _multiSpy->getSpyByIndex(bytesReceivedSignalIndex)->takeFirst(); + QVERIFY(arguments.at(1).toByteArray() == bytesOut); + _multiSpy->clearAllSignals(); + // Disconnect the link _link->disconnect(); @@ -196,27 +191,21 @@ void TCPLinkUnitTest::_connectSucceed_test(void) QCOMPARE(_multiSpy->checkOnlySignalByMask(disconnectedSignalMask | connected2SignalMask), true); arguments = _multiSpy->getSpyByIndex(connected2SignalIndex)->takeFirst(); QCOMPARE(arguments.at(0).toBool(), false); - _multiSpy->clearSignalsByMask(disconnectedSignalMask); - - // Make sure we get disconnected signals from the server side - QCOMPARE(serverSocket->waitForDisconnected(1000), true ); + _multiSpy->clearAllSignals(); // Try to connect again to make sure everything was cleaned up correctly from previous connection // Connect to the server QCOMPARE(_link->connect(), true); - // Wait for the connection to come through on server side - QCOMPARE(server->waitForNewConnection(1000), true); - serverSocket = server->nextPendingConnection(); - Q_ASSERT(serverSocket); - // Make sure we get the two different connected signals QCOMPARE(_multiSpy->waitForSignalByIndex(connectedSignalIndex, 1000), true); QCOMPARE(_multiSpy->checkOnlySignalByMask(connectedSignalMask | connected2SignalMask), true); arguments = _multiSpy->getSpyByIndex(connected2SignalIndex)->takeFirst(); QCOMPARE(arguments.at(0).toBool(), true); - _multiSpy->clearSignalsByMask(connectedSignalMask); - + _multiSpy->clearAllSignals(); + + server->quit(); + QTest::qWait(500); // Wait a little for server thread to terminate delete server; } diff --git a/src/qgcunittest/TCPLinkTest.h b/src/qgcunittest/TCPLinkTest.h index 120fa96..61a0fa8 100644 --- a/src/qgcunittest/TCPLinkTest.h +++ b/src/qgcunittest/TCPLinkTest.h @@ -43,6 +43,10 @@ class TCPLinkUnitTest : public QObject public: TCPLinkUnitTest(void); + +signals: + void waitForBytesWritten(int msecs); + void waitForReadyRead(int msecs); private slots: void init(void); diff --git a/src/qgcunittest/TCPLoopBackServer.cc b/src/qgcunittest/TCPLoopBackServer.cc new file mode 100644 index 0000000..07c1644 --- /dev/null +++ b/src/qgcunittest/TCPLoopBackServer.cc @@ -0,0 +1,65 @@ +/*===================================================================== + + QGroundControl Open Source Ground Control Station + + (c) 2009 - 2014 QGROUNDCONTROL PROJECT + + This file is part of the QGROUNDCONTROL project + + QGROUNDCONTROL is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + QGROUNDCONTROL is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QGROUNDCONTROL. If not, see . + + ======================================================================*/ + +#include "TCPLoopBackServer.h" + +TCPLoopBackServer::TCPLoopBackServer(QHostAddress hostAddress, quint16 port) : + _hostAddress(hostAddress), + _port(port), + _tcpSocket(NULL) +{ + moveToThread(this); + start(HighPriority); +} + +void TCPLoopBackServer::run(void) +{ + // Start the server side + _tcpServer = new QTcpServer(this); + Q_CHECK_PTR(_tcpServer); + + bool connected = QObject::connect(_tcpServer, SIGNAL(newConnection()), this, SLOT(_newConnection())); + Q_ASSERT(connected); + + Q_ASSERT(_tcpServer->listen(_hostAddress, _port)); + + // Fall into main event loop + exec(); +} + +void TCPLoopBackServer::_newConnection(void) +{ + Q_ASSERT(_tcpServer); + _tcpSocket = _tcpServer->nextPendingConnection(); + Q_ASSERT(_tcpSocket); + bool connected = QObject::connect(_tcpSocket, SIGNAL(readyRead()), this, SLOT(_readBytes())); + Q_ASSERT(connected); +} + +void TCPLoopBackServer::_readBytes(void) +{ + Q_ASSERT(_tcpSocket); + QByteArray bytesIn = _tcpSocket->read(_tcpSocket->bytesAvailable()); + Q_ASSERT(_tcpSocket->write(bytesIn) == bytesIn.count()); + Q_ASSERT(_tcpSocket->waitForBytesWritten(1000)); +} diff --git a/src/qgcunittest/TCPLoopBackServer.h b/src/qgcunittest/TCPLoopBackServer.h new file mode 100644 index 0000000..b433efc --- /dev/null +++ b/src/qgcunittest/TCPLoopBackServer.h @@ -0,0 +1,55 @@ +/*===================================================================== + + QGroundControl Open Source Ground Control Station + + (c) 2009 - 2014 QGROUNDCONTROL PROJECT + + This file is part of the QGROUNDCONTROL project + + QGROUNDCONTROL is free software: you can redistribute it and/or modify + it under the terms of the GNU General Public License as published by + the Free Software Foundation, either version 3 of the License, or + (at your option) any later version. + + QGROUNDCONTROL is distributed in the hope that it will be useful, + but WITHOUT ANY WARRANTY; without even the implied warranty of + MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the + GNU General Public License for more details. + + You should have received a copy of the GNU General Public License + along with QGROUNDCONTROL. If not, see . + + ======================================================================*/ + +#ifndef TCPLOOPBACKSERVER_H +#define TCPLOOPBACKSERVER_H + +#include +#include +#include + +class TCPLoopBackServer : public QThread +{ + Q_OBJECT + +public: + TCPLoopBackServer(QHostAddress hostAddress, quint16 port); + +signals: + void newConnection(void); + +protected: + virtual void run(void); + +private slots: + void _newConnection(void); + void _readBytes(void); + +private: + QHostAddress _hostAddress; + quint16 _port; + QTcpServer* _tcpServer; + QTcpSocket* _tcpSocket; +}; + +#endif \ No newline at end of file From c97c855764c1e8f881ae1f5b267f7f8e8c755f94 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sun, 15 Jun 2014 15:04:01 -0700 Subject: [PATCH 5/7] Can't call _socket methods from primary thread --- src/comm/TCPLink.cc | 1 - 1 file changed, 1 deletion(-) diff --git a/src/comm/TCPLink.cc b/src/comm/TCPLink.cc index 7eee21c..9cef75e 100644 --- a/src/comm/TCPLink.cc +++ b/src/comm/TCPLink.cc @@ -198,7 +198,6 @@ bool TCPLink::disconnect() if (_socket) { - _socket->disconnectFromHost(); _socketIsConnected = false; delete _socket; _socket = NULL; From 163383cb990441c7ab9720c8ff33c208957ebed9 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sun, 15 Jun 2014 15:28:17 -0700 Subject: [PATCH 6/7] Comments --- src/qgcunittest/TCPLoopBackServer.cc | 5 +++++ src/qgcunittest/TCPLoopBackServer.h | 5 +++++ 2 files changed, 10 insertions(+) diff --git a/src/qgcunittest/TCPLoopBackServer.cc b/src/qgcunittest/TCPLoopBackServer.cc index 07c1644..0198ade 100644 --- a/src/qgcunittest/TCPLoopBackServer.cc +++ b/src/qgcunittest/TCPLoopBackServer.cc @@ -23,6 +23,11 @@ #include "TCPLoopBackServer.h" +/// @file +/// @brief Simple TCP loop back server +/// +/// @author Don Gagne + TCPLoopBackServer::TCPLoopBackServer(QHostAddress hostAddress, quint16 port) : _hostAddress(hostAddress), _port(port), diff --git a/src/qgcunittest/TCPLoopBackServer.h b/src/qgcunittest/TCPLoopBackServer.h index b433efc..c403671 100644 --- a/src/qgcunittest/TCPLoopBackServer.h +++ b/src/qgcunittest/TCPLoopBackServer.h @@ -28,6 +28,11 @@ #include #include +/// @file +/// @brief Simple TCP loop back server +/// +/// @author Don Gagne + class TCPLoopBackServer : public QThread { Q_OBJECT From 83a092f348a6dc7c5687b456899f618ad9aa6b05 Mon Sep 17 00:00:00 2001 From: Don Gagne Date: Sun, 15 Jun 2014 15:28:46 -0700 Subject: [PATCH 7/7] deleteLater on _socket This way the delete happens on the right thread. --- src/comm/TCPLink.cc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/comm/TCPLink.cc b/src/comm/TCPLink.cc index 9cef75e..51c2223 100644 --- a/src/comm/TCPLink.cc +++ b/src/comm/TCPLink.cc @@ -199,7 +199,7 @@ bool TCPLink::disconnect() if (_socket) { _socketIsConnected = false; - delete _socket; + _socket->deleteLater(); // Make sure delete happens on correct thread _socket = NULL; emit disconnected();