add ping timer tied to NetworkPeer

libraries/networking/src/LimitedNodeList.cpp

@@ -472,7 +472,8 @@ void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) {
 
 SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType,
                                                    const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket,
-                                                   bool canAdjustLocks, bool canRez) {
+                                                   bool canAdjustLocks, bool canRez,
+                                                   const QUuid& connectionSecret) {
     NodeHash::const_iterator it = _nodeHash.find(uuid);
 
     if (it != _nodeHash.end()) {
@@ -482,11 +483,13 @@ SharedNodePointer LimitedNodeList::addOrUpdateNode(const QUuid& uuid, NodeType_t
         matchingNode->setLocalSocket(localSocket);
         matchingNode->setCanAdjustLocks(canAdjustLocks);
         matchingNode->setCanRez(canRez);
+        matchingNode->setConnectionSecret(connectionSecret);
 
         return matchingNode;
     } else {
         // we didn't have this node, so add them
-        Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket, canAdjustLocks, canRez);
+        Node* newNode = new Node(uuid, nodeType, publicSocket, localSocket, canAdjustLocks, canRez, connectionSecret);
+
         SharedNodePointer newNodePointer(newNode);
 
         _nodeHash.insert(UUIDNodePair(newNode->getUUID(), newNodePointer));

libraries/networking/src/LimitedNodeList.h

@@ -151,7 +151,8 @@ public:
 
     SharedNodePointer addOrUpdateNode(const QUuid& uuid, NodeType_t nodeType,
                                       const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket,
-                                      bool canAdjustLocks, bool canRez);
+                                      bool canAdjustLocks, bool canRez,
+                                      const QUuid& connectionSecret = QUuid());
 
     bool hasCompletedInitialSTUN() const { return _hasCompletedInitialSTUN; }
 

libraries/networking/src/NetworkPeer.cpp

@@ -25,7 +25,7 @@ NetworkPeer::NetworkPeer() :
     _lastHeardMicrostamp(usecTimestampNow()),
     _connectionAttempts(0)
 {
-    
+
 }
 
 NetworkPeer::NetworkPeer(const QUuid& uuid, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket) :
@@ -36,14 +36,14 @@ NetworkPeer::NetworkPeer(const QUuid& uuid, const HifiSockAddr& publicSocket, co
     _lastHeardMicrostamp(usecTimestampNow()),
     _connectionAttempts(0)
 {
-    
+
 }
 
 NetworkPeer::NetworkPeer(const NetworkPeer& otherPeer) : QObject() {
     _uuid = otherPeer._uuid;
     _publicSocket = otherPeer._publicSocket;
     _localSocket = otherPeer._localSocket;
-    
+
     _wakeTimestamp = otherPeer._wakeTimestamp;
     _lastHeardMicrostamp = otherPeer._lastHeardMicrostamp;
     _connectionAttempts = otherPeer._connectionAttempts;
@@ -57,7 +57,7 @@ NetworkPeer& NetworkPeer::operator=(const NetworkPeer& otherPeer) {
 
 void NetworkPeer::swap(NetworkPeer& otherPeer) {
     using std::swap;
-    
+
     swap(_uuid, otherPeer._uuid);
     swap(_publicSocket, otherPeer._publicSocket);
     swap(_localSocket, otherPeer._localSocket);
@@ -71,15 +71,33 @@ QByteArray NetworkPeer::toByteArray() const {
 
     QDataStream peerStream(&peerByteArray, QIODevice::Append);
     peerStream << *this;
-    
+
     return peerByteArray;
 }
 
+void NetworkPeer::startPingTimer() {
+    if (!_pingTimer) {
+        _pingTimer = new QTimer(this);
+
+        connect(_pingTimer, &QTimer::timeout, this, &NetworkPeer::pingTimerTimeout);
+
+        _pingTimer->start(UDP_PUNCH_PING_INTERVAL_MS);
+    }
+}
+
+void NetworkPeer::stopPingTimer() {
+    if (_pingTimer) {
+        _pingTimer->stop();
+        _pingTimer->deleteLater();
+        _pingTimer = NULL;
+    }
+}
+
 QDataStream& operator<<(QDataStream& out, const NetworkPeer& peer) {
     out << peer._uuid;
     out << peer._publicSocket;
     out << peer._localSocket;
-    
+
     return out;
 }
 
@@ -87,7 +105,7 @@ QDataStream& operator>>(QDataStream& in, NetworkPeer& peer) {
     in >> peer._uuid;
     in >> peer._publicSocket;
     in >> peer._localSocket;
-    
+
     return in;
 }
 

libraries/networking/src/NetworkPeer.h

@@ -12,8 +12,9 @@
 #ifndef hifi_NetworkPeer_h
 #define hifi_NetworkPeer_h
 
-#include <qobject.h>
-#include <quuid.h>
+#include <QtCore/QObject>
+#include <QtCore/QTimer>
+#include <QtCore/QUuid>
 
 #include "HifiSockAddr.h"
 
@@ -22,35 +23,38 @@ const int ICE_SERVER_DEFAULT_PORT = 7337;
 const int ICE_HEARBEAT_INTERVAL_MSECS = 2 * 1000;
 const int MAX_ICE_CONNECTION_ATTEMPTS = 5;
 
+const int UDP_PUNCH_PING_INTERVAL_MS = 25;
+
 class NetworkPeer : public QObject {
+    Q_OBJECT
 public:
     NetworkPeer();
     NetworkPeer(const QUuid& uuid, const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket);
-    
+
     // privatize copy and assignment operator to disallow peer copying
     NetworkPeer(const NetworkPeer &otherPeer);
     NetworkPeer& operator=(const NetworkPeer& otherPeer);
-    
+
     bool isNull() const { return _uuid.isNull(); }
-    
+
     const QUuid& getUUID() const { return _uuid; }
     void setUUID(const QUuid& uuid) { _uuid = uuid; }
-    
+
     void reset();
-    
+
     const HifiSockAddr& getPublicSocket() const { return _publicSocket; }
     virtual void setPublicSocket(const HifiSockAddr& publicSocket) { _publicSocket = publicSocket; }
     const HifiSockAddr& getLocalSocket() const { return _localSocket; }
     virtual void setLocalSocket(const HifiSockAddr& localSocket) { _localSocket = localSocket; }
-    
+
     quint64 getWakeTimestamp() const { return _wakeTimestamp; }
     void setWakeTimestamp(quint64 wakeTimestamp) { _wakeTimestamp = wakeTimestamp; }
-    
+
     quint64 getLastHeardMicrostamp() const { return _lastHeardMicrostamp; }
     void setLastHeardMicrostamp(quint64 lastHeardMicrostamp) { _lastHeardMicrostamp = lastHeardMicrostamp; }
-    
+
     QByteArray toByteArray() const;
-    
+
     int getConnectionAttempts() const  { return _connectionAttempts; }
     void incrementConnectionAttempts() { ++_connectionAttempts; }
     void resetConnectionAttemps() { _connectionAttempts = 0; }
@@ -60,18 +64,25 @@ public:
 
     float getOutboundBandwidth(); // in kbps
     float getInboundBandwidth(); // in kbps
-    
+
+    void startPingTimer();
+    void stopPingTimer();
+
     friend QDataStream& operator<<(QDataStream& out, const NetworkPeer& peer);
     friend QDataStream& operator>>(QDataStream& in, NetworkPeer& peer);
+signals:
+    void pingTimerTimeout();
 protected:
     QUuid _uuid;
-    
+
     HifiSockAddr _publicSocket;
     HifiSockAddr _localSocket;
-    
+
     quint64 _wakeTimestamp;
     quint64 _lastHeardMicrostamp;
-    
+
+    QTimer* _pingTimer = NULL;
+
     int _connectionAttempts;
 private:
     void swap(NetworkPeer& otherPeer);

libraries/networking/src/Node.cpp

@@ -42,12 +42,12 @@ const QString& NodeType::getNodeTypeName(NodeType_t nodeType) {
 }
 
 Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket,
-           const HifiSockAddr& localSocket, bool canAdjustLocks, bool canRez) :
+           const HifiSockAddr& localSocket, bool canAdjustLocks, bool canRez, const QUuid& connectionSecret) :
     NetworkPeer(uuid, publicSocket, localSocket),
     _type(type),
     _activeSocket(NULL),
     _symmetricSocket(),
-    _connectionSecret(),
+    _connectionSecret(connectionSecret),
     _linkedData(NULL),
     _isAlive(true),
     _pingMs(-1),  // "Uninitialized"
@@ -57,7 +57,7 @@ Node::Node(const QUuid& uuid, NodeType_t type, const HifiSockAddr& publicSocket,
     _canAdjustLocks(canAdjustLocks),
     _canRez(canRez)
 {
-    
+
 }
 
 Node::~Node() {
@@ -75,11 +75,11 @@ void Node::setPublicSocket(const HifiSockAddr& publicSocket) {
             // if the active socket was the public socket then reset it to NULL
             _activeSocket = NULL;
         }
-        
+
         if (!_publicSocket.isNull()) {
             qCDebug(networking) << "Public socket change for node" << *this;
         }
-        
+
         _publicSocket = publicSocket;
     }
 }
@@ -90,11 +90,11 @@ void Node::setLocalSocket(const HifiSockAddr& localSocket) {
             // if the active socket was the local socket then reset it to NULL
             _activeSocket = NULL;
         }
-        
+
         if (!_localSocket.isNull()) {
             qCDebug(networking) << "Local socket change for node" << *this;
         }
-        
+
         _localSocket = localSocket;
     }
 }
@@ -105,32 +105,39 @@ void Node::setSymmetricSocket(const HifiSockAddr& symmetricSocket) {
             // if the active socket was the symmetric socket then reset it to NULL
             _activeSocket = NULL;
         }
-        
+
         if (!_symmetricSocket.isNull()) {
             qCDebug(networking) << "Symmetric socket change for node" << *this;
         }
-        
+
         _symmetricSocket = symmetricSocket;
     }
 }
 
+void Node::setActiveSocket(HifiSockAddr* discoveredSocket) {
+    _activeSocket = discoveredSocket;
+
+    // we have an active socket, stop our ping timer
+    stopPingTimer();
+}
+
 void Node::activateLocalSocket() {
     qCDebug(networking) << "Activating local socket for network peer with ID" << uuidStringWithoutCurlyBraces(_uuid);
-    _activeSocket = &_localSocket;
+    setActiveSocket(&_localSocket);
 }
 
 void Node::activatePublicSocket() {
     qCDebug(networking) << "Activating public socket for network peer with ID" << uuidStringWithoutCurlyBraces(_uuid);
-    _activeSocket = &_publicSocket;
+    setActiveSocket(&_publicSocket);
 }
 
 void Node::activateSymmetricSocket() {
     qCDebug(networking) << "Activating symmetric socket for network peer with ID" << uuidStringWithoutCurlyBraces(_uuid);
-    _activeSocket = &_symmetricSocket;
+    setActiveSocket(&_symmetricSocket);
 }
 
 PacketSequenceNumber Node::getLastSequenceNumberForPacketType(PacketType packetType) const {
-   auto typeMatch = _lastSequenceNumbers.find(packetType); 
+   auto typeMatch = _lastSequenceNumbers.find(packetType);
    if (typeMatch != _lastSequenceNumbers.end()) {
         return typeMatch->second;
    } else {
@@ -145,7 +152,7 @@ QDataStream& operator<<(QDataStream& out, const Node& node) {
     out << node._localSocket;
     out << node._canAdjustLocks;
     out << node._canRez;
-    
+
     return out;
 }
 
@@ -156,7 +163,7 @@ QDataStream& operator>>(QDataStream& in, Node& node) {
     in >> node._localSocket;
     in >> node._canAdjustLocks;
     in >> node._canRez;
-    
+
     return in;
 }
 

libraries/networking/src/Node.h

@@ -37,16 +37,17 @@ namespace NodeType {
     const NodeType_t AudioMixer = 'M';
     const NodeType_t AvatarMixer = 'W';
     const NodeType_t Unassigned = 1;
-    
+
     void init();
     const QString& getNodeTypeName(NodeType_t nodeType);
 }
 
 class Node : public NetworkPeer {
     Q_OBJECT
-public: 
+public:
     Node(const QUuid& uuid, NodeType_t type,
-         const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket, bool canAdjustLocks, bool canRez);
+         const HifiSockAddr& publicSocket, const HifiSockAddr& localSocket,
+         bool canAdjustLocks, bool canRez, const QUuid& connectionSecret = QUuid());
     ~Node();
 
     bool operator==(const Node& otherNode) const { return _uuid == otherNode._uuid; }
@@ -54,7 +55,7 @@ public:
 
     char getType() const { return _type; }
     void setType(char type) { _type = type; }
-    
+
     const QUuid& getConnectionSecret() const { return _connectionSecret; }
     void setConnectionSecret(const QUuid& connectionSecret) { _connectionSecret = connectionSecret; }
 
@@ -70,12 +71,12 @@ public:
     int getClockSkewUsec() const { return _clockSkewUsec; }
     void updateClockSkewUsec(int clockSkewSample);
     QMutex& getMutex() { return _mutex; }
-    
+
     virtual void setPublicSocket(const HifiSockAddr& publicSocket);
     virtual void setLocalSocket(const HifiSockAddr& localSocket);
     const HifiSockAddr& getSymmetricSocket() const { return _symmetricSocket; }
     virtual void setSymmetricSocket(const HifiSockAddr& symmetricSocket);
-    
+
     const HifiSockAddr* getActiveSocket() const { return _activeSocket; }
 
     void setCanAdjustLocks(bool canAdjustLocks) { _canAdjustLocks = canAdjustLocks; }
@@ -83,7 +84,7 @@ public:
 
     void setCanRez(bool canRez) { _canRez = canRez; }
     bool getCanRez() { return _canRez; }
-    
+
     void activatePublicSocket();
     void activateLocalSocket();
     void activateSymmetricSocket();
@@ -91,7 +92,7 @@ public:
     void setLastSequenceNumberForPacketType(PacketSequenceNumber sequenceNumber, PacketType packetType)
         { _lastSequenceNumbers[packetType] = sequenceNumber; }
     PacketSequenceNumber getLastSequenceNumberForPacketType(PacketType packetType) const;
-    
+
     friend QDataStream& operator<<(QDataStream& out, const Node& node);
     friend QDataStream& operator>>(QDataStream& in, Node& node);
 
@@ -100,11 +101,13 @@ private:
     Node(const Node &otherNode);
     Node& operator=(Node otherNode);
 
+    void setActiveSocket(HifiSockAddr* discoveredSocket);
+
     NodeType_t _type;
-    
+
     HifiSockAddr* _activeSocket;
     HifiSockAddr _symmetricSocket;
-    
+
     QUuid _connectionSecret;
     NodeData* _linkedData;
     bool _isAlive;

libraries/networking/src/NodeList.cpp

@@ -82,6 +82,9 @@ NodeList::NodeList(char newOwnerType, unsigned short socketListenPort, unsigned
     // clear our NodeList when logout is requested
     connect(&AccountManager::getInstance(), &AccountManager::logoutComplete , this, &NodeList::reset);
 
+    // anytime we get a new node we will want to attempt to punch to it
+    connect(this, &LimitedNodeList::nodeAdded, this, &NodeList::startNodeHolePunch);
+
     // we definitely want STUN to update our public socket, so call the LNL to kick that off
     startSTUNPublicSocketUpdate();
 }
@@ -521,7 +524,7 @@ int NodeList::processDomainServerList(const QByteArray& packet) {
     setThisNodeCanRez(thisNodeCanRez);
 
     // pull each node in the packet
-    while(packetStream.device()->pos() < packet.size()) {
+    while (packetStream.device()->pos() < packet.size()) {
         // setup variables to read into from QDataStream
         qint8 nodeType;
         QUuid nodeUUID, connectionUUID;
@@ -537,16 +540,12 @@ int NodeList::processDomainServerList(const QByteArray& packet) {
             nodePublicSocket.setAddress(_domainHandler.getIP());
         }
 
-        SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket,
-                                                 nodeLocalSocket, canAdjustLocks, canRez);
-
         packetStream >> connectionUUID;
-        node->setConnectionSecret(connectionUUID);
-    }
 
-    // ping inactive nodes in conjunction with receipt of list from domain-server
-    // this makes it happen every second and also pings any newly added nodes
-    pingInactiveNodes();
+        SharedNodePointer node = addOrUpdateNode(nodeUUID, nodeType, nodePublicSocket,
+                                                 nodeLocalSocket, canAdjustLocks, canRez,
+                                                 connectionUUID);
+    }
 
     return readNodes;
 }
@@ -566,6 +565,10 @@ void NodeList::sendAssignment(Assignment& assignment) {
 }
 
 void NodeList::pingPunchForInactiveNode(const SharedNodePointer& node) {
+    if (node->getType() == NodeType::AudioMixer) {
+        flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SendAudioPing);
+    }
+
     // send the ping packet to the local and public sockets for this node
     QByteArray localPingPacket = constructPingPacket(PingType::Local);
     writeDatagram(localPingPacket, node, node->getLocalSocket());
@@ -577,19 +580,27 @@ void NodeList::pingPunchForInactiveNode(const SharedNodePointer& node) {
         QByteArray symmetricPingPacket = constructPingPacket(PingType::Symmetric);
         writeDatagram(symmetricPingPacket, node, node->getSymmetricSocket());
     }
+
+    node->incrementConnectionAttempts();
 }
 
-void NodeList::pingInactiveNodes() {
-    eachNode([this](const SharedNodePointer& node){
-        if (!node->getActiveSocket()) {
-            // we don't have an active link to this node, ping it to set that up
-            pingPunchForInactiveNode(node);
+void NodeList::startNodeHolePunch(const SharedNodePointer& node) {
+    // connect to the correct signal on this node so we know when to ping it
+    connect(node.data(), &Node::pingTimerTimeout, this, &NodeList::handleNodePingTimeout);
 
-            if (node->getType() == NodeType::AudioMixer) {
-                flagTimeForConnectionStep(LimitedNodeList::ConnectionStep::SendAudioPing);
-            }
-        }
-    });
+    // start the ping timer for this node
+    node->startPingTimer();
+
+    // ping this node immediately
+    pingPunchForInactiveNode(node);
+}
+
+void NodeList::handleNodePingTimeout() {
+    Node* senderNode = qobject_cast<Node*>(sender());
+
+    if (senderNode) {
+        pingPunchForInactiveNode(nodeWithUUID(senderNode->getUUID()));
+    }
 }
 
 void NodeList::activateSocketFromNodeCommunication(const QByteArray& packet, const SharedNodePointer& sendingNode) {

libraries/networking/src/NodeList.h

@@ -70,13 +70,15 @@ public:
 public slots:
     void reset();
     void sendDomainServerCheckIn();
-    void pingInactiveNodes();
     void handleDSPathQuery(const QString& newPath);
 signals:
     void limitOfSilentDomainCheckInsReached();
 private slots:
     void sendPendingDSPathQuery();
     void handleICEConnectionToDomainServer();
+
+    void startNodeHolePunch(const SharedNodePointer& node);
+    void handleNodePingTimeout();
 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);

libraries/networking/src/ThreadedAssignment.h

@@ -46,7 +46,6 @@ protected:
 
 private slots:
     void checkInWithDomainServerOrExit();
-
 };
 
 typedef QSharedPointer<ThreadedAssignment> SharedAssignmentPointer;