QT Serial Port Threading Issue

canrollas

Hi, I am currently trying to monitor a device that sends every 10ms after sending the start monitor command.
When I start monitoring there is no error in the program. When I send a stop monitoring command it suddenly crashes. I think my architectural design is wrong in threads. Can anyone help me with this?

This is my UML diagram of Program.
UML class.png

This is my start stop methods of my serialDevice.

This is my send message to serialThread.

Ekran görüntüsü 2022-10-06 111752.png

This is my SenderThread run method:

This is my ReaderThread run method:

And final note: my thread classes gets dynamically allocated pointers of serialport. For example

Ekran görüntüsü 2022-10-06 112202.png

SerialDevice.cpp:

#include "serialdevice.h"
#include <QMutex>

SerialDevice::SerialDevice()
{
    this->baudRate =9600;
    this->serialPortInfo = new QSerialPortInfo();
    this->serialPort = new QSerialPort();
    this->senderThread = new SenderThread();
    this->receiverThread = new ReceiverThread();
    this->readerSerialPort = new QSerialPort();
    connectionStatus = false;
    monitoringHasStarted = false;




}

QString SerialDevice::connectToCanBus(){
    qInfo()<<"Button has clicked!"<<Qt::endl;
    qInfo()<<"Button has clicked-2!"<<Qt::endl;


    if(connectionStatus==false){
        qInfo()<<"Button has clicked-3!"<<Qt::endl;

        foreach (auto looper, serialPortInfo->availablePorts()) {
            qInfo()<<"This is looping device on the machine:"<<looper.description()<<looper.portName();
            if(looper.description()=="Makersan USB to CAN Converter" || looper.description()=="STMicroelectronics Virtual COM Port" && looper.portName().contains("tty")==true || looper.portName().contains("COM")==true){
                qInfo()<<"The device has found in the loop:"<<looper.description();
                QString portname = looper.portName();

                serialPort->setPortName(portname);

                serialPort->setBaudRate(QSerialPort::Baud9600);
                serialPort->setDataBits(QSerialPort::Data8);
                serialPort->setParity(QSerialPort::NoParity);
                serialPort->setStopBits(QSerialPort::OneStop);
                serialPort->setFlowControl(QSerialPort::SoftwareControl);




                if(serialPort->open(QIODevice::ReadWrite)){
                    senderThread = new SenderThread();
                    senderThread->setSerialPort(this->serialPort);
                    int data = 0;
                    qInfo()<<"This is pointer value:"<<data;

                    senderThread->setCommandType(data);
                    qInfo()<<"This part has reached"<<Qt::endl;
                    senderThread->start();
                    qInfo()<<"Starting the thread is success!!";
                    connectionStatus = true;
                    receiverThread->setSerialPort(serialPort);
                    QThread::msleep(2);


                    return "success";

                }else{

                    return serialPort->errorString();
                }
            }else{


                continue;
            }

        }
        return serialPort->errorString();
    }else{
        return "Serial Port already connected!!";
    }

}
void SerialDevice::sendMessage(int *data){
    if(serialPort->isOpen()==true){
        senderThread->setCommandType(*data);
        senderThread->start(QThread::HighestPriority);

    }
}
QString SerialDevice::disconnectMyDevice(){
    if(connectionStatus == true){
        int data = 1;
        int *commandPointer;
        commandPointer = &data;
        this->sendMessage(commandPointer);
        this->connectionStatus = false;

        QThread::msleep(2);
        serialPort->close();

        return "success";
    }else{
        return "Error occured";
    }

}
void SerialDevice::startMonitoring(){

    int data = 2;
    int *commandType;
    commandType = &data;
    this->sendMessage(commandType);
    monitoringHasStarted = true;
    receiverThread->stopValue = false;
    receiverThread->start();


}
void SerialDevice::stopMonitoring(){

    int data = 3;
    int *commandType;
    commandType = &data;
    QMutex mutex;
    receiverThread->stopValue = true;
    receiverThread->terminate();
    this->sendMessage(commandType);


}
QString SerialDevice::getDeviceStatus(){
    return "connectionStatus";
}

SenderThread.cpp

#include "senderthread.h"

SenderThread::SenderThread()
{
}

QSerialPort *SenderThread::getSerialPort()
{
    return serialPort;
}

void SenderThread::setSerialPort(QSerialPort *newSerialPort)
{
    serialPort = newSerialPort;
}

int SenderThread::getCommandType()
{
    return commandType;
}

void SenderThread::setCommandType(int newCommandType)
{
    this->commandType = newCommandType;
}

void SenderThread::run(){
    if(commandType==0){
        // Start the device
        QByteArray b("0149");
        QByteArray bytes = QByteArray::fromHex(b);
        qInfo()<<"Starting the device"<<this->commandType<<Qt::endl;
        mutex.lock();

        serialPort->write(bytes);
        serialPort->waitForBytesWritten();
        qInfo()<<"Starting the device!"<<Qt::endl;
        mutex.unlock();

    }else if(commandType==1){
        // Stop the device
        QByteArray b("0236");
        qInfo()<<"Stopping the device"<<this->commandType<<Qt::endl;

        QByteArray bytes = QByteArray::fromHex(b);
        mutex.lock();

        serialPort->write(bytes);
        serialPort->waitForBytesWritten();
        mutex.unlock();


    }else if(commandType==2){
        // Start monitoring the device

        QByteArray b("0919");
        QByteArray bytes = QByteArray::fromHex(b);
        mutex.lock();

        serialPort->write(bytes);
        serialPort->waitForBytesWritten();
        qInfo()<<"Starting monitor the device"<<this->commandType<<Qt::endl;
        mutex.unlock();


    }else if (commandType == 3){
        // Stop monitoring the device
        QByteArray b("0A53");
        QByteArray bytes = QByteArray::fromHex(b);
        mutex.lock();

        serialPort->write(bytes);
        serialPort->waitForBytesWritten();
        qInfo()<<"Stoping monitor the device"<<this->commandType<<Qt::endl;
        mutex.unlock();

    }
}

Receiver.cpp

#include "receiverthread.h"

ReceiverThread::ReceiverThread()
{

}
void ReceiverThread::run(){
    qInfo()<<"Thread run is started:";

    while(true){

        mutex.lock();
           if(stopValue==true){
               qInfo()<<"The stop value initiated:";

               break;
           }
        QString *dataString = new QString();
        *dataString  = serialPort->readAll();
        if(*dataString!=""){
            qInfo()<<"This is message:"<<*dataString;
        }
        delete dataString;

        this->msleep(2);
        mutex.unlock();

    }
}

QSerialPort *ReceiverThread::getSerialPort() const
{
    return serialPort;
}

void ReceiverThread::setSerialPort(QSerialPort *newSerialPort)
{
    serialPort = newSerialPort;
}

jsulm

@canrollas Hi and welcome!
Is there a reason you're using threads? QSerialPort is asynchronous - usually threads are not needed.
If your app is crashing then the first thing to do is to run it through debugger and see what exactly happens (you can also post stack trace here).

Please post code as text, not pictures.

canrollas

@jsulm Hi, I updated the post with code segments. What you are saying is not correct in my situation. I have to override a thread because my serial device sends data every 10 ms and If I do not a clear device with reading -> Serial device will give an overflow error. And İf you try to get data in every 10ms without threads; your program and ram will be burned.

jsulm

@canrollas Did you actually tested without threads? I don't see why anything would overflow if you implement it properly. Or do you do any heavy calculations?
And did you run now through debugger to see why it is crashing?

J.Hilk

@canrollas no, you're incorrect, 10ms is plenty of time for the asynchronous variant of QSerialPort

other things to note:

your mutex does literally nothing. It would make sense here for getter/setters of commandType , but thats not what you're doing with it
Your QSerialPort instance is passed from outside and accessed inside run. Meaning it lies in the original thread and your thread tries to access it methods. Thats bound to cause issues. I'm surprised this works, one usually has tons of qtimer cannot be started from another thread with this.
why heap allocate QString for readAll call, that returns a "stack" QByteArray ?

canrollas

@J-Hilk Hi thanks for your reply: It makes sense but I can not think thread without this architectural design do you have to suggest changing my design to other types of designs? For example, I have to read and write data simultaneously under milliseconds. But the Qserialport does not allow two connections to the same port.

Kind regards.

J.Hilk

@canrollas lets start with the following:

your receiver is doing nothing but calling serialPort->readAll(); inside a while loop. That you can completely replace with a slot connected to the "readyRead" signal. That signal is emitted as soon as data is available at the port. So exactly the same.

your sender has actually no loop, so simply call the write command when you want to write the command.

and move everything into one class