Permanent QThread

[DavidPL]

@ambershark Ok i'll try to explain briefly and clearly what the thread is supposed to do. I'm developing a ground station. In the main window you see a interactive satellite map, state variables of the aircraft like velocity, altitude, etc, and communications options between the ground station and the aircraft. As I don't want to overload the main window and get it frozen, the thread i was talking about is meant to take care of the data transferred between the ground station and the vehicle. The communication is serial and the ground station can receive and send data. I want to check for new upcoming data or send data as fast as I can and for this reason I think about implementing the serial communications in a dedicated thread.

Thank you so much for all the help :).

[ambershark]

@DavidPL Ok so from the sounds of it you don't really need a tight loop in your thread.

If you are using something derived from QIODevice like a QSerialPort for communications then you can just put the QIODevice::readyRead signal on the thread you created with the non-subclassing method showed in that link above.

Then when you want to write, you just send a message to that threaded QObject with the data you want to write.

No timers or loops needed. I mean there is an indirect loop, the QEventLoop, but that is why it's easier to not subclass. Since you then do not have to deal with your own QEventLoop, it is handled for you.

[DavidPL]

@ambershark Sorry it took me so long to answer. I'll try what you've metioned previously.
Thanks :)

[DavidPL]

@ambershark I'm back. I've implemented what we discussed here and it works very well. The only thing that is annoying me is that I'm getting a warning in the debbuger console saying:

"QObject: Cannot create children for a parent that is in a different thread.
(Parent is QSerialPort(0x39905ad8), parent's thread is QThread(0x35c99f60), current thread is QThread(0x399047e8) kernel\qobject.cpp: 771"

I don't understand why is that happening. Also I want to mention that none of the tread's addresses displayed in the debugger correspond with the upper ones.

[mranger90]

Actually, that warning is not benign. You are creating an object whose parent belongs to a different thread. Depending on how things time out, you could get a mysterious crash during, say, deleteLater() if the threads shut down in a particular order.

[jsulm]

@DavidPL Please show your code. In your thread object you should create instances of your variables AFTER moving thread object to the thread.

[ambershark]

Chances are you just need to not parent an object you are passing to your thread. I.e. if you say auto *x = new QObject(this) instead just use auto *x = new QObject(). That way you don't have a parent that is in say your main thread and a child (the object you created) that was moved to another thread. You will then have to deal with your memory for x though as it won't have a parent to clean it up. If you don't delete it when you're done it will leak.

As @mranger90 mentioned though, do not ignore that warning, it is most definitely not benign.

[DavidPL]

@jsulm This is the code of mainwindow.cpp/.h and the object.cpp/.h.
mainwindow.h:

#ifndef MAINWINDOW_H
#define MAINWINDOW_H

#include <QMainWindow>

#include "serialcomm.h"
#include <QSerialPortInfo>
#include <QThread>

namespace Ui {
class MainWindow;
}

class MainWindow : public QMainWindow
{
    Q_OBJECT

public:
    explicit MainWindow(QWidget *parent = 0);
    ~MainWindow();

signals:
    void setup_serialport(const QString, int);
    void close_serialport();

public slots:
    void commtype_changed();
    void showresponse(const QString &s);

private slots:
    void on_pushButton_open_port_clicked();

private:
    Ui::MainWindow *ui;
    QStringList comm_types;
    QThread *thread_serial = new QThread;
    SerialComm *serialcomm = new SerialComm;
};

#endif // MAINWINDOW_H

mainwindow.cpp:

#include "mainwindow.h"
#include "ui_mainwindow.h"

MainWindow::MainWindow(QWidget *parent) :
    QMainWindow(parent),
    ui(new Ui::MainWindow)
{
    ui->setupUi(this);

    ui->map->setSource(QUrl(QStringLiteral("qrc:/qml/mapbox.qml")));

    // Set items for communications types:
    comm_types << "Wire" << "Bluetooth" << "Wifi" << "Radio";
    ui->comboBox_comm_type->addItems(comm_types);

    // Get info about available serial ports in the specified communication type:
    const auto serial_obj = QSerialPortInfo::availablePorts();
    switch(comm_types.indexOf(ui->comboBox_comm_type->currentText())){
    case 0:
        break;
    case 1:
        for(const QSerialPortInfo &serialports_info : serial_obj){
            if(serialports_info.description().indexOf("Bluetooth") != -1){
                ui->comboBox_serialports->addItem(serialports_info.portName());
            }
        }
        break;
    case 2:
        break;
    case 3:
        break;
    }

    // Default baudrate:
    ui->lineEdit_baudrate->setText("115200");

    // Connect all the GUI elements with the corresponding SLOTS:
    //connect(thread_serial, SIGNAL(started()), serialcomm, SLOT());
    connect(this, SIGNAL(setup_serialport(const QString, int)), serialcomm, SLOT(setup_serial(const QString, int)));
    connect(this, SIGNAL(close_serialport()), serialcomm, SLOT(stop_serial()));
    connect(serialcomm, SIGNAL(response(QString)), this, SLOT(showresponse(QString)));
    connect(ui->comboBox_comm_type, SIGNAL(activated(QString)), this, SLOT(commtype_changed()));
    connect(serialcomm, SIGNAL(finished()), thread_serial, SLOT(quit()));
    //connect(serialcomm, SIGNAL(finished()), serialcomm, SLOT(deleteLater()));
    //connect(thread_serial, SIGNAL(finished()), thread_serial, SLOT(deleteLater()));

    // Move the serial communications to a dedicated thread (assign a thread to serial communications in order to avoid the GUI from freezing):
    serialcomm->moveToThread(thread_serial);
}

MainWindow::~MainWindow()
{
    delete ui;
}

void MainWindow::commtype_changed(){
    // Update info about available serial ports in the specified communication type:
    const auto serial_obj = QSerialPortInfo::availablePorts();
    switch(comm_types.indexOf(ui->comboBox_comm_type->currentText())){
    case 0:
        ui->comboBox_serialports->clear();
        for(const QSerialPortInfo &serialports_info : serial_obj){
            ui->comboBox_serialports->addItem(serialports_info.portName());
        }
        break;
    case 1:
        ui->comboBox_serialports->clear();
        for(const QSerialPortInfo &serialports_info : serial_obj){
            if(serialports_info.description().indexOf("Bluetooth") != -1){
                ui->comboBox_serialports->addItem(serialports_info.portName());
            }
        }
        break;
    case 2:
        break;
    case 3:
        break;
    }
}

void MainWindow::showresponse(const QString &s){
    ui->label_edit_last_packreceived->setText(s);
}

void MainWindow::on_pushButton_open_port_clicked()
{
    // Clicking connect initializes the serial comms:
    if(QString::compare(ui->pushButton_open_port->text(), "Connect", Qt::CaseInsensitive) == 0){
        // Reconfiguration with new selected parameters:
        thread_serial->start();
        emit this->setup_serialport(ui->comboBox_serialports->currentText(), ui->lineEdit_baudrate->text().toInt());

        ui->pushButton_open_port->setText("Close");
        ui->bar_comm_status->setValue(100);
    } else{
        ui->pushButton_open_port->setText("Connect");
        ui->bar_comm_status->setValue(0);
        emit close_serialport();
    }
}

serialcomm.h (object created to read serial data):

#ifndef SERIALCOMM_H
#define SERIALCOMM_H

#include <QObject>
#include <QThread>
#include <QMutex>
#include <QSerialPort>
#include <QIODevice>

class SerialComm : public QObject
{
    Q_OBJECT
public:
    explicit SerialComm(QObject *parent = nullptr);

signals:
    void response(const QString &s);
    void finished();

public slots:
    void setup_serial(const QString &portName, int baudrate);
    void run_serial();
    void stop_serial();

private:
    QSerialPort serial;
    QString serial_portname;
    int serial_baudrate;
    int serial_timeout;
    QMutex serial_mutex;
    bool serial_stop = false;
};

#endif // SERIALCOMM_H

serialcomm.cpp:

#include "serialcomm.h"

SerialComm::SerialComm(QObject *parent) : QObject(parent)
{

}

void SerialComm::setup_serial(const QString &portName, int baudrate)
{
    // Serial port is configured and opened with the selected parameters:
    // Values selected in the GUI are passed:
    serial_portname = portName;
    serial_baudrate = baudrate;
    serial_timeout = 3000;

    // Reset stop condition:
    serial_stop = false;

    serial.setPortName(serial_portname);
    serial.setBaudRate(baudrate);

    // Open selected port:
    serial.open(QIODevice::ReadWrite);
    connect(&serial, SIGNAL(readyRead()), this, SLOT(run_serial()));

    // The rest of parameters are the default ones (no parity, 8 databits, 1 stopbit).
}

void SerialComm::run_serial()
{
    //if(!serial_stop) {
    if (!serial.isOpen()) { // Try to open the selected port in Read/write mode.
        //emit error(tr("Can't open %1, error code %2")
        //.arg(m_portName).arg(serial.error()));
        return;
    }

    //if (serial.waitForReadyRead(serial_timeout)) {
        QByteArray responseData = serial.readAll();
        //            while (serial.waitForReadyRead(10))
        //                responseData += serial.readAll();

        const QString response = QString::fromUtf8(responseData);
        emit this->response(response);
    //}
    //}
}

void SerialComm::stop_serial()
{
    disconnect(&serial, SIGNAL(readyRead()), this, SLOT(run_serial()));
    serial.close();
    emit this->finished();
    //serial_stop = true;
}

[ambershark]

Ok so the problem here is because you have a QSerialPort in your class definition (on the stack). This means that it's memory is in the main thread and not the thread you created for your SerialComm object.

Since you create the SerialComm object in your main thread it's memory is allocated in that thread and not in your target thread.

Once you moveToThread on your SerialComm object and it tried to use SerialComm::serial that creates that issue because the memory for it is not in the same thread.

The solution here is to allocate your memory after your thread is started inside a function called in the new thread. So you can connect a signal for thread start to an init type function in your SerialComm class. Then in there setup your serial variable. You need to change the definition to a pointer instead of on the stack and then just new it inside the thread.

That should take care of that warning.

[DavidPL]

@ambershark Ok now it works perfectly :). I thought that the movetoThread function pass all the variables and objects to the new thread. Only for curiosity, is it correct then that I have variables declared on the stack (like serial_baudrate or serial_portname) and use them on the new thread??.

[ambershark]

@DavidPL Sure you can do that. That problem is with things derived from QObject. Those have to be allocated on the same thread where they are referenced.

All your other stuff you can allocate the memory (stack or heap) in whatever thread you want.

Edit: do keep in mind if you write to memory in multiple threads you will crash. Those bugs are hard to find too. Make sure if you write to a piece of memory in multiple threads that any access is protected via a QMutex or some other synchronization object.

[SGaist]

One small adjustment: moveToThread will move the object and all its children so if you give your QSerialPort a parent at construction time (so allocate it on the heap), it should be properly moved when you move SerialComm.

[DavidPL]

@SGaist Oh ok, that's interesting to know. Thanks :).

[DavidPL]

Thank you very much guys, your help was truly useful ;).