QSerialPort readAll() does not work with 5.14.0 anymore

rasc

Hello,
after the update to Qt 5.14.0 or later e.g. 5.15.1 (from 5.13.2) my code for serial communication does not work anymore. I use QSerialPort for communication with a Zeiss microscopce for which I don't know all parameters for sure (I had to reverse engineer it a bit), but the code worked fine with 5.13.2 and before. The project is hosted on Github, see https://github.com/BrillouinMicroscopy/BrillouinAcquisition for the complete code.

My class for serial communication looks like this:

class com : public QSerialPort {
public:
	com() {};
	com(std::string terminator) : m_terminator(terminator) {};

	virtual qint64 writeToDevice(const char* data);

	std::string receive(std::string request);
	void send(std::string message);

protected:
	std::string m_terminator{ "\r" };
};

The function in question which stopped working after the update is this

std::string com::receive(std::string request) {
	request = request + m_terminator;
	writeToDevice(request.c_str());

	std::string response = "";
	if (waitForBytesWritten(1000)) {
		// read response
		if (waitForReadyRead(1000)) {
			QByteArray responseData = readAll();
			while (waitForReadyRead(50))
				responseData += readAll();

			response = responseData;
		}
	}

	return response;
}

I connect to the microscope like this (m_comObject is an object of type com)

m_comObject->setPortName("COM1");
if (!m_comObject->setBaudRate(QSerialPort::Baud9600)) {
	throw QString("Could not set BaudRate.");
}
if (!m_comObject->setFlowControl(QSerialPort::HardwareControl)) {
	throw QString("Could not set FlowControl.");
}
if (!m_comObject->setDataBits(QSerialPort::Data8)) {
	throw QString("Could not set DataBits.");
}
if (!m_comObject->setParity(QSerialPort::NoParity)) {
	throw QString("Could not set Parity.");
}
if (!m_comObject->setStopBits(QSerialPort::OneStop)) {
	throw QString("Could not set StopBits.");
}
m_isConnected = m_comObject->open(QIODevice::ReadWrite);
if (!m_isConnected) {
	throw QString("Could not open the serial port.");
}
m_comObject->clear();

Sending to the microscope still works fine (it reacts to the command), but Qt never receives any data. So reading from the microscope stopped working with the update for some reason.

I tried to figure out what has changed for QSerialPort from 5.13.2 to 5.14.0, but couldn't really find anything which I thought to be relevant. I would be grateful for any hint about what has changed and how I can circumvent the problem. Thanks a lot in advance.

JonB

@rasc
You may be right and there may be a problem in 5.14, I don't know. But if it were me trying to test for good/bad behaviour, I would at least get rid of all your waitFor...() calls and replace with signals/slots, at least while I tested/debugged.

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

HardwareControl

Are you sure in this flow control? Do you use and the RTS/CTS line pinouts?

You can also check on the received data after the waitForBytesWritten()... Also, make sure that the waitForReadyRead() triggered too.

rasc

@JonB I now tested the QSerialPort Terminal example https://doc.qt.io/qt-5/qtserialport-terminal-example.html which uses async communication with signal/slots and this does indeed work with 5.14 onwards as well. I guess I could make it work using signal/slots, but I still wonder why it doesn't work anymore when using the blocking approach.

@kuzulis I am not absolutely sure about the settings, since I determined them by try and error (and by recording the device communication when using the manufacturer software). However, communication with the microscope only worked when using HardwareControl, and it did work with 5.13.2. So I think these settings should be ok.
I will check whether waitforReadyRead() triggered.

JonB

@rasc
So if it works with signals/slots I assume there cannot be a pin control etc. issue.

The waitFor...()s are a bit "dodgy". It looks like you are reporting a change in their behaviour. If you want to stick with them you probably need to try to see where exactly it does not behave as expected in those blocking calls. For example, I have no idea whether upping the timeouts (e.g. your while (waitForReadyRead(50))) would improve the situation.

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

HardwareControl

If you use the TX/RX pins only, then the HardwareControl do not need (it even may cause a problems), because in this mode the serial port driver waits for the RTS/CTS pins handshake.

rasc

@JonB I am fine with switching to the signals/slots approach. I only used the blocking approach, because it was a bit faster/simpler to implement in the first place. I will anyway try to debug this a bit more, since I really wonder what broke here. But I will switch to async if I cannot fix the blocking approach. Increasing the timeout did not help.

@kuzulis I don't think the HardwareControl is the issue here, since the handshake works (I think). I can send data to the device and it reacts accordingly. Only receiving data does not work.

kuzulis

I just checked the QSP with the Qt 5.15.0 (there are no changes between 5.14.0 and 5.15.x) using the virtual com0com serial ports and the following simple example:

#include <QCoreApplication>
#include <QSerialPort>
#include <QDebug>

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);
    QSerialPort sp("COM3");
    qDebug() << "OPEN:" << sp.open(QIODevice::ReadWrite);
    for (;;) {
        sp.write("foo\r\n");
        if (sp.waitForBytesWritten(1000)) {
            QByteArray recv;
            while (sp.waitForReadyRead(1000))
                recv += sp.readAll();
            qDebug() << "DATA:" << recv;
        } else {
            qDebug() << "WR OOPS:" << sp.error();
        }
    }
    return a.exec();
}

and it does work for me.

This example opens the COM3, send the 'foo' data and waits for the response. On other side I had open the Putty on the COM4, and put some symbols there.. And I see all received data on the COM3.

rasc

I now checked with the blockingmaster example https://doc.qt.io/qt-5/qtserialport-blockingmaster-example.html. With 5.13.2 the example code works, with 5.14.0 onwards it does not. So it doesn't seem to be a problem of my code (I was a bit afraid that it could be a threading issue, but the blocking master example only uses a single thread and still doesn't work).

I see that waitForReadyRead(1000) always runs into a timeout no matter the value and never resolves, so readAll() is never called. However, when I run readAll() anyway, it correctly catches the answer from the device. So communication actually works, but waitForReadyRead() seems to be broken for me.

What makes this especially weird is that the readyRead() signal seems to be emitted correctly, since this code

QEventLoop loop;
auto connection = QWidget::connect(
    this,
    &QSerialPort::readyRead,
    &loop,
    &QEventLoop::quit
);
write(request.c_str());
loop.exec();

correctly runs through and reading afterwards gives (a part) of the devices answer. I am a bit lost now, why waitForReadyRead() does not work.

JonB

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

I am a bit lost now, why waitForReadyRead() does not work.

I wrote you earlier:

The waitFor...()s are a bit "dodgy".

You have never said what platform you are on. Docs used to say waitFor...s were "dodgy" under Windows at least, though I believe that has been removed, so maybe they are supposed to be robust now. But your experiences seem to indicate.....

rasc

@JonB said in QSerialPort readAll() does not work with 5.14.0 anymore:

I wrote you earlier:

The waitFor...()s are a bit "dodgy".

Yes, that's for sure true.
I am indeed on Windows, but it has worked fine so far until 5.14.0.

Moving to signal/slots is sure possible, but probably requires quite a bit of work. I call the receive function in a for loop to control a translation stage on my microscope and raster a sample, so it has to work synchronously at the moment. Moving to signal/slots means changing how I control the stage. Also, I wanted to understand why it broke.

Thanks for the help, I guess signal/slots is the only solution now.

JonB

@rasc
I take the point, and I hope my answering has not dissuaded others chiming in if they know. Personally I don't know if there is anything wrong in your wait code. You may wish to raise this as Qt bug if you feel they may be able to help.

mrjj

Hi
I would wait with Qt bug report until @kuzulis answers. After all he wrote the class. :)

rasc

@JonB

Personally I don't know if there is anything wrong in your wait code.

Well, I basically copied the example code and the blockingmaster example doesn't work anymore either. So something in the behavior of QSerialPort has changed from 5.13.2 to 5.14.0 onwards. If it is an intended change or not, I don't dare to judge. If @kuzulis wrote the class, I guess he is the best expert on this. :)

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

waitForReadyRead() seems to be broken for me.

It is possible that now the waitForBytesWritten() handles the read pending requests too.. So, you may try to check that the new data are available after the waitForBytesWritten() call.

kuzulis

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

I basically copied the example code and the blockingmaster example doesn't work anymore either.

For me it does work with the virtual com0com serial ports. :)

A main issue is that it is hard to reproduce this problem to create the auto-tests. And yes, the waitForXXX() methods it is devil methods, it would be good to remove this at all and keep only the asynchronous API.

@rasc said in QSerialPort readAll() does not work with 5.14.0 anymore:

So something in the behavior of QSerialPort has changed from 5.13.2 to 5.14.0

Yes, there are a main change is that now the I/O uses the Windows alertable functions, like {Read|Write}FileEx() instead of previous {Read|Write}File() && QWinOverlapepdIONotifier (which was buggy and removed too).

kuzulis

A better solution is to use the asynchronous approach with the signals. :)

UPD: The QSP already has the similar 'blocking' auto-test, but it does not reproduces the problem for me:

    // first stage
    senderPort.write(newlineArray);
    senderPort.waitForBytesWritten(waitMsecs);
    QTest::qSleep(waitMsecs);
    receiverPort.waitForReadyRead(waitMsecs);
    QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size()));

    receiverPort.write(receiverPort.readAll());
    receiverPort.waitForBytesWritten(waitMsecs);
    QTest::qSleep(waitMsecs);
    senderPort.waitForReadyRead(waitMsecs);
    QCOMPARE(senderPort.bytesAvailable(), qint64(newlineArray.size()));
    QCOMPARE(senderPort.readAll(), newlineArray);

kuzulis

This test passes for me with the com0com serial port (I have not the USB/Serial adapters):

void tst_QSerialPort::twoStageSynchronousLoopback()
{
    QSerialPort senderPort(m_senderPortName);
    QVERIFY(senderPort.open(QSerialPort::ReadWrite));

    QSerialPort receiverPort(m_receiverPortName);
    QVERIFY(receiverPort.open(QSerialPort::ReadWrite));

    const int waitMsecs = 50;

    // first stage
    QVERIFY(senderPort.write(newlineArray) > 0);
    QVERIFY(senderPort.waitForBytesWritten(waitMsecs));
    QTest::qSleep(waitMsecs);
    QVERIFY(receiverPort.waitForReadyRead(waitMsecs));
    QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size()));

    QVERIFY(receiverPort.write(receiverPort.readAll()) > 0);
    QVERIFY(receiverPort.waitForBytesWritten(waitMsecs));
    QTest::qSleep(waitMsecs);
    QVERIFY(senderPort.waitForReadyRead(waitMsecs));
    QCOMPARE(senderPort.bytesAvailable(), qint64(newlineArray.size()));
    QCOMPARE(senderPort.readAll(), newlineArray);

    // second stage
    QVERIFY(senderPort.write(newlineArray) > 0);
    QVERIFY(senderPort.waitForBytesWritten(waitMsecs));
    QTest::qSleep(waitMsecs);
    QVERIFY(receiverPort.waitForReadyRead(waitMsecs));
    QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size()));
    QVERIFY(receiverPort.write(receiverPort.readAll()) > 0);
    QVERIFY(receiverPort.waitForBytesWritten(waitMsecs));
    QTest::qSleep(waitMsecs);
    QVERIFY(senderPort.waitForReadyRead(waitMsecs));
    QCOMPARE(senderPort.bytesAvailable(), qint64(newlineArray.size()));
    QCOMPARE(senderPort.readAll(), newlineArray);
}

rasc

@kuzulis Thanks for all your feedback. I for sure trust you that your auto-tests all pass. However, they don't seem to test for the issue I have, which might very well be related to how the hardware I have behaves.

I followed your idea to test for available data after the waitForBytesWritten() call. I did

write(data);
if (waitForBytesWritten(1000)) {
	flush();		// just for testing, doesn't change anything
	auto available1 = bytesAvailable(); // <- yields 0
	waitForReadyRead(10000);	// <- runs into timeout
	auto available2 = bytesAvailable(); // <- yields correct length
	auto result = readAll();	// yields correct data
}

So the data is in the read buffer, but waitForReadyRead() is not notified about that.

JonB

@rasc
I assume you tested this under 5.15.1? Can you confirm to @kuzulis that you have also tried just this code under 5.13.2 and it does not timeout on your waitForReadyRead(10000); // <- runs into timeout?