[SOLVED] QSerialPort::waitForBytesWritten() returns false

frankiefrank

Simplified as much as I can - this is just for the error for reading data. I didn't include the writing code and the threading related stuff besides the minimal to reproduce.

SerialPortWorkerThread.h
@
class SerialPortWorkerThread : public QThread
{
Q_OBJECT

public:
SerialPortWorkerThread(QObject *parent = 0);
~SerialPortWorkerThread();
virtual void run();

private:
bool abortThread;
bool openPort(QSerialPort &serialPort, QString &errorMsg);
};
@

SerialPortWorkerThread.cpp
@
#include "serialportworkerthread.h"
#include <QDebug>

SerialPortWorkerThread::SerialPortWorkerThread(QObject *parent)
: QThread(parent), abortThread(false)
{
}

SerialPortWorkerThread::~SerialPortWorkerThread()
{
if (isRunning())
abortThread = true;
}

void SerialPortWorkerThread::run()
{
QSerialPort serialPort;
QString error;

bool opened = openPort(serialPort, error);
if (!opened)
qDebug() << QString("Error opening port: %1").arg(error);

while (opened && !abortThread)
{
bool haveData = serialPort.waitForReadyRead(15);
if (!haveData)
{
QString error = serialPort.errorString();
int errorId = serialPort.error();
//! skip the UNKNOWN
if (!error.contains("Unknown error", Qt::CaseInsensitive))
qDebug() << QString("Error %1 - %2").arg(errorId).arg(error);
}
else {
qDebug() << QString("Data was read!");
QByteArray data = serialPort.readAll();
}
}

qDebug() << "Serial Port Worker Thread - we got out of while loop";
}

bool SerialPortWorkerThread::openPort(QSerialPort &serialPort, QString &errorMsg)
{
errorMsg.clear();
if (serialPort.isOpen())
serialPort.close();

serialPort.setPortName("COM20");
bool success = serialPort.open(QIODevice::ReadWrite);

//! connection error - just quit here
if (!success) {
int errorId = serialPort.error();
QString errorDescription = serialPort.errorString();
errorMsg = tr("Could not open port: (errorId=%1) %2").arg(errorId).arg(errorDescription);

return false;
}

//! Try to apply settings one after another, storing potential error message
success = serialPort.setBaudRate(QSerialPort::Baud9600);
if (!success)
errorMsg = tr("Could not set baud rate.");

if (success)
success = serialPort.setDataBits(QSerialPort::Data8);
if (!success)
errorMsg = tr("Could not set data bits number.");

if (success)
success = serialPort.setParity(QSerialPort::NoParity);
if (!success)
errorMsg = tr("Could not set parity check option.");

if (success)
success = serialPort.setStopBits(QSerialPort::OneStop);
if (!success)
errorMsg = tr("Could not set stop bits.");

if (success)
success = serialPort.setFlowControl(QSerialPort::NoFlowControl);
if (!success)
errorMsg = tr("Could not set flow control mode.");

//! If necessary, close
if (!success)
serialPort.close();

return success;
}
@

Then I create a regular UI application, with a SerialPortWorkerThread* member and in the constructor I call:

@
serialPortWorker = new SerialPortWorkerThread(this);
serialPortWorker->start();
@

kuzulis

Ohh.. it is not simple example.. :)
Though yes, I found some problems with waitForReadyRead() in own research. I will try to prepare a patch. :(

frankiefrank

Thank you for confirming. Should I do anything in the Qt Bug Tracker or would you update any existing issues?

kuzulis

You can try a patch, see issue: https://bugreports.qt-project.org/browse/QTBUG-39314

frankiefrank

Thanks kuzulis. I will try this.

I hate to hijack you to another thread (which is way more creative than hijacking a thread) - but is there a chance you could help out over here?
http://qt-project.org/forums/viewthread/43156/

(if not, I'll work it out).

frankiefrank

Unfortunately still doesn't work.

I took qtserialport-stable from git and manually replaced the qserialport_win.cpp with the patched version. I then cleaned and rebuilt, with 5.3.0.

In my test project I removed
@QT += serialport@
and added
@
INCLUDEPATH += "C:/qtserialport-stable/src/serialport"
LIBS += "C:/build-qtserialport-Desktop_Qt_5_3_0_MSVC2010_OpenGL_32bit-Debug/lib/Qt5SerialPortd.lib"
@

For the first time data should be read, I get
"Error 12 - Overlapped I/O operation is in progress."
"Error 12 - The handle is invalid."

After that, the code returns one or two of the "overlapped" errors each time data is sent to the port and should be read.

kuzulis

[quote]
For the first time data should be read, I get
“Error 12 – Overlapped I/O operation is in progress.”
“Error 12 – The handle is invalid.”
[/quote]

Hmm.. This is strange problem.. Can you please provide a more simple example without threads? E.g. try to modify an "creadersync" example..

kuzulis

UPD: I just checked a patch with a modified "creadersync" example: http://paste.kde.org/pg353hzw4

I got a such output:

@
waitForReadyRead failed, 12, error: Unknown error
waitForReadyRead failed, 12, error: Unknown error
Data successfully received from port:
q
waitForReadyRead failed, 12, error: Протекает наложенное событие ввода/вывода.
waitForReadyRead failed, 12, error: Протекает наложенное событие ввода/вывода.
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
qqqqqqqq
Data successfully received from port:
q
waitForReadyRead failed, 12, error: Протекает наложенное событие ввода/вывода.
@

Where "Протекает наложенное событие ввода/вывода." is an "Overlapped I/O operation is in progress."

Thus, everything is fine. It seems, the error description has incorrect text (seems, the last GetLastError() is decrypted), possibly it needs to be fix in the future (but it isn't critical). To be guided by the error code number =12 (that there is QSerialPort::TimeoutError), and this is correct code.

PS: If to be honest, for me do not like your error text: "The handle is invalid.” Seems, that your code has some problems.. You trying to manage on closed device.. or something else..

frankiefrank

So you're saying I should ignore the Overlapped I/O string and just treat this is an incorrect error string? OK - that's better.

I see that indeed now with the patch and if I ignore this specific error, it should eliminate the problems I had in my mini project.

About the "The handle is invalid" - I got this error when my waitForReadyRead() calls followed a write() and waitForBytesWritten() calls. I indeed didn't get that error in your modified CReaderSync.

BUT I did manage to reproduce it based on your code with a few steps and without any threads :)

Create a new Qt Widgets Application and copy all of your suggested CReaderSync code to the constructor of the MainWindow.

Modification 1: open the port as QIODevice::ReadWrite.
Modification 2: Add the following code (picked up from CWriterSync and slightly reformatted output string) before starting the while loop - so it happens only once.

@
QByteArray writeData(QString::number(QDateTime::currentMSecsSinceEpoch()).toLatin1());
qint64 bytesWritten = serialPort.write(writeData);

if (bytesWritten == -1) {
qDebug() << QObject::tr("Failed to write the data to port, error: %1 - %2").arg(serialPort.error()).arg(serialPort.errorString()) << endl;
return;
} else if (bytesWritten != writeData.size()) {
qDebug() << QObject::tr("Failed to write all the data to port, error: %1 - %2").arg(serialPort.error()).arg(serialPort.errorString()) << endl;
return;
} else if (!serialPort.waitForBytesWritten(5000)) {
qDebug() << QObject::tr("Operation timed out or an error occurred for port, error: %1 - %2").arg(serialPort.error()).arg(serialPort.errorString()) << endl;
return;
}
@

Modification 3: I removed the return values (since I threw code into constructor) and changed standardoutput to qDebug() (to see messages in the Application Output in Qt Creator). Copied a copy of the patched Qt5SerialPortd.dll into the shadow build folder. Made it compile.

What I get here is the mentioned:
"waitForReadyRead failed, 12, error: Overlapped I/O operation is in progress."
"waitForReadyRead failed, 12, error: The handle is invalid."

kuzulis

Hmm.. I can not reproduce:

bq. “waitForReadyRead failed, 12, error: The handle is invalid.”

with our modifications (If I correctly understand): http://paste.kde.org/pt8uhw1xp

Quite possibly that problems with the driver of your device or something else. Because someone trying to have access to your device... I.e. someone earlier set this error, maybe you trying to access to other I/O resource? (file, pipe and so on)?

In any case, don't pay attention to the text of this error in case the timeout works correctly (is fired after 10 sec). :)

frankiefrank

You mean this error is not necessarily related to the specific QSerialPort I'm accessing? I'm not sure how that part works.

Anyway, I think this solves the issue for me - so thank you! Will the patch be integrated into the stable branch of QSerialPort and will be released as a part of 5.3.1?

kuzulis

You mean this error is not necessarily related to the specific QSerialPort I’m accessing?

Yes, because someone can do something on some I/O resources in context of current thread (maybe you have an some other code in your app, not only the constructor, as you mention??). And this maybe lead to the access to invalid descriptor of some resource (not necessarily that it will serial port).. I just assumption..

QSerialPort and will be released as a part of 5.3.1?

Yes, most likely in 5.3.1 if reviewers don't block this patch. You can vote or add comments for this patch here: https://bugreports.qt-project.org/browse/QTBUG-39314

kuzulis

Many thanks for your vote. :)

BTW: Is ready an other patch to display an correct error message for TimeoutError: https://codereview.qt-project.org/#change,86406

You can also try it. :)

frankiefrank

Thanks Kuzulis,

What's the best way to apply these kind of patches? Should I just replace the .cpp in the qtserialport-stable\src\serialport folder and then qmake, build and finally nmake install from the shadow build folder?

I'm a bit new to this patching business - usually I'm only working with the binary releases.

I looked in the Wiki page and I didn't understand this sentence:

add a new make "Build Step" and write to the "Make arguments" the install target

Could you explain exactly what I need to do here?

kuzulis

bq. What’s the best way to apply these kind of patches?

Usual, like:

[code]
$ git clone git://gitorious.org/qt/qtserialport.git
$ cd qtserialport
$ git fetch https://codereview.qt-project.org/p/qt/qtserialport refs/changes/06/86406/1 && git checkout FETCH_HEAD
[/code]

where "git fetch" it is an copy/paste string from this: https://codereview.qt-project.org/#change,86406 ,

patch set #1 with selected: checkout tab + Anonymoys HTTP + click to copy to clipboard.

After this manipulations, the current patch will be applied in your local copy of QtSerialPort and you can try it.

bq. Could you explain exactly what I need to do here?

This is an QtCreator's project property (build-step) in the build-page (you should click to build step to expand it).. By default there is just like:

bq. make: [ ]

where [ ] is the line edit. So, you should just write there install . And re-build project.

frankiefrank

Thanks again!

Is there any chance this will enter 5.3.1? I'm looking at the QT-BUG entry and the Fix Version/s field is still None.

kuzulis

bq. Is there any chance this will enter 5.3.1?

Yes, will be in 5.3.1.

bq. I’m looking at the QT-BUG entry and the Fix Version/s field is still None.

Oops.. I'm sorry, I forgot to close a bug. Done. :)