Merge pull request #1023 from DonLakeFlyer/LInkManager

Link manager unit test and fixes

qgroundcontrol.pro

@@ -669,7 +669,8 @@ HEADERS += \
 	src/qgcunittest/TCPLinkTest.h \
 	src/qgcunittest/TCPLoopBackServer.h \
 	src/qgcunittest/QGCUASFileManagerTest.h \
-    src/qgcunittest/PX4RCCalibrationTest.h
+    src/qgcunittest/PX4RCCalibrationTest.h \
+    src/qgcunittest/LinkManagerTest.h
 
 SOURCES += \
 	src/qgcunittest/UASUnitTest.cc \
@@ -684,8 +685,8 @@ SOURCES += \
 	src/qgcunittest/TCPLinkTest.cc \
 	src/qgcunittest/TCPLoopBackServer.cc \
 	src/qgcunittest/QGCUASFileManagerTest.cc \
-    src/qgcunittest/PX4RCCalibrationTest.cc
-
+    src/qgcunittest/PX4RCCalibrationTest.cc \
+    src/qgcunittest/LinkManagerTest.cc
 }
 
 #

src/CmdLineOptParser.cc

@@ -39,13 +39,24 @@ void ParseCmdLineOptions(int&           argc,                   ///< count of ar
 {
     // Start with all options off
     for (size_t iOption=0; iOption<cOpts; iOption++) {
-        *prgOpts[iOption].flag = false;
+        *prgOpts[iOption].optionFound = false;
     }
     
     for (int iArg=1; iArg<argc; iArg++) {
         for (size_t iOption=0; iOption<cOpts; iOption++) {
-            if (QString(argv[iArg]).compare(prgOpts[iOption].optionStr, Qt::CaseInsensitive) == 0) {
-                *prgOpts[iOption].flag = true;
+            bool found = false;
+            
+            QString arg(argv[iArg]);
+            QString optionStr(prgOpts[iOption].optionStr);
+            
+            if (arg.startsWith(QString("%1:").arg(optionStr), Qt::CaseInsensitive)) {
+                found = true;
+                prgOpts[iOption].optionArg = arg.right(arg.length() - (optionStr.length() + 1));
+            } else if (arg.compare(optionStr, Qt::CaseInsensitive) == 0) {
+                found = true;
+            }
+            if (found) {
+                *prgOpts[iOption].optionFound = true;
                 if (removeParsedOptions) {
                     for (int iShift=iArg; iShift<argc-1; iShift++) {
                         argv[iShift] = argv[iShift+1];

src/CmdLineOptParser.h

@@ -29,12 +29,14 @@
 #ifndef CMDLINEOPTPARSER_H
 #define CMDLINEOPTPARSER_H
 
+#include <QString>
 #include <cstring>
 
 /// @brief Structure used to pass command line options to the ParseCmdLineOptions function.
 typedef struct {
-    const char* optionStr;  ///< command line option, for example "--foo"
-    bool*       flag;       ///< if option is found this variable will be set to true
+    const char* optionStr;      ///< command line option, for example "--foo"
+    bool*       optionFound;    ///< if option is found this variable will be set to true
+    QString     optionArg;      ///< Option has additional argument, form is option:arg
 } CmdLineOpt_t;
 
 void ParseCmdLineOptions(int&           argc,

src/QGCApplication.cc

@@ -109,7 +109,7 @@ QGCApplication::QGCApplication(int &argc, char* argv[]) :
     bool fClearSettingsOptions = false; // Clear stored settings
     
     CmdLineOpt_t rgCmdLineOptions[] = {
-        { "--clear-settings",   &fClearSettingsOptions },
+        { "--clear-settings",   &fClearSettingsOptions, QString() },
         // Add additional command line option flags here
     };
     
@@ -136,13 +136,15 @@ QGCApplication::~QGCApplication()
 
 void QGCApplication::_initCommon(void)
 {
-    _createSingletons();
+
 }
 
 bool QGCApplication::_initForNormalAppBoot(void)
 {
     QSettings settings;
     
+    _createSingletons();
+    
     // Exit main application when last window is closed
     connect(this, SIGNAL(lastWindowClosed()), this, SLOT(quit()));
     

src/comm/LinkInterface.h

@@ -49,12 +49,14 @@ class LinkInterface : public QThread
 {
     Q_OBJECT
     
-    // Only LinkManager is allowed to _connect or _disconnect a link
+    // Only LinkManager is allowed to _connect, _disconnect or delete a link
     friend class LinkManager;
     
 public:
     LinkInterface() :
-        QThread(0)
+        QThread(0),
+        _ownedByLinkManager(false),
+        _deletedByLinkManager(false)
     {
         // Initialize everything for the data rate calculation buffers.
         inDataIndex = 0;
@@ -73,7 +75,9 @@ public:
     }
 
     virtual ~LinkInterface() {
-        emit this->deleteLink(this);
+        // LinkManager take ownership of Links once they are added to it. Once added to LinkManager
+        // user LinkManager::deleteLink to remove if necessary/
+        Q_ASSERT(!_ownedByLinkManager || _deletedByLinkManager);
     }
 
     /* Connection management */
@@ -138,6 +142,11 @@ public:
     {
         return getCurrentDataRate(outDataIndex, outDataWriteTimes, outDataWriteAmounts);
     }
+    
+    // These are left unimplemented in order to cause linker errors which indicate incorrect usage of
+    // connect/disconnect on link directly. All connect/disconnect calls should be made through LinkManager.
+    bool connect(void);
+    bool disconnect(void);
 
 public slots:
 
@@ -192,9 +201,6 @@ signals:
 
     void communicationUpdate(const QString& linkname, const QString& text);
 
-    /** @brief destroying element */
-    void deleteLink(LinkInterface* const link);
-
 protected:
 
     static const int dataRateBufferSize = 20; ///< Specify how many data points to capture for data rate calculations.
@@ -329,6 +335,9 @@ private:
      * @return True if connection could be terminated, false otherwise
      **/
     virtual bool _disconnect(void) = 0;
+    
+    bool _ownedByLinkManager;   ///< true: This link has been added to LinkManager, false: Link not added to LinkManager
+    bool _deletedByLinkManager; ///< true: Link being deleted from LinkManager, false: error, Links should only be deleted from LinkManager
 };
 
 #endif // _LINKINTERFACE_H_

src/comm/LinkManager.cc

@@ -63,8 +63,8 @@ void LinkManager::deleteInstance(void)
  *
  * This class implements the singleton design pattern and has therefore only a private constructor.
  **/
-LinkManager::LinkManager(QObject* parent) :
-    QGCSingleton(parent),
+LinkManager::LinkManager(QObject* parent, bool registerSingleton) :
+    QGCSingleton(parent, registerSingleton),
     _connectionsSuspended(false)
 {
     _links = QList<LinkInterface*>();
@@ -75,24 +75,23 @@ LinkManager::~LinkManager()
 {
     disconnectAll();
     
-    _dataMutex.lock();
     foreach (LinkInterface* link, _links) {
         Q_ASSERT(link);
-        link->deleteLater();
+        deleteLink(link);
     }
     _links.clear();
-    _dataMutex.unlock();
 }
 
 void LinkManager::add(LinkInterface* link)
 {
     Q_ASSERT(link);
     
+    // Take ownership for delete
+    link->_ownedByLinkManager = true;
+    
     _dataMutex.lock();
     
-    if (!_links.contains(link))
-    {
-        connect(link, SIGNAL(destroyed(QObject*)), this, SLOT(removeObj(QObject*)));
+    if (!_links.contains(link)) {
         _links.append(link);
         _dataMutex.unlock();
         emit newLink(link);
@@ -174,7 +173,7 @@ bool LinkManager::disconnectAll()
     foreach (LinkInterface* link, _links)
     {
         Q_ASSERT(link);
-        if (!link->disconnect()) {
+        if (!link->_disconnect()) {
             allDisconnected = false;
         }
     }
@@ -200,62 +199,30 @@ bool LinkManager::disconnectLink(LinkInterface* link)
     return link->_disconnect();
 }
 
-void LinkManager::removeObj(QObject* link)
-{
-    // Be careful of the fact that by the time this signal makes it through the queue
-    // the link object has already been destructed.
-    removeLink((LinkInterface*)link);
-}
-
-bool LinkManager::removeLink(LinkInterface* link)
-{
-    if(link)
-    {
-        _dataMutex.lock();
-        for (int i=0; i < _links.size(); i++)
-        {
-            if(link==_links.at(i))
-            {
-                _links.removeAt(i); //remove from link list
-            }
-        }
-        // Remove link from protocol map
-        QList<ProtocolInterface* > protocols = _protocolLinks.keys(link);
-        foreach (ProtocolInterface* proto, protocols)
-        {
-            _protocolLinks.remove(proto, link);
-        }
-        _dataMutex.unlock();
-
-        // Emit removal of link
-        emit linkRemoved(link);
-
-        return true;
-    }
-    return false;
-}
-
-/**
- * The access time is linear in the number of links.
- *
- * @param id link identifier to search for
- * @return A pointer to the link or NULL if not found
- */
-LinkInterface* LinkManager::getLinkForId(int id)
+void LinkManager::deleteLink(LinkInterface* link)
 {
+    Q_ASSERT(link);
+    
     _dataMutex.lock();
-    LinkInterface* linkret = NULL;
-    foreach (LinkInterface* link, _links)
-    {
-        Q_ASSERT(link);
-        
-        if (link->getId() == id)
-        {
-            linkret = link;
-        }
+    
+    Q_ASSERT(_links.contains(link));
+    _links.removeOne(link);
+    Q_ASSERT(!_links.contains(link));
+
+    // Remove link from protocol map
+    QList<ProtocolInterface* > protocols = _protocolLinks.keys(link);
+    foreach (ProtocolInterface* proto, protocols) {
+        _protocolLinks.remove(proto, link);
     }
+             
     _dataMutex.unlock();
-    return linkret;
+             
+    // Emit removal of link
+    emit linkDeleted(link);
+    
+    Q_ASSERT(link->_ownedByLinkManager);
+    link->_deletedByLinkManager = true;   // Signal that this is a valid delete
+    delete link;
 }
 
 /**
@@ -307,4 +274,3 @@ void LinkManager::setConnectionsSuspended(QString reason)
     _connectionsSuspendedReason = reason;
     Q_ASSERT(!reason.isEmpty());
 }
-

src/comm/LinkManager.h

@@ -42,6 +42,8 @@ This file is part of the PIXHAWK project
 #include "ProtocolInterface.h"
 #include "QGCSingleton.h"
 
+class LinkManagerTest;
+
 /**
  * The Link Manager organizes the physical Links. It can manage arbitrary
  * links and takes care of connecting them as well assigning the correct
@@ -60,15 +62,10 @@ public:
 
     ~LinkManager();
 
-    void run();
-
     QList<LinkInterface*> getLinksForProtocol(ProtocolInterface* protocol);
 
     ProtocolInterface* getProtocolForLink(LinkInterface* link);
 
-    /** @brief Get the link for this id */
-    LinkInterface* getLinkForId(int id);
-
     /** @brief Get a list of all links */
     const QList<LinkInterface*> getLinks();
 
@@ -86,15 +83,17 @@ public:
     
     /// @brief Sets the flag to allow new connections to be made
     void setConnectionsAllowed(void) { _connectionsSuspended = false; }
+    
+    /// @brief Deletes the specified link. Will disconnect if connected.
+    void deleteLink(LinkInterface* link);
 
 public slots:
-
+    /// @brief Adds the link to the LinkManager. LinkManager takes ownership of this object. To delete
+    //          it, call LinkManager::deleteLink.
     void add(LinkInterface* link);
+    
     void addProtocol(LinkInterface* link, ProtocolInterface* protocol);
 
-    void removeObj(QObject* obj);
-    bool removeLink(LinkInterface* link);
-
     bool connectAll();
     bool connectLink(LinkInterface* link);
 
@@ -103,11 +102,14 @@ public slots:
 
 signals:
     void newLink(LinkInterface* link);
-    void linkRemoved(LinkInterface* link);
+    void linkDeleted(LinkInterface* link);
     
 private:
     /// @brief All access to LinkManager is through LinkManager::instance
-    LinkManager(QObject* parent = NULL);
+    LinkManager(QObject* parent = NULL, bool registerSingleton = true);
+    
+    // LinkManager unit test is allowed to new LinkManager objects
+    friend class LinkManagerTest;
     
     static LinkManager* _instance;
     

src/comm/SerialLink.cc

@@ -15,7 +15,6 @@
 #include <QSerialPort>
 #include <QSerialPortInfo>
 #include "SerialLink.h"
-#include "LinkManager.h"
 #include "QGC.h"
 #include <MG.h>
 
@@ -72,9 +71,6 @@ SerialLink::SerialLink(QString portname, int baudRate, bool hardwareFlowControl,
     qDebug() << "create SerialLink " << portname << baudRate << hardwareFlowControl
              << parity << dataBits << stopBits;
     qDebug() << "m_portName " << m_portName;
-
-    LinkManager::instance()->add(this);
-    qDebug() << "link added to link manager";
 }
 
 void SerialLink::requestReset()

src/main.cc

@@ -112,8 +112,8 @@ int main(int argc, char *argv[])
     bool quietWindowsAsserts = false;   // Don't let asserts pop dialog boxes
     
     CmdLineOpt_t rgCmdLineOptions[] = {
-        { "--unittest",             &runUnitTests },
-        { "--no-windows-assert-ui", &quietWindowsAsserts },
+        { "--unittest",             &runUnitTests,          QString() },
+        { "--no-windows-assert-ui", &quietWindowsAsserts,   QString() },
         // Add additional command line option flags here
     };
     
@@ -140,7 +140,7 @@ int main(int argc, char *argv[])
         }
         
         // Run the test
-        int failures = AutoTest::run(argc-1, argv);
+        int failures = AutoTest::run(argc-1, argv, rgCmdLineOptions[0].optionArg);
         if (failures == 0)
         {
             qDebug() << "ALL TESTS PASSED";

src/qgcunittest/AutoTest.h

@@ -49,14 +49,16 @@ namespace AutoTest
 	}
     }
 
-    inline int run(int argc, char *argv[])
+    inline int run(int argc, char *argv[], QString& singleTest)
     { 
 	int ret = 0;
 	foreach (QObject* test, testList())
 	{
-        qgcApp()->destroySingletonsForUnitTest();
-        qgcApp()->createSingletonsForUnitTest();
-        ret += QTest::qExec(test, argc, argv);
+        if (singleTest.isEmpty() || singleTest == test->objectName()) {
+            qgcApp()->destroySingletonsForUnitTest();
+            qgcApp()->createSingletonsForUnitTest();
+            ret += QTest::qExec(test, argc, argv);
+        }
 	}
 	
 	return ret;

src/qgcunittest/LinkManagerTest.cc

+/*=====================================================================
+ 
+ QGroundControl Open Source Ground Control Station
+ 
+ (c) 2009 - 2014 QGROUNDCONTROL PROJECT <http://www.qgroundcontrol.org>
+ 
+ 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 <http://www.gnu.org/licenses/>.
+ 
+ ======================================================================*/
+
+/// @file
+///     @brief LinkManager Unit Test
+///
+///     @author Don Gagne <don@thegagnes.com>
+
+#include "LinkManagerTest.h"
+#include "MockLink.h"
+
+LinkManagerTest::LinkManagerTest(void) :
+    _linkMgr(NULL),
+    _multiSpy(NULL)
+{
+    
+}
+
+void LinkManagerTest::init(void)
+{
+    Q_ASSERT(_linkMgr == NULL);
+    Q_ASSERT(_multiSpy == NULL);
+    
+    _linkMgr = new LinkManager(NULL /* no parent */, false /* don't register singleton */);
+    Q_CHECK_PTR(_linkMgr);
+    
+    _rgSignals[newLinkSignalIndex] = SIGNAL(newLink(LinkInterface*));
+    _rgSignals[linkDeletedSignalIndex] = SIGNAL(linkDeleted(LinkInterface*));
+    
+    _multiSpy = new MultiSignalSpy();
+    QCOMPARE(_multiSpy->init(_linkMgr, _rgSignals, _cSignals), true);
+}
+
+void LinkManagerTest::cleanup(void)
+{
+    Q_ASSERT(_linkMgr);
+    Q_ASSERT(_multiSpy);
+    
+    delete _linkMgr;
+    delete _multiSpy;
+    
+    _linkMgr = NULL;
+    _multiSpy = NULL;
+}
+
+
+void LinkManagerTest::_instance_test(void)
+{
+    LinkManager *linkManager = new LinkManager(NULL /* no parent */, false /* don't register singleton */);
+    
+    // If the flag to not register singleton is not working this will cause QGCApplication to crash on
+    // desctrucion since it will try to de-reference a deleted singleton.
+    delete linkManager;
+}
+
+void LinkManagerTest::_add_test(void)
+{
+    Q_ASSERT(_linkMgr);
+    Q_ASSERT(_linkMgr->getLinks().count() == 0);
+    
+    MockLink* link = new MockLink();
+    _linkMgr->add(link);
+    
+    QList<LinkInterface*> links = _linkMgr->getLinks();
+    QCOMPARE(links.count(), 1);
+    QCOMPARE(dynamic_cast<MockLink*>(links[0]), link);
+}
+
+void LinkManagerTest::_delete_test(void)
+{
+    Q_ASSERT(_linkMgr);
+    Q_ASSERT(_linkMgr->getLinks().count() == 0);
+    
+    MockLink* link = new MockLink();
+    _linkMgr->add(link);
+    _linkMgr->deleteLink(link);
+    
+    QCOMPARE(_linkMgr->getLinks().count(), 0);
+}
+
+void LinkManagerTest::_addSignals_test(void)
+{
+    Q_ASSERT(_linkMgr);
+    Q_ASSERT(_linkMgr->getLinks().count() == 0);
+    Q_ASSERT(_multiSpy->checkNoSignals() == true);
+    
+    MockLink* link = new MockLink();
+    _linkMgr->add(link);
+    
+    QCOMPARE(_multiSpy->checkOnlySignalByMask(newLinkSignalMask), true);
+    QSignalSpy* spy = _multiSpy->getSpyByIndex(newLinkSignalIndex);
+    
+    // Check signal argument
+    QList<QVariant> signalArgs = spy->takeFirst();
+    QCOMPARE(signalArgs.count(), 1);
+    QObject* object = qvariant_cast<QObject *>(signalArgs[0]);
+    QVERIFY(object != NULL);
+    MockLink* signalLink = qobject_cast<MockLink*>(object);
+    QCOMPARE(signalLink, link);
+}
+
+void LinkManagerTest::_deleteSignals_test(void)
+{
+    Q_ASSERT(_linkMgr);
+    Q_ASSERT(_linkMgr->getLinks().count() == 0);
+    Q_ASSERT(_multiSpy->checkNoSignals() == true);
+    
+    MockLink* link = new MockLink();
+    _linkMgr->add(link);
+    _multiSpy->clearAllSignals();
+    
+    _linkMgr->deleteLink(link);
+    
+    QCOMPARE(_multiSpy->checkOnlySignalByMask(linkDeletedSignalMask), true);
+    QSignalSpy* spy = _multiSpy->getSpyByIndex(linkDeletedSignalIndex);
+    
+    // Best we can do is check the argument count. We can't check the contents of signal argument
+    // because the object has been deleted and any access to the variant to try to cast it back
+    // to MockLink* will cause the link to be de-referenced and hence crash.
+    QList<QVariant> signalArgs = spy->takeFirst();
+    QCOMPARE(signalArgs.count(), 1);
+}

src/qgcunittest/LinkManagerTest.h

+/*=====================================================================
+ 
+ QGroundControl Open Source Ground Control Station
+ 
+ (c) 2009 - 2014 QGROUNDCONTROL PROJECT <http://www.qgroundcontrol.org>
+ 
+ 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 <http://www.gnu.org/licenses/>.
+ 
+ ======================================================================*/
+
+#ifndef UASUNITTEST_H
+#define UASUNITTEST_H
+
+#include <QObject>
+#include <QtTest>
+
+#include "AutoTest.h"
+#include "LinkManager.h"
+#include "MultiSignalSpy.h"
+
+/// @file
+///     @brief LinkManager Unit Test
+///
+///     @author Don Gagne <don@thegagnes.com>
+
+class LinkManagerTest : public QObject
+{
+    Q_OBJECT
+    
+public:
+    LinkManagerTest(void);
+    
+private slots:
+    void init(void);
+    void cleanup(void);
+    
+    void _instance_test(void);
+    void _add_test(void);
+    void _delete_test(void);
+    void _addSignals_test(void);
+    void _deleteSignals_test(void);
+    
+private:
+    enum {
+        newLinkSignalIndex = 0,
+        linkDeletedSignalIndex,
+        maxSignalIndex
+    };
+    
+    enum {
+        newLinkSignalMask =     1 << newLinkSignalIndex,
+        linkDeletedSignalMask = 1 << linkDeletedSignalIndex,
+    };
+
+    LinkManager*        _linkMgr;
+    MultiSignalSpy*     _multiSpy;
+    static const size_t _cSignals = maxSignalIndex;
+    const char*         _rgSignals[_cSignals];
+
+};
+
+DECLARE_TEST(LinkManagerTest)
+
+#endif

src/qgcunittest/MockLink.cc

@@ -27,8 +27,6 @@
 #include <QDebug>
 #include <QFile>
 
-#include "LinkManager.h"
-
 #include <string.h>
 
 /// @file
@@ -53,7 +51,6 @@ MockLink::MockLink(void) :
     moveToThread(this);
     _loadParams();
     QObject::connect(this, &MockLink::_incomingBytes, this, &MockLink::_handleIncomingBytes);
-    LinkManager::instance()->add(this);
 }
 
 MockLink::~MockLink(void)

src/qgcunittest/UASUnitTest.cc

@@ -298,121 +298,3 @@ void UASUnitTest::signalWayPoint_test()
     uas = NULL;
     delete wp2;
 }
-
-void UASUnitTest::signalUASLink_test()
-{
-    QSignalSpy spy(uas, SIGNAL(modeChanged(int,QString,QString)));
-    uas->setMode(2, 0);
-    QCOMPARE(spy.count(), 0);// not solve for UAS not receiving message from UAS
-
-    QSignalSpy spyS(LinkManager::instance(), SIGNAL(newLink(LinkInterface*)));
-    SerialLink* link = new SerialLink();
-
-    LinkManager::instance()->add(link);
-    LinkManager::instance()->addProtocol(link, mav);
-    QCOMPARE(spyS.count(), 1);
-
-    LinkManager::instance()->add(link);
-    LinkManager::instance()->addProtocol(link, mav);
-    QCOMPARE(spyS.count(), 1);// not add SerialLink, exist in list
-
-    SerialLink* link2 = new SerialLink();
-    LinkManager::instance()->add(link2);
-    LinkManager::instance()->addProtocol(link2, mav);
-    QCOMPARE(spyS.count(), 2);// add SerialLink, not exist in list
-
-    QList<LinkInterface*> links = LinkManager::instance()->getLinks();
-    foreach(LinkInterface* link, links)
-    {
-        qDebug()<< link->getName();
-        qDebug()<< QString::number(link->getId());
-        QVERIFY(link != NULL);
-        uas->addLink(link);
-    }
-
-    SerialLink* ff = static_cast<SerialLink*>(uas->getLinks()->at(0));
-
-    QCOMPARE(ff->isConnected(), false);
-
-    QCOMPARE(ff->isRunning(), false);
-
-    QCOMPARE(ff->isFinished(), false);
-
-    QCOMPARE(links.count(), uas->getLinks()->count());
-    QCOMPARE(uas->getLinks()->count(), 2);
-
-    LinkInterface* ff99 = static_cast<LinkInterface*>(links.at(1));
-    LinkManager::instance()->removeLink(ff99);
-    delete link2;
-
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 1);
-    QCOMPARE(uas->getLinks()->count(), 1);
-
-    QCOMPARE(static_cast<LinkInterface*>(LinkManager::instance()->getLinks().at(0))->getId(),
-             static_cast<LinkInterface*>(uas->getLinks()->at(0))->getId());
-
-    SerialLink* link3 = new SerialLink();
-    LinkManager::instance()->add(link3);
-    LinkManager::instance()->addProtocol(link3, mav);
-    QCOMPARE(spyS.count(), 3);
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 2);
-
-    //all the links in LinkManager must be deleted because LinkManager::instance
-    //is static. 
-    LinkManager::instance()->removeLink(link3);
-    delete link3;
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 1);
-    LinkManager::instance()->removeLink(link);
-    delete link;
-
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 0);
-}
-
-void UASUnitTest::signalIdUASLink_test()
-{
-
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 0);
-    SerialLink* myLink = new SerialLink();
-    myLink->setPortName("COM 17");
-    LinkManager::instance()->add(myLink);
-    LinkManager::instance()->addProtocol(myLink, mav);
-
-    SerialLink* myLink2 = new SerialLink();
-    myLink2->setPortName("COM 18");
-    LinkManager::instance()->add(myLink2);
-    LinkManager::instance()->addProtocol(myLink2, mav);
-
-    SerialLink* myLink3 = new SerialLink();
-    myLink3->setPortName("COM 19");
-    LinkManager::instance()->add(myLink3);
-    LinkManager::instance()->addProtocol(myLink3, mav);
-
-    SerialLink* myLink4 = new SerialLink();
-    myLink4->setPortName("COM 20");
-    LinkManager::instance()->add(myLink4);
-    LinkManager::instance()->addProtocol(myLink4, mav);
-
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 4);
-
-    QList<LinkInterface*> links = LinkManager::instance()->getLinks();
-
-    LinkInterface* a = static_cast<LinkInterface*>(links.at(0));
-    LinkInterface* b = static_cast<LinkInterface*>(links.at(1));
-    LinkInterface* c = static_cast<LinkInterface*>(links.at(2));
-    LinkInterface* d = static_cast<LinkInterface*>(links.at(3));
-    QCOMPARE(a->getName(), QString("COM 17"));
-    QCOMPARE(b->getName(), QString("COM 18"));
-    QCOMPARE(c->getName(), QString("COM 19"));
-    QCOMPARE(d->getName(), QString("COM 20"));
-
-    LinkManager::instance()->removeLink(myLink4);
-    delete myLink4;
-    LinkManager::instance()->removeLink(myLink3);
-    delete myLink3;
-    LinkManager::instance()->removeLink(myLink2);
-    delete myLink2;
-    LinkManager::instance()->removeLink(myLink);
-    delete myLink;
-
-    QCOMPARE(LinkManager::instance()->getLinks().count(), 0);
-}

src/qgcunittest/UASUnitTest.h

@@ -54,8 +54,6 @@ private slots:
   void getWaypointList_test();
   void signalWayPoint_test();
   void getWaypoint_test();
-  void signalUASLink_test();
-  void signalIdUASLink_test();
 };
 
 DECLARE_TEST(UASUnitTest)

src/uas/UAS.cc

@@ -253,7 +253,6 @@ UAS::~UAS()
     writeSettings();
 
     _thread->quit();
-    _thread->wait();
 
     delete links;
     delete simulation;

src/ui/CommConfigurationWindow.cc

@@ -376,10 +376,7 @@ void CommConfigurationWindow::remove()
     action=NULL;
 
     if(link) {
-        LinkManager::instance()->removeLink(link); //remove link from LinkManager list
-        link->disconnect(); //disconnect port, and also calls terminate() to stop the thread
-        if (link->isRunning()) link->terminate(); // terminate() the serial thread just in case it is still running
-        link->wait(); // wait() until thread is stoped before deleting
+        LinkManager::instance()->disconnectLink(link);  // disconnect connection
         link->deleteLater();
     }
     link=NULL;

src/ui/DebugConsole.cc

@@ -183,7 +183,7 @@ void DebugConsole::addLink(LinkInterface* link)
 
     // Register for name changes
     connect(link, SIGNAL(nameChanged(QString)), this, SLOT(updateLinkName(QString)), Qt::UniqueConnection);
-    connect(link, SIGNAL(deleteLink(LinkInterface* const)), this, SLOT(removeLink(LinkInterface* const)), Qt::UniqueConnection);
+    connect(link, SIGNAL(linkDeleted(LinkInterface* const)), this, SLOT(removeLink(LinkInterface* const)), Qt::UniqueConnection);
 }
 
 void DebugConsole::removeLink(LinkInterface* const linkInterface)

src/ui/QGCMAVLinkLogPlayer.cc

@@ -327,11 +327,8 @@ bool QGCMAVLinkLogPlayer::loadLogFile(const QString& file)
 
     // If there's an existing MAVLinkSimulationLink() being used for an old file,
     // we replace it.
-    if (logLink)
-    {
-        LinkManager::instance()->disconnectLink(logLink);
-        LinkManager::instance()->removeLink(logLink);
-        logLink->deleteLater();
+    if (logLink) {
+        LinkManager::instance()->deleteLink(logLink);
     }
     logLink = new MAVLinkSimulationLink("");
 

src/ui/QGCToolBar.cc

@@ -219,7 +219,7 @@ void QGCToolBar::createUI()
     }
 
     connect(LinkManager::instance(), SIGNAL(newLink(LinkInterface*)), this, SLOT(addLink(LinkInterface*)));
-    connect(LinkManager::instance(), SIGNAL(linkRemoved(LinkInterface*)), this, SLOT(removeLink(LinkInterface*)));
+    connect(LinkManager::instance(), SIGNAL(linkDeleted(LinkInterface*)), this, SLOT(removeLink(LinkInterface*)));
 
     loadSettings();
 

src/ui/uas/UASListWidget.cc

@@ -63,7 +63,7 @@ UASListWidget::UASListWidget(QWidget *parent) : QWidget(parent),
 
     this->setVisible(false);
 
-    connect(LinkManager::instance(), SIGNAL(linkRemoved(LinkInterface*)), this, SLOT(removeLink(LinkInterface*)));
+    connect(LinkManager::instance(), SIGNAL(linkDeleted(LinkInterface*)), this, SLOT(removeLink(LinkInterface*)));
 
     // Listen for when UASes are added or removed. This does not manage the UASView
     // widgets that are displayed within this widget.