[SOLVED] Trying to get Ntp client to work

koahnig

I have dowloaded qntp project source .
Besides a couple of memsets it was compilable immediately. However, to get this to work is another issue.
The basics are fairly easy by using QUdpSocket. When trying to use the method NtpClient::sendRequest it returns false.

bool NtpClient::sendRequest(const QHostAddress &address, quint16 port) {
    if(mSocket->state() != QAbstractSocket::BoundState)
        return false;
    
    /* Initialize the NTP packet. */
    NtpPacket packet;
    memset(&packet, 0, sizeof(packet));
    packet.flags.mode = ClientMode;
    packet.flags.versionNumber = 4;
    packet.transmitTimestamp = NtpTimestamp::fromDateTime(QDateTime::currentDateTimeUtc());
    
    qDebug() << " state " << mSocket->state() << " packet size " << sizeof ( packet );
    
    /* Send it. */
    if(mSocket->writeDatagram(reinterpret_cast<const char *>(&packet), sizeof(packet), address, port) < 0)
    {
        qDebug() << " error " << mSocket->error() << " " << mSocket->errorString();
        return false;
    }
   
    return true;
}

The qDebug output is:

 state  QAbstractSocket::BoundState  packet size  48
 error  QAbstractSocket::NetworkError   "Unable to send a message"

I am running the test on Win7 64 bit. I have checked the firewalls. The windows one is switched off. The one of Bidefender should let UDP all through according to settings. In addition I have added also a special rule to Bitdefender to allow all network traffic for this application. Also switching off the Bitdefender firewall did not change a thing.

I have tried with different NTP server addresses and port 123. Always the same issue. However, even with wrong addresses or port I would expect that I can send something out.

Anyone an idea what to check in addition?

SGaist

Hi,

From what I can see you are trying to use the NTP reserved port (123), so you must have the credentials to do so. User available ports start at 1024

koahnig

@SGaist
Thanks for reply.

123 was the only definite port number I could find through Google.
Just tried 1024, 1025 and 0 with the very same result.

mcosta

Hi.

can we see how you open the socket?? which parameters?

koahnig

@mcosta
Here is a link to the trial program..
I basically changed the orginal project files for having an application and did some basic changes.
Recently I have used the default constructor which does use only default parameters. However, also using parameters did not help.

I have no experience with UDP sockets so far, I wonder if I do something very basic wrong.

koahnig

Finally solved.

Stupid user error or misleading Qt doc. Whereever you want to blame this.

QHostAddress is only valid for IP addresses. However, since no IP addresses are typically given for NTP servers, I had used names found in internet. RTFM carefully helps.

Well, I did not see a point to check out QHostAddress carefully, because with QTcpSockets I am using a mix of host names and ip addresses without caring. The parameter list of connectToHost has also QHostAddress. However, the other method contains QString as parameter. After years of using successfully there was no reason ....

QHostInfo has to be involved for resolving the lookup.

koahnig

@SGaist @mcosta
123 is possible to use. See also above

SGaist

If I understand you correctly, the behavior of QHostAddress changed between two versions of Qt ?

koahnig

@SGaist
I am not sure about this. Also I am not sure when QHostAddress has been implemented first.

Those prototype are available in QAbstractSocket

connectToHost(const QString &, quint16, OpenMode, NetworkLayerProtocol);
connectToHost(const QHostAddress &, quint16, OpenMode);

I have started to use QTcpSocket in 2007 and I am using the same mechanism with host names and IP addresses. Being careful, it is obvious that I am using the first prototype. I have seen also the second prototype many times.
When seeing the prototype using the ntp class, it was completely clear to me that the behaviour is identical to the behaviour of QTcpSocket. The "minor detail" of QHostAddress was overlooked. I saw no warning or anything. Therefore, I did not pay attention and looked in other areas where I did not have experience with.

Finally I thought that I missed the obvious and started this thread. Unfortunately, I missed to add the section with the creation of the QHostAddress object by using the contructor QHostAddress::QHostAddress(const QString & address) with a host name.
That would have given others the chance to point out my stupid mistake.

SGaist

QHostAddress dates back to Qt 3 so it's a pretty old class

So if I follow you correctly, you thought you were using the first one, while in practice the second one was called ?

koahnig

@SGaist
Basically yes. Simply running into the trap and did not pay attention.

SGaist

I'm surprised that it does an automatic conversion like that...

koahnig

@SGaist said:

I'm surprised that it does an automatic conversion like that...

Just try HostAddress.pro

QT       += core network
QT       -= gui
TARGET = HostAddress
CONFIG   += console
CONFIG   -= app_bundle
TEMPLATE = app
SOURCES += main.cpp

main.cpp:

#include <QCoreApplication>
#include <QHostAddress>

int main(int argc, char *argv[])
{
    QCoreApplication a(argc, argv);

    QHostAddress hadd ( "any.domain.com" );

    return a.exec();
}

This does not tell you a thing. Clearly by reading the docs that is wrong and you have to look up the ip address and use a proper ip address.
However, the constructor does notice that the input is wrong. I would expect to see at least a message. Probably the reason is that the class is rooting back to Qt3.

SGaist

Hi,

In fact the constructor taking a string doesn't detect anything particular. However using setAddress would return false so it might be a better option currently

koahnig

@SGaist
You are right. Also when reading all the details in the documentation it becomes obvious.

My point is that this is not really consistent with error reporting in other sections of Qt. In my opinion Qt's beauty is because of the error reporting, which helps to avoid problems. For instance when using a non-existing signal in a connect or the parameters are not fitting one receives a message reporting the problem. Certainly when one is not checking the connect's return value, the information might be lost. However, when you start to check the return value with an assert it is very powerful and you see immediately your problem.

For consistency I would expect to obtain at least a direct warning, when a non-IP address is handed over. Compared to other really good things in Qt, this is rather sloppy and forms a trap.

SGaist

Just talked with one the network kings: you should use isNull before using your QHostAddress object, then you'll know for sure that you have a valid address.

koahnig

@SGaist

Thanks for following up. Certainly I will take care this next time. I guess I will remember.