From 06a4f3ddf7d9f0da6ec881958ebaa8e4bd041cb6 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Thu, 25 Sep 2014 13:22:26 +0400 Subject: Query a serial number of USB device from Sysfs Tested on Linux with the USB (FTDI and TI) serial ports using Qt4 Change-Id: I8c088bf8b6fe440565cc37538ca7ef029651fb61 Reviewed-by: Sergey Belyashov --- src/serialport/qserialportinfo_unix.cpp | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/serialport/qserialportinfo_unix.cpp b/src/serialport/qserialportinfo_unix.cpp index 0ed8703..9a146de 100644 --- a/src/serialport/qserialportinfo_unix.cpp +++ b/src/serialport/qserialportinfo_unix.cpp @@ -169,6 +169,10 @@ QList availablePortsBySysfs() if (manufacturer.open(QIODevice::ReadOnly | QIODevice::Text)) serialPortInfo.d_ptr->manufacturer = QString::fromLatin1(manufacturer.readAll()).simplified(); + QFile serialNumber(QFileInfo(targetDir, QStringLiteral("serial")).absoluteFilePath()); + if (serialNumber.open(QIODevice::ReadOnly | QIODevice::Text)) + serialPortInfo.d_ptr->serialNumber = QString::fromLatin1(serialNumber.readAll()).simplified(); + QFile vendorIdentifier(QFileInfo(targetDir, QStringLiteral("idVendor")).absoluteFilePath()); if (vendorIdentifier.open(QIODevice::ReadOnly | QIODevice::Text)) { serialPortInfo.d_ptr->vendorIdentifier = QString::fromLatin1(vendorIdentifier.readAll()) -- cgit v1.2.1 From 738d20dbd952ccdf28187b64526733aab345031f Mon Sep 17 00:00:00 2001 From: Dyami Caliri Date: Wed, 24 Sep 2014 15:07:01 -0700 Subject: Fix access after delete in OS X QSPI Valgrind analysis shows that searchStringProperty was accessing memory after deletion. This was caused by constructing QCFString local var with CFStringRef, which does not retain object. Change-Id: I1e9571a5051aa6f0c3fbc732ac01e821069f8b02 Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_mac.cpp | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/src/serialport/qserialportinfo_mac.cpp b/src/serialport/qserialportinfo_mac.cpp index 03f95e7..865c3bf 100644 --- a/src/serialport/qserialportinfo_mac.cpp +++ b/src/serialport/qserialportinfo_mac.cpp @@ -72,8 +72,7 @@ static QCFType searchProperty(io_registry_entry_t ioRegistryEntry, static QString searchStringProperty(io_registry_entry_t ioRegistryEntry, const QCFString &propertyKey) { - const QCFString result(searchProperty(ioRegistryEntry, propertyKey).as()); - return QCFString::toQString(result); + return QCFString::toQString(searchProperty(ioRegistryEntry, propertyKey).as()); } static quint16 searchShortIntProperty(io_registry_entry_t ioRegistryEntry, -- cgit v1.2.1 From 54a07b350895b4fdd4143ac2ce9d2d99d86bba42 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Sat, 27 Sep 2014 23:22:04 +0400 Subject: Fix order of passed parameters into QCOMPARE According to documentation the QCOMPARE should accept to the first parameter an actual value, and to the second parameter an expected value. Change-Id: I64e762e779fa6a61401f358c4dd6097dacf7a33a Reviewed-by: Sergey Belyashov --- tests/auto/qserialport/tst_qserialport.cpp | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp index af767dd..9feccd4 100644 --- a/tests/auto/qserialport/tst_qserialport.cpp +++ b/tests/auto/qserialport/tst_qserialport.cpp @@ -470,27 +470,27 @@ void tst_QSerialPort::twoStageSynchronousLoopback() senderPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); receiverPort.waitForReadyRead(waitMsecs); - QCOMPARE(qint64(newlineArray.size()), receiverPort.bytesAvailable()); + QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); receiverPort.write(receiverPort.readAll()); receiverPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); senderPort.waitForReadyRead(waitMsecs); - QCOMPARE(qint64(newlineArray.size()), receiverPort.bytesAvailable()); - QCOMPARE(newlineArray, senderPort.readAll()); + QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); + QCOMPARE(senderPort.readAll(), newlineArray); // second stage senderPort.write(newlineArray); senderPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); receiverPort.waitForReadyRead(waitMsecs); - QCOMPARE(qint64(newlineArray.size()), receiverPort.bytesAvailable()); + QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); receiverPort.write(receiverPort.readAll()); receiverPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); senderPort.waitForReadyRead(waitMsecs); - QCOMPARE(qint64(newlineArray.size()), receiverPort.bytesAvailable()); - QCOMPARE(newlineArray, senderPort.readAll()); + QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); + QCOMPARE(senderPort.readAll(), newlineArray); } void tst_QSerialPort::synchronousReadWrite() @@ -516,7 +516,7 @@ void tst_QSerialPort::synchronousReadWrite() while ((readData.size() < writeData.size()) && receiverPort.waitForReadyRead(100)) readData.append(receiverPort.readAll()); - QCOMPARE(writeData, readData); + QCOMPARE(readData, writeData); } class AsyncReader : public QObject -- cgit v1.2.1 From 3a967ff35979196c8c97a066b5aa68c1b5d13494 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Sun, 28 Sep 2014 00:00:22 +0400 Subject: Fix typo in twoStageSynchronousLoopback test Commit f1761c1236edce428278f7a9e8aa1091097eaa57 introduce a typo for receiverPort and senderPort variables, that lead to failure of test. Change-Id: I18ecad78bb11d7bb218a674c3c3bf9c863a2b33c Reviewed-by: Samuel Gaist Reviewed-by: Sergey Belyashov --- tests/auto/qserialport/tst_qserialport.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp index 9feccd4..2b8de92 100644 --- a/tests/auto/qserialport/tst_qserialport.cpp +++ b/tests/auto/qserialport/tst_qserialport.cpp @@ -476,7 +476,7 @@ void tst_QSerialPort::twoStageSynchronousLoopback() receiverPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); senderPort.waitForReadyRead(waitMsecs); - QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); + QCOMPARE(senderPort.bytesAvailable(), qint64(newlineArray.size())); QCOMPARE(senderPort.readAll(), newlineArray); // second stage @@ -489,7 +489,7 @@ void tst_QSerialPort::twoStageSynchronousLoopback() receiverPort.waitForBytesWritten(waitMsecs); QTest::qSleep(waitMsecs); senderPort.waitForReadyRead(waitMsecs); - QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); + QCOMPARE(senderPort.bytesAvailable(), qint64(newlineArray.size())); QCOMPARE(senderPort.readAll(), newlineArray); } -- cgit v1.2.1 From 2787010bbbc6d7350b49c879568da4b23e06d0bd Mon Sep 17 00:00:00 2001 From: Rainer Keller Date: Fri, 26 Sep 2014 14:42:20 +0200 Subject: Handle onboard serial ports when parsing sysfs The device detection stopped if one of the sources reported at least one device. This made onboard serial ports not listed if a USB device was plugged in. Onboard serial ports are now detected and added to the list of devices. Change-Id: I8798e7f14073e19d9e206eb4d7cdd0b28bd8a0bb Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_unix.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/src/serialport/qserialportinfo_unix.cpp b/src/serialport/qserialportinfo_unix.cpp index 9a146de..9ff4431 100644 --- a/src/serialport/qserialportinfo_unix.cpp +++ b/src/serialport/qserialportinfo_unix.cpp @@ -202,6 +202,9 @@ QList availablePortsBySysfs() .toInt(&serialPortInfo.d_ptr->hasProductIdentifier, 16); } // TODO: Obtain more information about the device + } else if (targetPath.contains(QStringLiteral(".serial/tty/tty"))) { + // This condition matches onboard serial port on embedded devices. + // Keep those devices in the list } else { continue; } -- cgit v1.2.1 From dfdebc849400be43396b6600ac4c910e36869989 Mon Sep 17 00:00:00 2001 From: Sergey Belyashov Date: Fri, 26 Sep 2014 16:00:39 +0400 Subject: Fix segfault related to dynamic udev loading Problem was caused by conflict of libudev symbols and function pointers declared in qtudev_p.h. Task-number: QTBUG-40113 Change-Id: I599575e8a1b9ffe32295331d46d991422975f773 Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_unix.cpp | 7 +------ src/serialport/qtudev_p.h | 2 +- 2 files changed, 2 insertions(+), 7 deletions(-) diff --git a/src/serialport/qserialportinfo_unix.cpp b/src/serialport/qserialportinfo_unix.cpp index 9ff4431..0405a41 100644 --- a/src/serialport/qserialportinfo_unix.cpp +++ b/src/serialport/qserialportinfo_unix.cpp @@ -360,12 +360,7 @@ QList availablePortsByUdev() QList QSerialPortInfo::availablePorts() { - QList serialPortInfoList; - // TODO: Remove this condition once the udev runtime symbol resolution crash - // is fixed for Qt 4. -#if defined(LINK_LIBUDEV) || (QT_VERSION >= QT_VERSION_CHECK(5, 0, 0)) - serialPortInfoList = availablePortsByUdev(); -#endif + QList serialPortInfoList = availablePortsByUdev(); #ifdef Q_OS_LINUX if (serialPortInfoList.isEmpty()) diff --git a/src/serialport/qtudev_p.h b/src/serialport/qtudev_p.h index 09940ab..bd9e74d 100644 --- a/src/serialport/qtudev_p.h +++ b/src/serialport/qtudev_p.h @@ -65,7 +65,7 @@ extern "C" #define GENERATE_SYMBOL_VARIABLE(returnType, symbolName, ...) \ typedef returnType (*fp_##symbolName)(__VA_ARGS__); \ - fp_##symbolName symbolName; + static fp_##symbolName symbolName; #define RESOLVE_SYMBOL(symbolName) \ symbolName = (fp_##symbolName)resolveSymbol(udevLibrary, #symbolName); \ -- cgit v1.2.1 From 26d61928f5a50c74f843cd88276c5b18e1bed375 Mon Sep 17 00:00:00 2001 From: Manuele Conti Date: Wed, 1 Oct 2014 13:13:11 +0200 Subject: Remove custom baud rate filtering Some serial device have baud base that it is not multiple of baud rate selected, so we need to allow baud rate compliant with device. Change-Id: I7d3ce94f10d4382a29ff34bb18daebb650186c1c Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialport_unix.cpp | 10 +++++++--- 1 file changed, 7 insertions(+), 3 deletions(-) diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp index d555805..e80c0e8 100644 --- a/src/serialport/qserialport_unix.cpp +++ b/src/serialport/qserialport_unix.cpp @@ -504,9 +504,6 @@ QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions d if (::ioctl(descriptor, TIOCGSERIAL, ¤tSerialInfo) == -1) return decodeSystemError(); - if (currentSerialInfo.baud_base % baudRate != 0) - return QSerialPort::UnsupportedOperationError; - currentSerialInfo.flags &= ~ASYNC_SPD_MASK; currentSerialInfo.flags |= (ASYNC_SPD_CUST /* | ASYNC_LOW_LATENCY*/); currentSerialInfo.custom_divisor = currentSerialInfo.baud_base / baudRate; @@ -514,6 +511,13 @@ QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions d if (currentSerialInfo.custom_divisor == 0) return QSerialPort::UnsupportedOperationError; + if (currentSerialInfo.custom_divisor * baudRate != currentSerialInfo.baud_base) { + qWarning("Baud rate of serial port %s is set to %d instead of %d: divisor %f unsupported", + qPrintable(systemLocation), + currentSerialInfo.baud_base / currentSerialInfo.custom_divisor, + baudRate, (float)currentSerialInfo.baud_base / baudRate); + } + if (::ioctl(descriptor, TIOCSSERIAL, ¤tSerialInfo) == -1) return decodeSystemError(); -- cgit v1.2.1 From ac0422e8c9e74f2275129e3c7c69ef64299f07a9 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Mon, 29 Sep 2014 18:44:36 +0400 Subject: Fix reading on Windows at limited read buffer size In case the read buffer has a limited size then are impossible to read remainder which is still can be in driver's queue, since no readyRead signal emmitted and reading are stalled. Problem is that Windows does not fire the EV_RXCHAR event in case a driver's queue has ready to read remainder; this event will be triggered only when a new data are received. The solution is to start of asynchronous read operation for reading of possible remainder from the queue after doing QSP::read() from the user. Besides is necessary to meet conditions: - do not start reading in case a reading already is started - do not start reading in case is not in limited buffer size - do not start reading in case is a previous reading returns a less data than read buffer size or are not in the hardware flow control mode Tested on Windows 8 with virtual com0com serial ports using Qt5 and then Qt4. Task-number: QTBUG-41295 Change-Id: I01797e6f8d6006751244144fead3616b1de1b811 Reviewed-by: Robert Kurjata Reviewed-by: Sergey Belyashov --- src/serialport/qserialport.cpp | 2 +- src/serialport/qserialport_unix.cpp | 5 +++++ src/serialport/qserialport_unix_p.h | 1 + src/serialport/qserialport_win.cpp | 35 +++++++++++++++++++++++++++--- src/serialport/qserialport_win_p.h | 2 ++ src/serialport/qserialport_wince.cpp | 5 +++++ src/serialport/qserialport_wince_p.h | 1 + tests/auto/qserialport/tst_qserialport.cpp | 35 ++++++++++++++++++++++++++++++ 8 files changed, 82 insertions(+), 4 deletions(-) diff --git a/src/serialport/qserialport.cpp b/src/serialport/qserialport.cpp index ffc763e..c815dd0 100644 --- a/src/serialport/qserialport.cpp +++ b/src/serialport/qserialport.cpp @@ -1343,7 +1343,7 @@ bool QSerialPort::setBreakEnabled(bool set) qint64 QSerialPort::readData(char *data, qint64 maxSize) { Q_D(QSerialPort); - return d->readBuffer.read(data, maxSize); + return d->readData(data, maxSize); } /*! diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp index e80c0e8..b082275 100644 --- a/src/serialport/qserialport_unix.cpp +++ b/src/serialport/qserialport_unix.cpp @@ -383,6 +383,11 @@ void QSerialPortPrivate::startWriting() setWriteNotificationEnabled(true); } +qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) +{ + return readBuffer.read(data, maxSize); +} + bool QSerialPortPrivate::waitForReadyRead(int msecs) { Q_Q(QSerialPort); diff --git a/src/serialport/qserialport_unix_p.h b/src/serialport/qserialport_unix_p.h index 1213c30..408fdc5 100644 --- a/src/serialport/qserialport_unix_p.h +++ b/src/serialport/qserialport_unix_p.h @@ -120,6 +120,7 @@ public: bool setBreakEnabled(bool set); void startWriting(); + qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msecs); bool waitForBytesWritten(int msecs); diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp index 18cef1e..3174db5 100644 --- a/src/serialport/qserialport_win.cpp +++ b/src/serialport/qserialport_win.cpp @@ -98,6 +98,7 @@ QSerialPortPrivate::QSerialPortPrivate(QSerialPort *q) , readChunkBuffer(ReadChunkSize, 0) , readyReadEmitted(0) , writeStarted(false) + , readStarted(false) , communicationNotifier(new QWinEventNotifier(q)) , readCompletionNotifier(new QWinEventNotifier(q)) , writeCompletionNotifier(new QWinEventNotifier(q)) @@ -173,6 +174,7 @@ void QSerialPortPrivate::close() writeCompletionNotifier->setEnabled(false); communicationNotifier->setEnabled(false); + readStarted = false; readBuffer.clear(); writeStarted = false; @@ -267,8 +269,10 @@ bool QSerialPortPrivate::clear(QSerialPort::Directions directions) Q_Q(QSerialPort); DWORD flags = 0; - if (directions & QSerialPort::Input) + if (directions & QSerialPort::Input) { flags |= PURGE_RXABORT | PURGE_RXCLEAR; + readStarted = false; + } if (directions & QSerialPort::Output) { flags |= PURGE_TXABORT | PURGE_TXCLEAR; writeStarted = false; @@ -320,6 +324,21 @@ void QSerialPortPrivate::startWriting() } } +qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) +{ + const qint64 result = readBuffer.read(data, maxSize); + // We need try to start async reading to read a remainder from a driver's queue + // in case we have a limited read buffer size. Because the read notification can + // be stalled since Windows do not re-triggered an EV_RXCHAR event if a driver's + // buffer has a remainder of data ready to read until a new data will be received. + if (readBufferMaxSize + && result > 0 + && (result == readBufferMaxSize || flowControl == QSerialPort::HardwareControl)) { + startAsyncRead(); + } + return result; +} + bool QSerialPortPrivate::waitForReadyRead(int msecs) { Q_Q(QSerialPort); @@ -523,14 +542,18 @@ bool QSerialPortPrivate::_q_completeAsyncCommunication() bool QSerialPortPrivate::_q_completeAsyncRead() { const qint64 bytesTransferred = handleOverlappedResult(QSerialPort::Input, readCompletionOverlapped); - if (bytesTransferred == qint64(-1)) + if (bytesTransferred == qint64(-1)) { + readStarted = false; return false; + } if (bytesTransferred > 0) { readBuffer.append(readChunkBuffer.left(bytesTransferred)); if (!emulateErrorPolicy()) emitReadyRead(); } + readStarted = false; + // start async read for possible remainder into driver queue if ((bytesTransferred == ReadChunkSize) && (policy == QSerialPort::IgnorePolicy)) return startAsyncRead(); @@ -578,6 +601,9 @@ bool QSerialPortPrivate::startAsyncRead() { Q_Q(QSerialPort); + if (readStarted) + return true; + DWORD bytesToRead = policy == QSerialPort::IgnorePolicy ? ReadChunkSize : 1; if (readBufferMaxSize && bytesToRead > (readBufferMaxSize - readBuffer.size())) { @@ -590,8 +616,10 @@ bool QSerialPortPrivate::startAsyncRead() } initializeOverlappedStructure(readCompletionOverlapped); - if (::ReadFile(handle, readChunkBuffer.data(), bytesToRead, NULL, &readCompletionOverlapped)) + if (::ReadFile(handle, readChunkBuffer.data(), bytesToRead, NULL, &readCompletionOverlapped)) { + readStarted = true; return true; + } QSerialPort::SerialPortError error = decodeSystemError(); if (error != QSerialPort::NoError) { @@ -603,6 +631,7 @@ bool QSerialPortPrivate::startAsyncRead() return false; } + readStarted = true; return true; } diff --git a/src/serialport/qserialport_win_p.h b/src/serialport/qserialport_win_p.h index cf30c18..8db3a74 100644 --- a/src/serialport/qserialport_win_p.h +++ b/src/serialport/qserialport_win_p.h @@ -87,6 +87,7 @@ public: bool setBreakEnabled(bool set); void startWriting(); + qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msec); bool waitForBytesWritten(int msec); @@ -130,6 +131,7 @@ public: QByteArray readChunkBuffer; bool readyReadEmitted; bool writeStarted; + bool readStarted; QWinEventNotifier *communicationNotifier; QWinEventNotifier *readCompletionNotifier; QWinEventNotifier *writeCompletionNotifier; diff --git a/src/serialport/qserialport_wince.cpp b/src/serialport/qserialport_wince.cpp index c06e748..d6c58aa 100644 --- a/src/serialport/qserialport_wince.cpp +++ b/src/serialport/qserialport_wince.cpp @@ -351,6 +351,11 @@ void QSerialPortPrivate::startWriting() notifyWrite(); } +qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) +{ + return readBuffer.read(data, maxSize); +} + bool QSerialPortPrivate::waitForReadyRead(int msec) { if (!readBuffer.isEmpty()) diff --git a/src/serialport/qserialport_wince_p.h b/src/serialport/qserialport_wince_p.h index 91bba3c..be08bc1 100644 --- a/src/serialport/qserialport_wince_p.h +++ b/src/serialport/qserialport_wince_p.h @@ -86,6 +86,7 @@ public: bool setBreakEnabled(bool set); void startWriting(); + qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msec); bool waitForBytesWritten(int msec); diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp index 2b8de92..0fdde48 100644 --- a/tests/auto/qserialport/tst_qserialport.cpp +++ b/tests/auto/qserialport/tst_qserialport.cpp @@ -115,6 +115,10 @@ private slots: void asynchronousWriteByTimer_data(); void asynchronousWriteByTimer(); +#ifdef Q_OS_WIN + void readBufferOverflow(); +#endif + protected slots: void handleBytesWrittenAndExitLoopSlot(qint64 bytesWritten); void handleBytesWrittenAndExitLoopSlot2(qint64 bytesWritten); @@ -667,5 +671,36 @@ void tst_QSerialPort::asynchronousWriteByTimer() QCOMPARE(receiverPort.readAll(), alphabetArray); } +#ifdef Q_OS_WIN +void tst_QSerialPort::readBufferOverflow() +{ + clearReceiver(); + + QSerialPort senderPort(m_senderPortName); + QVERIFY(senderPort.open(QSerialPort::WriteOnly)); + + QSerialPort receiverPort(m_receiverPortName); + QVERIFY(receiverPort.open(QSerialPort::ReadOnly)); + + const int readBufferSize = alphabetArray.size() / 2; + receiverPort.setReadBufferSize(readBufferSize); + QCOMPARE(receiverPort.readBufferSize(), qint64(readBufferSize)); + + QCOMPARE(senderPort.write(alphabetArray), qint64(alphabetArray.size())); + QVERIFY2(senderPort.waitForBytesWritten(100), "Waiting for bytes written failed"); + + QByteArray readData; + while (receiverPort.waitForReadyRead(100)) { + QVERIFY(receiverPort.bytesAvailable() > 0); + readData += receiverPort.readAll(); + } + + QCOMPARE(readData, alphabetArray); + + // No more bytes available + QVERIFY(receiverPort.bytesAvailable() == 0); +} +#endif + QTEST_MAIN(tst_QSerialPort) #include "tst_qserialport.moc" -- cgit v1.2.1 From 9c88ad89801596e1d94acc4f32ff55c34118a66f Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Fri, 3 Oct 2014 13:21:17 +0400 Subject: Restart of async reading when a QSP::clear() is called The method QSP::clear() can stall reading in case of following situations: - at the moment when PurgeComm abort of previously started asynchronous reading operation - when a serial port is in hardware flow control mode - when a serial port has a limited read buffer size Therefore is necessary restart of asynchronous reading to enable of the read sequence. Change-Id: I7a722a1ee20ecba0dd631da96ca81d2937d7ca6b Reviewed-by: Robert Kurjata Reviewed-by: Sergey Belyashov --- src/serialport/qserialport_win.cpp | 6 +++++ tests/auto/qserialport/tst_qserialport.cpp | 42 ++++++++++++++++++++++++++++++ 2 files changed, 48 insertions(+) diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp index 3174db5..84d1db7 100644 --- a/src/serialport/qserialport_win.cpp +++ b/src/serialport/qserialport_win.cpp @@ -282,6 +282,12 @@ bool QSerialPortPrivate::clear(QSerialPort::Directions directions) return false; } + // We need start async read because a reading can be stalled. Since the + // PurgeComm can abort of current reading sequence, or a port is in hardware + // flow control mode, or a port has a limited read buffer size. + if (directions & QSerialPort::Input) + startAsyncRead(); + return true; } diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp index 0fdde48..a7dc5f3 100644 --- a/tests/auto/qserialport/tst_qserialport.cpp +++ b/tests/auto/qserialport/tst_qserialport.cpp @@ -117,6 +117,7 @@ private slots: #ifdef Q_OS_WIN void readBufferOverflow(); + void readAfterInputClear(); #endif protected slots: @@ -700,6 +701,47 @@ void tst_QSerialPort::readBufferOverflow() // No more bytes available QVERIFY(receiverPort.bytesAvailable() == 0); } + +void tst_QSerialPort::readAfterInputClear() +{ + clearReceiver(); + + QSerialPort senderPort(m_senderPortName); + QVERIFY(senderPort.open(QSerialPort::WriteOnly)); + + QSerialPort receiverPort(m_receiverPortName); + QVERIFY(receiverPort.open(QSerialPort::ReadOnly)); + + const int readBufferSize = alphabetArray.size() / 2; + receiverPort.setReadBufferSize(readBufferSize); + QCOMPARE(receiverPort.readBufferSize(), qint64(readBufferSize)); + + const int waitMsecs = 100; + + // First write more than read buffer size + QCOMPARE(senderPort.write(alphabetArray), qint64(alphabetArray.size())); + QVERIFY2(senderPort.waitForBytesWritten(waitMsecs), "Waiting for bytes written failed"); + + // Wait for first part of data into read buffer + while (receiverPort.waitForReadyRead(waitMsecs)); + QCOMPARE(receiverPort.bytesAvailable(), qint64(readBufferSize)); + // Wait for second part of data into driver's FIFO + QTest::qSleep(waitMsecs); + + QVERIFY(receiverPort.clear(QSerialPort::Input)); + QCOMPARE(receiverPort.bytesAvailable(), qint64(0)); + + // Second write less than read buffer size + QCOMPARE(senderPort.write(newlineArray), qint64(newlineArray.size())); + QVERIFY2(senderPort.waitForBytesWritten(waitMsecs), "Waiting for bytes written failed"); + + while (receiverPort.waitForReadyRead(waitMsecs)); + QCOMPARE(receiverPort.bytesAvailable(), qint64(newlineArray.size())); + QCOMPARE(receiverPort.readAll(), newlineArray); + + // No more bytes available + QVERIFY(receiverPort.bytesAvailable() == 0); +} #endif QTEST_MAIN(tst_QSerialPort) -- cgit v1.2.1 From d9a29ff7cc479e8a9e6d418de6408866f0dce4b6 Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Tue, 30 Sep 2014 23:00:03 +0400 Subject: Allow for deviceInstanceIdentifier() to return an upper case string Change-Id: I91977b1aa4a8e5bd8321efc5cfda375c9d7deff7 Reviewed-by: Sergey Belyashov Reviewed-by: Denis Shienkov --- src/serialport/qserialportinfo_win.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/serialport/qserialportinfo_win.cpp b/src/serialport/qserialportinfo_win.cpp index 8250d2a..d41c196 100644 --- a/src/serialport/qserialportinfo_win.cpp +++ b/src/serialport/qserialportinfo_win.cpp @@ -146,7 +146,7 @@ static QString deviceInstanceIdentifier(DEVINST deviceInstanceNumber) outputBuffer.size(), 0) != CR_SUCCESS) { return QString(); } - return toStringAndTrimNullCharacter(outputBuffer); + return toStringAndTrimNullCharacter(outputBuffer).toUpper(); } static DEVINST parentDeviceInstanceNumber(DEVINST childDeviceInstanceNumber) @@ -290,7 +290,7 @@ static QString deviceSerialNumber(const QString &instanceIdentifier, QString result = parseDeviceSerialNumber(instanceIdentifier); if (result.isEmpty()) { const DEVINST parentNumber = parentDeviceInstanceNumber(deviceInstanceNumber); - const QString parentInstanceIdentifier = deviceInstanceIdentifier(parentNumber).toUpper(); + const QString parentInstanceIdentifier = deviceInstanceIdentifier(parentNumber); result = parseDeviceSerialNumber(parentInstanceIdentifier); } return result; @@ -327,7 +327,7 @@ QList QSerialPortInfo::availablePorts() serialPortInfo.d_ptr->description = deviceDescription(deviceInfoSet, &deviceInfoData); serialPortInfo.d_ptr->manufacturer = deviceManufacturer(deviceInfoSet, &deviceInfoData); - const QString instanceIdentifier = deviceInstanceIdentifier(deviceInfoData.DevInst).toUpper(); + const QString instanceIdentifier = deviceInstanceIdentifier(deviceInfoData.DevInst); serialPortInfo.d_ptr->serialNumber = deviceSerialNumber(instanceIdentifier, deviceInfoData.DevInst); -- cgit v1.2.1 From 62dfdeb3642250bdb642dbf607a8c7b95e57835e Mon Sep 17 00:00:00 2001 From: Denis Shienkov Date: Sat, 19 Jul 2014 19:25:19 +0400 Subject: Fix QSP::bytesToWrite() on Windows After calling the WriteFile() function, QSP::bytesToWrite() should return zero, but did return the size of writeBuffer. On Windows, we must not to cut a size of the writeBuffer until the asynchronous write operation has completed. So we need to make use of an additional actualBytesToWrite variable, the value of which is increased when new data is added to the writeBuffer, and decreased after calling the WriteFile() function. This change also entails the modification of the QSP::writeData() method and deleting the QSP::startWriting() method. Now all platform-dependent code (related to startWriting(), and to copying of memory to writeBuffer) resides in the new QSP::writeData() method. But this modification does not change the behavior on platforms other than Windows. Tested on Windows with the virtual com0com serial ports using Qt5. Change-Id: I35c1428ad374c0709d6c352a93c552898e947bde Reviewed-by: Oswald Buddenhagen Reviewed-by: Sergey Belyashov --- src/serialport/qserialport.cpp | 8 ++----- src/serialport/qserialport_unix.cpp | 19 ++++++++++----- src/serialport/qserialport_unix_p.h | 4 +++- src/serialport/qserialport_win.cpp | 45 ++++++++++++++++++++++++------------ src/serialport/qserialport_win_p.h | 5 +++- src/serialport/qserialport_wince.cpp | 19 ++++++++++----- src/serialport/qserialport_wince_p.h | 4 +++- 7 files changed, 68 insertions(+), 36 deletions(-) diff --git a/src/serialport/qserialport.cpp b/src/serialport/qserialport.cpp index c815dd0..1bffba0 100644 --- a/src/serialport/qserialport.cpp +++ b/src/serialport/qserialport.cpp @@ -1226,7 +1226,7 @@ qint64 QSerialPort::bytesAvailable() const qint64 QSerialPort::bytesToWrite() const { Q_D(const QSerialPort); - return d->writeBuffer.size() + QIODevice::bytesToWrite(); + return d->bytesToWrite() + QIODevice::bytesToWrite(); } /*! @@ -1360,11 +1360,7 @@ qint64 QSerialPort::readLineData(char *data, qint64 maxSize) qint64 QSerialPort::writeData(const char *data, qint64 maxSize) { Q_D(QSerialPort); - - ::memcpy(d->writeBuffer.reserve(maxSize), data, maxSize); - if (!d->writeBuffer.isEmpty()) - d->startWriting(); - return maxSize; + return d->writeData(data, maxSize); } void QSerialPort::setError(QSerialPort::SerialPortError serialPortError, const QString &errorString) diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp index b082275..f3ddc4f 100644 --- a/src/serialport/qserialport_unix.cpp +++ b/src/serialport/qserialport_unix.cpp @@ -377,12 +377,6 @@ bool QSerialPortPrivate::setBreakEnabled(bool set) return true; } -void QSerialPortPrivate::startWriting() -{ - if (!isWriteNotificationEnabled()) - setWriteNotificationEnabled(true); -} - qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) { return readBuffer.read(data, maxSize); @@ -885,6 +879,19 @@ inline bool QSerialPortPrivate::initialize(QIODevice::OpenMode mode) return true; } +qint64 QSerialPortPrivate::bytesToWrite() const +{ + return writeBuffer.size(); +} + +qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize) +{ + ::memcpy(writeBuffer.reserve(maxSize), data, maxSize); + if (!writeBuffer.isEmpty() && !isWriteNotificationEnabled()) + setWriteNotificationEnabled(true); + return maxSize; +} + bool QSerialPortPrivate::updateTermios() { Q_Q(QSerialPort); diff --git a/src/serialport/qserialport_unix_p.h b/src/serialport/qserialport_unix_p.h index 408fdc5..14e3cea 100644 --- a/src/serialport/qserialport_unix_p.h +++ b/src/serialport/qserialport_unix_p.h @@ -119,7 +119,6 @@ public: bool sendBreak(int duration); bool setBreakEnabled(bool set); - void startWriting(); qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msecs); @@ -137,6 +136,9 @@ public: bool startAsyncWrite(); bool completeAsyncWrite(); + qint64 bytesToWrite() const; + qint64 writeData(const char *data, qint64 maxSize); + static QString portNameToSystemLocation(const QString &port); static QString portNameFromSystemLocation(const QString &location); diff --git a/src/serialport/qserialport_win.cpp b/src/serialport/qserialport_win.cpp index 84d1db7..9635325 100644 --- a/src/serialport/qserialport_win.cpp +++ b/src/serialport/qserialport_win.cpp @@ -105,6 +105,7 @@ QSerialPortPrivate::QSerialPortPrivate(QSerialPort *q) , startAsyncWriteTimer(0) , originalEventMask(0) , triggeredEventMask(0) + , actualBytesToWrite(0) { ::ZeroMemory(&communicationOverlapped, sizeof(communicationOverlapped)); communicationOverlapped.hEvent = ::CreateEvent(NULL, FALSE, FALSE, NULL); @@ -179,6 +180,7 @@ void QSerialPortPrivate::close() writeStarted = false; writeBuffer.clear(); + actualBytesToWrite = 0; readyReadEmitted = false; parityErrorOccurred = false; @@ -276,6 +278,7 @@ bool QSerialPortPrivate::clear(QSerialPort::Directions directions) if (directions & QSerialPort::Output) { flags |= PURGE_TXABORT | PURGE_TXCLEAR; writeStarted = false; + actualBytesToWrite = 0; } if (!::PurgeComm(handle, flags)) { q->setError(decodeSystemError()); @@ -316,20 +319,6 @@ bool QSerialPortPrivate::setBreakEnabled(bool set) return true; } -void QSerialPortPrivate::startWriting() -{ - Q_Q(QSerialPort); - - if (!writeStarted) { - if (!startAsyncWriteTimer) { - startAsyncWriteTimer = new QTimer(q); - q->connect(startAsyncWriteTimer, SIGNAL(timeout()), q, SLOT(_q_startAsyncWrite())); - startAsyncWriteTimer->setSingleShot(true); - } - startAsyncWriteTimer->start(0); - } -} - qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) { const qint64 result = readBuffer.read(data, maxSize); @@ -649,8 +638,10 @@ bool QSerialPortPrivate::_q_startAsyncWrite() return true; initializeOverlappedStructure(writeCompletionOverlapped); + + const int writeBytes = writeBuffer.nextDataBlockSize(); if (!::WriteFile(handle, writeBuffer.readPointer(), - writeBuffer.nextDataBlockSize(), + writeBytes, NULL, &writeCompletionOverlapped)) { QSerialPort::SerialPortError error = decodeSystemError(); @@ -662,6 +653,7 @@ bool QSerialPortPrivate::_q_startAsyncWrite() } } + actualBytesToWrite -= writeBytes; writeStarted = true; return true; } @@ -702,6 +694,29 @@ void QSerialPortPrivate::emitReadyRead() emit q->readyRead(); } +qint64 QSerialPortPrivate::bytesToWrite() const +{ + return actualBytesToWrite; +} + +qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize) +{ + Q_Q(QSerialPort); + + ::memcpy(writeBuffer.reserve(maxSize), data, maxSize); + actualBytesToWrite += maxSize; + + if (!writeBuffer.isEmpty() && !writeStarted) { + if (!startAsyncWriteTimer) { + startAsyncWriteTimer = new QTimer(q); + q->connect(startAsyncWriteTimer, SIGNAL(timeout()), q, SLOT(_q_completeAsyncWrite())); + startAsyncWriteTimer->setSingleShot(true); + } + startAsyncWriteTimer->start(0); + } + return maxSize; +} + void QSerialPortPrivate::handleLineStatusErrors() { Q_Q(QSerialPort); diff --git a/src/serialport/qserialport_win_p.h b/src/serialport/qserialport_win_p.h index 8db3a74..c6a8330 100644 --- a/src/serialport/qserialport_win_p.h +++ b/src/serialport/qserialport_win_p.h @@ -86,7 +86,6 @@ public: bool sendBreak(int duration); bool setBreakEnabled(bool set); - void startWriting(); qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msec); @@ -114,6 +113,9 @@ public: bool emulateErrorPolicy(); void emitReadyRead(); + qint64 bytesToWrite() const; + qint64 writeData(const char *data, qint64 maxSize); + static QString portNameToSystemLocation(const QString &port); static QString portNameFromSystemLocation(const QString &location); @@ -141,6 +143,7 @@ public: OVERLAPPED writeCompletionOverlapped; DWORD originalEventMask; DWORD triggeredEventMask; + qint64 actualBytesToWrite; private: bool initialize(QIODevice::OpenMode mode); diff --git a/src/serialport/qserialport_wince.cpp b/src/serialport/qserialport_wince.cpp index d6c58aa..0a442dd 100644 --- a/src/serialport/qserialport_wince.cpp +++ b/src/serialport/qserialport_wince.cpp @@ -345,12 +345,6 @@ bool QSerialPortPrivate::setBreakEnabled(bool set) return true; } -void QSerialPortPrivate::startWriting() -{ - // trigger write sequence - notifyWrite(); -} - qint64 QSerialPortPrivate::readData(char *data, qint64 maxSize) { return readBuffer.read(data, maxSize); @@ -592,6 +586,19 @@ bool QSerialPortPrivate::notifyWrite() return true; } +qint64 QSerialPortPrivate::bytesToWrite() const +{ + return writeBuffer.size(); +} + +qint64 QSerialPortPrivate::writeData(const char *data, qint64 maxSize) +{ + ::memcpy(writeBuffer.reserve(maxSize), data, maxSize); + if (!writeBuffer.isEmpty()) + notifyWrite(); + return maxSize; +} + void QSerialPortPrivate::processIoErrors(bool error) { Q_Q(QSerialPort); diff --git a/src/serialport/qserialport_wince_p.h b/src/serialport/qserialport_wince_p.h index be08bc1..d212d3a 100644 --- a/src/serialport/qserialport_wince_p.h +++ b/src/serialport/qserialport_wince_p.h @@ -85,7 +85,6 @@ public: bool sendBreak(int duration); bool setBreakEnabled(bool set); - void startWriting(); qint64 readData(char *data, qint64 maxSize); bool waitForReadyRead(int msec); @@ -105,6 +104,9 @@ public: bool notifyRead(); bool notifyWrite(); + qint64 bytesToWrite() const; + qint64 writeData(const char *data, qint64 maxSize); + static QString portNameToSystemLocation(const QString &port); static QString portNameFromSystemLocation(const QString &location); -- cgit v1.2.1