QSerialPort, reads very slowly and lost the last string

yczo

Hi, I'm trying to make a communication between a microcontroller and a Qt program.

When the Microcontroller read the string R00, responds with the next 23 strings (sent one by one, ending in '\n'):

c0+mA+yC+t199+i1F+B0+b1+nThe Name+gGenGroup+v10.1+f0.33+sR+\n

...
...
till 23 strings..
c21+mO+y7+t004+i7F+B5+b6+nJuan    +gCircuito+v31.1+f1.33+sS+\n
c22+mA+y8+t308+i8F+B6+b7+nRamon   +gGenGroup+v32.1+f2.33+sO+\n

the first three, are sent with 1 second delay, but the rest witout delay.

The problem is that, the program use too much time to read the the following 20 (are sent in mili seconds), and the last string is not readed

please, any help will be wellcomed.

Here is my code, the issue is in the "void readPort::process()" routine

#ifndef SERIALPORT_H
#define SERIALPORT_H
#include <QObject>
#include <QThread>
#include <QtSerialPort/QSerialPort>
#include <QtSerialPort/QSerialPortInfo>
#include<QDebug>
#define maxPort 41
#define askId       "cb?"
#define iDAck       "cbr"


class writePort : public QObject{ //AC1
    Q_OBJECT

    public:
        writePort();
        ~writePort();


    public slots:
        void process();

    signals:
        void finished();
        void error(QString err);

    private:

}; //EOAC1
//***************************************
class readPort : public QObject{ //AC2
    Q_OBJECT

    public:
        readPort(QSerialPort *_serial);
        ~readPort();
        QSerialPort *serial;

    public slots:
        void process();

    signals:
        void finished();
        void error(QString err);

    private:

}; //EOAC2

//BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
//** base ********************************************
//BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB

class SerialPort : public QObject
{
    Q_OBJECT
public:
    //infoPort infPrt;
    SerialPort(QObject *parent);
    ~SerialPort();
    void makeCnx();
    QSerialPort serial;
    ::readPort *readPrt;
    ::writePort *writePrt;
    QThread *thRead;
    QThread *thWrite;

public slots:
//    void updateMsg(QString newMsg);

signals:
    void received(QString rcv);
    void conInf(QString cnxInfo);


private:

  //  void run();
  //  char *read();
  //  void write(char *outBuffer);
};

#endif // SERIALPORT_H
//*****************************************************************************************

#include "serialport.h"
#include<iostream>
using namespace std;

//********* //AC1******************
writePort::writePort(){

}
//***************************
writePort::~writePort(){

}

//***************************
void writePort::process() {
    // allocate resources using new here
/*    while(serial.waitForReadyRead(30)){
       // QThread::msleep(100);
         cerr << "Hello World!" << endl;

    }*/
    emit finished();
}
//***** //EOAC1 **********************
//***************************
//********* //AC2******************
readPort::readPort(QSerialPort *_serial) : serial(_serial){

}
//***************************
readPort::~readPort(){

}

//***************************
void readPort::process() {
    // allocate resources using new here
    serial->write("R00");
/*    serial.flush();
    serial.clear();
    serial.clearError();*/

    QString incomming;

    qint8 i=0;
    while(1){

    if(serial->waitForReadyRead(20)){
                while(serial->canReadLine()) {
                    incomming = serial->readLine();
                    qDebug() << incomming << " " << i;
                    i++;
                }



     }//wait for readyRead

        //    serial.flush();
/*        QStringList msgs = incomming.split('\n');
        foreach(const QString &msg, msgs){
            if (msg != "") qDebug() << msg + '\n' << " " << i++;
        }
        incomming = "";*/
    }
    emit finished();
}
//***** //EOAC2 **********************
//BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
//** base ********************************************
//BBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBBB
SerialPort::SerialPort(QObject *parent): QObject(parent)
{

    makeCnx();
    thRead = new QThread;
    readPrt = new readPort(&serial);
    readPrt->moveToThread(thRead);
    qDebug() << " 1Read" << connect(readPrt, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
    qDebug() << connect(thRead, SIGNAL(started()), readPrt, SLOT(process()));
    qDebug() << connect(readPrt, SIGNAL(finished()), thRead, SLOT(quit()));
    qDebug() << connect(readPrt, SIGNAL(finished()), readPrt, SLOT(deleteLater()));
    qDebug() << connect(thRead, SIGNAL(finished()), thRead, SLOT(deleteLater()));
    thRead->start();

    thWrite = new QThread;
    writePrt = new writePort();
    writePrt->moveToThread(thRead);
    qDebug() << " 1Write" << connect(writePrt, SIGNAL(error(QString)), this, SLOT(errorString(QString)));
    qDebug() << connect(thWrite, SIGNAL(started()), writePrt, SLOT(process()));
    qDebug() << connect(writePrt, SIGNAL(finished()), thWrite, SLOT(quit()));
    qDebug() << connect(writePrt, SIGNAL(finished()), writePrt, SLOT(deleteLater()));
    qDebug() << connect(thWrite, SIGNAL(finished()), thWrite, SLOT(deleteLater()));
    thWrite->start();


}
//*******************************************************
void SerialPort::makeCnx(){

    //serial = new QSerialPort;

    qint8 p = 0;
    while(p < maxPort){
        QString portType = "COM";
        QString port = portType + QString::number(p);
        serial.setPortName(port);

        if (serial.open(QIODevice::ReadWrite)){
            qDebug() << port << " Open";
            qDebug() << "Baud? " << serial.setBaudRate(QSerialPort::Baud19200);
            qDebug() << "Data bits? " << serial.setDataBits(QSerialPort::Data8);
            qDebug() << "Parity? " << serial.setParity(QSerialPort::NoParity);
            qDebug() << "Stop Bits? " << serial.setStopBits(QSerialPort::OneStop);
            qDebug() << "Flow Control? " << serial.setFlowControl(QSerialPort::NoFlowControl);
            serial.write(askId);
            if(serial.waitForReadyRead(1000)){
                qDebug() << endl << "Response: " << serial.readAll();



            }
                else{
                qDebug() << "time out";
                serial.close();
                }

            return;
        }
        else {
            qDebug() << port << " Not Ready";
        }
        p++;
    }
}
//*********************************************************
SerialPort::~SerialPort(){
   // thRead->finished();
   // thWrite->finished();
    readPrt->~readPort();
    writePrt->~writePort();
}

yczo

Update: one more thing, only once had readed 23 strings, the other times only 22.
On the other hand whenever click-X to exit, the Program crash (no normal exit) ... no idea.

koahnig

@yczo

There are a couple of general things to consider:
The baud rate used to communicate has certainly a major effect. Also the speed of the device you are talking to.
You do not report the baud rate here in your post.

Also how do you run the program? In debug mode with the debugger running it may considerably longer than in release mode.

Your routine

void readPort::process()

is simply an endless loop and the emit finished is not processed. Please check with your code.

You are using canReadLine() for detection of when to read. You can use alternatively readyRead() which will be triggered any time something to read is available. At least during trials to get it to work this might be more helpful. My suspicion is that the last line does not have the line feed triggering the end of a line.

yczo

First time, thank you very much. Yes I'm in debug mode.
The baud rate is serial.setBaudRate(QSerialPort::Baud19200);

My suspicion is that the last line does not have the line feed triggering the end of a line.

Hummm I'm not sure about that, the microcontroller send "\n" at the end of each string

this is the last string

char msgPrueba22[] ="c4+mA+y8+t308+i8F+B6+b7+nRamon   +gGenGroup+v32.1+f2.33+sO+\n";

Greetings

kuzulis

Huh.. I'm surprized that your code get something work.... Your spagetty-code should not work at all, it should get crashed...

Do not use threads at all if you don't know how to use it.
Use async approach with the QSP::readyRead() signal.
QSP::waitForReadyRead() (as well the QSP::readyRead() signal) does not guarantee that you get whole string, it can be even 1 byte - N bytes.

yczo

@kuzulis

Thank you very much for answer. Can you please show me an example over how can I a bidirectional communication make, that not freeze the gui?

I have not understand:

Use async approach with the QSP::readyRead() signal.

Greetings

kshegunov

@kuzulis

Your spagetty-code should not work at all, it should get crashed

Race conditions are usually happening rarely enough, so thread-unsafe code will run fine most of the time ... until it crashes gloriously that is ...

@yczo

Can you please show me an example over how can I a bidirectional communication make, that not freeze the gui?

You should fix your threading before anything else. You can't use objects common to two different threads like you do unless they're thread-safe, which is not the case here. Put one (and only one) worker object that will do the reading and writing from/to the serial port and don't touch the serial port object from a different thread; the serial port class is reentrant, but it's not thread-safe.

After doing that, use the async API (as @kuzulis pointed out) - this means you connect the QSerialPort's signals to slots in your worker object and those slots will be executed when something happens with the port, like data pending for reading. You don't need or wish to use endless loops in this case either. And you actually have the ability to stop your threads and graciously exit, instead of waiting for the OS to forcefully terminate you application (or rather the thread(s) you're running).

Kind regards.

? Offline

@kuzulis said:

Do not use threads at all if you don't know how to use it.

Come on, he's just about to learn it ;-)

kuzulis

Come on, he's just about to learn it ;-)

Well.. but I wouldn't to start learn it for such (real?) task. I would try to simplify everything and to throw out threads. ;)

class ProtocolHandler : public QObject
{
    Q_OBJECT
public:
    explicit ProtocolHandler(QObject *parent = nullptr) 
        : QObject(parent)
        , sp(new QSerialPort(this))
    {
        connect(sp, &QSerialPort::readyRead, this, &ProtocolHandler::parseReceivedData);
    }

    bool start()
    {
        sp->setPortName("COMn");
        // set parameters
        return so->open(QIODevice::ReadWrite);
    }

    void stop() 
    {
        sp->close();
    }

    // connect this slot e.g. to button::clicked (ot to something else) if you want to send data to device
    void writeData(const QByteArray &data)
    {
        sp->write(data);
    }

signals:
    void receivedLine(const QByteArray &line);

private:
    void parseReceivedData()
    {
        if (!sp->canReadLine()))
            return;

        const QByteArray line = sp->readLine();
        emit receivedLine(line);
    }
    QSerialPort *sp;
}

koahnig

@kuzulis said:

Well.. but I wouldn't to start learn it for such (real?) task. I would try to simplify everything and to throw out threads. ;)

You are completely right with your statement, but only at back sight.
The issue is that it is not obvious that are no threads required. Starters with too much background know that there have to be done things in parallel, especially with serial comms and lower baud rates, because you do not want to loose bytes. Therefore threads are the way to go.

YangPengbo

This is my program to avoid losting the last string.

void MainWindow::readyReadSlot()
{

static QByteArray byteArray;         //静态变量！！在串口只发送一半的时候用来累加数据

QByteArray temp = m_reader.readLine();

   if(!temp.isEmpty())
   {
       byteArray+=temp;
       if(byteArray.contains("\n"))
       {
           int __RealPulse ,__Speed;

           sscanf( byteArray, "*%d,%f\n", &__RealPulse, &__Speed);

           this->ui->lineEdit->setText(byteArray);

           byteArray.clear();

       }
   }

}