From c6f6dbd8458cf5521fd225cf83b67d2bd93bd01b Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 16 Jul 2015 09:59:15 -0700 Subject: [PATCH 1/4] Fix QByteArray from rqw data --- assignment-client/src/AssignmentClientMonitor.cpp | 2 +- assignment-client/src/avatars/AvatarMixerClientData.cpp | 4 +++- libraries/avatars/src/AvatarHashMap.cpp | 6 ++++-- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/assignment-client/src/AssignmentClientMonitor.cpp b/assignment-client/src/AssignmentClientMonitor.cpp index 9190b9be3e..0a39d80a61 100644 --- a/assignment-client/src/AssignmentClientMonitor.cpp +++ b/assignment-client/src/AssignmentClientMonitor.cpp @@ -203,7 +203,7 @@ void AssignmentClientMonitor::checkSpares() { } void AssignmentClientMonitor::handleChildStatusPacket(QSharedPointer packet) { - QUuid senderID = QUuid::fromRfc4122(QByteArray::fromRawData(packet->getData(), NUM_BYTES_RFC4122_UUID)); + QUuid senderID = QUuid::fromRfc4122(QByteArray::fromRawData(packet->getPayload(), NUM_BYTES_RFC4122_UUID)); auto nodeList = DependencyManager::get(); diff --git a/assignment-client/src/avatars/AvatarMixerClientData.cpp b/assignment-client/src/avatars/AvatarMixerClientData.cpp index 00ec6ad220..de2e1ac143 100644 --- a/assignment-client/src/avatars/AvatarMixerClientData.cpp +++ b/assignment-client/src/avatars/AvatarMixerClientData.cpp @@ -15,7 +15,9 @@ int AvatarMixerClientData::parseData(NLPacket& packet) { // compute the offset to the data payload - return _avatar.parseDataFromBuffer(QByteArray::fromRawData(packet.getPayload(), packet.getPayloadSize())); + QByteArray byteArray = QByteArray::fromRawData(packet.getPayload() + packet.pos(), + packet.bytesLeftToRead()); + return _avatar.parseDataFromBuffer(byteArray); } bool AvatarMixerClientData::checkAndSetHasReceivedFirstPackets() { diff --git a/libraries/avatars/src/AvatarHashMap.cpp b/libraries/avatars/src/AvatarHashMap.cpp index 2ad997c306..3243ff3f9a 100644 --- a/libraries/avatars/src/AvatarHashMap.cpp +++ b/libraries/avatars/src/AvatarHashMap.cpp @@ -61,6 +61,8 @@ void AvatarHashMap::processAvatarDataPacket(QSharedPointer packet, Sha while (packet->bytesLeftToRead()) { QUuid sessionUUID = QUuid::fromRfc4122(packet->read(NUM_BYTES_RFC4122_UUID)); + QByteArray byteArray = QByteArray::fromRawData(packet->getPayload() + packet->pos(), + packet->bytesLeftToRead()); if (sessionUUID != _lastOwnerSessionUUID) { AvatarSharedPointer avatar = _avatarHash.value(sessionUUID); if (!avatar) { @@ -68,12 +70,12 @@ void AvatarHashMap::processAvatarDataPacket(QSharedPointer packet, Sha } // have the matching (or new) avatar parse the data from the packet - int bytesRead = avatar->parseDataFromBuffer(QByteArray::fromRawData(packet->getPayload(), packet->pos())); + int bytesRead = avatar->parseDataFromBuffer(byteArray); packet->seek(packet->pos() + bytesRead); } else { // create a dummy AvatarData class to throw this data on the ground AvatarData dummyData; - int bytesRead = dummyData.parseDataFromBuffer(QByteArray::fromRawData(packet->getPayload(), packet->pos())); + int bytesRead = dummyData.parseDataFromBuffer(byteArray); packet->seek(packet->pos() + bytesRead); } } From 8424e91ae18d042dd881762f97e41b61c61df00b Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 16 Jul 2015 10:00:01 -0700 Subject: [PATCH 2/4] Added sequence number to packets --- libraries/networking/src/LimitedNodeList.cpp | 52 +++++++++++--------- libraries/networking/src/LimitedNodeList.h | 1 + libraries/networking/src/udt/Packet.h | 4 +- 3 files changed, 32 insertions(+), 25 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 720bb2eee0..a43b8b089d 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -216,6 +216,20 @@ bool LimitedNodeList::packetSourceAndHashMatch(const NLPacket& packet, SharedNod return false; } +qint64 LimitedNodeList::writePacket(const NLPacket& packet, const Node& destinationNode) { + if (!destinationNode.getActiveSocket()) { + return 0; + } + + // TODO Move to transport layer when ready + if (SEQUENCE_NUMBERED_PACKETS.contains(packet.getType())) { + PacketSequenceNumber sequenceNumber = getNextSequenceNumberForPacket(destinationNode.getUUID(), packet.getType()); + const_cast(packet).writeSequenceNumber(sequenceNumber); + } + + return writePacket(packet, *destinationNode.getActiveSocket(), destinationNode.getConnectionSecret()); +} + qint64 LimitedNodeList::writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret) { if (!NON_SOURCED_PACKETS.contains(packet.getType())) { @@ -248,14 +262,7 @@ qint64 LimitedNodeList::writeDatagram(const QByteArray& datagram, const HifiSock } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const Node& destinationNode) { - const HifiSockAddr* activeSocket = destinationNode.getActiveSocket(); - if (!activeSocket) { - // we don't have a socket to send to, return 0 - return 0; - } - - // use the node's active socket as the destination socket - return sendUnreliablePacket(packet, *activeSocket, destinationNode.getConnectionSecret()); + return writePacket(packet, destinationNode); } qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiSockAddr& sockAddr, @@ -264,30 +271,29 @@ qint64 LimitedNodeList::sendUnreliablePacket(const NLPacket& packet, const HifiS } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const Node& destinationNode) { - const HifiSockAddr* activeSocket = destinationNode.getActiveSocket(); - if (!activeSocket) { - // we don't have a socket to send to, return 0 - return 0; - } - - // use the node's active socket as the destination socket - return sendPacket(std::move(packet), *activeSocket, destinationNode.getConnectionSecret()); + // Keep unique_ptr alive during write + auto result = writePacket(*packet, destinationNode); + return result; } qint64 LimitedNodeList::sendPacket(std::unique_ptr packet, const HifiSockAddr& sockAddr, const QUuid& connectionSecret) { - return writePacket(*packet, sockAddr, connectionSecret); + // Keep unique_ptr alive during write + auto result = writePacket(*packet, sockAddr, connectionSecret); + return result; } qint64 LimitedNodeList::sendPacketList(NLPacketList& packetList, const Node& destinationNode) { - const HifiSockAddr* activeSocket = destinationNode.getActiveSocket(); - if (!activeSocket) { - // we don't have a socket to send to, return 0 - return 0; + qint64 bytesSent = 0; + + // close the last packet in the list + packetList.closeCurrentPacket(); + + while (!packetList._packets.empty()) { + bytesSent += sendPacket(std::move(packetList.takeFront()), destinationNode); } - // use the node's active socket as the destination socket - return sendPacketList(packetList, *activeSocket, destinationNode.getConnectionSecret()); + return bytesSent; } qint64 LimitedNodeList::sendPacketList(NLPacketList& packetList, const HifiSockAddr& sockAddr, diff --git a/libraries/networking/src/LimitedNodeList.h b/libraries/networking/src/LimitedNodeList.h index a2cbac7348..f3acb81ef1 100644 --- a/libraries/networking/src/LimitedNodeList.h +++ b/libraries/networking/src/LimitedNodeList.h @@ -248,6 +248,7 @@ protected: LimitedNodeList(LimitedNodeList const&); // Don't implement, needed to avoid copies of singleton void operator=(LimitedNodeList const&); // Don't implement, needed to avoid copies of singleton + qint64 writePacket(const NLPacket& packet, const Node& destinationNode); qint64 writePacket(const NLPacket& packet, const HifiSockAddr& destinationSockAddr, const QUuid& connectionSecret = QUuid()); qint64 writeDatagram(const QByteArray& datagram, const HifiSockAddr& destinationSockAddr); diff --git a/libraries/networking/src/udt/Packet.h b/libraries/networking/src/udt/Packet.h index 1d499e2b00..740fa7f663 100644 --- a/libraries/networking/src/udt/Packet.h +++ b/libraries/networking/src/udt/Packet.h @@ -66,7 +66,8 @@ public: HifiSockAddr& getSenderSockAddr() { return _senderSockAddr; } const HifiSockAddr& getSenderSockAddr() const { return _senderSockAddr; } - + + void writeSequenceNumber(SequenceNumber seqNum); SequenceNumber readSequenceNumber() const; bool readIsControlPacket() const; @@ -98,7 +99,6 @@ protected: // Header writers void writePacketTypeAndVersion(PacketType::Value type); - void writeSequenceNumber(SequenceNumber seqNum); PacketType::Value _type; // Packet type PacketVersion _version; // Packet version From ae8e79750b3eda23a8c13b203c870dc2ac589455 Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 16 Jul 2015 10:00:54 -0700 Subject: [PATCH 3/4] Stop ping timer in handle node kill --- libraries/networking/src/LimitedNodeList.cpp | 1 + 1 file changed, 1 insertion(+) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index a43b8b089d..46682d60fb 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -401,6 +401,7 @@ void LimitedNodeList::processKillNode(NLPacket& packet) { void LimitedNodeList::handleNodeKill(const SharedNodePointer& node) { qCDebug(networking) << "Killed" << *node; + node->stopPingTimer(); emit nodeKilled(node); } From bf637f8f18de55bdf43ca7345f87ee31b7bc38ae Mon Sep 17 00:00:00 2001 From: Atlante45 Date: Thu, 16 Jul 2015 10:50:05 -0700 Subject: [PATCH 4/4] Fix broadcast to nodes --- libraries/networking/src/LimitedNodeList.cpp | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/libraries/networking/src/LimitedNodeList.cpp b/libraries/networking/src/LimitedNodeList.cpp index 46682d60fb..badfbef60a 100644 --- a/libraries/networking/src/LimitedNodeList.cpp +++ b/libraries/networking/src/LimitedNodeList.cpp @@ -497,8 +497,8 @@ unsigned int LimitedNodeList::broadcastToNodes(std::unique_ptr packet, unsigned int n = 0; eachNode([&](const SharedNodePointer& node){ - if (node->getActiveSocket() && destinationNodeTypes.contains(node->getType())) { - writePacket(*packet, *node->getActiveSocket(), node->getConnectionSecret()); + if (node && destinationNodeTypes.contains(node->getType())) { + writePacket(*packet, *node); ++n; } });