Create object in another object

jsulm

@Damian7546 said in Create object in another object:

m_outFrameQueue = outFrameQueue;

And is outFrameQueue a valid pointer?

Damian7546

This is my declaration
QQueue<RfidFrame*> *m_outFrameQueue;

With RfidFrame I want to prepare my frame to send:

#include <QObject>

class RfidFrame : public QObject
{
    Q_OBJECT
public:
    static const quint8 FRAME_ADDR_RFiD = 0x01;
    static const int INDEX_ADDR_RFiD = 0;
    static const int INDEX_DATA_LENGTH = 1;
    static const int INDEX_DATA_CMD = 2;
    static const int INDEX_FIRST_DATA_BYTE = 3;
    static const int INDEX_FIRST_ID_DATA = 5;

    explicit RfidFrame(QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, quint8 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, qint8 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, quint16 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, qint16 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, quint32 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, qint32 data, QObject *parent = nullptr);
    explicit RfidFrame(quint8 addr, quint8 cmd, QByteArray data, QObject *parent = nullptr);
    ~RfidFrame();

    quint16 CalculateCRC2_in(QByteArray data);

private:
    QByteArray m_buffer;

};

For example sending this frame I do in this way

RfidFrame *frameToSend = new RfidFrame(addr, C_WriteSourceRSx, data);
    m_outFrameQueue->enqueue(frameToSend);

Like I said, in main.cpp it works...

Damian7546

@jsulm probably it is not a problem with initialisation.

I added to my code first frame to send like below:

QQueue<RfidFrame*> outFrameQueue;

    RfidFrame *frameToSend = new RfidFrame(1, 1, 1);
    outFrameQueue.enqueue(frameToSend);

    QThread *threadRfidSerial = new QThread();
    RfidSerialWorker *rfidSerialWorker = new RfidSerialWorker(&outFrameQueue);
    RfidFrameProcessor *rfidFrameProcessor = new RfidFrameProcessor(&outFrameQueue);

    rfidSerialWorker->moveToThread(threadRfidSerial);
    QObject::connect(rfidSerialWorker, SIGNAL(frameReceived(RfidFrame*)), rfidFrameProcessor, SLOT(FrameIncoming(RfidFrame*)));
    QObject::connect(rfidSerialWorker, SIGNAL(workRequested()), threadRfidSerial, SLOT(start()));
    QObject::connect(threadRfidSerial, SIGNAL(started()), rfidSerialWorker, SLOT(doWork()));
    QObject::connect(rfidSerialWorker, SIGNAL(finished()), threadRfidSerial, SLOT(quit()), Qt::DirectConnection);


    rfidSerialWorker->abort();
    threadRfidSerial->wait(); // If the thread is not running, this will immediately return.
    rfidSerialWorker->requestWork();

Christian Ehrlicher

@Damian7546 said in Create object in another object:

rfidSerialWorker->abort();
threadRfidSerial->wait(); // If the thread is not running, this will immediately return.
rfidSerialWorker->requestWork();

What should this do? Where is this called? Directly after the connect() statements? How is this supposed to work then and why a thread for this stuff at all?

Damian7546

Please see my connect functions and below:

void RfidSerialWorker::requestWork()
{
    mutex.lock();
    _working = true;
    _abort = false;
    qDebug()<<"Request worker start in Thread "<<thread()->currentThreadId();
    mutex.unlock();


    emit workRequested();
}
void RfidSerialWorker::abort()
{
    mutex.lock();
    if (_working) {
        _abort = true;
        qDebug()<<"Request worker aborting in Thread "<<thread()->currentThreadId();
    }
    mutex.unlock();
}

Christian Ehrlicher

This does not answer my questions - why do you call abort() directly after creating all the stuff and then block the main event loop until the thread is finished - why (and why a thread at all - what's the advantage except strange problems like yours?)

Damian7546

I simplified my code as below: And still the same problem.

RfidSystemController::RfidSystemController( QObject *parent)
    : QObject(parent)
{

    QQueue<RfidFrame*> outFrameQueue;
    QThread *threadRfidSerial = new QThread();
    RfidSerialWorker *rfidSerialWorker = new RfidSerialWorker(&outFrameQueue);
    RfidFrameProcessor *rfidFrameProcessor = new RfidFrameProcessor(&outFrameQueue);

    rfidSerialWorker->moveToThread(threadRfidSerial);
    connect(rfidSerialWorker, SIGNAL(frameReceived(RfidFrame*)), rfidFrameProcessor, SLOT(FrameIncoming(RfidFrame*)));
    threadRfidSerial->start();
    connect(threadRfidSerial, SIGNAL(started()), rfidSerialWorker, SLOT(doWork()));

}

And class where is a problem :

#include "rfidserialworker.h"
#include <QTimer>
#include <QEventLoop>
#include <QThread>
#include <QDebug>
#include <QFile>

RfidSerialWorker::RfidSerialWorker(QQueue<RfidFrame*> *outFrameQueue, QObject *parent) :
    QObject(parent)
{
    m_Serial = nullptr;
    m_outFrameQueue = outFrameQueue;
}

RfidSerialWorker::~RfidSerialWorker()
{
    if(m_Serial != nullptr)
        delete m_Serial;
}

void RfidSerialWorker::doWork()
{

    qDebug()<<"Starting worker process in Thread "<<thread()->currentThreadId();

    // Serial Port Initialization
    m_Serial = new QSerialPort();
    m_Serial->setPortName("COM1");
    m_Serial->setBaudRate(QSerialPort::Baud9600);
    m_Serial->setDataBits(QSerialPort::Data8);
    m_Serial->setParity(QSerialPort::NoParity);
    m_Serial->setStopBits(QSerialPort::OneStop);
    m_Serial->setFlowControl(QSerialPort::NoFlowControl);

    QObject::connect(m_Serial,SIGNAL(readyRead()),this, SLOT(recive()));
    m_Serial->open(QIODevice::ReadWrite);
    qDebug() << "SerialPort Status: " << m_Serial->isOpen();

    emit comPortStatus(m_Serial->isOpen());


    while(1)
    {
            if(!m_outFrameQueue->isEmpty())
            {
                RfidFrame *outFrame = m_outFrameQueue->dequeue();
                sendFrame(outFrame);
                delete outFrame;

            }
            else
            {
                m_Serial->waitForReadyRead(10);
            }
    }
    if(m_Serial != nullptr)
    {
        m_Serial->close();
        qDebug() << "SerialPort Closed";
        delete m_Serial;
        qDebug() << "SerialPort destroyed";

    }
    qDebug()<<"Worker process finished in Thread "<<thread()->currentThreadId();
    emit finished();

}
void RfidSerialWorker::recive()
{
    receivedData = receivedData + m_Serial->readAll();
    if(!receivedData.isEmpty() && receivedData.count() > 2 )
    {
        if(receivedData.count() == quint8(receivedData[RfidFrame::INDEX_DATA_LENGTH] ) )
        {
            qDebug() << "We have the full frame";
            receivedFrame = receivedData;
            receivedData.clear();
            parseRecivedData();
        }
    }
}

In While loop should works like : While sending buffer is not empty then send frame, otherwise waitForReadyRead(10) to give a chance to call recive() function from connect(m_Serial,SIGNAL(readyRead()),this, SLOT(recive())); signal. ...

jsulm

@Damian7546 Again: why do you need a thread? Qt is assynchronous framework, QSerialPort is also assynchronous...

Damian7546

I would like to separate above task form others. This project is for learning programming purposes only.
In my example using thread it is a mistake ?

jsulm

@Damian7546 said in Create object in another object:

In my example using thread it is a mistake ?

You should avoid threads if they are not needed as it is easy to do mistakes and the code becomes more complex. As I wrote: Qt is an asynchronous framework, that means you usually do not need threads. QSerialPort is also asynchronous - you can use it without threads and it will still not block your main thread. Threads are usually used to move heavy calculations to other threads, but I don't see any heavy calculations in your code.

Christian Ehrlicher

@Damian7546 said in Create object in another object:

This project is for learning programming purposes only

Then start with writing clean code without threads. Write a good handler class to handle your incoming data. Once this is done this class can be easily moved into a separate thread if really needed.

Damian7546

anyway, can anyone demonstrate why the above code doesn't work when created from the constructor? but works with the main.cpp one?

jsulm

@Damian7546 If your application crashes then the first thing to do is: use the debugger.
So, please run your app in debugger and post the stack trace after the crash here...

Christian Ehrlicher

Your code crashes in the dtor of RfidSerialWorker (if doWork() would reach it's end), also it's using a blocking approach instead signals and slots to wait for data, the strange while(1) - loop does never exits and at last ít's accessing outFrameQueue from two different threads without proper locking mechanisms (and before you ask - read and understand at least https://doc.qt.io/qt-6/threads-synchronizing.html before fiddling around with threads for no reason).

Damian7546

@Christian-Ehrlicher so I will do like you propose.

Damian7546

I think that no a problem with thread.

I remove thread and create my object like below:

  QQueue<RfidFrame*> outFrameQueue;
    RfidSerialWorker *rfidSerialWorker = new RfidSerialWorker(&outFrameQueue);
    RfidFrameProcessor *rfidFrameProcessor = new RfidFrameProcessor(&outFrameQueue);

    connect(rfidSerialWorker, SIGNAL(frameReceived(RfidFrame*)), rfidFrameProcessor, SLOT(FrameIncoming(RfidFrame*)));
    connect(rfidFrameProcessor, SIGNAL(frameSend()), rfidSerialWorker, SLOT(frameToSend()));
    rfidSerialWorker->initSerialPort();

    //tests - try send frame cyclically
    QTimer *timer = new QTimer(this);
    QObject::connect(timer, SIGNAL(timeout()), rfidFrameProcessor , SLOT(update_power_ind()));
    timer->start(2000);

Function which one try send frame is below:

void RfidFrameProcessor::update_power_ind()
{
    readSourceRSx(1,9);
}
void RfidFrameProcessor::readSourceRSx(quint8 addr, quint8 source)
{

    QByteArray data;
    data.append(source);
    RfidFrame *frameToSend = new RfidFrame(addr, C_ReadSourceRSx, data);
    qDebug() << "Get lenght from frameToSend: " << frameToSend->GetDataLength();
    m_outFrameQueue->enqueue(frameToSend);
    emit frameSend();

}

and problem is in the same step:
m_outFrameQueue->enqueue(frameToSend); despite the fact that in frameToSend I have data like you can see in below in Application Output.

jsulm

@Damian7546 said in Create object in another object:

and problem is in the same step

And again: is m_outFrameQueue a valid pointer?

Damian7546

@jsulm In my opinion yes, but I am beginner.... Please explain me.
.h

class RfidFrameProcessor : public QObject
{
    Q_OBJECT
public:
    explicit RfidFrameProcessor(QQueue<RfidFrame*> *outFrameQueue, QObject *parent = nullptr);
private:
    QQueue<RfidFrame*> *m_outFrameQueue;
}

.cpp

RfidFrameProcessor::RfidFrameProcessor(QQueue<RfidFrame*> *outFrameQueue,QObject *parent)
    : QObject(parent)
{
    m_outFrameQueue = outFrameQueue;
}

jsulm

@Damian7546 said in Create object in another object:

m_outFrameQueue = outFrameQueue;

I laready asked that before: is outFrameQueue a valid pointer?

Damian7546

Ok,
I create in this way:

 QQueue<RfidFrame*> outFrameQueue;
    RfidSerialWorker *rfidSerialWorker = new RfidSerialWorker(&outFrameQueue);
    RfidFrameProcessor *rfidFrameProcessor = new RfidFrameProcessor(&outFrameQueue);

But in class

   Q_OBJECT
public:
    explicit RfidFrameProcessor(QQueue<RfidFrame*> *outFrameQueue, QObject *parent = nullptr);
private:
    QQueue<RfidFrame*> *m_outFrameQueue;