Signals/Slots timing issue in an handshake communication protocol

[Andrew23]

My application has to communicate with a custom device through a handshake protocol which provides for a response (ACK or NACK) for every packet/command sent in both directions. Every packets/commands start with 0x02 marker, then two byte for the size, then the type, then data and finally the close marker 0x03. I've created a separate class for the serial communication with `QSerialPort` and created the signals/slots needed:


connect( myInterface, &MyInterface::SendPacket, serialConnection, &SerialConnection::SendPacket, Qt::DirectConnection );
connect( myInterface, &MyInterface::SendAck, serialConnection, &SerialConnection::SendAck, Qt::DirectConnection );
connect( myInterface, &MyInterface::SendNack, serialConnection, &SerialConnection::SendNack, Qt::DirectConnection );
connect( serialConnection, &SerialConnection::ReceivePacket, myInterface, &MyInterface::ReceivePacket, Qt::DirectConnection );
connect( serialConnection, &SerialConnection::ReceiveAck, myInterface, &MyInterface::ReceiveAck, Qt::DirectConnection );
connect( serialConnection, &SerialConnection::ReceiveNack, myInterface, &MyInterface::ReceiveNack, Qt::DirectConnection );

Following my serial communication class (I've omitted the unnecessary parts of code) :

serialconnection.cpp

SerialConnection::SerialConnection( QWidget *parent ) :
QWidget( parent ),
serial( new QSerialPort )
{
connect( serial, &QSerialPort::readyRead, this, &SerialConnection::ReadData );
}

SerialConnection::~SerialConnection()
{

}

void SerialConnection::SendAck()
{
QByteArray ack;

[...]

serial->write( ack );
qDebug() << "-> ack";
}

void SerialConnection::SendNack()
{
QByteArray nack;

[...]

serial->write( nack );
qDebug() << "-> nack";
}

void SerialConnection::SendPacket( QByteArray packet )
{
serial->write( packet );
qDebug() << "-> " + packet.toHex();
}

void SerialConnection::ReadData()
{
[...]
if ( packet.at( 3 ) == COMMUNICATION_USB_PACKET_TYPE_ACK )
{
qDebug() << "<- ack";
emit ReceiveAck();
}
else if ( packet.at( 3 ) == COMMUNICATION_USB_PACKET_TYPE_NACK )
{
qDebug() << "<- nack";
emit ReceiveNack();
}
else
{
qDebug() << "<- packet: " + packet.toHex();
emit ReceivePacket( packet );
}
}

and my interface class:

myinterface.cpp

MyInterface::MyInterface( QWidget *parent ) :
QWidget( parent ),
ui( new Ui::MyInterface )
{
ui->setupUi( this );

connect( ui->pushButtonTest, &QPushButton::clicked, this, &MyInterface::Test );
}


void Onemytis2Interface::ReceiveAck()
{
ackReceived = true;
}

void Onemytis2Interface::ReceiveNack()
{
nackReceived = true;
}

void Onemytis2Interface::ReceivePacket( QByteArray packet )
{
switch ( packet.at( 4 ) )
{
case COMMUNICATION_USB_RX_COMMAND_DEVICE_SERIAL_NUMBER_ GET:
{
if ( packet.size() == COMMUNICATION_USB_RX_COMMAND_DEVICE_SERIAL_NUMBER_ GET_SIZE )
{
emit SendAck();
deviceSerialNumber = packet.mid( 5, 16 );
ui->labelSNValue->setText( deviceSerialNumber );
requestReceived = true;
}
else
{
emit SendNack();
}

break;
}

case COMMUNICATION_USB_RX_COMMAND_DEVICE_TYPE_GET:
{
if ( packet.size() == COMMUNICATION_USB_RX_COMMAND_DEVICE_TYPE_GET_SIZE )
{
emit SendAck();
deviceType = packet.mid( 5, 16 );
ui->labelModelValue->setText( deviceType );
requestReceived = true;
}
else
{
emit SendNack();
}

break;
}

case COMMUNICATION_USB_RX_COMMAND_TEST:
{

break;
}

default:
{
qDebug() << "ERROR";
break;
}
}
}


void MyInterface::Test()
{
RequestType();
RequestSerialNumber();
}


void MyInterface::RequestType()
{
QByteArray request;
QByteArray cmdSize;
uint16_t cmdSizeTmp;
int retry;

request.append( ( char ) COMMUNICATION_USB_PACKET_STX );
request.append( ( char ) 0x00 );
request.append( ( char ) 0x00 );
request.append( ( char ) COMMUNICATION_USB_PACKET_TYPE_COMMAND );
request.append( ( char ) COMMUNICATION_USB_TX_COMMAND_DEVICE_TYPE_GET );
request.append( ( char ) COMMUNICATION_USB_PACKET_ETX );

cmdSizeTmp = request.length();
cmdSize.append( *( char * )&cmdSizeTmp );
cmdSize.append( *( ( char * )&cmdSizeTmp + 1 ) );
request.replace( 1, 2, cmdSize );

ackReceived = false;
nackReceived = false;
requestReceived = false;
emit SendPacket( request );

retry = 0;

while ( ackReceived == false && nackReceived == false )
{
retry++;
QThread::msleep( 1 );

if ( retry > COMMUNICATION_USB_PACKET_WAIT_MAX_MS )
{
qDebug() << "timeout type response";
break;
}
}

if ( ackReceived == true )
{
retry = 0;

while ( requestReceived == false )
{
retry++;
QThread::msleep( 1 );

if ( retry > COMMUNICATION_USB_PACKET_WAIT_MAX_MS )
{
qDebug() << "timeout type request";
break;
}
}
}
else
{
ui->labelTypeValue->setText( "error" );
}
}


void MyInterface::RequestSerialNumber()
{
QByteArray request;
QByteArray cmdSize;
uint16_t cmdSizeTmp;
int retry;

request.append( ( char ) COMMUNICATION_USB_PACKET_STX );
request.append( ( char ) 0x00 );
request.append( ( char ) 0x00 );
request.append( ( char ) COMMUNICATION_USB_PACKET_TYPE_COMMAND );
request.append( ( char ) COMMUNICATION_USB_TX_COMMAND_DEVICE_SERIAL_NUMBER_ GET );
request.append( ( char ) COMMUNICATION_USB_PACKET_ETX );

cmdSizeTmp = request.length();
cmdSize.append( *( char * )&cmdSizeTmp );
cmdSize.append( *( ( char * )&cmdSizeTmp + 1 ) );
request.replace( 1, 2, cmdSize );

ackReceived = false;
nackReceived = false;
requestReceived = false;
emit SendPacket( request );

retry = 0;

while ( ackReceived == false && nackReceived == false )
{
retry++;
QThread::msleep( 1 );

if ( retry > COMMUNICATION_USB_PACKET_WAIT_MAX_MS )
{
qDebug() << "timeout sn response";
break;
}
}

if ( ackReceived == true )
{
retry = 0;

while ( requestReceived == false )
{
retry++;
QThread::msleep( 1 );

if ( retry > COMMUNICATION_USB_PACKET_WAIT_MAX_MS )
{
qDebug() << "timeout sn response";
break;
}
}
}
else
{
ui->labelSNValue->setText( "error" );
}
}

Now, if I click the Test push button, I expect the following transactions:


-> 020600030803 // The interface send the request packet emitting signal in RequestType()
<- ack // The custom device respond with ack, we are waiting it in the RequestType()
<- packet: 02160003034f74686563000000000000000000000003 //The custom device send to interface the requested packet
-> ack // The interface respond with ack
-> 020600030703 // The interface send the request packet emitting signal in RequestSerialNumber()
<- ack // The custom device respond with ack, we are waiting it in the RequestSerialNumber()
<- packet: 02160003034f74686563000000000000000000000003 //The custom device send to interface the requested packet
-> ack // The interface respond with ack

but the interface doesn't work as I expect. Usign the `qDebug` I get


-> 020600030803 // The interface send the request packet emitting signal in RequestType()
timeout type response // I've a timeout waiting the ack/nack for the RequestType() command
-> 020600030703 // The interface send the request packet emitting signal in RequestSerialNumber()
timeout sn response // I've a timeout waiting the ack/nack for the RequestSerialNumber() command
<- ack // HERE I HAVE THE FIRST ACK OF THE RequestType() COMMAND
<- packet: 02160003034f74686563000000000000000000000003
-> ack
<- nack

As you can see, I receive the ack for the first command `RequestType()` AFTER the `Test` function terminates and it means after I send two packet, and this cause an error in communication. The state variable `ackReceived`/`nackReceived` never change after emit the `SendPacket` signal if I remain in the call function scope, also if a use a `Qt::DirectConnection` between signals and slots.

I've not much experience in the Qt signal/slot context and I want to understand what goin on in my code to achieve my target.

Anyone can give me some suggestions and/or clarifications?

[ChrisW67]

Two things.

No data is sent or received until your code returns to the Qt event loop. From what I can see, your interface code goes into a busy wait loop more-or-less guaranteeing that Qt never gets a chance.

When you send or receive data (serial or network) it may not arrive at the other end in the same chunks it was sent. You may get neat whole packets but it would be unwise to rely on this. You seem to be using ReadData to handle incoming bytes. That code must accommodate partial packets and not (as appears to be the case) assume a whole packet at the start of the buffer.