Refactor the speed configuration for *nix

The internal implementation of this is a little tangled. It is
reasonable to simplify it:

1. Remove the setBaudRate_helper() method, as it is equivalent to
   setStandardBaudRate() method.

2. Now the setStandardBaudRate() and the setCustomBaudRate() are
   self-sufficient methods (can setup a speed and an error codes
   internally) and return a boolean values.

Also added the "loopback" test for check of different speeds.

Tested with the on-board and the USB serial ports.

Change-Id: I4320c2e29ad42a394e07753cbaea804d0faf6b78
Reviewed-by: Sergey Belyashov <Sergey.Belyashov@gmail.com>
Reviewed-by: Denis Shienkov <denis.shienkov@gmail.com>

3 files changed, 98 insertions, 84 deletions

diff --git a/src/serialport/qserialport_p.h b/src/serialport/qserialport_p.h
index 070a316..cf8cb79 100644
--- a/src/serialport/qserialport_p.h
+++ b/src/serialport/qserialport_p.h
@@ -249,12 +249,8 @@ public:
     bool initialize(QIODevice::OpenMode mode);
     bool updateTermios();
 
-    QSerialPortErrorInfo setBaudRate_helper(qint32 baudRate,
-            QSerialPort::Directions directions);
-    QSerialPortErrorInfo setCustomBaudRate(qint32 baudRate,
-            QSerialPort::Directions directions);
-    QSerialPortErrorInfo setStandardBaudRate(qint32 baudRate,
-            QSerialPort::Directions directions);
+    bool setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions);
+    bool setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions);
 
     bool isReadNotificationEnabled() const;
     void setReadNotificationEnabled(bool enable);
diff --git a/src/serialport/qserialport_unix.cpp b/src/serialport/qserialport_unix.cpp
index b0f339a..87833da 100644
--- a/src/serialport/qserialport_unix.cpp
+++ b/src/serialport/qserialport_unix.cpp
@@ -409,121 +409,112 @@ bool QSerialPortPrivate::setBaudRate()
         && setBaudRate(outputBaudRate, QSerialPort::Output));
 }
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setBaudRate_helper(qint32 baudRate,
-        QSerialPort::Directions directions)
+bool QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
-    if ((directions & QSerialPort::Input) && ::cfsetispeed(&currentTermios, baudRate) < 0)
-        return getSystemError();
-
-    if ((directions & QSerialPort::Output) && ::cfsetospeed(&currentTermios, baudRate) < 0)
-        return getSystemError();
-
-    return QSerialPortErrorInfo(QSerialPort::NoError);
-}
-
-#if defined(Q_OS_LINUX)
-
-QSerialPortErrorInfo
-QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions)
-{
-    struct serial_struct currentSerialInfo;
-
-    ::memset(&currentSerialInfo, 0, sizeof(currentSerialInfo));
-
-    if ((::ioctl(descriptor, TIOCGSERIAL, &currentSerialInfo) != -1)
-            && (currentSerialInfo.flags & ASYNC_SPD_CUST)) {
-        currentSerialInfo.flags &= ~ASYNC_SPD_CUST;
-        currentSerialInfo.custom_divisor = 0;
-        if (::ioctl(descriptor, TIOCSSERIAL, &currentSerialInfo) == -1)
-            return getSystemError();
+#ifdef Q_OS_LINUX
+    // try to clear custom baud rate
+    struct serial_struct serial;
+    ::memset(&serial, 0, sizeof(serial));
+    if (::ioctl(descriptor, TIOCGSERIAL, &serial) != -1) {
+        if (serial.flags & ASYNC_SPD_CUST) {
+            serial.flags &= ~ASYNC_SPD_CUST;
+            serial.custom_divisor = 0;
+            // we don't check on errors because a driver can has not this feature
+            ::ioctl(descriptor, TIOCSSERIAL, &serial);
+        }
     }
+#endif
 
-    return setBaudRate_helper(baudRate, directions);
-}
+    if ((directions & QSerialPort::Input) && ::cfsetispeed(&currentTermios, baudRate) < 0) {
+        setError(getSystemError());
+        return false;
+    }
 
-#else
+    if ((directions & QSerialPort::Output) && ::cfsetospeed(&currentTermios, baudRate) < 0) {
+        setError(getSystemError());
+        return false;
+    }
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setStandardBaudRate(qint32 baudRate, QSerialPort::Directions directions)
-{
-    return setBaudRate_helper(baudRate, directions);
+    return updateTermios();
 }
 
-#endif
-
 #if defined(Q_OS_LINUX)
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
+bool QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
-    Q_UNUSED(directions);
-
-    struct serial_struct currentSerialInfo;
-
-    ::memset(&currentSerialInfo, 0, sizeof(currentSerialInfo));
+    struct serial_struct serial;
 
-    if (::ioctl(descriptor, TIOCGSERIAL, &currentSerialInfo) == -1)
-        return getSystemError();
+    if (::ioctl(descriptor, TIOCGSERIAL, &serial) == -1) {
+        setError(getSystemError());
+        return false;
+    }
 
-    currentSerialInfo.flags &= ~ASYNC_SPD_MASK;
-    currentSerialInfo.flags |= (ASYNC_SPD_CUST /* | ASYNC_LOW_LATENCY*/);
-    currentSerialInfo.custom_divisor = currentSerialInfo.baud_base / baudRate;
+    serial.flags &= ~ASYNC_SPD_MASK;
+    serial.flags |= (ASYNC_SPD_CUST /* | ASYNC_LOW_LATENCY*/);
+    serial.custom_divisor = serial.baud_base / baudRate;
 
-    if (currentSerialInfo.custom_divisor == 0)
-        return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError, QSerialPort::tr("No suitable custom baud rate divisor"));
+    if (serial.custom_divisor == 0) {
+        setError(QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError,
+                                      QSerialPort::tr("No suitable custom baud rate divisor")));
+        return false;
+    }
 
-    if (currentSerialInfo.custom_divisor * baudRate != currentSerialInfo.baud_base) {
+    if (serial.custom_divisor * baudRate != serial.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);
+            serial.baud_base / serial.custom_divisor,
+            baudRate, (float)serial.baud_base / baudRate);
     }
 
-    if (::ioctl(descriptor, TIOCSSERIAL, &currentSerialInfo) == -1)
-        return getSystemError();
+    if (::ioctl(descriptor, TIOCSSERIAL, &serial) == -1) {
+        setError(getSystemError());
+        return false;
+    }
 
-    return setBaudRate_helper(B38400, directions);
+    return setStandardBaudRate(B38400, directions);
 }
 
 #elif defined(Q_OS_MAC)
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
+bool QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     Q_UNUSED(directions);
 
 #if defined(MAC_OS_X_VERSION_10_4) && (MAC_OS_X_VERSION_MIN_REQUIRED >= MAC_OS_X_VERSION_10_4)
-    if (::ioctl(descriptor, IOSSIOSPEED, &baudRate) == -1)
-        return getSystemError();
+    if (::ioctl(descriptor, IOSSIOSPEED, &baudRate) == -1) {
+        setError(getSystemError());
+        return false;
+    }
 
-    return QSerialPortErrorInfo(QSerialPort::NoError);
+    return true;
+#else
+    setError(QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError,
+                                  QSerialPort::tr("Custom baud rate is not supported")));
+    return false;
 #endif
-
-    return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError, QSerialPort::tr("Custom baud rate is not supported"));
 }
 
 #elif defined(Q_OS_QNX)
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
+bool QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     // On QNX, the values of the 'Bxxxx' constants are set to 'xxxx' (i.e.
     // B115200 is defined to '115200'), which means that literal values can be
     // passed to cfsetispeed/cfsetospeed, including custom values, provided
     // that the underlying hardware supports them.
-    return setBaudRate_helper(baudRate, directions);
+    return setStandardBaudRate(baudRate, directions);
 }
 
 #else
 
-QSerialPortErrorInfo
-QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
+bool QSerialPortPrivate::setCustomBaudRate(qint32 baudRate, QSerialPort::Directions directions)
 {
     Q_UNUSED(baudRate);
     Q_UNUSED(directions);
 
-    return QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError);
+    setError(QSerialPortErrorInfo(QSerialPort::UnsupportedOperationError,
+                                  QSerialPort::tr("Custom baud rate is not supported")));
+    return false;
 }
 
 #endif
@@ -537,15 +528,9 @@ bool QSerialPortPrivate::setBaudRate(qint32 baudRate, QSerialPort::Directions di
 
     const qint32 unixBaudRate = QSerialPortPrivate::settingFromBaudRate(baudRate);
 
-    const QSerialPortErrorInfo error = (unixBaudRate > 0)
-        ? setStandardBaudRate(unixBaudRate, directions)
-        : setCustomBaudRate(baudRate, directions);
-
-    if (error.errorCode == QSerialPort::NoError)
-        return updateTermios();
-
-    setError(error);
-    return false;
+    return (unixBaudRate > 0)
+            ? setStandardBaudRate(unixBaudRate, directions)
+            : setCustomBaudRate(baudRate, directions);
 }
 
 bool QSerialPortPrivate::setDataBits(QSerialPort::DataBits dataBits)
diff --git a/tests/auto/qserialport/tst_qserialport.cpp b/tests/auto/qserialport/tst_qserialport.cpp
index 947047a..22f3b65 100644
--- a/tests/auto/qserialport/tst_qserialport.cpp
+++ b/tests/auto/qserialport/tst_qserialport.cpp
@@ -112,6 +112,9 @@ private slots:
 
     void clearAfterOpen();
 
+    void loopBack_data();
+    void loopBack();
+
 protected slots:
     void handleBytesWrittenAndExitLoopSlot(qint64 bytesWritten);
     void handleBytesWrittenAndExitLoopSlot2(qint64 bytesWritten);
@@ -916,5 +919,35 @@ void tst_QSerialPort::clearAfterOpen()
     QCOMPARE(senderPort.error(), QSerialPort::NoError);
 }
 
+void tst_QSerialPort::loopBack_data()
+{
+    QTest::addColumn<int>("baudRate");
+
+    QTest::newRow("9600 N 1 None") << 9600;
+    QTest::newRow("115200 N 1 None") << 115200;
+    QTest::newRow("14400 N 1 None") << 14400;
+}
+
+// This test works with the connected Rx and Tx pins.
+void tst_QSerialPort::loopBack()
+{
+    QFETCH(int, baudRate);
+
+    QSerialPort serialPort(m_senderPortName);
+    QVERIFY(serialPort.open(QSerialPort::ReadWrite));
+
+    QVERIFY(serialPort.setBaudRate(baudRate));
+    QCOMPARE(serialPort.baudRate(), baudRate);
+
+    QCOMPARE(serialPort.write(alphabetArray), qint64(alphabetArray.size()));
+    QVERIFY(serialPort.waitForBytesWritten(500));
+
+    do {
+        QVERIFY(serialPort.waitForReadyRead(500));
+    } while (serialPort.bytesAvailable() < alphabetArray.size());
+
+    QCOMPARE(serialPort.readAll(), alphabetArray);
+}
+
 QTEST_MAIN(tst_QSerialPort)
 #include "tst_qserialport.moc"