The original text reads:
The RTPS protocol version 2.5 defines the following special values.
PROTOCOLVERSION_1_0
PROTOCOLVERSION_1_1
PROTOCOLVERSION_1_0 PROTOCOLVERSION_1_1
PROTOCOLVERSION_2_1
PROTOCOLVERSION_2_2
PROTOCOLVERSION_2_2
PROTOCOLVERSION_2_4

Two PROTOCOLVERSION_2_2 appeared
I think it should be PROTOCOLVERSION_2_3

The InfoSource is meant to hold the same information inside the RTPS message such that an RTPS message can be used to relay RTPS submessages from other sources and the RTPS submessages are interpreted in the correct content of the originating Participant.

The RTPS Header Extension submessage was added to extend the RTPS Header in a way that did not break interoperability with existing systems. So logically the information inside RTPS Header Extension belongs in the RTPS Header. Therefore it should also be added to the InfoSource

Primarily targets environments that don't have multicast. Implementations support using a "discovery" service. In some cases it operates "transparently".

The SIG and vendors have discussed this and the conclusion was that it would be best to add tome description in the RTPS spec that explains how the discovery service would work and interoperate.

IDL value annotation for GAP is @value(8x08) instead of @value(0x08).

Table 8.42 defines the elements that comprise a DataFrag submessage. It includes the dataSize as one of those elements.

Section 9.4.5.5 must map all elements of the submessage to the PSM types. However, it is lacking a dataSize. Instead it includes a sampleSize. SampleSize should be renamed dataSize and perhaps should use a well-defined type from 9.3.2 such as MessageLength_t.

Section 8.3.3.1.2 indicates which version of the protocol is specified in this document.
No other place in the document should attempt to do the same, all others should refer back to 8.3.3.1.2. 8.3.3.1.2 should define the constant "PROTOCOLVERSION" as a symbol that indicates the current version (2.6). Add a reference to section 8.6 which defines how different versions can interoperate.

In Table 8.2:
In the cell to the right of ProtocolVersion_t, remove the text from "The following values are reserved..." until the end of the cell. Replace it with a reference to 8.3.3.1.2.

In Section 8.3.5.3:
Remove the text from "The RTPS protocol version X.Y defines..." until the end of this subsection. Replace it with a reference to 8.3.3.1.2.

In Section 9.3.2.1:
Remove the IDL comment

// The implementations following this version of the document
// implement protocol version 2.4

The spec uses two numbering schemes for bits:

For example 9.4.1 (Overall Structure)

0...2...........7...............15.............23. .............31
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

But then in 9.4.4 (Mapping of the RTPS Header)

0...2...........8...............16.............24.............. 32
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+

It should be consistent. The more correct scheme appears to be:

0...2...........8...............16.............24. .............32 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 

With this scheme the number represents the number of bits that appear before the number (the "." positions). The other scheme seems inconsistent. "0" is placed ahead of the bit of that order, but '7' and the rest are placed after.

LivelinessFlag in the last paragraph should be GroupInfoFlag, i.e.,
"The GroupInfoFlag is represented with the literal ‘G’. G=1 means the HeartBeat includes the currentGSN,
firstGSN, and lastGSN. The value of the LivelinessFlag can be obtained from the expression:"
should be changed to
"The GroupInfoFlag is represented with the literal ‘G’. G=1 means the HeartBeat includes the currentGSN,
firstGSN, and lastGSN. The value of the GroupInfoFlag can be obtained from the expression:"

Details available to RTF members

In the wire representation figures of 9.4.5.4 and 9.4.5.5, `SequenceNumber writerSN` is shown as:

+---------------+---------------+---------------+---------------+
+           SequenceNumber       writerSN                       +
+---------------+---------------+---------------+---------------+

as if it only occupies 4 bytes.

In all other places (9.4.2.6, 9.4.5.6, 9.4.5.7, 9.4.5.8, 9.4.5.14, and 9.6.4.7), SequenceNumbers are represented explicitly as 64-bit values:

+---------------+---------------+---------------+---------------+
|                                                               |
+           SequenceNumber       gapStart                       +
|                                                               |
+---------------+---------------+---------------+---------------+

In hindsight, the plus signs does indicate that the field is larger than it looks, but unless there is a clear reason, can those representations be unified to remove potential confusion and provide better readability?

`PID__DOMAIN_ID` in table 9.18 has an extra underscore. It should be `PID_DOMAIN_ID`.

Section 9.3.1.2 lists the three pre-defined values of the entity key field in EntityId_t as:

• ‘8 for user-defined entities.
• ‘11’ for built-in entities.
• ‘01’ for vendor-specific entities

The user-defined entities definition should be '00'.

Table 8.42 gives the Structure of the DataFrag Submessage. The last field is specified to have type "SerializedPayload". If this would be the case, the contents of that field should be decodable as a SerializedPayload object, as specified in Section 10, but really the field seems to contain just a fragment of a SerializedPayload. A complete series of fragments must be concatenated to get a valid SerializedPayload. Therefore, the type should be changed to indicate that it is a fragment, not a whole object.

A similar problem exists in Section 9.4.5.5 DataFrag Submessage.

More generally, there seems to be confusion in terminology between SerializedPayload / SerializedData / SerializedDataFragment.

The latter two seem to be suitable for use in the payload field of Data and DataFrag submessages, respectively, and they are defined in 8.3.5.17 -.18, but SerializedPayload is not defined in 8.3.5 at all.

Section 9.4.2 (PSM) again does not define a mapping for SerializedData / SerializedDataFragment, but for SerializedPayload instead.

Please clarify the relationship between the aforementioned terms.

At the wire-protocol level currently only DataWriters and DataReaders have Partitions.

It would be useful to have a similar concept/Qos at the DomainParticipant level. This would allow system deployments to control better participant matching.

While the same could be implemented by adding the the partitions to all Readers and Writers in the Participant the process would be tedious and far less efficient. For example with Participant partitions it would be possible for Participants that do not have a matching partition to avoid doing EndpointDiscovery.

In previous versions, ChangeForReader was specified and illustrated. The ChangeFromWriter (8.4.10.5) is still in place including its illustrated use (8.4.12.3). Is ChangeForReader removed on purpose or by accident? In my opinion, ChangeForReader should be defined as before.

The if statement on the second line should start on a new line.
Instead of:
```
ReceiverReceiver.unicastReplyLocatorList =
InfoReply.unicastLocatorList if ( MulticastFlag )

else

```

In the version 2.3 specs, a gap is inserted in case a cache change isn't available in the history cache. In the 2.5 specs, only the highest previously sent sequence number is taken into account to determine if agap should be inserted.
The shown logical actions will fail if the cache change with a_change_seq_num can't be located in the cache. in which case a gap should be inserted as well as in the 2.3 specs. Considering the explanation given for sending a gap because of the HISTORY QoS set to KEEP_LAST in the last paragraph on page 82, one should definitely consider the situation that a cache change is missing.

In 8.4.7.3 and 8.4.7.5, the variable highestSentChangeSN is used for the highest sequence number sent. In 8.4.8.1.4 and subsequent paragraphs, the variable highest_sent_seq_num is used for the same purpose.
Also, there is a spelling error in the variable's name higuest_sent_seq_num instead of highest_sent_seq_num.

On initialization of the ReaderLocator, the highestSentChangeSN is set to SEQUENCE_NUMBER_INVALID which is not defined in this specification. In 8.4.7.2 RTPS StatelessWriter the function unsent_changes_reset is defined as resetting highestSentChangeSN to the value 0. Either define SEQUENCE_NUMBER_INVALID as being the value 0 and use it also in function unsent_changes_reset or use the value 0 instead of SEQUENCE_NUMBER_INVALID. Or, is there a good erason to make a distinction between SEQUENCE_NUMBER_INVALID and 0?

In 8.3.8.6 Heartbeat, the fields writerSet and secureWriterSet are defined. The equivalent wire presentation is missing however.

Both the parameter id and length fields are defined as short. These should be unsigned short.

The masks used in table 9.6 should be written as 0x8000 and 0x4000 instead of 8000 and 4000

Using the header_extension submessage to pass the message's CRC seems to me the wrong place. It would be more logical to add the CRC at the end of the message for obvious reasons. First processing the whole potentially large message and then inserting the CRC somewhere in the front, requires extra resources. Also, this makes it unnecessary to first use zero's for the value of the CRC field while calculating the message's CRC. Adding the CRC just to the end of the message, maybe in a submessage of its own if you like, is a much better solution.

Moreover, the receiver has been designed in such a way that while processing a message it can hand off processed submessgaes right away, which is rather nice. With a message CRC ckheck, either in the header or the tail of the messgae, mitigates this feature since one first has to process the whole message before deciding if the message, and therefor the submessages, is valid. Iff some CRC checking should be implemented, do this at the level of a submessage.

With a 128 or just 64 bit CRC bit checksum, the designer clearly had huge message sizes in mind, which doesn't seem a good design decision to me.

The line

The value of the UExtension4Flag can be obtained from the expression:
T = SubmessageHeader.flags & 0x04

should read:

The value of the TimestampFlag can be obtained from the expression:
T = SubmessageHeader.flags & 0x04

In the bullet list, the text of second item seems to be incomplete. It now reads:

• The GUIDs of Endpoint Groups within a Participant

The algorithms for 32 and 64 bit crc checks are defined but the one for 128 bits is missing.

Sorry, had this flag checked by accident.

The RTPS V2.5 spec explains how to implement the Group ordering/coherency feature in section 8.7.5 and 8.7.6. In these sections it is explained that the scope of the coherent update (i.e. which coherent Writers the coherent Publishers contains) is communicated by means of the GroupDigest, which is the first part of an MD5 hash of an ordered list of CDR encoded EntityIDs of those Writers.

In order for the Subscribing side to validate whether all relevant Writers have successfully been discovered, it will create a digest from all EntityIDs of the writers discovered so far, and then compare that to the Digest received from the publisher. Only when the two digests are identical you know that you have successfully discovered all participating Writers.

Problem for this is that you need to know which Writers belong to which Publisher: you can't just calculate a digest over all discovered Writers because not all discovered Writers might be part of the same group coherent Publisher.

That means I need to to know the GROUP_GUID or at the very least the GROUP_ENTITY_ID of the Publisher of a Writer in order for me to determine which discovered Writer to include in the digest I calculate to do the comparison.

However, it is not clear from the spec how this GROUP_GUID or this GROUP_ENTITY_ID is communicated from the publishing side to the subscribing side. The spec introduces a separate PID for both the GROUP_GUID and and the GROUP_ENTITY_ID, but nowhere does it say which one to send out from the Publisher side, or to which message to attach this information. This information is necessary however in order to build an interoperable group coherent implementation.

There is an inherent DoS network amplification attack that exploits peer-to-peer discovery. See https://issues.omg.org/browse/DDSIRTP26-6

This issue should be addressed by DDS-Security. Likely using some pre-shared key mechanics to protect all messages not otherwise protected. For example, the authentication handshakes.

RTPS version 2.3 updated modified Time_t to have the 'seconds' be of type unsigned long so that there would be no rollover in 2038.

This matches when it is shown in the IDL definition in 9.3.2.1 'IDL Definitions'

However Section 9.4.2.9 Timestamps illustrates the 'Timestamp' sub-message element with a 'long' instead of 'unsigned long' for the seconds part. It should be changed to illustrate 'unsigned long' as in:

Timestamp:

0...2...........8...............16.............24. .............32 
+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+-+ 
|                 unsigned long    seconds                      | 
+---------------+---------------+---------------+---------------+ 
|                 unsigned long    fraction                     | 
+---------------+---------------+---------------+---------------+