From 734798a365c7081aca44f034eb4c07b3811e8231 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Wed, 27 May 2015 15:50:31 -0700 Subject: [PATCH] fix move of connection steps to LNL --- interface/src/ui/DomainConnectionDialog.cpp | 34 +++++----- libraries/audio-client/src/AudioClient.cpp | 4 +- libraries/networking/src/LimitedNodeList.cpp | 55 ++++++++++++++-- libraries/networking/src/LimitedNodeList.h | 37 ++++++++++- libraries/networking/src/NodeList.cpp | 66 +++++--------------- libraries/networking/src/NodeList.h | 31 --------- 6 files changed, 122 insertions(+), 105 deletions(-) diff --git a/interface/src/ui/DomainConnectionDialog.cpp b/interface/src/ui/DomainConnectionDialog.cpp index f5fdc530d5..c0471dc5e1 100644 --- a/interface/src/ui/DomainConnectionDialog.cpp +++ b/interface/src/ui/DomainConnectionDialog.cpp @@ -28,19 +28,21 @@ DomainConnectionDialog::DomainConnectionDialog(QWidget* parent) : // setup a QTableWidget so we can populate it with our values QTableWidget* timeTable = new QTableWidget; + timeTable->setEditTriggers(QAbstractItemView::NoEditTriggers); + const QStringList TABLE_HEADERS = QStringList() << "Name" << "Timestamp (ms)" << "Delta (ms)" << "Time elapsed (ms)"; timeTable->setColumnCount(TABLE_HEADERS.size()); // ask the NodeList for the current values for connection times - QMap times = DependencyManager::get()->getLastConnectionTimes(); + QMap times = DependencyManager::get()->getLastConnectionTimes(); timeTable->setRowCount(times.size()); timeTable->setHorizontalHeaderLabels(TABLE_HEADERS); // setup our data with the values from the NodeList - quint64 firstStepTime = times[NodeList::ConnectionStep::LookupAddress] / USECS_PER_MSEC; + quint64 firstStepTime = times.firstKey() / USECS_PER_MSEC; quint64 lastStepTime = firstStepTime; int thisRow = 0; @@ -48,25 +50,23 @@ DomainConnectionDialog::DomainConnectionDialog(QWidget* parent) : const QMetaObject &nodeListMeta = NodeList::staticMetaObject; QMetaEnum stepEnum = nodeListMeta.enumerator(nodeListMeta.indexOfEnumerator("ConnectionStep")); - for (int i = 0; i < stepEnum.keyCount(); i++) { - NodeList::ConnectionStep step = static_cast(i); + foreach(quint64 timestamp, times.keys()) { + // When did this step occur, how long since the last step, how long since the start? + quint64 stepTime = timestamp / USECS_PER_MSEC; + quint64 delta = (stepTime - lastStepTime); + quint64 elapsed = stepTime - firstStepTime; - if (times.contains(step)) { - // When did this step occur, how long since the last step, how long since the start? - quint64 stepTime = times[step] / USECS_PER_MSEC; - quint64 delta = (stepTime - lastStepTime); - quint64 elapsed = stepTime - firstStepTime; + lastStepTime = stepTime; - lastStepTime = stepTime; + // setup the columns for this row in the table + int stepIndex = (int) times.value(timestamp); - // setup the columns for this row in the table - timeTable->setItem(thisRow, 0, new QTableWidgetItem(stepEnum.valueToKey(i))); - timeTable->setItem(thisRow, 1, new QTableWidgetItem(QString::number(stepTime))); - timeTable->setItem(thisRow, 2, new QTableWidgetItem(QString::number(delta))); - timeTable->setItem(thisRow, 3, new QTableWidgetItem(QString::number(elapsed))); + timeTable->setItem(thisRow, 0, new QTableWidgetItem(stepEnum.valueToKey(stepIndex))); + timeTable->setItem(thisRow, 1, new QTableWidgetItem(QString::number(stepTime))); + timeTable->setItem(thisRow, 2, new QTableWidgetItem(QString::number(delta))); + timeTable->setItem(thisRow, 3, new QTableWidgetItem(QString::number(elapsed))); - ++thisRow; - } + ++thisRow; } // setup a horizontal box layout diff --git a/libraries/audio-client/src/AudioClient.cpp b/libraries/audio-client/src/AudioClient.cpp index 2bf25a3575..5c123b3bd0 100644 --- a/libraries/audio-client/src/AudioClient.cpp +++ b/libraries/audio-client/src/AudioClient.cpp @@ -875,7 +875,7 @@ void AudioClient::handleAudioInput() { _stats.sentPacket(); - nodeList->flagTimeForConnectionStep(NodeList::ConnectionStep::SendFirstAudioPacket); + nodeList->flagTimeForConnectionStep(NodeList::ConnectionStep::SendAudioPacket); int packetBytes = currentPacketPtr - audioDataPacket; nodeList->writeDatagram(audioDataPacket, packetBytes, audioMixer); @@ -923,7 +923,7 @@ void AudioClient::sendMuteEnvironmentPacket() { } void AudioClient::addReceivedAudioToStream(const QByteArray& audioByteArray) { - DependencyManager::get()->flagTimeForConnectionStep(NodeList::ConnectionStep::ReceiveFirstAudioPacket); + DependencyManager::get()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::ReceiveFirstAudioPacket); if (_audioOutput) { // Audio output must exist and be correctly set up if we're going to process received audio diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index bd412be414..76bdc5d089 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -61,6 +61,8 @@ LimitedNodeList::LimitedNodeList(unsigned short socketListenPort, unsigned short firstCall = false; } + qRegisterMetaType("ConnectionStep"); + _nodeSocket.bind(QHostAddress::AnyIPv4, socketListenPort); qCDebug(networking) << "NodeList socket is listening on" << _nodeSocket.localPort(); @@ -636,6 +638,8 @@ void LimitedNodeList::sendSTUNRequest() { QUuid randomUUID = QUuid::createUuid(); memcpy(stunRequestPacket + packetIndex, randomUUID.toRfc4122().data(), NUM_TRANSACTION_ID_BYTES); + flagTimeForConnectionStep(ConnectionStep::SendSTUNRequest); + _nodeSocket.writeDatagram((char*) stunRequestPacket, sizeof(stunRequestPacket), _stunSockAddr.getAddress(), _stunSockAddr.getPort()); } @@ -693,18 +697,20 @@ bool LimitedNodeList::processSTUNResponse(const QByteArray& packet) { QHostAddress newPublicAddress = QHostAddress(stunAddress); if (newPublicAddress != _publicSockAddr.getAddress() || newPublicPort != _publicSockAddr.getPort()) { - if (!_hasCompletedInitialSTUN) { - // if we're here we have definitely completed our initial STUN sequence - stopInitialSTUNUpdate(true); - } - _publicSockAddr = HifiSockAddr(newPublicAddress, newPublicPort); qCDebug(networking, "New public socket received from STUN server is %s:%hu", _publicSockAddr.getAddress().toString().toLocal8Bit().constData(), _publicSockAddr.getPort()); + if (!_hasCompletedInitialSTUN) { + // if we're here we have definitely completed our initial STUN sequence + stopInitialSTUNUpdate(true); + } + emit publicSockAddrChanged(_publicSockAddr); + + flagTimeForConnectionStep(ConnectionStep::SetPublicSocketFromSTUN); } return true; @@ -741,6 +747,9 @@ void LimitedNodeList::startSTUNPublicSocketUpdate() { const int STUN_INITIAL_UPDATE_INTERVAL_MSECS = 250; _initialSTUNTimer->start(STUN_INITIAL_UPDATE_INTERVAL_MSECS); + + // send an initial STUN request right away + sendSTUNRequest(); } } } @@ -760,6 +769,8 @@ void LimitedNodeList::stopInitialSTUNUpdate(bool success) { // we have changed the publicSockAddr, so emit our signal emit publicSockAddrChanged(_publicSockAddr); + + flagTimeForConnectionStep(ConnectionStep::SetPublicSocketFromSTUN); } assert(_initialSTUNTimer); @@ -847,3 +858,37 @@ bool LimitedNodeList::getLocalServerPortFromSharedMemory(const QString key, quin return true; } } + +void LimitedNodeList::flagTimeForConnectionStep(ConnectionStep connectionStep) { + QMetaObject::invokeMethod(this, "flagTimeForConnectionStep", + Q_ARG(ConnectionStep, connectionStep), + Q_ARG(quint64, usecTimestampNow())); +} + +void LimitedNodeList::flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp) { + if (!_areConnectionTimesComplete) { + QWriteLocker writeLock(&_connectionTimeLock); + + if (connectionStep == ConnectionStep::LookupAddress) { + // we clear the current times if the user just fired off a lookup + _lastConnectionTimes.clear(); + _areConnectionTimesComplete = false; + } + + // anything > than sending the first DS check should not come before the DS check in, so we drop those + // this handles the case where you lookup an address and get packets in the existing domain before changing domains + if (connectionStep > LimitedNodeList::ConnectionStep::SendDSCheckIn + && (_lastConnectionTimes.key(ConnectionStep::SendDSCheckIn) == 0 + || timestamp <= _lastConnectionTimes.key(ConnectionStep::SendDSCheckIn))) { + return; + } + + // if there is no time for existing step add a timestamp on the first call for each ConnectionStep + _lastConnectionTimes[timestamp] = connectionStep; + + // if this is a received audio packet we consider our connection times complete + if (connectionStep == ConnectionStep::ReceiveFirstAudioPacket) { + _areConnectionTimesComplete = true; + } + } +} diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index faaeac3c57..990508e89d 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -80,8 +80,30 @@ namespace PingType { class LimitedNodeList : public QObject, public Dependency { Q_OBJECT SINGLETON_DEPENDENCY - public: + + enum ConnectionStep { + LookupAddress = 1, + HandleAddress, + SendSTUNRequest, + SetPublicSocketFromSTUN, + SetICEServerHostname, + SetICEServerSocket, + SendICEServerHearbeat, + ReceiveDSPeerInformation, + SendPingsToDS, + SetDomainHostname, + SetDomainSocket, + SendDSCheckIn, + ReceiveDSList, + SendAudioPing, + SetAudioMixerSocket, + SendAudioPacket, + ReceiveFirstAudioPacket + }; + + Q_ENUMS(ConnectionStep); + const QUuid& getSessionUUID() const { return _sessionUUID; } void setSessionUUID(const QUuid& sessionUUID); @@ -204,6 +226,11 @@ public: void putLocalPortIntoSharedMemory(const QString key, QObject* parent, quint16 localPort); bool getLocalServerPortFromSharedMemory(const QString key, quint16& localPort); + const QMap getLastConnectionTimes() const + { QReadLocker readLock(&_connectionTimeLock); return _lastConnectionTimes; } + void flagTimeForConnectionStep(ConnectionStep connectionStep); + + public slots: void reset(); void eraseAllNodes(); @@ -269,6 +296,12 @@ protected: QPointer _initialSTUNTimer; int _numInitialSTUNRequests = 0; bool _hasCompletedInitialSTUN = false; + quint64 _firstSTUNTime = 0; + quint64 _publicSocketUpdateTime = 0; + + mutable QReadWriteLock _connectionTimeLock { }; + QMap _lastConnectionTimes; + bool _areConnectionTimesComplete = false; template void eachNodeHashIterator(IteratorLambda functor) { @@ -279,6 +312,8 @@ protected: functor(it); } } +private slots: + void flagTimeForConnectionStep(ConnectionStep connectionStep, quint64 timestamp); }; #endif // hifi_LimitedNodeList_h diff --git a/libraries/networking/src/NodeList.cpp b/libraries/networking/src/NodeList.cpp index 20d9604c59..b7870bcdea 100644 --- a/libraries/networking/src/NodeList.cpp +++ b/libraries/networking/src/NodeList.cpp @@ -82,8 +82,6 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned // clear our NodeList when logout is requested connect(&AccountManager::getInstance(), &AccountManager::logoutComplete , this, &NodeList::reset); - qRegisterMetaType("NodeList::ConnectionStep"); - // we definitely want STUN to update our public socket, so call the LNL to kick that off startSTUNPublicSocketUpdate(); } @@ -230,15 +228,18 @@ void NodeList::processNodeData(const HifiSockAddr& senderSockAddr, const QByteAr case PacketTypeUnverifiedPingReply: { qCDebug(networking) << "Received reply from domain-server on" << senderSockAddr; - // for now we're unsafely assuming this came back from the domain - if (senderSockAddr == _domainHandler.getICEPeer().getLocalSocket()) { - qCDebug(networking) << "Connecting to domain using local socket"; - _domainHandler.activateICELocalSocket(); - } else if (senderSockAddr == _domainHandler.getICEPeer().getPublicSocket()) { - qCDebug(networking) << "Conecting to domain using public socket"; - _domainHandler.activateICEPublicSocket(); - } else { - qCDebug(networking) << "Reply does not match either local or public socket for domain. Will not connect."; + if (_domainHandler.getIP().isNull()) { + // for now we're unsafely assuming this came back from the domain + if (senderSockAddr == _domainHandler.getICEPeer().getLocalSocket()) { + qCDebug(networking) << "Connecting to domain using local socket"; + _domainHandler.activateICELocalSocket(); + } else if (senderSockAddr == _domainHandler.getICEPeer().getPublicSocket()) { + qCDebug(networking) << "Conecting to domain using public socket"; + _domainHandler.activateICEPublicSocket(); + } else { + qCDebug(networking) << "Reply does not match either local or public socket for domain. Will not connect."; + } + } } case PacketTypeStunResponse: { @@ -348,7 +349,7 @@ void NodeList::sendDomainServerCheckIn() { } } - flagTimeForConnectionStep(NodeList::ConnectionStep::SendFirstDSCheckIn); + flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SendDSCheckIn); if (!isUsingDTLS) { writeUnverifiedDatagram(domainServerPacket, _domainHandler.getSockAddr()); @@ -467,7 +468,7 @@ void NodeList::handleICEConnectionToDomainServer() { _domainHandler.getICEPeer().resetConnectionAttemps(); - flagTimeForConnectionStep(NodeList::ConnectionStep::SendFirstICEServerHearbeat); + flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SendICEServerHearbeat); LimitedNodeList::sendHeartbeatToIceServer(_domainHandler.getICEServerSockAddr(), _domainHandler.getICEClientID(), @@ -476,7 +477,7 @@ void NodeList::handleICEConnectionToDomainServer() { qCDebug(networking) << "Sending ping packets to establish connectivity with domain-server with ID" << uuidStringWithoutCurlyBraces(_domainHandler.getICEDomainID()); - flagTimeForConnectionStep(NodeList::ConnectionStep::SendFirstPingsToDS); + flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SendPingsToDS); // send the ping packet to the local and public sockets for this node QByteArray localPingPacket = constructPingPacket(PingType::Local, false, _domainHandler.getICEClientID()); @@ -493,7 +494,7 @@ int NodeList::processDomainServerList(const QByteArray& packet) { // this is a packet from the domain server, reset the count of un-replied check-ins _numNoReplyDomainCheckIns = 0; - DependencyManager::get()->flagTimeForConnectionStep(NodeList::ConnectionStep::ReceiveFirstDSList); + DependencyManager::get()->flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::ReceiveDSList); // if this was the first domain-server list from this domain, we've now connected if (!_domainHandler.isConnected()) { @@ -585,7 +586,7 @@ void NodeList::pingInactiveNodes() { pingPunchForInactiveNode(node); if (node->getType() == NodeType::AudioMixer) { - flagTimeForConnectionStep(NodeList::ConnectionStep::SendFirstAudioPing); + flagTimeForConnectionStep(NodeList::ConnectionStep::SendAudioPing); } } }); @@ -613,36 +614,3 @@ void NodeList::activateSocketFromNodeCommunication(const QByteArray& packet, con flagTimeForConnectionStep(NodeList::ConnectionStep::SetAudioMixerSocket); } } - -void NodeList::flagTimeForConnectionStep(NodeList::ConnectionStep connectionStep) { - QMetaObject::invokeMethod(this, "flagTimeForConnectionStep", - Q_ARG(NodeList::ConnectionStep, connectionStep), - Q_ARG(quint64, usecTimestampNow())); -} - -void NodeList::flagTimeForConnectionStep(NodeList::ConnectionStep connectionStep, quint64 timestamp) { - QWriteLocker writeLock(&_connectionTimeLock); - - if (connectionStep == NodeList::ConnectionStep::LookupAddress) { - // we clear the current times if the user just fired off a lookup - _lastConnectionTimes.clear(); - } - - if (!_lastConnectionTimes.contains(connectionStep)) { - // if there is no time for existing step add a timestamp on the first call for each NodeList::ConnectionStep - _lastConnectionTimes[connectionStep] = timestamp; - } else { - // if the existing time for this step is before the nearest sibling before then replace it - // this handles the case where audio comes in after an address lookup after the previous times have been cleared - quint64 currentTime = _lastConnectionTimes[connectionStep]; - - // go down the sibling steps and check if the registered time is actually before the sibling - for (int i = ((int) connectionStep) - 1; i >= 0; i--) { - NodeList::ConnectionStep thisStep = static_cast(i); - if (_lastConnectionTimes.contains(thisStep) && _lastConnectionTimes[thisStep] >= currentTime) { - _lastConnectionTimes[connectionStep] = timestamp; - break; - } - } - } -} diff --git a/libraries/networking/src/NodeList.h b/libraries/networking/src/NodeList.h index d52dd6e101..67810180f4 100644 --- a/libraries/networking/src/NodeList.h +++ b/libraries/networking/src/NodeList.h @@ -45,28 +45,6 @@ class NodeList : public LimitedNodeList { SINGLETON_DEPENDENCY public: - - enum ConnectionStep { - LookupAddress, - HandleAddress, - SetICEServerHostname, - SetICEServerSocket, - SendFirstICEServerHearbeat, - ReceiveDSPeerInformation, - SendFirstPingsToDS, - SetDomainHostname, - SetDomainSocket, - ForcedSTUNRequest, - SendFirstDSCheckIn, - ReceiveFirstDSList, - SendFirstAudioPing, - SetAudioMixerSocket, - SendFirstAudioPacket, - ReceiveFirstAudioPacket - }; - - Q_ENUMS(ConnectionStep); - NodeType_t getOwnerType() const { return _ownerType; } void setOwnerType(NodeType_t ownerType) { _ownerType = ownerType; } @@ -85,11 +63,6 @@ public: int processDomainServerList(const QByteArray& packet); - const QMap getLastConnectionTimes() const - { QReadLocker readLock(&_connectionTimeLock); return _lastConnectionTimes; } - void flagTimeForConnectionStep(NodeList::ConnectionStep connectionStep); - void resetConnectionTimes() { QWriteLocker writeLock(&_connectionTimeLock); _lastConnectionTimes.clear(); } - void setAssignmentServerSocket(const HifiSockAddr& serverSocket) { _assignmentServerSocket = serverSocket; } void sendAssignment(Assignment& assignment); @@ -104,7 +77,6 @@ signals: private slots: void sendPendingDSPathQuery(); void handleICEConnectionToDomainServer(); - void flagTimeForConnectionStep(NodeList::ConnectionStep connectionStep, quint64 timestamp); private: NodeList() : LimitedNodeList(0, 0) { assert(false); } // Not implemented, needed for DependencyManager templates compile NodeList(char ownerType, unsigned short socketListenPort = 0, unsigned short dtlsListenPort = 0); @@ -126,9 +98,6 @@ private: int _numNoReplyDomainCheckIns; HifiSockAddr _assignmentServerSocket; - mutable QReadWriteLock _connectionTimeLock { }; - QMap _lastConnectionTimes; - friend class Application; };