Update threading in Rotator Controller for #1346

plugins/feature/gs232controller/gs232controller.cpp

@@ -49,12 +49,12 @@ const char* const GS232Controller::m_featureIdURI = "sdrangel.feature.gs232contr
 const char* const GS232Controller::m_featureId = "GS232Controller";
 
 GS232Controller::GS232Controller(WebAPIAdapterInterface *webAPIAdapterInterface) :
-    Feature(m_featureIdURI, webAPIAdapterInterface)
+    Feature(m_featureIdURI, webAPIAdapterInterface),
+    m_thread(nullptr),
+    m_worker(nullptr)
 {
     qDebug("GS232Controller::GS232Controller: webAPIAdapterInterface: %p", webAPIAdapterInterface);
     setObjectName(m_featureId);
-    m_worker = new GS232ControllerWorker();
-    m_worker->moveToThread(&m_thread);
     m_state = StIdle;
     m_errorMessage = "GS232Controller error";
     m_selectedPipe = nullptr;
@@ -124,22 +124,22 @@ GS232Controller::~GS232Controller()
         &GS232Controller::networkManagerFinished
     );
     delete m_networkManager;
-    if (m_worker->isRunning()) {
     stop();
-    }
-
-    delete m_worker;
 }
 
 void GS232Controller::start()
 {
     qDebug("GS232Controller::start");
 
-    m_worker->reset();
+    m_thread = new QThread(this);
+    m_worker = new GS232ControllerWorker();
+    m_worker->moveToThread(m_thread);
+    QObject::connect(m_thread, &QThread::started, m_worker, &GS232ControllerWorker::startWork);
+    QObject::connect(m_thread, &QThread::finished, m_worker, &QObject::deleteLater);
+    QObject::connect(m_thread, &QThread::finished, m_thread, &QThread::deleteLater);
     m_worker->setMessageQueueToFeature(getInputMessageQueue());
-    bool ok = m_worker->startWork();
+    m_thread->start();
-    m_state = ok ? StRunning : StError;
+    m_state = StRunning;
-    m_thread.start();
 
     GS232ControllerWorker::MsgConfigureGS232ControllerWorker *msg = GS232ControllerWorker::MsgConfigureGS232ControllerWorker::create(m_settings, true);
     m_worker->getInputMessageQueue()->push(msg);
@@ -150,10 +150,14 @@ void GS232Controller::start()
 void GS232Controller::stop()
 {
     qDebug("GS232Controller::stop");
-    m_worker->stopWork();
     m_state = StIdle;
-    m_thread.quit();
+    if (m_thread)
-    m_thread.wait();
+    {
+        m_thread->quit();
+        m_thread->wait();
+        m_thread = nullptr;
+        m_worker = nullptr;
+    }
 }
 
 bool GS232Controller::handleMessage(const Message& cmd)
@@ -404,7 +408,9 @@ void GS232Controller::applySettings(const GS232ControllerSettings& settings, boo
     GS232ControllerWorker::MsgConfigureGS232ControllerWorker *msg = GS232ControllerWorker::MsgConfigureGS232ControllerWorker::create(
         settings, force
     );
+    if (m_worker) {
         m_worker->getInputMessageQueue()->push(msg);
+    }
 
     if (settings.m_useReverseAPI)
     {

plugins/feature/gs232controller/gs232controller.h

@@ -169,7 +169,7 @@ public:
     static const char* const m_featureId;
 
 private:
-    QThread m_thread;
+    QThread *m_thread;
     GS232ControllerWorker *m_worker;
     GS232ControllerSettings m_settings;
     QHash<QObject*, GS232ControllerSettings::AvailableChannelOrFeature> m_availableChannelOrFeatures;

plugins/feature/gs232controller/gs232controllerworker.cpp

@@ -32,7 +32,6 @@ MESSAGE_CLASS_DEFINITION(GS232ControllerReport::MsgReportAzAl, Message)
 
 GS232ControllerWorker::GS232ControllerWorker() :
     m_msgQueueToFeature(nullptr),
-    m_running(false),
     m_device(nullptr),
     m_serialPort(this),
     m_socket(this),
@@ -48,33 +47,15 @@ GS232ControllerWorker::GS232ControllerWorker() :
 
 GS232ControllerWorker::~GS232ControllerWorker()
 {
+    qDebug() << "GS232ControllerWorker::~GS232ControllerWorker";
     m_inputMessageQueue.clear();
 }
 
-void GS232ControllerWorker::reset()
+void GS232ControllerWorker::startWork()
 {
-    QMutexLocker mutexLocker(&m_mutex);
+    qDebug() << "GS232ControllerWorker::startWork";
-    m_inputMessageQueue.clear();
-    m_lastAzimuth = -1.0f;
-    m_lastElevation = -1.0f;
-    m_spidSetOutstanding = false;
-    m_spidSetSent = false;
-    m_spidStatusSent = false;
-}
-
-bool GS232ControllerWorker::startWork()
-{
-    QMutexLocker mutexLocker(&m_mutex);
     connect(&m_inputMessageQueue, SIGNAL(messageEnqueued()), this, SLOT(handleInputMessages()));
-    connect(thread(), SIGNAL(started()), this, SLOT(started()));
+    connect(thread(), SIGNAL(finished()), this, SLOT(stopWork()));
-    connect(thread(), SIGNAL(finished()), this, SLOT(finished()));
-    m_running = true;
-    return m_running;
-}
-
-// startWork() is called from main thread. Serial ports on Linux need to be opened/closed on worker thread
-void GS232ControllerWorker::started()
-{
     connect(&m_serialPort, &QSerialPort::readyRead, this, &GS232ControllerWorker::readData);
     connect(&m_socket, &QTcpSocket::readyRead, this, &GS232ControllerWorker::readData);
     if (m_settings.m_connection == GS232ControllerSettings::TCP) {
@@ -84,18 +65,14 @@ void GS232ControllerWorker::started()
     }
     connect(&m_pollTimer, SIGNAL(timeout()), this, SLOT(update()));
     m_pollTimer.start(1000);
-    disconnect(thread(), SIGNAL(started()), this, SLOT(started()));
+    // Handle any messages already on the queue
+    handleInputMessages();
 }
 
 void GS232ControllerWorker::stopWork()
 {
-    QMutexLocker mutexLocker(&m_mutex);
+    qDebug() << "GS232ControllerWorker::stopWork";
     disconnect(&m_inputMessageQueue, SIGNAL(messageEnqueued()), this, SLOT(handleInputMessages()));
-}
-
-void GS232ControllerWorker::finished()
-{
-    // Close serial port as USB/controller activity can create RFI
     if (m_device && m_device->isOpen()) {
         m_device->close();
     }
@@ -103,8 +80,6 @@ void GS232ControllerWorker::finished()
     disconnect(&m_socket, &QTcpSocket::readyRead, this, &GS232ControllerWorker::readData);
     m_pollTimer.stop();
     disconnect(&m_pollTimer, SIGNAL(timeout()), this, SLOT(update()));
-    m_running = false;
-    disconnect(thread(), SIGNAL(finished()), this, SLOT(finished()));
 }
 
 void GS232ControllerWorker::handleInputMessages()
@@ -123,7 +98,6 @@ bool GS232ControllerWorker::handleMessage(const Message& cmd)
 {
     if (MsgConfigureGS232ControllerWorker::match(cmd))
     {
-        QMutexLocker mutexLocker(&m_mutex);
         MsgConfigureGS232ControllerWorker& cfg = (MsgConfigureGS232ControllerWorker&) cmd;
 
         applySettings(cfg.getSettings(), cfg.getForce());

plugins/feature/gs232controller/gs232controllerworker.h

@@ -58,8 +58,7 @@ public:
 
     GS232ControllerWorker();
     ~GS232ControllerWorker();
-    void reset();
+    void startWork();
-    bool startWork();
     void stopWork();
     bool isRunning() const { return m_running; }
     MessageQueue *getInputMessageQueue() { return &m_inputMessageQueue; }
@@ -71,7 +70,6 @@ private:
     MessageQueue *m_msgQueueToFeature; //!< Queue to report channel change to main feature object
     GS232ControllerSettings m_settings;
     bool m_running;
-    QRecursiveMutex m_mutex;
     QIODevice *m_device;
     QSerialPort m_serialPort;
     QTcpSocket m_socket;
@@ -95,8 +93,6 @@ private:
     void setAzimuthElevation(float azimuth, float elevation);
 
 private slots:
-    void started();
-    void finished();
     void handleInputMessages();
     void readData();
     void update();