QSerialPort doesn't work in QThread

Alien

Hello,
I want to use QSerialPort inside a QThread unfortunately serial port inside a thread doesn't work the code compile with no error and application run and write function of QSerialPort return number of bytes it sends but the receiver can't get anything if I put QSerialPort out of the thread everything work perfect.

I use QT SDK 5.11 ,QT Creator 4.6.1 GCC 5.3.1 in Ubuntu 18.04 LTS and virtual port socat
the code is :
CThread.h

#ifndef CTHREAD_H
#define CTHREAD_H

#include <QObject>
#include <QThread>
#include <QtSerialPort/QtSerialPort>

class CThread : public QThread
{
    Q_OBJECT
private :
    bool m_cancel;
    static const QString PORT_NAME;
    static const int BAUD_RATE;

    QSerialPort *m_serialPort;

    void init();
public:
    explicit CThread();
    virtual ~CThread() override;


    // QThread interface
protected:
    void run() override;
};

#endif // CTHREAD_H

CThread.cpp

#include "cthread.h"
#include <QDebug>




const QString CThread::PORT_NAME="/dev/pts/1";
const int CThread::BAUD_RATE = 38400;



void CThread::init()
{
    m_cancel=false;
    m_serialPort = new QSerialPort();
    m_serialPort->setPortName(PORT_NAME);
    m_serialPort->setBaudRate(BAUD_RATE);
    m_serialPort->open(QIODevice::ReadWrite);
}

CThread::CThread()
{

}

CThread::~CThread()
{
    qDebug()<<"~CThread";
    m_cancel = true;
//    wait(500);
    if(m_serialPort->isOpen())
        m_serialPort->close();
    if(m_serialPort!=nullptr)
    {
        delete m_serialPort;
    }
    m_serialPort = nullptr;
}

void CThread::run()
{
    int i=10;
    init();
    char data[] = {0x00,0x01,0x02,0x03,0x04,0x05};
    if(m_serialPort->isOpen())
    while (i)
    {
        qDebug()<<m_serialPort->write(data,6);
        i--;

    }
    qDebug()<<"end of thread";
}

main.cpp

#include <QCoreApplication>
#include <QObject>
#include <QThread>
#include "cthread.h"

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    CThread thread;
    QObject::connect(&thread,&QThread::finished,&thread,&QObject::deleteLater);
    thread.start();

    return a.exec();
}

I really appreciate your help

Regards,

JonB

@Alien said in QSerialPort doesn't work in QThread:

but the receiver can't get anything

I don't see where your code attempts to do anything to do with the serial port in the main thread (not the CThread) --- other than deleteLater() --- if that is what your problem is? So I conclude the code as posted works fine?

J.Hilk

@Alien

please read through this post

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

and redo your threading. Overwriting run is for your use case inadequate/not recommended.

aha_1980

@Alien

~~Your main problem is that you create the serial port in one thread, but use it in another one.~~

~~Reason: CThread::init() is run in the calling thread, but CThread::run() runs in a new thread. It should already work if you create the serial port object within run()~~

Regards

J.Hilk

@aha_1980
actually, init is called inside of run, 2nd line of the run function.

aha_1980

@J.Hilk good catch, init() is private. would have been too easy...

J.Hilk

I would try and move this

const QString CThread::PORT_NAME="/dev/pts/1";
const int CThread::BAUD_RATE = 38400;

either in the header or in the constructor of the class, IIRC "Global" variables inside the cpp part of the class may be initiated after the functions are already called!?

Alien

Guys thank you for replying me,
I searched around this problem before posting the question I know that run function will execute in another thread that's why I put init function inside the run function @aha_1980 is it wrong to put init function in run or is it wrong to have it private ?
@J-Hilk "Overwriting run is for your use case inadequate/not recommended." what is recommended?
I can't get your point could you please give me more information? (Also I change PORT_NAME and BAUD_RATE still the application doesnt' work)
@JonB does it work for you? could you please put the code here ?

J.Hilk

@Alien

I would imagen, that your QSerialPort will not only send data once, but your goal is a communication between your app and your hardware.

Howver by overwriting Run, you will end up implementing your very own SIGNAL/SLOT handling, data passing between threads etc.

By going with Worker-class QThread::moveToThread, you end up passing a lot of work to the framework.

Also it has the nice benefit of, you can create your QSerialport in its own class non threaded, make sure, everything works as you want it to and move it later into its own Thread without nearly any changes insde of your code.

Alien

@J-Hilk I've read how-to-really-truly-use-qthreads-the-full-explanation I can't understand why she wrote
"
The main thing to keep in mind when using a QThread is that it’s not a thread. It’s a wrapper around a thread object. This wrapper provides the signals, slots and methods to easily use the thread object within a Qt project. This should immediately show why the recommended way of using QThreads in the documentation, namely to sub-class it and implement your own run() function, is very wrong.
"
If overriding run function is very wrong why QT documentation examples introduce this way even in QT 5.11 ? Also I use Maya Posch approach still my app can't use serial port in a thread

VRonin

You are probably just deleting the serial port too early. The easy but ugly way to solve would be adding m_serialPort->waitForBytesWritten after m_serialPort->write.

Your CThread destructor is a race condition. It is executed in the main thread and you are calling methods on m_serialPort and even worse deleting it directly.

I really recommend you follow @J-Hilk 's advice https://forum.qt.io/topic/91681/qserialport-doesn-t-work-in-qthread/4

Alien

@VRonin said in QSerialPort doesn't work in QThread:

m_serialPort->waitForBytesWritten

~~what do you recommend me to use qserial in qthread?~~

J.Hilk

@Alien said in QSerialPort doesn't work in QThread:

If overriding run function is very wrong why QT documentation examples introduce this way even in QT 5.11 ? Also I use Maya Posch approach still my app can't use serial port in a thread

it doesn't,
Check the details from the QThread docu:
http://doc.qt.io/qt-5/qthread.html#details

cleary a worker approach, to overwrite QThread is the 2nd example, and in my opinion should be removed. But thats far from my call and probably still there for legacy reasons.

This was change to the docs was made with Qt5.9 iirc.

Buckwheat

Hello and welcome @Alien ,

I am glad your read May's blog and redid your threading. The worker thread approach is a best way to setup the threads. With your serial port in the worker object you will only need QThread to run the event queue which is VERY important to QSerialPort event handling. As you have already found out, overriding "run" will block event queue processing.

One thing I did not see is where you are connecting to the readyRead signal. This will allow your worker object to read the data from the serialport as it arrives.

If you are still having issues, post your new threading model so we can see where the disconnect is happening and our eyes can give you insight.

Alien

Thanks for all of your help

If my undrestanding is right its a bad idea to use aserialport inside a thread while qserialport is async itself also when I implement qthread's run function actually I disrupt the event dispatcher of that thread so signal and slot can't work properly for qserialport in thread due to the infinite while inside the run.

If my undrestanig is still wrong please help me and make me aware of that
Yours,

Alien

@Buckwheat how to run event queue inside my thread?
A snippet code would be helpfull

VRonin

call QThread::exec() inside run().
I still suggest you convert to the worker object design though

Buckwheat

@Alien,

QThread has an event queue and will run freely if you do NOT override run! Using the technique outlined by May will allow the thread's event queue to run. You just need to connect your signals for the serial port and things will run freely. Here is a sample for starting a GNSS receiver in a thread (using May's method):

*** NOTE: This is really pseudo code as it comes from our proprietary code ***

   mpQ_GnssRcvrThread = new QThread;

   // The receiver code is implemented in an event-based fashion as a QObject
   // but will be owned by a dedicated thread, to ensure independence from
   // risk of blocking operations being used by other parts of the system.
   mpQ_GnssRcvr->moveToThread (mpQ_GnssRcvrThread);

   // Trigger worker start upon thread start
   connect (mpQ_GnssRcvrThread, &QThread::started, mpQ_GnssRcvr, &GNSSReceiver::start);

   // Delete
   connect (mpQ_GnssRcvrThread, &QThread::finished, mpQ_GnssRcvr, &GNSSReceiver::deleteLater);
   connect (mpQ_GnssRcvrThread, &QThread::finished, mpQ_GnssRcvrThread, QThread::deleteLater);

Now in the GNSSReceiver object (derived from QObject):
   mQ_SerialPort= new QSerialPort (this);
   mQ_SerialPort->open (...);

   // Connect data handling
   connect (mQ_SerialPort, &QSerialPort::readyRead, this, &handleCommsChan);

In handleCommsChan:

QByteArray Q_Data = mQ_SerialPort->readAll ();

...
do stuff
...

The GNSSReceiver object runs inside of the QThread event loop. Writing to the serial port can be done asynchronously. The only Qt object that has issues with worker objects is QTimer. They need to be created in the thread space and assert errors if you try to start/stop them outside of the thread.

I use this technique for serial port, network, and serial bus (CAN) interfacing (asynchronous objects). I am liking QFuture for worker threads (functions to do something) currently.

@VRonin, as you know, if you get rid of run (or at least use run to initialize your thread and call QThread::exec on exiting) the thread works. I prefer to treat the thread as a container and just call object->moveToThread (new QThread) myself for most things.