Architecture problem using threads, custom events and signals/slots

[yellowmat]

I don't know the "good-way-programming" with thread so I don't know if it is correct to do the way I do. I didn't it was a problem to create an instance in one thread and use it in another ... I didn't knew since it brings me trouble with data sharing and so on

[jacek]

I don't know the "good-way-programming" with thread so I don't know if it is correct to do the way I do. I didn't it was a problem to create an instance in one thread and use it in another ...

It looks like QextSerialPort has some protection against problems with multiple threads, but still I think that you should instantiate it in CSerialDataFrameDecoder::run().

I'm not sure why did you split the decoding in two parts (CSerialDataFrameDecoder::run() and CSerialPort::customEvent()). If you create a separate thread for decoding why don't you let it do the whole job?

Another thing is that in several places you create a temporary frame object just to copy it later:

CSerialDataFrame aFrame;
aFrame.setSerialDataFrameValue(frame);
aFrame.setSerialDataFrameStatus(CSerialDataFrame::SerialDataFrameStatusIncompleteHasJustTheEndingPart);
this->serialDataFrameDecoded(aFrame);

CSerialDataFrame aFrame;
aFrame.setSerialDataFrameValue(frame);
aFrame.setSerialDataFrameStatus(CSerialDataFrame::SerialDataFrameStatusIncompleteHasJustTheEndingPart);
this->serialDataFrameDecoded(aFrame);

To copy to clipboard, switch view to plain text mode 

Wouldn't it be easier to allocate that CSerialDataFrame on the heap?

CSerialDataFrame *aFrame = new CSerialDataFrame( QDeepCopy<QString>( frame ),
                                                 CSerialDataFrame::SerialDataFrameStatusIncompleteHasJustTheEndingPart );
serialDataFrameDecoded( aFrame );
 
//...
 
void CSerialDataFrameDecoder::serialDataFrameDecoded( CSerialDataFrame * frame )
{
    // Allocate some memory for the custom event
    QCustomEvent* ce = new QCustomEvent( 1002, data );
 
    // Send the event
    QApplication::postEvent( objectOwner, ce );
    // The allocated memory for the custum event is freed by Qt so the receiver neither the sender MUST NOT free it
}

CSerialDataFrame *aFrame = new CSerialDataFrame( QDeepCopy<QString>( frame ),
                                                 CSerialDataFrame::SerialDataFrameStatusIncompleteHasJustTheEndingPart );
serialDataFrameDecoded( aFrame );

//...

void CSerialDataFrameDecoder::serialDataFrameDecoded( CSerialDataFrame * frame )
{
    // Allocate some memory for the custom event
    QCustomEvent* ce = new QCustomEvent( 1002, data );
    
    // Send the event
    QApplication::postEvent( objectOwner, ce );
    // The allocated memory for the custum event is freed by Qt so the receiver neither the sender MUST NOT free it
}

To copy to clipboard, switch view to plain text mode 

And the last thing --- you should define that "1002" as constant.

[yellowmat]

Ok.

I'll do all those changes in CSerialDataFrameDecoder and keep you informed.

Thanks for your help, it is precious.