I'm doing it wrong...again

mzimmers

Hi all -

I have another thread going on this topic, but that's more of a how-to. I think I need a lesson in Qt design.

I'm writing an app that communicates with a remote device via UDP sockets. The rub is that the host system may have multiple network interfaces, and the program needs to determine the correct one to use. My method for this seemed simple enough at first: get a list of the interfaces, try each one, and select the one that responds appropriately.

The asynchronous nature of Qt socket reads/writes has complicated this, though. I'd like to run a routine at startup (from the c'tor, even) that does this, but all I can do is the sends; the reads will be blocked until I exit my startup routine. How do I get around this?

Thanks...

kshegunov

@mzimmers said in I'm doing it wrong...again:

I'd like to run a routine at startup (from the c'tor, even) that does this, but all I can do is the sends; the reads will be blocked until I exit my startup routine. How do I get around this?

Use a queued call. QMetaObject::invokeMethod with Qt::QueuedConnection is your friend in these situations. You can queue the sends until after your init goes (as it may not be appropriate in the constructors), and receive the replies in a (private) slot where you determine which one is the valid interface.

As a side note, have you considered the case where you may get multiple responses (i.e. interfaces) and thus need to handle this separately after? Is it possible or significant?

mzimmers

Are you suggesting I queue my sends? Not sure I understand how this would work.

I was thinking that I'd try one interface, give it some period of time (<1 second) to return something valid, then move onto the next interface. If none work out, I'd give the user a message and terminate. Are you suggesting that I do this in parallel instead -- sending on all the interfaces, and then waiting for a response from any of them?

EDIT: even though I don't fully understand this, I decided to experiment a little with it. I don't seem to be properly forming the call to invokeMethod():

QUdpSocket *m_sock = nullptr;
...
qint64 UdpSocket::send(string str) {...}
...
qint64 bytesSent;
rc = QMetaObject::invokeMethod(m_sock, "send", Qt::QueuedConnection,
                               Q_RETURN_ARG(qint64, bytesSent),
                               Q_ARG(std::string, str));

Can anyone see what I'm doing wrong here? I'm getting the error message: "QMetaObject::invokeMethod: No such method QUdpSocket::send(std::string)"

SGaist

Hi,

From the looks of it, you didn't make that method Q_INVOKABLE.

mzimmers

No, I didn't. (I'd never seen that before.) Unfortunately, that didn't change the outcome -- I'm still getting the error message.

kshegunov

Sorry I wrote that in passing as I was leaving work. I could've and probably should've elaborated more.

@mzimmers said in I'm doing it wrong...again:

Are you suggesting I queue my sends? Not sure I understand how this would work.

No, not the sends themselves, they're already async. I meant that the constructor may not be the best place to do them (depending on your code), so if you want to postpone them for after the constructor you can do that through QMetaObject::invokeMethod. I had such a case today, where I needed to call a virtual method, but the constructor is less than an ideal place for that to happen ...

I was thinking that I'd try one interface, give it some period of time (<1 second) to return something valid, then move onto the next interface. If none work out, I'd give the user a message and terminate. Are you suggesting that I do this in parallel instead -- sending on all the interfaces, and then waiting for a response from any of them?

Very much so, yes. The other way can get very latent for no good reason. Say you have 3 interfaces configured and want to try them sequentially, this means waiting for 2 seconds + the response time. If you do that in parallel it's only the response time and/or a timeout (depending on the answers to the questions from my previous post). I was thinking more - collect the responses in a slot and decide what to do with them thereafter. Something like (pseudocode):

int interfacesNumber;
UdpSocket * sockets = new UdpSocket[interfacesNumber];
for (...)  {
    // Configure and send here
    // Collect responses in a slot
    QObject::connect(udpSocket, &QUdpSocket::datagramPending, this, std::bind(&MyClass::handleResponder, this, udpSocket));
    QObject::connect(&timeoutTimer, &QTimer::timeout, udpSocket, &QObject::deleteLater);
}

Where the slot could be something like:

void MyClass::handleResponder(QUdpSocket * socket)
{
    QObject::disconnect(&timeoutTimer, nullptr, socket, nullptr); //< Disconnected everything so it doesn't get destroyed later
    // Save the socket for later ...
}

kshegunov

@mzimmers said in I'm doing it wrong...again:

I'm still getting the error message.

You can't return (values) from queued connections, it breaks determinism and as such is not allowed.

mzimmers

I removed the return argument and changed the type to void. Still getting the error. Is passing arguments permitted?

kshegunov

@mzimmers said in I'm doing it wrong...again:

Still getting the error. Is passing arguments permitted?

Yes, if they're declared and registered as metatypes (which also implies public default constructor, public copy constructor and destructor). See Q_DECLARE_METATYPE and qRegisterMetaType in the docs for the full picture.

mzimmers

Hmm...I'm already registering it (in main):

    qRegisterMetaType<std::string>("std::string");

I added this in my header file:

    Q_DECLARE_METATYPE(std::string)

Is this not correct?

kshegunov

Yes, for most cases. It's correct, but the alias you provided isn't matching exactly. You do this:

qRegisterMetaType<std::string>("std::string");

But the function prototype spells:

qint64 UdpSocket::send(string str) {...}

So string is unknown. If you expand the namespace, then you need to give the unqualified class name as a string argument. So it'd be like this in your case:

qRegisterMetaType<std::string>("string");

Also make sure you object really has this method, as I see one m_sock being QUdpSocket, while the function prototype is for UdpSocket.

mzimmers

Yep...I should have specified "this" as the first argument in the invokeMethod() call. Seems to be working now. Here are some snippets:

 UdpSocket::UdpSocket(QObject *parent) : QObject(parent)
{
    bool rc;
    // set up addresses.
    m_addrRecv.setAddress(QHostAddress::AnyIPv4);
    m_addrSend.setAddress(MCAST_GROUP);

    rc = QMetaObject::invokeMethod(this, "sendDiscoveryMsgs", Qt::QueuedConnection);
}	
void UdpSocket::sendDiscoveryMsgs()
{
   qnil = QNetworkInterface::allInterfaces();

    // for each viable interface, create and configure a socket.
    for (it = qnil.begin(); it != qnil.end(); ++it)
    {
        sock = new QUdpSocket;
        sock->bind(m_addrRecv, MCAST_PORT, QAbstractSocket::ShareAddress | QAbstractSocket::ReuseAddressHint);
        QObject::connect(sock, &QUdpSocket::readyRead, this, &UdpSocket::checkResponse);
        m_sock = sock; // the send() below uses m_sock.
        send(str);
    }
}
void UdpSocket::checkResponse()
{
    int rc = recv();
    if (rc == 0) // got a valid response
    {
        if (m_msgStr.find(MsgTypeText[MSG_DISCOVERY_ACK]) != string::npos)
        {
            m_sock = qobject_cast<QUdpSocket *>(sender());
            m_qni = new QNetworkInterface;
            *m_qni = m_socketInterfaceMap[m_sock];
            QObject::disconnect(m_sock, &QUdpSocket::readyRead, this, &UdpSocket::checkResponse);
            QObject::connect(m_sock, &QUdpSocket::readyRead, this, &UdpSocket::recv);
            QObject::connect(m_sock, &QUdpSocket::disconnected, this, &UdpSocket::reconnect);
        }
    }
}

I have a minor memory leak in that I don't destroy the unused sockets after I find the right one, but this way I don't have to maintain a list of the sockets while I'm trying to determine the "good" one.

Thanks for the help.