Re: Re: Kad Update

Hi Peter, thanks for you answers! I have a few more questions to ask you:
1.- About UUID:

+ UInt128 id; //!QUuid readUuid(); and void writeUuid (const QUuid &);.

I’ve been reading the QUuid class documentation and it seems hard linked to a special format of a kind of ids used in different OS. Just look at the constructors! Besides that, I will need bit level access into each byte and that class doesn’t support it. I don’t even have access to the data after constructing a QUuid object as it seems to be private to the class.

2.-  About the payload specification:

Payload:

0..15	id	Kad ID of the peer.
16..19	ip; 	Sender or Receiver IP address.
20..21	udpPort	Incoming UDP Port used in Kad network.
22..23	tcpPort	Incoming TCP Port used in eDonkey network.
24	notUsed	Reserved for future use.

But that info better be added to the docs of PeerInfoPacket and then only add a reference to it from the template instantiations:\n\nPayload:\n\nsee PeerInfoPacket\n\n
\nWhat do you think of this?

\n+#ifndef __PROTOCOLS__KAD__PACKETS__PEER__INFO__PACKET
\n+#define __PROTOCOLS__KAD__PACKETS__PEER__INFO__PACKET

\nPlease use _ for word separator or . and __ for directory separator. I’ve fixed this and similar ones in your patch.

\nI guess you’ll need getters for PeerInfoPacket and a ctor that takes arguments to initialize the payload fields. It’ll become obvious you have to add them when you write the unit tests.

\n+\t\tUInt128\t\t\tid; //!< Kad ID.
\n…

\nWhy don’t you take QUuid instead ? It’s a 128bit UUID struct with a bunch of useful functions. BinaryReader and BinaryWriter can be get the new QUuid readUuid(); and void writeUuid (const QUuid &);.

\nPlease also protect Imports.h with #ifndef. That have slipped last time. It’s not too bad but it’s better.

\n+\t// Utils namespace conflicts with Protocols::Kad::Utils !!!! :S

\nYou’ve resolved that well and it’s great you’ve written a comment about the issues.

\n+
\n+ \\todo Find the way to support both protocol types (compressed and plain).

\nI might have an idea. Let’s implement only uncompressed packets. We’ll have anyway a PacketFactory that will read and possibly write packets, so the factory can take care of compressing/uncompressing the rawData and modifying the header field. Can it be that a specific bit in the packet type is flipped to indicate compressed payload?

\nBest regards,

\nPeter

\n

\nDavid Alfonso wrote:Hi all, I’ve been working on another kind of packets which contains the information of a peer. I’ve also corrected some errors from my last revision.

\nTo-Do:

\n

    \n

  • Make tests.
  • \n

  • Keep on studying the Kad protocol and packets.
  • \n

  • Implement more packets.
  • \n

  • Support packet compression.
  • \n

\nThere’s a diff attached to this post. You should apply it over the last revision of the Kad branch (”,1] ); //–>

Payload:

see PeerInfoPacket

What documents are you referring?? All the information about PeerInfoPacket is in the header (i.e., in the template instantiation)…

Note: I guess your solution for compressing packets is great. Of course, the header Protocol field has to be changed to KadPackedProtocol if the packet is compressed.

+
+ \todo Find the way to support both protocol types (compressed and plain).

I might have an idea. Let’s implement only uncompressed packets. We’ll have anyway a PacketFactory that will read and possibly write packets, so the factory can take care of compressing/uncompressing the rawData and modifying the header field. Can it be that a specific bit in the packet type is flipped to indicate compressed payload?

Regards,David

Would you like to post a relpy?


This post is a reply to:
Re: Kad Update
Hi David, Thanks for the patch! It’s quite neat :-)! I just applied it on your Kad branch: http://bzr.calitko.org/developers/david/calitko-Kad and the latest revision there is 81. Here are my answers to your notes/questions in (more...)

Follow-ups:
Re: Re: Re: Kad Update
Hi David, [1]. If you feel that QUuid won’t do, then stick to your UInt128. Later we can think about generalizing the concept into a generic class for ints of random (more...)