QSerialPort troubles on Windows 10

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:

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!