QSerialPort discovery in a non GUI QThread?

CJha

Hi, I am writing a program where I have to discover an MCU connected to the PC through a USB port. I am using QSerialPort for that and in the process of discovering I am using bool QSerialPort::waitForReadyRead(int msecs) which is a reimplementation of bool QIODevice::waitForReadyRead(int msecs). Since this function clearly states:

Warning: Calling this function from the main (GUI) thread might cause your user interface to freeze.

I want to implement the QSerialPort connection in a different thread as I have observed the freezing of the GUI if I use this function in the GUI thread. I have tried different approaches and in each approach, I am getting either unreliable output or error.

Approach 1: QObject::moveToThread(QThread *targetThread) with QSerialPort as a member of QObject:

MCU Header

class MCU : public QObject
{
    Q_OBJECT

public:
    explicit MCU();
    ~MCU() override;

signals:
    void deviceFound(QSerialPortInfo portInfo);

public slots:
    void findDevice(quint16 vid, quint16 pid, QString handshake, int handshakeWait, QSerialPort::BaudRate baudRate);

private:
    QSerialPort port;
    QPointer<QThread> myThread{nullptr};
};

MCU Cpp

MCU::MCU()
{
    myThread = new QThread;
    this->moveToThread(myThread);
    myThread->start();
}

MCU::~MCU()
{
    if(!myThread.isNull())
        myThread->quit();
}

void MCU::findDevice(quint16 vid, quint16 pid, QString handshake, int handshakeWait, QSerialPort::BaudRate baudRate)
{
    if(port.isOpen()) {
        port.clear();
        port.close();
    }

    auto portList = QSerialPortInfo::availablePorts();
    if(!portList.isEmpty()) {
        for(auto ii = 0; ii < portList.length(); ++ii) {
            if((portList[ii].vendorIdentifier() == vid) && (portList[ii].productIdentifier() == pid)) {
                if(port.isOpen()) {
                    port.clear();
                    port.close();
                }
                port.setPort(portList[ii]);
                bool portOpen = port.open(QIODevice::ReadWrite);
                if(portOpen) {
                    bool baudSuccess = port.setBaudRate(baudRate, QSerialPort::AllDirections);
                    if(baudSuccess) {
                        port.setDataTerminalReady(true);
                        QString handshakeReceived{""};
                        bool readAvailable = port.waitForReadyRead(handshakeWait);
                        while(readAvailable) {
                            handshakeReceived.append(QString::fromUtf8(port.readAll()));
                            readAvailable = port.waitForReadyRead(handshakeWait);
                        }
                        if(handshakeReceived == handshake) {
                            emit deviceFound(portList[ii]);
                            break;
                        }
                        else {
                            if(port.isOpen()) {
                                port.clear();
                                port.close();
                            }
                        }
                    }
                    else {
                        if(port.isOpen()) {
                            port.clear();
                            port.close();
                        }
                    }
                }
            }
        }
    }
}

Method Reliability: Unreliable method, rarely it finds the connected MCU although the parameters are correct and work when it is a GUI thread.
Warnings: Gives an output warning

QObject: Cannot create children for a parent that is in a different thread.

(Parent is QSerialPort(0xf037eff560), parent's thread is QThread(0x2b741fdbf30), current thread is QThread(0x2b741ff38d0)

Approach 2: QObject::moveToThread(QThread *targetThread) with QSerialPort as a std::unique_ptr member of QObject:

MCU Header

class MCU : public QObject
{
    Q_OBJECT

public:
    explicit MCU();
    ~MCU() override;

signals:
    void deviceFound(QSerialPortInfo portInfo);

public slots:
    void findDevice(quint16 vid, quint16 pid, QString handshake, int handshakeWait, QSerialPort::BaudRate baudRate);

private:
    std::unique_ptr<QSerialPort> port{nullptr};
    QPointer<QThread> myThread{nullptr};
};

MCU Cpp

MCU::MCU()
{
    myThread = new QThread;
    this->moveToThread(myThread);
    myThread->start();
}

MCU::~MCU()
{
    if(port != nullptr) {
        if(port->isOpen()) {
            port->clear();
            port->close();
        }
        port.reset();
    }
    if(!myThread.isNull())
        myThread->quit();
}

void MCU::findDevice(quint16 vid, quint16 pid, QString handshake, int handshakeWait, QSerialPort::BaudRate baudRate)
{
    if(port != nullptr) {
        if(port->isOpen()) {
            port->clear();
            port->close();
        }
        port.reset();
    }

    auto portList = QSerialPortInfo::availablePorts();
    if(!portList.isEmpty()) {
        for(auto ii = 0; ii < portList.length(); ++ii) {
            if((portList[ii].vendorIdentifier() == vid) && (portList[ii].productIdentifier() == pid)) {
                if(port != nullptr) {
                    if(port->isOpen()) {
                        port->clear();
                        port->close();
                    }
                    port.reset();
                }
                port = std::make_unique<QSerialPort>(portList[ii]);
                bool portOpen = port->open(QIODevice::ReadWrite);
                if(portOpen) {
                    bool baudSuccess = port->setBaudRate(baudRate, QSerialPort::AllDirections);
                    if(baudSuccess) {
                        port->setDataTerminalReady(true);
                        QString handshakeReceived{""};
                        bool readAvailable = port->waitForReadyRead(handshakeWait);
                        while(readAvailable) {
                            handshakeReceived.append(QString::fromUtf8(port->readAll()));
                            readAvailable = port->waitForReadyRead(handshakeWait);
                        }
                        if(handshakeReceived == handshake) {
                            emit deviceFound(portList[ii]);
                            break;
                        }
                        else {
                            if(port != nullptr) {
                                if(port->isOpen()) {
                                    port->clear();
                                    port->close();
                                }
                                port.reset();
                            }
                        }
                    }
                    else {
                        if(port != nullptr) {
                            if(port->isOpen()) {
                                port->clear();
                                port->close();
                            }
                            port.reset();
                        }
                    }
                }
            }
        }
    }
}

Method Reliability: Very reliable method, always finds the connected MCU.
Error: Gives an error when the application is closed:

ASSERT failure in QCoreApplication::sendEvent: "Cannot send events to objects owned by a different thread. Current thread 0x0x16d3dbc2c50. Receiver '' (of type 'QSerialPort') was created in thread 0x0x16d3dbd8d00", file C:\Users\qt\work\qt\qtbase\src\corelib\kernel\qcoreapplication.cpp, line 540

I have tried other methods as well (e.g. QThread *QThread::create()) , in any case when accessing QSerialPort in a different thread I get either of the 2 warnings or error. I also tried the Qt Serial Port Examples but I am unable to find the bool QSerialPort::waitForReadyRead(int msecs) being used in a different thread while the documentation clearly states that it should be in a non-GUI thread.

I do not care if the QSerialPort is running in a GUI thread or a separate thread since the communication over the serial port is very short and can be easily handled by the main GUI thread. But I do have to run the bool QSerialPort::waitForReadyRead(int msecs) in a different thread to figure out if it's my MCU because this function takes many seconds to receive the data over the serial port and if not run in a separate thread then it will block the main GUI thread.

Is there any way I can solve either the warning or the error in the above two approaches, or have a completely different structure to discover my MCU in a different thread without warnings and errors?

JonB

@CJha
I know nothing about MCUs. But your problems seem to come from using a secondary thread. And that because you choose to use port.waitForReadyRead(), which as you say will block the thread it is in (the UI thread in your case if you don't create a secondary thread).

Nearly every time people start using threads they get into difficulty. Try not to if they are not necessary. I never use the waitFor...() calls. They are just blocking wrappers on top of the inherently asynchronous nature of Qt calls. Here you want asynchronicty, so just rewrite code logic to slot onto the readyRead() signal and then you won't need a secondary thread while the main UI does not block.

CJha

@JonB Hi, I need bool QSerialPort::waitForReadyRead(int msecs) to verify that I am connecting to the correct MCU. My MCUs are programmed to send a special text over the Serial Port as soon as the communication is established with it.
If I do not use the bool QSerialPort::waitForReadyRead(int msecs) then I will have to connect to all the MCUs where the vendor and product identifiers are the same (which can be multiple from my own hardware) and then wait to receive a signal back and somehow verify the correct one and then break and re-establish the correct connection. All this will take time and in between if the user plugs or unplugs something then my entire calculation for finding the correct MCU will be wrong and it will be a mess.

JonB

@CJha
You never need waitForReadyRead(). Like I said it's only a wrapper on top of the signal to make it block and wait. It is implemented as something like (https://codebrowser.dev/qt5/qtserialport/src/serialport/qserialport_unix.cpp.html#_ZN18QSerialPortPrivate16waitForReadyReadEi):

while (QElapsedTimer.elapsed())
    qt_poll_msecs()

No reason why you cannot do this without the QEventLoop/waitFor...().

and it will be a mess.

Your current code is a mess because it does not deal with the secondary thread correctly.

CJha

@JonB

Your current code is a mess because it does not deal with the secondary thread correctly.

It is dealing with the secondary thread correctly, the secondary thread is created and destroyed without any issues or memory leaks, and the communication with the secondary thread is all based on signal-slot mechanism and is of type Qt::QueuedConnection. Thread is not the problem the problem is the limitations of the QSerialPort.

I will try the QEventLoop method, but it will need a timeout because of how the serial communication happens.

JonB

@CJha said in QSerialPort discovery in a non GUI QThread?:

It is dealing with the secondary thread correctly,

Approach 1:

QObject: Cannot create children for a parent that is in a different thread.

Approach 2:

ASSERT failure in QCoreApplication::sendEvent: "Cannot send events to objects owned by a different thread.

Your definition of "dealing with the secondary thread correctly" is different from mine then.

Since you already know you are right and it needs a secondary thread with waitFor...() rather than the normal Qt asynchronous approach with readyRead() signal and non-blocking slot there is not much for me to say, so I leave you to it.

CJha

@JonB

You are only taking half of the warning/error messages, the rest of it says:

Approach 1:

(Parent is QSerialPort(0xf037eff560), parent's thread is QThread(0x2b741fdbf30), current thread is QThread(0x2b741ff38d0)

Approach 2:

Current thread 0x0x16d3dbc2c50. Receiver '' (of type 'QSerialPort') was created in thread 0x0x16d3dbd8d00"

And I mentioned

Thread is not the problem the problem is the limitations of the QSerialPort.

In both cases QSerialPort is causing the issue, if I take it out and replace it with a simple QObject then the threads are fine. So how is dealing with the thread an issue here? As you mentioned:

Your current code is a mess because it does not deal with the secondary thread correctly.

Sure putting QSerialPort in a thread can be an issue but the thread itself is not an issue. That's all I am trying to point out.

J.Hilk

@CJha There are a couple of issues with both of your approaches, I assume because you're laking some knowledge about QThread and or QSerialPort.
That is fine, no-one can know everything.

I'll help you fix the jamming of the gun, but I lack the time and patience these days to show you all the ins and outs of gun safety.

:
give your SerialPort instance the correct parent instead of none

MCU::MCU(QObject *parent) : QObject(parent), port(this)
{

CJha

@J-Hilk Thank you, I will try the method :)

CJha

@JonB Thanks, the QEventLoop seems to be working well in initial tests. I had to create extra slots and use a timer to synchronize but my GUI is responsive while the QSerialPort discovery in the main GUI thread is working as desired.

JonB

@CJha
For the record. You will see I changed my post above to show what QSerialPort::waitForReadyRead() seems to use in its code. This is not the QEventLoop guess which I originally entered. QEventLoop plays better with not blocking the UI than that code. I do not know why waitForReadyRead() does not seem to use that, or how you using that differs from what they do.

CJha

@JonB Yes I noticed the change. I am not sure why they do it the way they are doing it. I am starting a single shot timer and immediately after I am starting the event loop which exits with exit code 1 with the readyRead() signal received or with exit code 0 if the timer runs out. If I have the exit code of 1 I read the port and start the process again till the exit code of 0 is received. Seems simple and works every time.