1

I would like to send and receive a structure using QTCPSocket.

the System does have a central Socket Server and a lot of Qt Client Applications. Each client sends a struct which is broadcast to the rest by the central socket server.

Connector extends QTcpSocket

this is my method for connecting

void Connector::Open(QString address, int port) // It works!
{
    connectToHost(address, port);
    this->onWaiting();

    connect(this, SIGNAL(connected()), this, SLOT(connected()));
    connect(this, SIGNAL(disconnected()), this, SLOT(disconnected()));
    connect(this, SIGNAL(readyRead()), this, SLOT(readyRead()));
    connect(this, SIGNAL(bytesWritten(qint64)), this, SLOT(bytesWritten(qint64)));
}

This is my method for sending data

void Connector::SendData(const void* data,qint64 len) // It works!
{
    writeData((char*)data,len);
    waitForBytesWritten();
}

this is for receiving

void Connector::readyRead()  // Don't work
{
    Information n; //My struct 
    memcpy(&n, data.data(), data.size());
    onInformationReceived(n);
}

But the information received is always invalid. Is that correct?

roalz
  • 2,699
  • 3
  • 25
  • 42
Daniel Santos
  • 14,328
  • 21
  • 91
  • 174

1 Answers1

3

There are some problems:

  1. The readyRead() slot can be invoked with any number of bytes available for reading. The only guarantee you have is that it's more than zero bytes, although there's no reason to count on it in practice. You have to check bytesAvailable. It can be one byte. It can be a megabyte.

    See e.g. this answer and that answer for narrative, and that one for code.

    At the very minimum, you'd need:

    void Connector::readyRead() {
      if (bytesAvailable() < sizeof(Information)) return;
      Information info;
      auto const data = read(sizeof(Information));
      memcpy(&info, data.constData(), data.size());
      onInformationReceived(info);
    }
    
  2. Sending a struct as an opaque binary over the wire will not work. There are no guarantees that the receiving end will lay out the struct the same way. You can't even know what the binary format of the data is, unless you refer to your compiler ABI documentation.

    See e.g. this answer for how to do it properly.

    At the very minimum, you'd need to implement the QDataStream streaming operators for Information, and then use it on both ends of the connection:

    QDataStream & operator<<(QDataStream &, const Information &);
    QDataStream & operator>>(QDataStream &, Information &);
    
    void Connector::send(const Information & info) {
      auto dev = this; // We're a QIODevice :(
      QDataStream stream{dev};
      stream << info;
      dev.waitForBytesWritten(); // this is superbly bad!
    }
    
    void Connector::readyRead() {
      auto dev = this; // We're a QIODevice :(
      QDataStream stream{dev};
      stream.startTransaction();
      Information info;
      dev >> info;
      if (! stream.commitTransaction()) return;
      onInformationReceived(info);
    }
    
  3. You shouldn't be extending QTcpSocket. Your class should be a QObject that holds a QTcpSocket by value.

  4. You should never block.

Thus, a fixed up Connector class might look as follows. Again, this answer describes how to use QDataStream correctly to account for compatibility with future updates of your code. But also see this answer for how to leverage the read transactions in Qt 5.7's QDataStream.

class Connector : public QObject {
  Q_OBJECT
  QTcpDevice m_dev;
  QDataStream m_str{&m_dev};
  void onReadyRead() {
    m_str.startTransaction();
    Information info;
    m_str >> info;
    if (! stream.commitTransaction()) return;
    onInformationReceived(info);
  }
  void onBytesWritten() {
    if (m_dev.bytesToWrite() == 0)
      emit allDataSent();
  }
  void onInformationReceived(const Information &);
public:
  Connector(const QString & address, int port, QObject * parent = nullptr) :
    QObject{parent}
  {
    connect(&m_dev, &QAbstractSocket::connected, this, &Connector::connected);
    connect(&m_dev, &QAbstractSocket::disconnected, this, &Connector::disconnected);
    connect(&m_dev, &QIODevice::readyRead, this, onReadyRead);
    connect(&m_dev, &QIODevice::bytesWritten, this, onBytesWritten);
    m_dev.connect(address, port);
  }
  void send(const Information & info) {
    m_str << info;
  }
  Q_SIGNAL void connected();
  Q_SIGNAL void disconnected();
  Q_SIGNAL void allDataSent();
}
Community
  • 1
  • 1
Kuba hasn't forgotten Monica
  • 95,931
  • 16
  • 151
  • 313