0

I'm writing a simple network application. Client sending to server message server printing it in QTextEdit and responding to client . I'm using QTcpServer and QTcpSocket. There is a problem I can't solve. Receiving data is quint16 + QTime + QString which is sending as QByteArrey. I use quint16 for receiving size of data blocks. And for some reason when client send to server

next block size: 16 (quint16 value)
block size: 18 

Server get:

next block size: 30073  (quint16 value)
block size: 18 

As you can see, for some reason server getting from QDataStrem wrong variable value and it is always 30073. I don't understand why?

void Widget::slotSendToServer()
{
    logTextEdit->append("slotSendToServer()");
    QByteArray arrBlock;
    QDataStream serverSendStream(&arrBlock, QIODevice::ReadWrite);
    QString messageStr = messageLineEdit->text();

    serverSendStream << quint16(0) << QTime::currentTime()
                     << messageStr;

    serverSendStream.device()->seek(0);
    serverSendStream << (quint16)(arrBlock.size() - sizeof(quint16));

    qDebug() << "next_block_size:"
             <<(quint16)(arrBlock.size() - sizeof(quint16)) 
             << endl
             << "full size of Byte arrey:" << arrBlock.size();

    tcpSocket->write(arrBlock);
    messageLineEdit->clear();
}



void Widget::slotReadClient()
{
    logTextEdit->append("slotReadClient()");
    QTcpSocket *tcpSocket = (QTcpSocket*)sender();
    QDataStream clientReadStream(tcpSocket);

    while(true)
    {
        if (!next_block_size)
        {
            if (tcpSocket->bytesAvailable() < sizeof(quint16))
            {
                break;
            }
            clientReadStream >> next_block_size;
        }

        if (tcpSocket->bytesAvailable() < next_block_size)
        {
            break;
        }
        QTime   time;
        QString messageTextStr;
        clientReadStream >> time >> messageTextStr;

        QString messageCompleteStr =
                time.toString() + " " + "Client has sent - " 
                                + messageTextStr;
        logTextEdit->append("Message received: ");
        logTextEdit->append(messageCompleteStr);

        next_block_size = 0;

        sendToClient(tcpSocket,
                     "Server Response: Received \"" 
                      + messageTextStr + "\"");
    }
}
Ilya Glushchenko
  • 317
  • 1
  • 6
  • 13

2 Answers2

2

You should ensure that the variable next_block_size is initialized to 0 each time the socket is connected.

If you don't reuse the same QTcpSocket object, this can be done in your Widget class constructor, or if you do, in a slot connected to signal connected().

alexisdm
  • 29,448
  • 6
  • 64
  • 99
-1

I have no idea why you did it it so complicated. This should work:

void Widget::slotSendToServer()
{
    logTextEdit->append("slotSendToServer()");
    QByteArray arrBlock;
    QDataStream serverSendStream(tcpSocket);

    serverSendStream << QTime::currentTime()
                     << messageLineEdit->text();
    messageLineEdit->clear();
}

void Widget::slotReadClient()
{
    logTextEdit->append("slotReadClient()");
    QTcpSocket *tcpSocket = (QTcpSocket*)sender();
    QDataStream clientReadStream(tcpSocket);

    QTime time;
    QString message;

    clientReadStream >> time >> message;

    emit YourSignal(time, message);
}

You don't have to worry about sizes, QDataStream keep track of it, you only have to maintain same sequence of read as it was done on write, so your buffer arrBlock is waste of code.


According to documentation my code should block when not all data are available at the moment of reading and this is only disadvantage. For small data like date and some string it will never happen.

About your code, you have messed up your reader with while loop for sure, so here is correction without this loop.

void Widget::slotReadClient()
{
    logTextEdit->append("slotReadClient()");
    QTcpSocket *tcpSocket = (QTcpSocket*)sender();
    QDataStream clientReadStream(tcpSocket);

    if (next_block_size==0) {
        // need to read data size
        if (tcpSocket->bytesAvailable() < sizeof(quint16))
            return; // wait for next signal
        // next_block_size must be type of qint16
        clientReadStream >> next_block_size;
    }
    if (tcpSocket->bytesAvailable() < next_block_size) {
        // not enought data to complete read immediately
        return;  // wait for next signal
    }
    QTime time;
    QString messageTextStr;
    clientReadStream >> time >> messageTextStr;
    QString messageCompleteStr =
                time.toString() + " " + "Client has sent - " 
                                + messageTextStr;
    logTextEdit->append("Message received: ");
    logTextEdit->append(messageCompleteStr);

    // mark that data read has been finished
    next_block_size = 0;
}
Marek R
  • 32,568
  • 6
  • 55
  • 140
  • So, even if I have a large file I steel don't care about sizes? – Ilya Glushchenko Jun 30 '13 at 05:29
  • if you are using QDataStream to write/read data than no. It embeds all required information. With large chunks of data there might be a problem with blocking thread, but your example doesn't have this problem. – Marek R Jun 30 '13 at 08:20
  • 1
    This won't work. The slot is called when some data is received, not when all the data required to read the message, so, `QDataStream::status()` would return `QDataStream::ReadPastEnd`, if there is not enough bytes available. That's why you have to use a way to determine if the whole message has arrived, by prepending the size or appending a end-of-message flag to the message. – alexisdm Jun 30 '13 at 10:29
  • size info is needed only to prevent blocking thread, see my update – Marek R Jun 30 '13 at 12:06
  • 1
    The documentation doesn't say that `readData` will block, only that it will return the bytes that are already available (which can be 0), or return -1 to indicate an error or a closed socket. And the function source code shows that, if any amount of data is received, it will return immediately (And IMHO, by design with the Qt API, blocking would mean that the user should be able to specify a timeout, which is only possible through the `waitForReadyRead` function). – alexisdm Jun 30 '13 at 12:29
  • 1
    Also the while loop is necessary, because if several complete messages are received at the same time only one `readyRead()` signal will be emitted, so you won't have any occasion to reread theses messages until some new data arrive and a new `readyRead()` signal is emitted. – alexisdm Jun 30 '13 at 12:35