repairs to ping-reply, removal of deconstructPacketHeader to simply return UUID

animation-server/src/AnimationServer.cpp

@@ -835,10 +835,8 @@ void AnimationServer::readPendingDatagrams() {
                 int headerBytes = numBytesForPacketHeader(receivedPacket);
                 // PacketType_JURISDICTION, first byte is the node type...
                 if (receivedPacket.data()[headerBytes] == NodeType::VoxelServer && ::jurisdictionListener) {
-                    QUuid nodeUUID;
-                    deconstructPacketHeader(receivedPacket, nodeUUID);
                     
-                    SharedNodePointer matchedNode = NodeList::getInstance()->nodeWithUUID(nodeUUID);
+                    SharedNodePointer matchedNode = NodeList::getInstance()->sendingNodeForPacket(receivedPacket);
                     if (matchedNode) {
                         ::jurisdictionListener->queueReceivedPacket(matchedNode, receivedPacket);
                     }

assignment-client/src/audio/AudioMixer.cpp

@@ -213,12 +213,10 @@ void AudioMixer::processDatagram(const QByteArray& dataByteArray, const HifiSock
     if (mixerPacketType == PacketTypeMicrophoneAudioNoEcho
         || mixerPacketType == PacketTypeMicrophoneAudioWithEcho
         || mixerPacketType == PacketTypeInjectAudio) {
-        QUuid nodeUUID;
-        deconstructPacketHeader(dataByteArray, nodeUUID);
 
         NodeList* nodeList = NodeList::getInstance();
 
-        SharedNodePointer matchingNode = nodeList->nodeWithUUID(nodeUUID);
+        SharedNodePointer matchingNode = nodeList->sendingNodeForPacket(dataByteArray);
 
         if (matchingNode) {
             nodeList->updateNodeWithData(matchingNode.data(), senderSockAddr, dataByteArray);

assignment-client/src/avatars/AvatarMixer.cpp

@@ -142,11 +142,9 @@ void AvatarMixer::processDatagram(const QByteArray& dataByteArray, const HifiSoc
     
     switch (packetTypeForPacket(dataByteArray)) {
         case PacketTypeAvatarData: {
-            QUuid nodeUUID;
-            deconstructPacketHeader(dataByteArray, nodeUUID);
             
             // add or update the node in our list
-            SharedNodePointer avatarNode = nodeList->nodeWithUUID(nodeUUID);
+            SharedNodePointer avatarNode = nodeList->sendingNodeForPacket(dataByteArray);
             
             if (avatarNode) {
                 // parse positional data from an node
@@ -156,11 +154,9 @@ void AvatarMixer::processDatagram(const QByteArray& dataByteArray, const HifiSoc
             break;
         }
         case PacketTypeAvatarIdentity: {
-            QUuid nodeUUID;
-            deconstructPacketHeader(dataByteArray, nodeUUID);
             
             // check if we have a matching node in our list
-            SharedNodePointer avatarNode = nodeList->nodeWithUUID(nodeUUID);
+            SharedNodePointer avatarNode = nodeList->sendingNodeForPacket(dataByteArray);
             
             if (avatarNode && avatarNode->getLinkedData()) {
                 AvatarMixerClientData* nodeData = reinterpret_cast<AvatarMixerClientData*>(avatarNode->getLinkedData());
@@ -170,7 +166,7 @@ void AvatarMixer::processDatagram(const QByteArray& dataByteArray, const HifiSoc
                     QByteArray identityPacket = byteArrayWithPopluatedHeader(PacketTypeAvatarIdentity);
                     
                     QByteArray individualByteArray = nodeData->identityByteArray();
-                    individualByteArray.replace(0, NUM_BYTES_RFC4122_UUID, nodeUUID.toRfc4122());
+                    individualByteArray.replace(0, NUM_BYTES_RFC4122_UUID, avatarNode->getUUID().toRfc4122());
                     
                     identityPacket.append(individualByteArray);
                     

assignment-client/src/octree/OctreeServer.cpp

@@ -463,10 +463,7 @@ void OctreeServer::processDatagram(const QByteArray& dataByteArray, const HifiSo
 
     PacketType packetType = packetTypeForPacket(dataByteArray);
     
-    QUuid nodeUUID;
+    SharedNodePointer matchingNode = nodeList->sendingNodeForPacket(dataByteArray);
-    deconstructPacketHeader(dataByteArray, nodeUUID);
-    
-    SharedNodePointer matchingNode = nodeList->nodeWithUUID(nodeUUID);
 
     if (packetType == getMyQueryMessageType()) {
         bool debug = false;
@@ -487,7 +484,7 @@ void OctreeServer::processDatagram(const QByteArray& dataByteArray, const HifiSo
             }
             OctreeQueryNode* nodeData = (OctreeQueryNode*) matchingNode->getLinkedData();
             if (nodeData && !nodeData->isOctreeSendThreadInitalized()) {
-                nodeData->initializeOctreeSendThread(this, nodeUUID);
+                nodeData->initializeOctreeSendThread(this, matchingNode->getUUID());
             }
         }
     } else if (packetType == PacketTypeJurisdictionRequest) {

domain-server/src/DomainServer.cpp

@@ -229,7 +229,7 @@ void DomainServer::readAvailableDatagrams() {
     
     QByteArray receivedPacket;
     NodeType_t nodeType;
-    QUuid nodeUUID;
+   
 
     while (nodeList->getNodeSocket().hasPendingDatagrams()) {
         receivedPacket.resize(nodeList->getNodeSocket().pendingDatagramSize());
@@ -244,7 +244,7 @@ void DomainServer::readAvailableDatagrams() {
                 QDataStream packetStream(receivedPacket);
                 packetStream.skipRawData(numBytesForPacketHeader(receivedPacket));
                 
-                deconstructPacketHeader(receivedPacket, nodeUUID);
+                QUuid nodeUUID = uuidFromPacketHeader(receivedPacket);
                 
                 packetStream >> nodeType;
                 packetStream >> nodePublicAddress >> nodeLocalAddress;

interface/src/DatagramProcessor.cpp

@@ -40,9 +40,6 @@ void DatagramProcessor::processDatagrams() {
         _packetCount++;
         _byteCount += incomingPacket.size();
         
-        QUuid nodeUUID;
-        deconstructPacketHeader(incomingPacket, nodeUUID);
-        
         if (packetVersionMatch(incomingPacket)) {
             // only process this packet if we have a match on the packet version
             switch (packetTypeForPacket(incomingPacket)) {
@@ -87,7 +84,7 @@ void DatagramProcessor::processDatagrams() {
                         printf("got PacketType_VOXEL_DATA, sequence:%d flightTime:%d\n", sequence, flightTime);
                     }
                     
-                    SharedNodePointer matchedNode = NodeList::getInstance()->nodeWithUUID(nodeUUID);
+                    SharedNodePointer matchedNode = NodeList::getInstance()->sendingNodeForPacket(incomingPacket);
                     
                     if (matchedNode) {
                         // add this packet to our list of voxel packets and process them on the voxel processing
@@ -103,7 +100,7 @@ void DatagramProcessor::processDatagrams() {
                 case PacketTypeKillAvatar:
                 case PacketTypeAvatarIdentity: {
                     // update having heard from the avatar-mixer and record the bytes received
-                    SharedNodePointer avatarMixer = nodeList->nodeWithUUID(nodeUUID);
+                    SharedNodePointer avatarMixer = nodeList->sendingNodeForPacket(incomingPacket);
                     
                     if (avatarMixer) {
                         avatarMixer->setLastHeardMicrostamp(usecTimestampNow());

libraries/shared/src/NodeList.cpp

@@ -142,13 +142,9 @@ void NodeList::setDomainHostname(const QString& domainHostname) {
     }
 }
 
-void NodeList::timePingReply(const QByteArray& packet) {
+void NodeList::timePingReply(const QByteArray& packet, const SharedNodePointer& sendingNode) {
-    QUuid nodeUUID;
-    deconstructPacketHeader(packet, nodeUUID);
     
-    SharedNodePointer matchingNode = nodeWithUUID(nodeUUID);
+    if (sendingNode) {
-    
-    if (matchingNode) {
         QDataStream packetStream(packet);
         packetStream.skipRawData(numBytesForPacketHeader(packet));
         
@@ -165,13 +161,13 @@ void NodeList::timePingReply(const QByteArray& packet) {
         quint64 othersExprectedReply = ourOriginalTime + oneWayFlightTime;
         int clockSkew = othersReplyTime - othersExprectedReply;
         
-        matchingNode->setPingMs(pingTime / 1000);
+        sendingNode->setPingMs(pingTime / 1000);
-        matchingNode->setClockSkewUsec(clockSkew);
+        sendingNode->setClockSkewUsec(clockSkew);
         
         const bool wantDebug = false;
         
         if (wantDebug) {
-            qDebug() << "PING_REPLY from node " << *matchingNode << "\n" <<
+            qDebug() << "PING_REPLY from node " << *sendingNode << "\n" <<
             "                     now: " << now << "\n" <<
             "                 ourTime: " << ourOriginalTime << "\n" <<
             "                pingTime: " << pingTime << "\n" <<
@@ -200,11 +196,16 @@ void NodeList::processNodeData(const HifiSockAddr& senderSockAddr, const QByteAr
             break;
         }
         case PacketTypePingReply: {
+            SharedNodePointer sendingNode = sendingNodeForPacket(packet);
+            
+            if (sendingNode) {
                 // activate the appropriate socket for this node, if not yet updated
-            activateSocketFromNodeCommunication(senderSockAddr);
+                activateSocketFromNodeCommunication(packet, sendingNode);
                 
                 // set the ping time for this node for stat collection
-            timePingReply(packet);
+                timePingReply(packet, sendingNode);
+            }
+            
             break;
         }
         case PacketTypeStunResponse: {
@@ -252,8 +253,7 @@ SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID) {
 }
 
 SharedNodePointer NodeList::sendingNodeForPacket(const QByteArray& packet) {
-    QUuid nodeUUID;
+    QUuid nodeUUID = uuidFromPacketHeader(packet);
-    deconstructPacketHeader(packet, nodeUUID);
     
     // return the matching node, or NULL if there is no match
     return nodeWithUUID(nodeUUID);
@@ -590,10 +590,12 @@ void NodeList::sendAssignment(Assignment& assignment) {
     _nodeSocket.writeDatagram(packet, assignmentServerSocket->getAddress(), assignmentServerSocket->getPort());
 }
 
-QByteArray NodeList::constructPingPacket() {
+QByteArray NodeList::constructPingPacket(PingType_t pingType) {
     QByteArray pingPacket = byteArrayWithPopluatedHeader(PacketTypePing);
     
     QDataStream packetStream(&pingPacket, QIODevice::Append);
+    
+    packetStream << pingType;
     packetStream << usecTimestampNow();
     
     return pingPacket;
@@ -615,11 +617,13 @@ QByteArray NodeList::constructPingReplyPacket(const QByteArray& pingPacket) {
 }
 
 void NodeList::pingPublicAndLocalSocketsForInactiveNode(const SharedNodePointer& node) {
-    QByteArray pingPacket = constructPingPacket();
     
     // send the ping packet to the local and public sockets for this node
-    writeDatagram(pingPacket, node);
+    QByteArray localPingPacket = constructPingPacket(PingType::Local);
-    writeDatagram(pingPacket, node);
+    writeDatagram(localPingPacket, node, node->getLocalSocket());
+    
+    QByteArray publicPingPacket = constructPingPacket(PingType::Public);
+    writeDatagram(publicPingPacket, node, node->getPublicSocket());
 }
 
 SharedNodePointer NodeList::addOrUpdateNode(const QUuid& uuid, char nodeType,
@@ -703,20 +707,20 @@ const HifiSockAddr* NodeList::getNodeActiveSocketOrPing(const SharedNodePointer&
     return NULL;
 }
 
-void NodeList::activateSocketFromNodeCommunication(const HifiSockAddr& nodeAddress) {
+void NodeList::activateSocketFromNodeCommunication(const QByteArray& packet, const SharedNodePointer& sendingNode) {
+    // deconstruct this ping packet to see if it is a public or local reply
+    QDataStream packetStream(packet);
+    packetStream.skipRawData(numBytesForPacketHeader(packet));
     
-    foreach (const SharedNodePointer& node, _nodeHash) {
+    quint8 pingType;
-        if (!node->getActiveSocket()) {
+    packetStream >> pingType;
-            // check both the public and local addresses for each node to see if we find a match
+    
-            // prioritize the private address so that we prune erroneous local matches
+    // if this is a local or public ping then we can activate a socket
-            if (node->getPublicSocket() ==  nodeAddress) {
+    // we do nothing with agnostic pings, those are simply for timing
-                node->activatePublicSocket();
+    if (pingType == PingType::Local) {
-                break;
+        sendingNode->activateLocalSocket();
-            } else if (node->getLocalSocket() == nodeAddress) {
+    } else if (pingType == PingType::Public) {
-                node->activateLocalSocket();
+        sendingNode->activatePublicSocket();
-                break;
-            }
-        }
     }
 }
 

libraries/shared/src/NodeList.h

@@ -54,6 +54,13 @@ typedef QSharedPointer<Node> SharedNodePointer;
 typedef QHash<QUuid, SharedNodePointer> NodeHash;
 Q_DECLARE_METATYPE(SharedNodePointer)
 
+typedef quint8 PingType_t;
+namespace PingType {
+    const PingType_t Agnostic = 0;
+    const PingType_t Local = 1;
+    const PingType_t Public = 2;
+}
+
 class NodeList : public QObject {
     Q_OBJECT
 public:
@@ -100,7 +107,7 @@ public:
     void setAssignmentServerSocket(const HifiSockAddr& serverSocket) { _assignmentServerSocket = serverSocket; }
     void sendAssignment(Assignment& assignment);
 
-    QByteArray constructPingPacket();
+    QByteArray constructPingPacket(PingType_t pingType = PingType::Agnostic);
     QByteArray constructPingReplyPacket(const QByteArray& pingPacket);
     void pingPublicAndLocalSocketsForInactiveNode(const SharedNodePointer& node);
 
@@ -158,8 +165,8 @@ private:
     bool _hasCompletedInitialSTUNFailure;
     unsigned int _stunRequestsSinceSuccess;
 
-    void activateSocketFromNodeCommunication(const HifiSockAddr& nodeSockAddr);
+    void activateSocketFromNodeCommunication(const QByteArray& packet, const SharedNodePointer& sendingNode);
-    void timePingReply(const QByteArray& packet);
+    void timePingReply(const QByteArray& packet, const SharedNodePointer& sendingNode);
     void resetDomainData(char domainField[], const char* domainData);
     void domainLookup();
     void clear();

libraries/shared/src/PacketHeaders.cpp

@@ -95,8 +95,7 @@ bool packetVersionMatch(const QByteArray& packet) {
         PacketType mismatchType = packetTypeForPacket(packet);
         int numPacketTypeBytes = arithmeticCodingValueFromBuffer(packet.data());
        
-        QUuid nodeUUID;
+        QUuid nodeUUID = uuidFromPacketHeader(packet);
-        deconstructPacketHeader(packet, nodeUUID);
         
         qDebug() << "Packet mismatch on" << packetTypeForPacket(packet) << "- Sender"
             << nodeUUID << "sent" << qPrintable(QString::number(packet[numPacketTypeBytes])) << "but"
@@ -120,8 +119,8 @@ int numBytesForPacketHeaderGivenPacketType(PacketType type) {
     return (int) ceilf((float)type / 255) + NUM_STATIC_HEADER_BYTES;
 }
 
-void deconstructPacketHeader(const QByteArray& packet, QUuid& senderUUID) {
+QUuid uuidFromPacketHeader(const QByteArray& packet) {
-    senderUUID = QUuid::fromRfc4122(packet.mid(numBytesArithmeticCodingFromBuffer(packet.data()) + sizeof(PacketVersion),
+    return QUuid::fromRfc4122(packet.mid(numBytesArithmeticCodingFromBuffer(packet.data()) + sizeof(PacketVersion),
                                          NUM_BYTES_RFC4122_UUID));
 }
 

libraries/shared/src/PacketHeaders.h

@@ -76,7 +76,7 @@ int numBytesForPacketHeader(const QByteArray& packet);
 int numBytesForPacketHeader(const char* packet);
 int numBytesForPacketHeaderGivenPacketType(PacketType type);
 
-void deconstructPacketHeader(const QByteArray& packet, QUuid& senderUUID);
+QUuid uuidFromPacketHeader(const QByteArray& packet);
 
 PacketType packetTypeForPacket(const QByteArray& packet);
 PacketType packetTypeForPacket(const char* packet);