Permanent QThread

DavidPL

Hi there!!. I'm developing a GUI in QT. I need some code to run in a dedicated thread and this code must run indefinetely in loop until a condition is met (the user clicks a button in the mainwindow). I create a QObject class with the mentioned code contained in a function of it and in the mainwindow create an object of Qthread and the mentioned class. Then I move the object to the thread but i only get one run over the code and then exits. How can i set a loop ??
Thanks in advance.

ambershark

@DavidPL You can just use a repeating QTimer that kicks off a function that runs your code every X amount of milliseconds. Keep in mind the tighter your loop/timer kick off the more CPU you will use. If you enter a tight loop i.e. for (;;) { } you will end up with 100% cpu at least on one of your cores.

Then you can also handle an quit signal on the thread. That way it will exit nicely when you ask it to by sending the signal.

VRonin

https://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/

DavidPL

@ambershark First of all thanks for your answer. I understand what you said but there is one thing that is giving me a serious headache jaja. The problem is that the first time i tried to implement the QObject in the QThread I forgot to define the connections to shut down the thread and the object when finishing.->connect(&worker, SIGNAL("Finished()"), &Workerthread, SLOT(quit());
In that situation the code was runned in loop and i thought that the previous connection forced the thread to stop running the worker's object function. Now i comment that line but i only get one run over the worker function. So i have 3 wuestions:

How was i able to get the thread running over the funtion multiple times?.
What does the quit() thread's slot?.
QTimer is the only way, I mean if possible i would prefer other solutions.

Sorry for bodering you but i'm newbie :).

SGaist

Hi,

There are several ways:

subclass QThread and reimplement the run method using a while loop with an exit variable
use a QTimer to call your method at regular interval
Use a QTimer with 0 for timeout to queue a call to your method again.

It stops the event loop started by the default implementation of QThread
No it's not, take a look at #1

DavidPL

@SGaist Ok I implemented a QTimer object but then i realized that i don't see the advantages of using a QThread. I think the main purpose of implementing a thread is relaxing the tasks that the mainthread (the one that controls the main window) is taking care of. And by implementing the QTimer you are overloading the mainthread as it is executing the timer. On the other hand, as stated in the link https://mayaposch.wordpress.com/2011/11/01/how-to-really-truly-use-qthreads-the-full-explanation/ passed by @VRonin subclassing QThread is not a good implementation, isn't it?.

By the way, is there any possibility of checking the current active threads in Windows like, for example, searching in the task manager?.

Thanks for your help.

SGaist

It depends on how many times per second you trigger your timer and what it does.

Whether subclassing QThread is bad ? It depends all on what that thread is supposed to do. You also have to know what you are doing regarding signals and slots handling in that use case.

I don't remember if there's such a possibility.

ambershark

@DavidPL I believe you can find thread information in the newer task managers (win 10+ probably). I can't be 100% sure though since I'm really not a big windows user.

You can run resource monitor and it should have that type of information too.

And finally, the easiest way is just in a debugger.

Subclassing QThread sounds like something you're looking for in this example. Like @SGaist said above it isn't necessarily bad, it depends on what you want to do. If you are concerned with the event loop dealing with your QTimer then you are doing enough in your thread that you probably want to subclass. Keep in mind doing that much in your thread without sleep/yield is going to eat up a cpu. Most people will not run software that eats up a cpu while it is supposed to be idle.

If you give us more details of what your thread is doing we can give you a good way to handle it, or some advice if it isn't something you should be doing. :)

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.