From fa1825fa32f3e9b57f8f93b9de476bc48f7e4e12 Mon Sep 17 00:00:00 2001 From: Stephen Birarda Date: Fri, 7 Feb 2014 10:20:50 -0800 Subject: [PATCH] repairs to ping-reply, removal of deconstructPacketHeader to simply return UUID --- animation-server/src/AnimationServer.cpp | 4 +- assignment-client/src/audio/AudioMixer.cpp | 4 +- assignment-client/src/avatars/AvatarMixer.cpp | 10 +-- assignment-client/src/octree/OctreeServer.cpp | 7 +- domain-server/src/DomainServer.cpp | 4 +- interface/src/DatagramProcessor.cpp | 7 +- libraries/shared/src/NodeList.cpp | 74 ++++++++++--------- libraries/shared/src/NodeList.h | 13 +++- libraries/shared/src/PacketHeaders.cpp | 9 +-- libraries/shared/src/PacketHeaders.h | 2 +- 10 files changed, 65 insertions(+), 69 deletions(-) diff --git a/animation-server/src/AnimationServer.cpp b/animation-server/src/AnimationServer.cpp index 466ed7cf93..78966da2b5 100644 --- a/animation-server/src/AnimationServer.cpp +++ b/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); } diff --git a/assignment-client/src/audio/AudioMixer.cpp b/assignment-client/src/audio/AudioMixer.cpp index e9c0aafabb..3a736fd51c 100644 --- a/assignment-client/src/audio/AudioMixer.cpp +++ b/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); diff --git a/assignment-client/src/avatars/AvatarMixer.cpp b/assignment-client/src/avatars/AvatarMixer.cpp index 5b085fbcd2..9971bb51af 100644 --- a/assignment-client/src/avatars/AvatarMixer.cpp +++ b/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(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); diff --git a/assignment-client/src/octree/OctreeServer.cpp b/assignment-client/src/octree/OctreeServer.cpp index 10dc22fa96..4eff13f658 100644 --- a/assignment-client/src/octree/OctreeServer.cpp +++ b/assignment-client/src/octree/OctreeServer.cpp @@ -463,10 +463,7 @@ void OctreeServer::processDatagram(const QByteArray& dataByteArray, const HifiSo PacketType packetType = packetTypeForPacket(dataByteArray); - QUuid nodeUUID; - deconstructPacketHeader(dataByteArray, nodeUUID); - - SharedNodePointer matchingNode = nodeList->nodeWithUUID(nodeUUID); + SharedNodePointer matchingNode = nodeList->sendingNodeForPacket(dataByteArray); 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) { diff --git a/domain-server/src/DomainServer.cpp b/domain-server/src/DomainServer.cpp index e847143eaf..6ce71cae01 100644 --- a/domain-server/src/DomainServer.cpp +++ b/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; diff --git a/interface/src/DatagramProcessor.cpp b/interface/src/DatagramProcessor.cpp index e3f46c52d4..36f03c39f0 100644 --- a/interface/src/DatagramProcessor.cpp +++ b/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()); diff --git a/libraries/shared/src/NodeList.cpp b/libraries/shared/src/NodeList.cpp index 6b9ef4f406..ed3e37dae6 100644 --- a/libraries/shared/src/NodeList.cpp +++ b/libraries/shared/src/NodeList.cpp @@ -142,13 +142,9 @@ void NodeList::setDomainHostname(const QString& domainHostname) { } } -void NodeList::timePingReply(const QByteArray& packet) { - QUuid nodeUUID; - deconstructPacketHeader(packet, nodeUUID); +void NodeList::timePingReply(const QByteArray& packet, const SharedNodePointer& sendingNode) { - SharedNodePointer matchingNode = nodeWithUUID(nodeUUID); - - if (matchingNode) { + if (sendingNode) { 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); - matchingNode->setClockSkewUsec(clockSkew); + sendingNode->setPingMs(pingTime / 1000); + 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: { - // activate the appropriate socket for this node, if not yet updated - activateSocketFromNodeCommunication(senderSockAddr); - - // set the ping time for this node for stat collection - timePingReply(packet); + SharedNodePointer sendingNode = sendingNodeForPacket(packet); + + if (sendingNode) { + // activate the appropriate socket for this node, if not yet updated + activateSocketFromNodeCommunication(packet, sendingNode); + + // set the ping time for this node for stat collection + timePingReply(packet, sendingNode); + } + break; } case PacketTypeStunResponse: { @@ -252,8 +253,7 @@ SharedNodePointer NodeList::nodeWithUUID(const QUuid& nodeUUID) { } SharedNodePointer NodeList::sendingNodeForPacket(const QByteArray& packet) { - QUuid nodeUUID; - deconstructPacketHeader(packet, nodeUUID); + QUuid nodeUUID = uuidFromPacketHeader(packet); // 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); - writeDatagram(pingPacket, node); + QByteArray localPingPacket = constructPingPacket(PingType::Local); + 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) { - - foreach (const SharedNodePointer& node, _nodeHash) { - if (!node->getActiveSocket()) { - // 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 (node->getPublicSocket() == nodeAddress) { - node->activatePublicSocket(); - break; - } else if (node->getLocalSocket() == nodeAddress) { - node->activateLocalSocket(); - break; - } - } +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)); + + quint8 pingType; + packetStream >> pingType; + + // if this is a local or public ping then we can activate a socket + // we do nothing with agnostic pings, those are simply for timing + if (pingType == PingType::Local) { + sendingNode->activateLocalSocket(); + } else if (pingType == PingType::Public) { + sendingNode->activatePublicSocket(); } } diff --git a/libraries/shared/src/NodeList.h b/libraries/shared/src/NodeList.h index 1a035e91d1..3a71ee7620 100644 --- a/libraries/shared/src/NodeList.h +++ b/libraries/shared/src/NodeList.h @@ -54,6 +54,13 @@ typedef QSharedPointer SharedNodePointer; typedef QHash 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 timePingReply(const QByteArray& packet); + void activateSocketFromNodeCommunication(const QByteArray& packet, const SharedNodePointer& sendingNode); + void timePingReply(const QByteArray& packet, const SharedNodePointer& sendingNode); void resetDomainData(char domainField[], const char* domainData); void domainLookup(); void clear(); diff --git a/libraries/shared/src/PacketHeaders.cpp b/libraries/shared/src/PacketHeaders.cpp index 99066cb66b..16e1b3ce78 100644 --- a/libraries/shared/src/PacketHeaders.cpp +++ b/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; - deconstructPacketHeader(packet, nodeUUID); + QUuid nodeUUID = uuidFromPacketHeader(packet); qDebug() << "Packet mismatch on" << packetTypeForPacket(packet) << "- Sender" << nodeUUID << "sent" << qPrintable(QString::number(packet[numPacketTypeBytes])) << "but" @@ -120,9 +119,9 @@ int numBytesForPacketHeaderGivenPacketType(PacketType type) { return (int) ceilf((float)type / 255) + NUM_STATIC_HEADER_BYTES; } -void deconstructPacketHeader(const QByteArray& packet, QUuid& senderUUID) { - senderUUID = QUuid::fromRfc4122(packet.mid(numBytesArithmeticCodingFromBuffer(packet.data()) + sizeof(PacketVersion), - NUM_BYTES_RFC4122_UUID)); +QUuid uuidFromPacketHeader(const QByteArray& packet) { + return QUuid::fromRfc4122(packet.mid(numBytesArithmeticCodingFromBuffer(packet.data()) + sizeof(PacketVersion), + NUM_BYTES_RFC4122_UUID)); } PacketType packetTypeForPacket(const QByteArray& packet) { diff --git a/libraries/shared/src/PacketHeaders.h b/libraries/shared/src/PacketHeaders.h index 691dee861b..ad669daece 100644 --- a/libraries/shared/src/PacketHeaders.h +++ b/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);