How to implement a QThread that runs forever{} with a QWaitCondition but still needs to catch another Slot while doing that

Question

I implemented a class that can write data to a serial port via a QQueue and read from it by a slot. I use QAsyncSerial for this which in turn uses boost::asio with a callback. The class is moved to a thread and its start() method is executed when the QThread emits "started()"

The problem is that I dequeue the QQueue in the start()-method using forever {} and a QWaitCondition. While this is running (which obviously runs forever) the slot connected to the dataReceived signal of QAsyncSerial can not be called, thus I never read anything from the serial port.

What is the usual approach to this problem?

SerialPortHandler::SerialPortHandler(SerialPort serialPort, QObject *parent) : QObject(parent), serialPort(serialPort)
{
    m_enqueueMessageMutex = new QMutex();
    m_messageQueue = new QQueue<BaseMessage*>();
    m_waitCondition = new QWaitCondition();
    serial.open(serialPort.deviceName(), 2400);
    connect(&serial, SIGNAL(dataReceived(QByteArray)), this, SLOT(serialSlotReceivedData(QByteArray)));
}

void SerialPortHandler::serialSlotReceivedData(QByteArray line)
{
    qDebug() << QString(line).toAscii();
}

void SerialPortHandler::sendTestPing()
{
    PingMessage *msg = new PingMessage();
    enqueueMessage(msg);
}

void SerialPortHandler::enqueueMessage(BaseMessage *msg)
{
    QMutexLocker locker(m_enqueueMessageMutex);
    m_messageQueue->enqueue(msg);
    m_waitCondition->wakeAll();
}

void SerialPortHandler::start()
{
    if (!serial.isOpen())
        return;

    forever {
        m_enqueueMessageMutex->lock();
        if (m_messageQueue->isEmpty())
            m_waitCondition->wait(m_enqueueMessageMutex);
        BaseMessage *msg = m_messageQueue->dequeue();
        serial.write(msg->encodeForWriting());
        m_enqueueMessageMutex->unlock();
    }
}

The changed QAsyncSerial callback used by boost::asio:

void QAsyncSerial::readCallback(const char *data, size_t size)
{
    emit dataReceived(QByteArray::fromRawData(data, (int) size));
}

Edit:

I solved this problem with another approach. I ditched QAsyncSerial and instead used the CallbackAsyncSerial which is also distributed by QAsyncSerial directly. Now the callback used by boost::asio is the serialSlotReceivedData "slot". This "solves" the problem since the callback is called in the thread boost::asio runs in. Since it has its own thread it doesn't matter that the thread SerialPortHandler runs in is blocked by the forever loop.

New code: (since QAsyncSerial is something like a wrapper to CallbackAsyncSerial only some trivial things changed)

SerialPortHandler::SerialPortHandler(SerialPort serialPort, QObject *parent) : QObject(parent), serialPort(serialPort)
{
    m_enqueueMessageMutex = new QMutex();
    m_messageQueue = new QQueue<BaseMessage*>();
    m_waitCondition = new QWaitCondition();
    /* serial is now CallbackAsyncSerial and not QAsyncSerial */
    serial.open(QString(serialPort.deviceName()).toStdString(), 2400);
    serial.setCallback(bind(&SerialPortHandler::serialSlotReceivedData, this, _1, _2));

    m_messageProcessingState = MessageProcessingState::Inactive;
}

void SerialPortHandler::start()
{
    if (!serial.isOpen())
        return;

    forever {
        m_enqueueMessageMutex->lock();

        if (m_messageQueue->isEmpty())
            m_waitCondition->wait(m_enqueueMessageMutex);

        BaseMessage *msg = m_messageQueue->dequeue();
        QByteArray encodedMessage = msg->encodeForWriting();
        serial.write(encodedMessage.constData(), encodedMessage.length());

        m_enqueueMessageMutex->unlock();
    }
}

I don't know if I completely understood your problem, but perhaps you need a QThread with an own event loop, look into QThread's documentation for more info. — Christian Rau, May 23 '11 at 19:42
I checked the documentation again - but doesn't QThread always run in its own event loop? — Strayer, May 23 '11 at 20:00
No, only if you start it's `exec` method, I think. But then you cannot use `run`, you have to do your work ba event handling. — Christian Rau, May 23 '11 at 22:57

score 3 · Accepted Answer · answered May 24 '11 at 10:09

1) Create slot in your thread, for example onMessageReady(), which will do the job.

2) Create a signal indicates that new message ready, and emit it each time you creating a new message.

3) Connect them using QueuedConnection and call your thread's exec function.

This won't block your thread, as WaitforObject does, and you will handle all incoming signals.

something like this:

SerialPortHandler: public QThread
{
  Q_OBJECT
...
signals:
    void sNewMessageReady();
slots:
    void onNewMessageReady();
    void serialSlotReceivedData(QByteArray);
};

SerialPortHandler::SerialPortHandler(SerialPort serialPort, QObject *parent) : QThread(parent), serialPort(serialPort)
{
    m_enqueueMessageMutex = new QMutex();
    m_messageQueue = new QQueue<BaseMessage*>();
    serial.open(serialPort.deviceName(), 2400);
    connect(&serial, SIGNAL(dataReceived(QByteArray)), this, SLOT(serialSlotReceivedData(QByteArray)));
    connect(this, SIGNAL(sNewMessageReady()), this, SLOT(onNewMessageReady()),Qt::QueuedConnection);
}

void SerialPortHandler::enqueueMessage(BaseMessage *msg)
{
    QMutexLocker locker(m_enqueueMessageMutex);
    m_messageQueue->enqueue(msg);
    emit sNewMessageReady();
}


void SerialPortHandler::onNewMessageReady()
{
    QMutexLocker locker(m_enqueueMessageMutex);
    BaseMessage *msg = m_messageQueue->dequeue();
    serial.write(msg->encodeForWriting());
}

after all simply call thread's exec() method, you don't need to reimplement run() and to use QWaitCondotion at all.

Since it will take some time until I work on this project again and this answer has example code which seems correct I mark this as the most helpful answer. Otherwise this is basically what Jeremy Friesner suggested. Thanks for the help to all of you! — Strayer, May 24 '11 at 12:56

score 2 · Answer 2 · answered May 23 '11 at 19:47

2

This is kind of a shot in the dark, since I'm pretty new to using Qt and I don't know the "usual" approaches to problems like this, but perhaps a call to QCoreApplication::processEvents within the loop would help.

answered May 23 '11 at 19:47

gnovice

125,304
15
256
359

1

This DOES work indeed. When I let the QWaitCondition timeout and then just call processEvents the slot is "called" I'll let this question open for a while if someone else has another suggestion because I'm sure I'm just using the wrong approach here. – Strayer May 23 '11 at 20:05
1

@Strayer: It looks like the other answers suggest better solutions, which I myself am going to use. I had a similar problem as yours which I had previously "band-aided" with a call to `processEvents`, but it's beginning to look like an inferior workaround now. Thanks for asking the question! ;) – gnovice May 24 '11 at 15:29
This is a "busy wait", waking up just to see that there's nothing to do, most of the time. Waste of resource, but also slower reaction to events (the slot is only called at the end of the timeout). – David Faure Dec 14 '20 at 18:32

score 2 · Answer 3 · answered May 24 '11 at 01:58

Unless it's strictly necessary for some reason, I'd get rid of the QWaitCondition. Instead, when the have enqueueMessage() emit a (Qt) signal after it has appended new data to the QQueue, and have your worker thread receive that signal (along with whatever other signals it needs to receive) in the usual Qt way. Then your problem goes away, with no timeouts or other hackery needed.

(optional optimization: have the serial port only emit the signal if the QQueue was empty before it added the new data, and have the main thread's corresponding slot read from the QQueue until the QQueue is empty -- that can cut down on the number of signals that need to be sent)

In retrospect this makes a lot more sense than my approach (and my workaround, which I added to the question), it even saves me the trouble to think about how to break the forever loop when I want to stop the thread. I think I will try this even with my new approach. I still want to skip QAsyncSerial because it just adds _another_ layer between the Serial Port and my application. — Strayer, May 24 '11 at 10:07

How to implement a QThread that runs forever{} with a QWaitCondition but still needs to catch another Slot while doing that

3 Answers3

Linked