QSerialPort troubles on Windows 10

[gav007]

QSerialPort failed to handle long communication with many exchanges. The communication starts correctly and after a while (sometimes 2 or 3 exchanges, sometimes about 300 exchanges), we observe that the developed client sounds to slow down.

As a 1st implementation, the handy signals-slots system was used. At reception of the readyRead signal, simplified code :

QByteArray data( pPort->readAll() );
QByteArray answer( nextStep( data ) );
pPort->write( answer );
pPort->waitForBytesWritten( xxx );
emit dataRead( data );
emit dataSent( data );

After a while, the 1st symptom is the other device starting to resend its requests, just as if the answer was not correctly sent.
If the communication lasts long enough, the timeout in the waitForBytesWritten call starts to expire.

Other observation : If the emission of the 2 dataRead and dataSent signals (at the end of the code) is removed, the communication lasts longer !

As a 2d implementation, a QThread is created to manage the full communication in a blocking mode : simplified code :

pPort->setPortName(mSettings.name);
pPort->setBaudRate(mSettings.baudRate);
pPort->setDataBits(mSettings.dataBits);
pPort->setParity(mSettings.parity);
pPort->setStopBits(mSettings.stopBits);
pPort->setFlowControl(mSettings.flowControl);
if ( pPort ->open( QIODevice::ReadWrite ) )
{
    bool bRunning = true;
    while ( bRunning)
    {
        if ( pPort->waitForReadyRead() )
        {
            QByteArray data( pPort->readAll() );
            QByteArray sentData( nextStep( data, bRunning ) );
            pPort->waitForBytesWritten( xxx );
        }
    }
    QThread::sleep(1);
    pPort->close();
}

Again, we observe a longer communication, but it still fails.

As a 3d implementation, the above code for a blocking-mode communication is used in a simple console application where the Qt process loop is NOT used.

int main(int argc, char *argv[])
{
    //QCoreApplication a(argc, argv);
    SerialComm comm( "COM9" );
    comm.run();
    //return a.exec();
    return 0;
}

where the run() function is the same as the above code.

And IT WORKS !!!!

One explanation for this is the addition of much latency in the signals/slots system.
Have we misunderstood the way to use QSerialPort ?

Additional info :
OS : Windows 10 Pro
Kit : Desktop Qt 5.11.0 MinGW 32bit

Thanks in advance for any support. Regards.

[aha_1980]

Hi @gav007,

while it's not impossible that you've hit a bug like QTBUG-36758, most problems come from user code (Believe me, I have used QSerialPort quite often, blocking and non-blocking way).

QByteArray data( pPort->readAll() );
QByteArray answer( nextStep( data ) );
pPort->write( answer );
pPort->waitForBytesWritten( xxx );
emit dataRead( data );

Sigh. waitFor... in a slot, no, that's not the way it was intended! Use the proper signal for bytes written, please!

Also, connect the error signal to a slot and evaluate all errors.

Another question is: do you need to wait for the bytes written? It probably just tells you the data has left Qt's buffer and is now stucking in some OS/driver buffers. If you have a handshake based communication, you have to wait for the answer coming in anyway.

One explanation for this is the addition of much latency in the signals/slots system.

Unlikely, as the data is buffered in background.

Regards

[gav007]

@aha_1980

Ooops ! Right. The waitForBytesWritten call was originally not present. It has been added during the investigation phase.
That's also the reason why I do not believe to have hit the mentionned bug (however, I will update my Qt environment and check again).
The provided code is simplified. The error management has been removed for the presentation. And no error is raised.
In Linux I also often used the QSerialPort class without any trouble. This is the 1st time I use it with the Windows 10 OS. By the way, I will check this code in Linux ...
Could the MinGW development kit be incriminated ?

Many thanks for the help !

[aha_1980]

@gav007 said in QSerialPort troubles on Windows 10:

This is the 1st time I use it with the Windows 10 OS. By the way, I will check this code in Linux ...

Yes, please do to isolate the issue.

Could the MinGW development kit be incriminated ?

I think not ... but maybe @kuzulis has an idea?

[kuzulis]

@gav007 ,

Try to do a simple loopback test at first (just connect the RX and TX pins).

[gav007]

Unfortunately it did not work under Linux either (Ubuntu 18.04 + Qt 5.9.5). That's weird !
Error and symptoms are identical.

[kuzulis]

This means that an issue somewhere in your code or your HW.

[gav007]

The trouble appears on 2 different HW. Hence the HW should not be at fault.
I would rather point the SW. Here is the full class code :

#ifndef OPLUSCOMM_H
#define OPLUSCOMM_H

#include <QObject>
#include <QSerialPort>


class OctoPlusComm : public QSerialPort
{

    Q_OBJECT

signals:

    void dataRead( const QByteArray & data );
    void dataSent( const QByteArray & data );

    void newCommunicationStep( const QString & step );

public:

    struct Settings {
        QString name;
        qint32 baudRate;
        QString stringBaudRate;
        QSerialPort::DataBits dataBits;
        QString stringDataBits;
        QSerialPort::Parity parity;
        QString stringParity;
        QSerialPort::StopBits stopBits;
        QString stringStopBits;
        QSerialPort::FlowControl flowControl;
        QString stringFlowControl;
        bool localEchoEnabled;
    };

    enum CommunicationStep {
        IdleStep,
        StartCommandStep,
        ROMVersionStep,
        EEPROMSizeStep,
        RAMSizeStep,
        UploadStep
    };

    OctoPlusComm(QObject * parent = nullptr);
    virtual ~OctoPlusComm();

    bool openPort( const Settings & settings );

    static const QString stepText( const CommunicationStep step );

private slots:

    void readAll();

private:

    void nextStep();
    void setCommunicationStep( const CommunicationStep step );
    bool checkReception();
    bool checkStartCommand();
    bool checkRomVersion( QByteArray & frame );
    bool checkEepromSize( QByteArray & frame );
    bool checkRamSize( QByteArray & frame );
    bool uploadInProgress();

private:

    CommunicationStep mCommStep;

    QByteArray mReceptionBuffer;

};

#endif // OPLUSCOMM_H

With the implementation :

OctoPlusComm::OctoPlusComm(QObject * parent) : QSerialPort(parent), mCommStep(IdleStep)
{
    connect(this, &OctoPlusComm::readyRead, this, &OctoPlusComm::readAll);
}

OctoPlusComm::~OctoPlusComm() {}

bool OctoPlusComm::openPort( const Settings & settings )
{
    setPortName(settings.name);
    setBaudRate(settings.baudRate);
    setDataBits(settings.dataBits);
    setParity(settings.parity);
    setStopBits(settings.stopBits);
    setFlowControl(settings.flowControl);
    if ( !open(QIODevice::ReadWrite) ) {
        setErrorString("Failed to open serial port");
        emit errorOccurred(QSerialPort::OpenError);
        return false;
    }
    mCommStep = IdleStep;
    mReceptionBuffer.clear();
    return true;
}

void OctoPlusComm::readAll()
{
    qDebug() << "Ready to read";

    QByteArray newData = QSerialPort::readAll();
    mReceptionBuffer.append(newData);

    emit dataRead( newData );

    if ( checkReception() )
    {
        nextStep();
    }
}

void OctoPlusComm::nextStep()
{
    bool bOk = true;
    QByteArray nextFrame;
    nextFrame.clear();
    bOk = checkStartCommand() || checkRomVersion( nextFrame ) || checkEepromSize( nextFrame ) ||
            checkRamSize( nextFrame ) || uploadInProgress();
    mReceptionBuffer.clear();
    if ( bOk )
    {
        if ( nextFrame.isEmpty() )
        {
            nextFrame.append(COMM_RECEIVE_OK);
            nextFrame.append(COMM_REC_OK_CHECKSUM);
            nextFrame.append(COMM_FRAME_END);
            qDebug() << "Ack reception";
        }
        write( nextFrame );
        emit dataSent( nextFrame );
    }
    else
    {
        QByteArray request;
        request.append(mReceptionBuffer.at(1));
        request.append(mReceptionBuffer.at(2));
        setErrorString(tr("Unknown request : %1").arg(Utility::displayBuffer(request)));
        emit errorOccurred(QSerialPort::UnknownError);
    }
}

void OctoPlusComm::setCommunicationStep( const CommunicationStep step )
{
    mCommStep = step;
    emit newCommunicationStep( stepText( mCommStep ) );
}

bool OctoPlusComm::checkReception()
{
    if (mReceptionBuffer.size() > 0)
    {
        if ( mReceptionBuffer.endsWith(0x0D) )
        {
            qDebug() << "Reception complete";
            int n = mReceptionBuffer.lastIndexOf('#');
            if ( n > 0 )
            {
                mReceptionBuffer.remove(0, n);
                qDebug() << "Reception cleaned (" << n << ") : " << Utility::displayBuffer(mReceptionBuffer);
            }
            return true;
        }

        qDebug() << "Reception incomplete";
    }
    else
    {
        qDebug() << "Empty reception !";
    }
    return false;
}

bool OctoPlusComm::checkStartCommand()
{
    if ( mReceptionBuffer.startsWith('#') &&
         (mReceptionBuffer.indexOf(COMM_START_COMMAND, 1) == 1) )
    {
        qDebug() << "Start command received";
        setCommunicationStep( StartCommandStep );
        return true;
    }
//    setErrorString(tr("Expect start command, received %1").arg(Utility::displayBuffer(mReceptionBuffer)));
//    emit errorOccurred(QSerialPort::UnknownError);
    return false;
}

bool OctoPlusComm::checkRomVersion( QByteArray & frame )
{
    if ( mReceptionBuffer.startsWith('#') &&
         (mReceptionBuffer.indexOf(COMM_ROM_VERSION_COMMAND, 1) == 1) )
    {
        qDebug() << "ROM version request";
        setCommunicationStep( ROMVersionStep );

        frame.append(COMM_RECEIVE_OK);
        frame.append(COMM_ROM_VERSION_RESPONSE);
        frame.append(COMM_FRAME_END);
        return true;
    }
    return false;
}

bool OctoPlusComm::checkEepromSize( QByteArray & frame )
{
    if ( mReceptionBuffer.startsWith('#') &&
         (mReceptionBuffer.indexOf(COMM_EEPROM_SIZE_COMMAND, 1) == 1) )
    {
        qDebug() << "EEPROM size request";
        setCommunicationStep( EEPROMSizeStep );

        frame.append(COMM_RECEIVE_OK);
        frame.append(COMM_EEPROM_SIZE_RESPONSE);
        frame.append(COMM_FRAME_END);
        return true;
    }
    return false;
}

bool OctoPlusComm::checkRamSize( QByteArray & frame )
{
    if ( mReceptionBuffer.startsWith('#') &&
         (mReceptionBuffer.indexOf(COMM_RAM_SIZE_COMMAND, 1) == 1) )
    {
        qDebug() << "RAM size request";

        setCommunicationStep( RAMSizeStep );

        frame.append(COMM_RECEIVE_OK);
        frame.append(COMM_RAM_SIZE_RESPONSE);
        frame.append(COMM_FRAME_END);
        return true;
    }
    return false;
}

bool OctoPlusComm::uploadInProgress()
{
    if ( mReceptionBuffer.startsWith('#') &&
         (mReceptionBuffer.indexOf(COMM_UPLOAD_COMMAND, 1) == 1) )
    {
        qDebug() << "Upload data";

        setCommunicationStep( UploadStep );

        return true;
    }
    return false;
}

const QString OctoPlusComm::stepText( const CommunicationStep step )
{
    switch ( step )
    {
    case IdleStep:          return tr("Idle");
    case StartCommandStep:  return tr("Start command");
    case ROMVersionStep:    return tr("ROM version check");
    case EEPROMSizeStep:    return tr("EEPROM size check");
    case RAMSizeStep:       return tr("RAM size check");
    case UploadStep:        return tr("Data upload");
    }
    return tr("???");
}

Does anyone see what I would do wrong ?
Many thanks for any support.

[kuzulis]

As I sad before, you should to create a simple loopback test at first, to make sure that all data transferred/received. Because nobody will be check your code.

[gav007]

I wrote the loopback test. As expected it worked. The reported ticket is related to a timing issue. The loopback test has nothing to do with this. I lost my time.

I added some delay measurements (OK, it's intrusive but at least, it gives some clues) and observe a delay of about 10 msec between the write operation and the reception of the bytesWritten signal and sometimes a delay of the same level between the readAll call and the write operation. Those delays are observed even if the signals are emitted after the read/write operations.
If a remove all the emitted signals, those delays are no more observed. The communication is then successful.
I then check what is done with the dataRead and the dataSent signals : these are only display operations in QPlainTextEdit widget :

mTextConsole->insertPlainText( QString(data) );

mHexConsole->insertPlainText( QString(data.toHex(' ')) );

This should not justify 10 msec delays.

So. Since I could not reach a good confidence level with QSerialPort, I abandoned the Qt framework for JAVA. Now I got my solution. In consequence this ticket can be closed for me.

This is my first disappointing experience with Qt ! Too bad !

Thanks for the support.
Best regards.

[J.Hilk]

@gav007
I don't think, that 20 ms to create a QString from raw data and than inserting it to a plain textview + (eventual) repaint of the the ui is totoally unreasonable. Especially, if you're using the old (qt4) styled syntax for QObject::connect. Those come with a noticeable overhead during execution.

This is the reason why I usually move my IO-communication in its own thread.
That said, giving your connects a 5th parameter Qt::QueuedConnection may already be enough to make your Qt-solution workable.

[kuzulis]

@gav007 said in QSerialPort troubles on Windows 10:

observe a delay of about 10 msec between the write operation and the reception of the bytesWritten signal

Your words is empty without of a real example. In my case I got an expected delay in this example:

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

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    QSerialPort sp("COM1");

    // it opens with 9600 bauds by default
    if (!sp.open(QIODevice::ReadWrite))
        return 0;

    QElapsedTimer et;

    auto write = [&]() {
        et.start();
        QByteArray ba(256, 0);
        sp.write(ba);
    };

    QObject::connect(&sp, &QSerialPort::bytesWritten, [&](qint64 bytes) {
        qDebug() << et.elapsed();
        write();
    });

    write();

    return a.exec();
}

It printed out:

265
265
266
266
265

that is right, because according to this calculation, the expected delay should be:

baudrate = 9600 bits per second
bytes per second = 9600 / (1start bit + 8 data bits + 1 stop bit) = 960 bytes per second
256 bytes in seconds = 256 / 960 = 0.266(6)

• If I try to send ~10 bytes, then I got ~10 msecs. It is true.
• if I try to send ~1 byte, then I got ~0-1 msec. It is true.

So, I don't know what are you talking about.

[gav007]

@kuzulis said in QSerialPort troubles on Windows 10:

Because nobody will be check your code.

@kuzulis said in QSerialPort troubles on Windows 10:

Your words is empty without of a real example

How surprising you can be !

Again, if you read my post, I'm more worried about the 10 msec for 2 insertions in QPlainTextEdit widget than the delay between the write operation and the bytesWritten signal.
Where you're right, I did not mention the size of the handled buffer : it varies between 4 and 12 bytes. Not enough to spend 10 msec for sending !

So @kuzulis, do not worry about my problem. I'm sure there will be people on this forum willing to help me. Thanks.

@J-Hilk ,
Thanks for this remark. I would not expect the QSerialPort signals to be managed in the same thread as the GUI elements. Hence using the default AutoConnection type should make the difference. However, I forced all the connections between the QSerialPort object and the GUI elements to be queued and unfortunately the behavior remains the same.

Best regards

[kuzulis]

@gav007 said in QSerialPort troubles on Windows 10:

it varies between 4 and 12 bytes. Not enough to spend 10 msec for sending !

Again, your words is empty. I already have provided the test above, you can try it with your HW, with your baudrate && data length.

@gav007 said in QSerialPort troubles on Windows 10:

do not worry about my problem

I'm do not worry, good luck!