QCanBusDevice: More error codes and messages on failures

More error messages can now be logged with the logging framework
or directly accessed with the errorOccurred() signal.

The new error codes QCanBusDevice::OperationError and
QCanBusDevice::TimeoutError were introduced to signal situations
where the device is not able to perform a specific operation
respectively when an operation timed out.

[ChangeLog][QCanBusDevice] Added the QCanBusDevice::OperationError
and QCanBusDevice::TimeoutError codes to signal wrong operation
respectively timeout errors.

Task-number: QTBUG-70449
Change-Id: Ifa0831bd0947b624579ae8662df10a2a9ce38714
Reviewed-by: Alex Blasche <alexander.blasche@qt.io>

3 files changed, 181 insertions, 55 deletions

diff --git a/src/serialbus/qcanbusdevice.cpp b/src/serialbus/qcanbusdevice.cpp
index 713dc52..cb9dd69 100644
--- a/src/serialbus/qcanbusdevice.cpp
+++ b/src/serialbus/qcanbusdevice.cpp
@@ -73,6 +73,11 @@ Q_LOGGING_CATEGORY(QT_CANBUS, "qt.canbus")
     \value ConfigurationError   An error occurred when attempting to set a configuration
                                 parameter.
     \value UnknownError         An unknown error occurred.
+    \value OperationError       An operation was attempted while the device was in
+                                a state that did not permit it. This enum was introduced
+                                in Qt 5.14.
+    \value TimeoutError         An timeout occurred while waiting for frames written or
+                                received. This enum was introduced in Qt 5.14.
 */
 
 /*!
@@ -248,7 +253,7 @@ QCanBusDevice::QCanBusDevice(QObject *parent) :
     CAN bus implementations must use this function to update the device's
     error state.
 
-    \sa error(), errorOccurred()
+    \sa error(), errorOccurred(), clearError()
 */
 void QCanBusDevice::setError(const QString &errorText, CanBusError errorId)
 {
@@ -261,6 +266,24 @@ void QCanBusDevice::setError(const QString &errorText, CanBusError errorId)
 }
 
 /*!
+    \since 5.14
+    Clears the error id and the human readable description of the last
+    device error.
+
+    CAN bus implementations must use this function to update the device's
+    error state.
+
+    \sa error(), errorOccurred(), setError()
+*/
+void QCanBusDevice::clearError()
+{
+    Q_D(QCanBusDevice);
+
+    d->errorText.clear();
+    d->lastError = NoError;
+}
+
+/*!
     Appends \a newFrames to the internal list of frames which can be
     accessed using \l readFrame() and emits the \l framesReceived()
     signal.
@@ -569,8 +592,14 @@ void QCanBusDevice::clear(QCanBusDevice::Directions direction)
 {
     Q_D(QCanBusDevice);
 
-    if (Q_UNLIKELY(d->state != ConnectedState))
+    if (Q_UNLIKELY(d->state != ConnectedState)) {
+        const QString error = tr("Cannot clear buffers as device is not connected.");
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        setError(error, CanBusError::OperationError);
         return;
+    }
+
+    clearError();
 
     if (direction & Direction::Input) {
         QMutexLocker(&d->incomingFramesGuard);
@@ -607,31 +636,46 @@ bool QCanBusDevice::waitForFramesWritten(int msecs)
                              "recursively. Check that no slot containing waitForFramesReceived() "
                              "is called in response to framesWritten(qint64) or "
                              "errorOccurred(CanBusError) signals.");
+        setError(tr("QCanBusDevice::waitForFramesWritten() must not be called recursively."),
+                 CanBusError::OperationError);
         return false;
     }
 
     QScopedValueRollback<bool> guard(d_func()->waitForWrittenEntered);
     d_func()->waitForWrittenEntered = true;
 
-    if (d_func()->state != ConnectedState)
+    if (Q_UNLIKELY(d_func()->state != ConnectedState)) {
+        const QString error = tr("Cannot wait for frames written as device is not connected.");
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        setError(error, CanBusError::OperationError);
         return false;
+    }
 
     if (!framesToWrite())
         return false; // nothing pending, nothing to wait upon
 
+    enum { Written = 0, Error, Timeout };
     QEventLoop loop;
-    connect(this, &QCanBusDevice::framesWritten, &loop, [&]() { loop.exit(0); });
-    connect(this, &QCanBusDevice::errorOccurred, &loop, [&]() { loop.exit(1); });
+    connect(this, &QCanBusDevice::framesWritten, &loop, [&]() { loop.exit(Written); });
+    connect(this, &QCanBusDevice::errorOccurred, &loop, [&]() { loop.exit(Error); });
     if (msecs >= 0)
-        QTimer::singleShot(msecs, &loop, [&]() { loop.exit(2); });
+        QTimer::singleShot(msecs, &loop, [&]() { loop.exit(Timeout); });
 
-    int result = 0;
+    int result = Written;
     while (framesToWrite() > 0) {
         // wait till all written or time out
         result = loop.exec(QEventLoop::ExcludeUserInputEvents);
-        if (result > 0)
+        if (Q_UNLIKELY(result == Timeout)) {
+            const QString error = tr("Timeout (%1 ms) during wait for frames written.").arg(msecs);
+            setError(error, CanBusError::TimeoutError);
+            qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        }
+
+        if (result > Written)
             return false;
     }
+
+    clearError();
     return true;
 }
 
@@ -660,25 +704,39 @@ bool QCanBusDevice::waitForFramesReceived(int msecs)
                              "recursively. Check that no slot containing waitForFramesReceived() "
                              "is called in response to framesReceived() or "
                              "errorOccurred(CanBusError) signals.");
+        setError(tr("QCanBusDevice::waitForFramesReceived() must not be called recursively."),
+                 CanBusError::OperationError);
         return false;
     }
 
     QScopedValueRollback<bool> guard(d_func()->waitForReceivedEntered);
     d_func()->waitForReceivedEntered = true;
 
-    if (d_func()->state != ConnectedState)
+    if (Q_UNLIKELY(d_func()->state != ConnectedState)) {
+        const QString error = tr("Cannot wait for frames received as device is not connected.");
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        setError(error, CanBusError::OperationError);
         return false;
+    }
 
+    enum { Received = 0, Error, Timeout };
     QEventLoop loop;
-
-    connect(this, &QCanBusDevice::framesReceived, &loop, [&]() { loop.exit(0); });
-    connect(this, &QCanBusDevice::errorOccurred, &loop, [&]() { loop.exit(1); });
+    connect(this, &QCanBusDevice::framesReceived, &loop, [&]() { loop.exit(Received); });
+    connect(this, &QCanBusDevice::errorOccurred, &loop, [&]() { loop.exit(Error); });
     if (msecs >= 0)
-        QTimer::singleShot(msecs, &loop, [&]() { loop.exit(2); });
+        QTimer::singleShot(msecs, &loop, [&]() { loop.exit(Timeout); });
 
     int result = loop.exec(QEventLoop::ExcludeUserInputEvents);
 
-    return result == 0;
+    if (Q_UNLIKELY(result == Timeout)) {
+        const QString error = tr("Timeout (%1 ms) during wait for frames received.").arg(msecs);
+        setError(error, CanBusError::TimeoutError);
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+    }
+
+    if (result == Received)
+        clearError();
+    return result == Received;
 }
 
 /*!
@@ -734,8 +792,14 @@ QCanBusFrame QCanBusDevice::readFrame()
 {
     Q_D(QCanBusDevice);
 
-    if (Q_UNLIKELY(d->state != ConnectedState))
+    if (Q_UNLIKELY(d->state != ConnectedState)) {
+        const QString error = tr("Cannot read frame as device is not connected.");
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        setError(error, CanBusError::OperationError);
         return QCanBusFrame(QCanBusFrame::InvalidFrame);
+    }
+
+    clearError();
 
     QMutexLocker locker(&d->incomingFramesGuard);
 
@@ -758,8 +822,14 @@ QVector<QCanBusFrame> QCanBusDevice::readAllFrames()
 {
     Q_D(QCanBusDevice);
 
-    if (Q_UNLIKELY(d->state != ConnectedState))
+    if (Q_UNLIKELY(d->state != ConnectedState)) {
+        const QString error = tr("Cannot read frame as device is not connected.");
+        qCWarning(QT_CANBUS, "%ls", qUtf16Printable(error));
+        setError(error, CanBusError::OperationError);
         return QVector<QCanBusFrame>();
+    }
+
+    clearError();
 
     QMutexLocker locker(&d->incomingFramesGuard);
 
@@ -835,6 +905,8 @@ bool QCanBusDevice::connectDevice()
         return false;
     }
 
+    clearError();
+
     //Connected is set by backend -> might be delayed by event loop
     return true;
 }
diff --git a/src/serialbus/qcanbusdevice.h b/src/serialbus/qcanbusdevice.h
index 8457b08..1844f4b 100644
--- a/src/serialbus/qcanbusdevice.h
+++ b/src/serialbus/qcanbusdevice.h
@@ -60,7 +60,9 @@ public:
         WriteError,
         ConnectionError,
         ConfigurationError,
-        UnknownError
+        UnknownError,
+        OperationError,
+        TimeoutError
     };
     Q_ENUM(CanBusError)
 
@@ -167,6 +169,7 @@ Q_SIGNALS:
 protected:
     void setState(QCanBusDevice::CanBusDeviceState newState);
     void setError(const QString &errorText, QCanBusDevice::CanBusError);
+    void clearError();
 
     void enqueueReceivedFrames(const QVector<QCanBusFrame> &newFrames);
 
diff --git a/tests/auto/qcanbusdevice/tst_qcanbusdevice.cpp b/tests/auto/qcanbusdevice/tst_qcanbusdevice.cpp
index 24127d9..e0b30f0 100644
--- a/tests/auto/qcanbusdevice/tst_qcanbusdevice.cpp
+++ b/tests/auto/qcanbusdevice/tst_qcanbusdevice.cpp
@@ -84,8 +84,11 @@ public:
 
     bool writeFrame(const QCanBusFrame &data)
     {
-        if (state() != QCanBusDevice::ConnectedState)
+        if (state() != QCanBusDevice::ConnectedState) {
+            setError(QStringLiteral("Cannot write frame as device is not connected"),
+                     QCanBusDevice::OperationError);
             return false;
+        }
 
         if (writeBufferUsed) {
             enqueueOutgoingFrame(data);
@@ -174,7 +177,9 @@ void tst_QCanBusDevice::initTestCase()
     QSignalSpy stateSpy(device.data(), &QCanBusDevice::stateChanged);
 
     QVERIFY(!device->connectDevice()); // first connect triggered to fail
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QVERIFY(device->connectDevice());
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
     QCOMPARE(stateSpy.count(), 4);
     QCOMPARE(stateSpy.at(0).at(0).value<QCanBusDevice::CanBusDeviceState>(),
@@ -236,7 +241,8 @@ void tst_QCanBusDevice::write()
     stateSpy.clear();
     QVERIFY(stateSpy.isEmpty());
 
-    device->writeFrame(frame);
+    QVERIFY(!device->writeFrame(frame));
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
     QCOMPARE(spy.count(), 0);
 
     device->connectDevice();
@@ -247,7 +253,8 @@ void tst_QCanBusDevice::write()
     QCOMPARE(stateSpy.at(1).at(0).value<QCanBusDevice::CanBusDeviceState>(),
              QCanBusDevice::ConnectedState);
 
-    device->writeFrame(frame);
+    QVERIFY(device->writeFrame(frame));
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QCOMPARE(spy.count(), 1);
 }
 
@@ -260,6 +267,7 @@ void tst_QCanBusDevice::read()
     stateSpy.clear();
 
     const QCanBusFrame frame1 = device->readFrame();
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
 
     QVERIFY(device->connectDevice());
     QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
@@ -271,6 +279,7 @@ void tst_QCanBusDevice::read()
 
     device->triggerNewFrame();
     const QCanBusFrame frame2 = device->readFrame();
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QVERIFY(!frame1.frameId());
     QVERIFY(!frame1.isValid());
     QVERIFY(frame2.frameId());
@@ -281,6 +290,12 @@ void tst_QCanBusDevice::readAll()
 {
     enum { FrameNumber = 10 };
     device->disconnectDevice();
+    QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::UnconnectedState, 5000);
+
+    const QVector<QCanBusFrame> empty = device->readAllFrames();
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
+    QVERIFY(empty.isEmpty());
+
     QVERIFY(device->connectDevice());
     QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
 
@@ -288,21 +303,30 @@ void tst_QCanBusDevice::readAll()
         device->triggerNewFrame();
 
     const QVector<QCanBusFrame> frames = device->readAllFrames();
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QCOMPARE(FrameNumber, frames.size());
     QVERIFY(!device->framesAvailable());
 }
 
 void tst_QCanBusDevice::clearInputBuffer()
 {
-    if (device->state() != QCanBusDevice::ConnectedState) {
-        QVERIFY(device->connectDevice());
-        QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
-    }
+    device->disconnectDevice();
+    QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::UnconnectedState, 5000);
+
+    device->clear(QCanBusDevice::Input);
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
+
+    QVERIFY(device->connectDevice());
+    QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
+
+    device->clear(QCanBusDevice::Input);
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
 
     for (int i = 0; i < 10; ++i)
         device->triggerNewFrame();
 
     device->clear(QCanBusDevice::Input);
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
 
     QVERIFY(!device->framesAvailable());
 }
@@ -311,11 +335,17 @@ void tst_QCanBusDevice::clearOutputBuffer()
 {
     // this test requires buffered writing
     device->setWriteBuffered(true);
+    device->disconnectDevice();
+    QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::UnconnectedState, 5000);
 
-    if (device->state() != QCanBusDevice::ConnectedState) {
-        QVERIFY(device->connectDevice());
-        QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
-    }
+    device->clear(QCanBusDevice::Output);
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
+
+    QVERIFY(device->connectDevice());
+    QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
+
+    device->clear(QCanBusDevice::Output);
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
 
     // first test buffered writing, frames will be written after some delay
     QSignalSpy spy(device.data(), &QCanBusDevice::framesWritten);
@@ -329,6 +359,7 @@ void tst_QCanBusDevice::clearOutputBuffer()
         device->writeFrame(QCanBusFrame(0x123, "output"));
 
     device->clear(QCanBusDevice::Output);
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QTRY_VERIFY_WITH_TIMEOUT(spy.count() == 0, 5000);
 }
 
@@ -522,6 +553,10 @@ void tst_QCanBusDevice::tst_bufferingAttribute()
 
 void tst_QCanBusDevice::tst_waitForFramesReceived()
 {
+    device->disconnectDevice();
+    QVERIFY(!device->waitForFramesReceived(100));
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
+
     if (device->state() != QCanBusDevice::ConnectedState) {
         QVERIFY(device->connectDevice());
         QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
@@ -531,9 +566,10 @@ void tst_QCanBusDevice::tst_waitForFramesReceived()
     QVERIFY(device->triggerNewFrame());
     QVERIFY(device->framesAvailable());
 
-    // frame is available, function blocks and times out
-    bool result = device->waitForFramesReceived(2000);
-    QVERIFY(!result);
+    // frame is already available, but no new frame comes in
+    // while function blocks, therefore times out
+    QVERIFY(!device->waitForFramesReceived(2000));
+    QCOMPARE(device->error(), QCanBusDevice::TimeoutError);
 
     QCanBusFrame frame = device->readFrame();
     QVERIFY(frame.isValid());
@@ -543,16 +579,16 @@ void tst_QCanBusDevice::tst_waitForFramesReceived()
     QElapsedTimer elapsed;
     elapsed.start();
     // no pending frame (should trigger active wait & timeout)
-    result = device->waitForFramesReceived(5000);
+    QVERIFY(!device->waitForFramesReceived(5000));
     QVERIFY(elapsed.hasExpired(4000)); // should have caused time elapse
-    QVERIFY(!result);
+    QCOMPARE(device->error(), QCanBusDevice::TimeoutError);
 
     QTimer::singleShot(2000, [&]() { device->triggerNewFrame(); });
     elapsed.restart();
     // frame will be inserted after 2s
-    result = device->waitForFramesReceived(8000);
+    QVERIFY(device->waitForFramesReceived(8000));
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QVERIFY(!elapsed.hasExpired(8000));
-    QVERIFY(result);
 
     frame = device->readFrame();
     QVERIFY(frame.isValid());
@@ -564,9 +600,8 @@ void tst_QCanBusDevice::tst_waitForFramesReceived()
     });
     elapsed.restart();
     // error will be inserted after 2s
-    result = device->waitForFramesReceived(8000);
+    QVERIFY(!device->waitForFramesReceived(8000));
     QVERIFY(!elapsed.hasExpired(8000));
-    QVERIFY(!result);
     QCOMPARE(device->errorString(), QStringLiteral("TriggerError"));
     QCOMPARE(device->error(), QCanBusDevice::ReadError);
 
@@ -580,56 +615,71 @@ void tst_QCanBusDevice::tst_waitForFramesReceived()
     QObject::connect(device.data(), &QCanBusDevice::framesReceived, [this, &handleCounter]() {
         handleCounter++;
         // this should trigger a recursion which we want to catch
-        device->waitForFramesReceived(5000);
+        QVERIFY(!device->waitForFramesReceived(5000));
+        // Only the first two frames create a recursion, as the outer
+        // waitForFramesReceived() will immediately exit once at least
+        // one frame was received. Therefore the third frame here leads
+        // to TimeoutError, as no further frame is received.
+        if (handleCounter < 3) {
+            QCOMPARE(device->error(), QCanBusDevice::OperationError);
+        } else {
+            QCOMPARE(device->error(), QCanBusDevice::TimeoutError);
+        }
     });
-    result = device->waitForFramesReceived(8000);
-    QVERIFY(result);
+    QVERIFY(device->waitForFramesReceived(8000));
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QTRY_COMPARE_WITH_TIMEOUT(handleCounter, 3, 5000);
 }
 
 void tst_QCanBusDevice::tst_waitForFramesWritten()
 {
+    device->disconnectDevice();
+    QVERIFY(!device->waitForFramesWritten(100));
+    QCOMPARE(device->error(), QCanBusDevice::OperationError);
+
     if (device->state() != QCanBusDevice::ConnectedState) {
+        QVERIFY(!device->waitForFramesWritten(100));
+        QCOMPARE(device->error(), QCanBusDevice::OperationError);
+
         QVERIFY(device->connectDevice());
         QTRY_VERIFY_WITH_TIMEOUT(device->state() == QCanBusDevice::ConnectedState, 5000);
     }
 
     device->setWriteBuffered(false);
-    bool result = device->waitForFramesWritten(1000);
-    QVERIFY(!result); // no buffer, waiting not possible
+    QVERIFY(!device->waitForFramesWritten(1000)); // no buffer, waiting not possible
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
 
     device->setWriteBuffered(true);
 
     QVERIFY(device->framesToWrite() == 0);
-    result = device->waitForFramesWritten(1000);
-    QVERIFY(!result); // nothing in buffer, nothing to wait for
+    QVERIFY(!device->waitForFramesWritten(1000)); // nothing in buffer, nothing to wait for
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
 
     QCanBusFrame frame;
     frame.setPayload(QByteArray("testData"));
 
     // test error case
     QTimer::singleShot(500, [&]() {
-        device->emulateError(QStringLiteral("TriggerWriteError"), QCanBusDevice::ReadError);
+        device->emulateError(QStringLiteral("TriggerWriteError"), QCanBusDevice::WriteError);
     });
     device->writeFrame(frame);
     QElapsedTimer elapsed;
     elapsed.start();
 
     // error will be triggered
-    result = device->waitForFramesWritten(8000);
+    QVERIFY(!device->waitForFramesWritten(8000));
     QVERIFY(!elapsed.hasExpired(8000));
-    QVERIFY(!result);
     QCOMPARE(device->errorString(), QStringLiteral("TriggerWriteError"));
-    QCOMPARE(device->error(), QCanBusDevice::ReadError);
+    QCOMPARE(device->error(), QCanBusDevice::WriteError);
 
     // flush remaining frames out to reset the test
     QTRY_VERIFY_WITH_TIMEOUT(device->framesToWrite() == 0, 10000);
 
     // test timeout
     device->writeFrame(frame);
-    result = device->waitForFramesWritten(500);
+    QVERIFY(!device->waitForFramesWritten(500));
+    QCOMPARE(device->error(), QCanBusDevice::TimeoutError);
     QVERIFY(elapsed.hasExpired(500));
-    QVERIFY(!result);
 
     // flush remaining frames out to reset the test
     QTRY_VERIFY_WITH_TIMEOUT(device->framesToWrite() == 0, 10000);
@@ -637,9 +687,9 @@ void tst_QCanBusDevice::tst_waitForFramesWritten()
     device->writeFrame(frame);
     device->writeFrame(frame);
     elapsed.restart();
-    result = device->waitForFramesWritten(8000);
+    QVERIFY(device->waitForFramesWritten(8000));
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QVERIFY(!elapsed.hasExpired(8000));
-    QVERIFY(result);
 
     // flush remaining frames out to reset the test
     QTRY_VERIFY_WITH_TIMEOUT(device->framesToWrite() == 0, 10000);
@@ -652,10 +702,11 @@ void tst_QCanBusDevice::tst_waitForFramesWritten()
     QObject::connect(device.data(), &QCanBusDevice::framesWritten, [this, &handleCounter]() {
         handleCounter++;
         // this should trigger a recursion which we want to catch
-        device->waitForFramesWritten(5000);
+        QVERIFY(!device->waitForFramesWritten(5000));
+        QCOMPARE(device->error(), QCanBusDevice::OperationError);
     });
-    result = device->waitForFramesWritten(8000);
-    QVERIFY(result);
+    QVERIFY(device->waitForFramesWritten(8000));
+    QCOMPARE(device->error(), QCanBusDevice::NoError);
     QTRY_COMPARE_WITH_TIMEOUT(handleCounter, 3, 5000);
 
     device->setWriteBuffered(false);