Signal Slot problem in a thread [EDIT-UPDATE]

[yczo]

Hello folks, I have an object moved to a thread on MainWindow. The issue is that the connection from signal, to the slot

connect(this,SIGNAL(writeCom(QString)),serPort,SLOT(updateOutBuff(QString)),Qt::QueuedConnection);

do not work, don't run, the slot do not execute, but qDebug say that the connection is made.

In theory, I must see qDebug() << "update msg" << newMsg;, executed from the slot, with newMsg, sended from MainWindow: emit writeCom(prueba2);
but it does not.

[EDIT-UPDATE]
The problem comes when the instruction

serPort->moveToThread(thSer);

is executed.

I tested the connection to the slot without moving to the thread, and works fine

[/EDIT-UPDATE]

Like always, any help is welcome.
Thanks in advance

(from MainWindow)
    serPort = new SerialPort();
    thSer = new QThread; //thRead

    serPort->moveToThread(thSer);
    //qDebug() << " 1Read" << connect(readPrt, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
    connect(thSer, SIGNAL(started()), serPort, SLOT(process()));
    connect(serPort, SIGNAL(finished()), thSer, SLOT(quit()));
    connect(serPort, SIGNAL(finished()), serPort, SLOT(deleteLater()));
    connect(thSer, SIGNAL(finished()), thSer, SLOT(deleteLater()));
    thSer->start();
    connect(serPort,SIGNAL(received(QString)),this,SLOT(debug(QString)),Qt::QueuedConnection);
    //thr qDebug() << "WinRead connected" << connect(serPort->winRead,SIGNAL(received(QString)),this,SLOT(debug(QString)),Qt::QueuedConnection);
    qDebug() << "connected" << connect(this,SIGNAL(writeCom(QString)),serPort,SLOT(updateOutBuff(QString)),Qt::QueuedConnection);  /// qDebug() answer true

          char prueba2[] = "That is a probe of a long string, to see, how it works";

          emit writeCom(prueba2);

and the next object:

#ifndef SERIALPORT_H
#define SERIALPORT_H
#include <windows.h>
#include <QObject>
#include <QThread>
#include <QtSerialPort/QSerialPort>
#include <QtSerialPort/QSerialPortInfo>
#include<QDebug>
#define maxPort 41
#define askId       "cb?"
#define iDAck       "cbr"
#define maxSB 254
#define minComPort 0
#define maxComPort 41
#define baudRate CBR_19200


//SerialPort (BASE) ********************************************************

class SerialPort : public QObject
{
    Q_OBJECT
public:
  //WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
    char vacio[1];
    QString conMsgs,qWriteBuf;    //connection messages (error or success)
    char readBuffer[maxSB];
    HANDLE hComm;   //-------------------------
    char port2Config[16];
    BOOL status, exit = false;
    DWORD dwEventMask;  //event mask to trigger
    DWORD nBytesRead;
  //  ::readWP *winRead;
 //   ::writeWP *winWrite;
    void makeWinCnx();


    void write(char *outBuff);
    char* read();

  //WWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWWW
    //infoPort infPrt;
    SerialPort();
    ~SerialPort();
    void makeCnx();
    QSerialPort serial;

    QThread *thRead;
    QThread *thWrite;

public slots:
    void updateMsg(QString newMsg);
    void process();


signals:
    void received(QString rcv);
    void conInf(QString cnxInfo);
    void finished();


private:
    bool genericWindows = false;
  //  void run();
  //  char *read();
  //  void write(char *outBuffer);
};

#endif // SERIALPORT_H

//***********************************************************

#include "serialport.h"
#include<iostream>
using namespace std;



//SerialPort (Base) *****************************************************
void SerialPort::updateMsg(QString newMsg){  //write here
    qDebug() << "update msg" << newMsg;
    newMsg = newMsg + '\0';

    qWriteBuf = newMsg;

}
//-----------------------------------------------------------------------
void SerialPort::process(){
    makeWinCnx();

    char *rcv = new char[maxSB];
    qWriteBuf = "";
    while(!exit){


        if (qWriteBuf != "" ){

            string sWB = qWriteBuf.toStdString().c_str();
            char cWB[maxSB];
            qint8 in = 0;
            while(in < sWB.size()){
                cWB[in] = sWB[in];
                in++;
            }
            cWB[in] = '\0';
            write(cWB);
            qWriteBuf = "";
        }

        *rcv = '\0';

       // status = SetCommMask(hComm, EV_RXCHAR); //Configure Windows to Monitor the serial device for Character Reception
       // qDebug() << "wait: " << WaitCommEvent(hComm, &dwEventMask, NULL); //Wait for


        rcv = read();
        if (*rcv != '\0') {
            QString qRcv(rcv);
            emit received(qRcv);
        }
    }  //von while


    emit finished();
}
//-----------------------------------------------------------------------
SerialPort::SerialPort()
{
    genericWindows = true;
    exit = false;
}
//...(I avoid the connection functions)

[jsulm]

It's probably because you block the event loop in your thread:

while(!exit)

[VRonin]

Also, this:

connect(serPort, SIGNAL(finished()), thSer, SLOT(quit()));
connect(serPort, SIGNAL(finished()), serPort, SLOT(deleteLater()));
connect(thSer, SIGNAL(finished()), thSer, SLOT(deleteLater()));

Is a race condition (serPort needs thSer event loop to delete itself)
change connect(serPort, SIGNAL(finished()), thSer, SLOT(quit())); to connect(serPort, SIGNAL(destroyed()), thSer, SLOT(quit()));

Finally, if you are using Qt5 have a look at the new connect syntax that checks connections at compile time

[kshegunov]

@VRonin said:

Is a race condition (serPort needs thSer event loop to delete itself)
change connect(serPort, SIGNAL(finished()), thSer, SLOT(quit())); to connect(serPort, SIGNAL(destroyed()), thSer, SLOT(quit()));

It's enough to switch the order of connect statements.

connect(serPort, SIGNAL(finished()), serPort, SLOT(deleteLater()));
connect(serPort, SIGNAL(finished()), thSer, SLOT(quit()));

Is perfectly fine, as pending delete events will be processed before exiting the event loop.

[VRonin]

@kshegunov said:

It's enough to switch the order of connect statements.

True given the current implementation. I thought the order of execution of slots should not be considered deterministic (even if it has been deterministic at least since Qt4). Or does that rely on the fact that the first connection is implicitly a directconnection while the second is a queuedconnection? if that is the case then the order of connection doesn't matter anyway

[kshegunov]

@VRonin said:

I thought the order of execution of slots should not be considered deterministic

When across threads it can't be deterministic. When using Qt::DirectConnection (I think it's documented) the order is in the order of connects.

Or does that rely on the fact that the first connection is implicitly a directconnection while the second is a queuedconnection? if that is the case then the order of connection doesn't matter anyway

Nope, I was thinking of something else and got tangled up posting and reposting events in my head. Actually the order shouldn't matter anyway. I was thinking about this when I wrote the above:

When this signal is emitted, the event loop has already stopped running. No more events will be processed in the thread, except for deferred deletion events. This signal can be connected to QObject::deleteLater(), to free objects in that thread.

Basically the thread will process the last pending deferred deletions just before actually exiting the stack.

Kind regards.

[VRonin]

Looks like QThread can take care of itself (I always used the destroyed signal, I won't in the future). scarp my initial answer

[yczo]

That is strange, I made the connectionsin the following order and all works

    serPort = new SerialPort();
     qDebug() << "connected" << connect(this,SIGNAL(writeCom(QString)),serPort,SLOT(updateMsg(QString)),Qt::QueuedConnection);
    thSer = new QThread; //thRead

 //   serPort->moveToThread(thSer);
    //qDebug() << " 1Read" << connect(readPrt, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
    connect(thSer, SIGNAL(started()), serPort, SLOT(process()));
    connect(serPort, SIGNAL(finished()), thSer, SLOT(quit()));
    connect(serPort, SIGNAL(finished()), serPort, SLOT(deleteLater()));
    connect(thSer, SIGNAL(finished()), thSer, SLOT(deleteLater()));

I have the while(), becauseI thought by reading the serial port (in paralel with MainWindow)until the condition (exit) is true. Why is not right?

void SerialPort::process(){
while(!exit) {
//read here serial port
}
emit finished();

}

geetings and thank you very much

[jsulm]

@yczo Because while you're in the loop the event loop is blocked and no signals will be emited:

while(!exit){
    ...
        rcv = read();
        if (*rcv != '\0') {
            QString qRcv(rcv);
            emit received(qRcv); // This signal will be emited after while loop is finished!
        }
    }  //von while

[kshegunov]

@jsulm

Because while you're in the loop the event loop is blocked and no signals will be emited

Not true. Signals are emitted fine without an event loop (I sometimes use them like this when subclassing QThread). Also direct signal-slot connections will work fine. The thing that will not work is a deferred invocation (a queued connection) as it requires an active event loop.

@yczo

@jsulm has already given you the answer in his first post - you're blocking the event loop, so there will be no queued slots executed until you unblock it. This:

connect(this,SIGNAL(writeCom(QString)),serPort,SLOT(updateOutBuff(QString)),Qt::QueuedConnection);

requires a running event loop to work properly.

Kind regards.

[yczo]

A question: Then, I can not make infinite loops in a thread??? but I need to make an infinite loop in paralel with MainWindow, Is there another technique to do?

How can i make an infinite loop in paralele with MainWindow then?

Greetings, and thank you very much.

[SGaist]

Hi,

Take a look at the mandelbrot example.

[kshegunov]

@yczo

but I need to make an infinite loop in paralel with MainWindow, Is there another technique to do?

There is. Use the asynchronous API, as you were advised in your other thread. Connect the QSerialPort::readyRead signal to a slot in your worker object that will do the reading from the serial port.

As for loops there can be loops made to run through the event queue but I see no good justification to do that here.

Kind regards.