serial Communication using Threading

Christian Ehrlicher

What's your question?
As always when using threads for asnyc operations in Qt - why?
'QByteArray *sendData = new QByteArray();' - it's not used and for sure not needed on the stack

JonB

@Vijaykarthikeyan
Is there a question here?

Your read_data() assumes that after reading a single byte it can then immediately read further bytes (serial->read(sizeof(MyStruct)-1)). You cannot assume this will be the case (and you do not even check the size of any returned data there, which you absolutely should). This is not the way it works. It is your job to write code which accumulates data received --- possibly through multiple calls to read_data()/serial->read() --- until enough bytes have been received to form a complete structure.

Vijaykarthikeyan

@Vijaykarthikeyan said in serial Communication using Threading:

#include "mainwindow.h"
#include <QThread>
#include <thread>

MainWindow::MainWindow(QWidget *parent)
    : QMainWindow(parent)
{
    serial = new QSerialPort(this);
    serial->setPortName("COM7");
    serial->setBaudRate(QSerialPort::Baud115200);
    serial->setDataBits(QSerialPort::Data8);
    serial->setParity(QSerialPort::NoParity);
    serial->setStopBits(QSerialPort::OneStop);
    serial->setFlowControl(QSerialPort::NoFlowControl);

    if (serial->open(QIODevice::ReadWrite))
    {
        qDebug() << "Port opened";

        QThread::msleep(1000);
        timer = new QTimer(this);
        connect(timer, &QTimer::timeout, this, &MainWindow::send_data);
        connect(timer, &QTimer::timeout, this, &MainWindow::read_data);
        timer->start(500);

    }
    else
    {
        qDebug() << "Port not opened";
    }

}


MainWindow::~MainWindow()
{

}

void MainWindow::send_data()
{

    QByteArray *sendData = new QByteArray();
        MyStruct sendStruct ;
        AHRS ahrs_data;
        ahrs_data.yaw =50.66;
        ahrs_data.pitch=20.77;
        ahrs_data.roll=17.89;

        memcpy(sendStruct.payload, &ahrs_data, sizeof(AHRS));
        sendStruct.magic=0xFD;
        sendStruct.length=sizeof(AHRS);
        sendStruct.sequence=1;
        sendStruct.sysId=1;
        sendStruct.compId=2;
        sendStruct.msgId=1;

        //sendData->append(reinterpret_cast<const char*>(&sendStruct), sizeof(MyStruct));
        serial->write(reinterpret_cast<const char*>(&sendStruct), sizeof(MyStruct));

        delete sendData;
}

void MainWindow::read_data()
{
        if(serial->bytesAvailable())
        {
        QByteArray receivedData,receivedData2;
        MyStruct receivedStruct ;
        receivedData2 = serial->read(1);

        if(receivedData2.contains(253))
        {

            receivedData = serial->read(sizeof(MyStruct)-1);
            memcpy(&receivedStruct, receivedData2+receivedData.constData(), sizeof(MyStruct));
            AHRS ahrs_data;
            memcpy(&ahrs_data,receivedStruct.payload,sizeof(AHRS));
            qDebug()<<ahrs_data.yaw<<"\t"<<ahrs_data.pitch<<"\t"<<ahrs_data.roll;

        }

        else
        {
        }

        }

}

I have been trying to do serial port communication using same single serial port where I successfully implemented sending and receiving the struct data. There is no problem in it. But, Now., I want to use Threading .The receiving logic should move to Thread so there's no interruption for sending the struct data. I want to use same serial port. How to create a thread within the same MainWindow.cpp without any need of separate class

Christian Ehrlicher

@Vijaykarthikeyan said in serial Communication using Threading:

Now., I want to use Threading .The receiving logic should move to Thread so there's no interruption for sending the struct data.

Why? 'I want' is no reason. And as we already said - it's not needed at all.

Vijaykarthikeyan

@JonB It works perfectly..it is checking that number 253 .If it matches, it should read the remaining data from the stream. IT gives output like this:

50.66 20.77 17.89
50.66 20.77 17.89

My question which I forgot to add is that I want my read_data() to work in a separate thread. I worked out in separate Class/file. But,it needs the same serial port. So,I'm quering that how to implement threading within the same MainWindow.cpp so I could pass the serial in read_data() function which is running in the thread

JonB

@Vijaykarthikeyan

It works perfectly

Then you are "fortunate". The code should be corrected as I wrote. If you insist on implementing it your way, why do you not check the return result of receivedData = serial->read(sizeof(MyStruct)-1);?

read_data() would be better implemented as a slot attached to readyRead() rather than your QTimer principle.

As @Christian-Ehrlicher has said, with readyRead() signals there is no need to use a thread here. Everyone wants to get into threads when they are (a) not needed and (b) the cause of coding mistakes (just read how many posts there are about thread problems).

Vijaykarthikeyan

@Christian-Ehrlicher I'm little bit worried of performance. Is it fine to send and receive at the same time. Will it send and receive concurrently (or) parallely without any lag. As, I want much reduced latency. Also, I gone through that threading will be the good way for multiple tasks

Christian Ehrlicher

We already said mulitple times: Qt is asynchronous - your data will be sent in the background already now. No need for threads, esp. not when you did not notice a bottleneck (and don't have much clue about threading).

Vijaykarthikeyan

@JonB When using ready read() signal..it throws the error like this:

Port opened
50.66 20.77 17.89
-4.21704e+37 9.11138e-41 1.95399e-13
-4.21704e+37 9.11138e-41 1.56319e-13
50.66 20.77 17.89
-4.21704e+37 -1.99953e+18 3.76588e-13

That's Why i choose timer for receiving data

Vijaykarthikeyan

@Christian-Ehrlicher I have done a sample threading example creating 2 separate classes,called the thread in main class. But, it is not using mainWindow.cpp members and functions.

But,here,in this case I want to access the port which is in use by mainWindow.cpp..

There's where I find the bottle neck

SGaist

Hi,

As is usual with serial ports, do you have a proper protocol in place to know when you have received a full frame of your data ? Size is no such protocol, time as well.

The proper way do handle data is to cumulate what you read until you have a frame and then process it.

Don't involve threads in there until it is proven by a real benchmark that you need it. As my fellow already wrote, Qt is asynchronous and does not need threading for most tasks such as serial port handling.

Vijaykarthikeyan

@SGaist Yes.. It's the universal Mavlink message format. By implementing the above mentioned method. It is correctly sending and receiving the data. This is different from other format of receiving data. We could only have the required data based upon the the reading of the pre determined size as it is sending continously and we dont know when will that magic number 253..it could appear from anywhere but after the pack of data.That's why it is reading the stream based upon the size

JonB

@Vijaykarthikeyan said in serial Communication using Threading:

@JonB When using ready read() signal..it throws the error like this:

readyRead() will not throw an error or be in an error case. Your code will be incorrect. I explained what you need to do for correct code for asynchronous serial port reading.

Anyway, we cannot convince you of this. If you are determined to use threading then go ahead.

Vijaykarthikeyan

@JonB No..I'm with the side of Qt community.I seek the community for a small doubt?that's all. If you clarify me or telling me the solution, Ill gladly accept it

vrooktreat

The proper way do handle data is to cumulate what you read until you have a frame and then process it.
plus whatsapp

project free tv

Vijaykarthikeyan

@vrooktreat yes..That's why I have read a struct size of data

JonB

@Vijaykarthikeyan
No, you do not accumulate the data received as you should. I have previously said that you just assume receivedData = serial->read(sizeof(MyStruct)-1); will work, you don't bother checking and you don't care. And it assumes read() will always immediately return the rest of the "packet" after the initial byte of 253 arrives, which is just not what Qt/serial port guarantees. @vrooktreat is saying the same.

I am not saying that correcting that will necessarily resolve whatever your issue is (though it likely will for whatever your implementation of readyRead() slot was which went wrong), but it's not correct/makes an unnecessary assumption.

Vijaykarthikeyan

@JonB Ok. I agree with you. I'm doing the wrong way. But,replacing timer with readyRead() signal throws the garbage..this is my 1st issue.If my code implementation is wrong..please correct me.

My aim is to send mavlink like struct data from one device to my device.For that,the 1st byte should be the magic number(in this case, I have used 253(0xFD in hexadecimal)).If and only if matches, it reads through the remaining bytes which is the size of my struct. We can't send/receive struct data type through serial port.That's Why Im using QByteArray.

So,as far as your suggestion, How to cumulate the data? I know this issue is out of text thing which I have asked . Im ready to receive your coorrections

jsulm

@Vijaykarthikeyan said in serial Communication using Threading:

But,replacing timer with readyRead() signal throws the garbage

What does this mean? How does your code using readyRead() look like now? Do you accumulate the data?
You need to accumulate data you get every time readyRead() is emitted until you get a whole frame of data. Just like @SGaist and @JonB suggested.

Vijaykarthikeyan

@Vijaykarthikeyan said in serial Communication using Threading:

@jsulm When using ready read() signal..it throws the error like this:

Port opened
50.66 20.77 17.89
-4.21704e+37 9.11138e-41 1.95399e-13
-4.21704e+37 9.11138e-41 1.56319e-13
50.66 20.77 17.89
-4.21704e+37 -1.99953e+18 3.76588e-13

That's Why i choose timer for receiving data

Already said that corrections are welcomed.. thank you