Skip to content

Commit

Permalink
Adjust GAP messages so FastDDS always accepts them, while still compl…
Browse files Browse the repository at this point in the history
…ying with the spec.

Always set gapList.base as the exclusive endpoint of the contiguous range of sequence numbers in the GAP message. Before, we set gapList.base as the inclusive endpoint. This was not against the spec, but it meant that we could send messages where gap_start == gapList.base, in which case some DDS implementations (at least FastDDS) refused to process the GAP message. The new implementation also complies with the spec.
  • Loading branch information
ohuopio committed Mar 14, 2024
1 parent 08e9edb commit 2cc61da
Show file tree
Hide file tree
Showing 2 changed files with 20 additions and 18 deletions.
18 changes: 11 additions & 7 deletions src/messages/submessages/gap.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ use super::{
submessage_kind::SubmessageKind,
};
/// This Submessage is sent from an RTPS Writer to an RTPS Reader and
/// indicates to the RTPS Reader that a range of sequence numbers
/// is no longer relevant. The set may be a contiguous range of
/// sequence numbers or a specific set of sequence numbers.
/// indicates to the RTPS Reader that a set of sequence numbers
/// is no longer relevant. The set may contain a contiguous range of sequence
/// numbers and/or a noncontiguous collection of sequence numbers.
#[derive(Debug, PartialEq, Eq, Clone, Readable, Writable)]
pub struct Gap {
/// Identifies the Reader Entity that is being informed of the
Expand All @@ -29,15 +29,19 @@ pub struct Gap {
/// numbers applies.
pub writer_id: EntityId,

/// Identifies the first sequence number in the interval of
/// Identifies the first sequence number in the contiguous range of
/// irrelevant sequence numbers
pub gap_start: SequenceNumber,

/// Identifies the last sequence number in the interval of irrelevant
/// sequence numbers.
/// Identifies the last sequence number in the contiguous range of irrelevant
/// sequence numbers as defined by the spec: gap_list.base is the exclusive
/// endpoint of this range. Therefore, the range contains the sequence
/// numbers which satisfy gap_start <= sn < gap_list.base.
///
/// Identifies an additional list of sequence numbers that are
/// irrelevant.
/// irrelevant. Note that gap_list.base may or may not be included in this
/// list (and if it is, then the contiguous range of irrelevant
/// sequence numbers is actually larger than gap_start <= sn < gap_list.base).
pub gap_list: SequenceNumberSet,
}

Expand Down
20 changes: 9 additions & 11 deletions src/rtps/message.rs
Original file line number Diff line number Diff line change
Expand Up @@ -447,18 +447,16 @@ impl MessageBuilder {
) -> Self {
match (irrelevant_sns.first(), irrelevant_sns.last()) {
(Some(&gap_start), Some(&_last_sn)) => {
// Determine the contiguous range (starting from gap_start) of irrelevant
// seqnums
let mut range_end = gap_start;
while irrelevant_sns.contains(&range_end.plus_1()) {
range_end = range_end.plus_1();
// Determine the contiguous range of irrelevant seqnums starting from gap_start.
// Do this by finding the first seqnum which is larger than gap_start but is not
// included in irrelevant_sns. That is, find the
// exclusive endpoint of the contiguous range.
let mut range_endpoint_excl = gap_start.plus_1();
while irrelevant_sns.contains(&range_endpoint_excl) {
range_endpoint_excl = range_endpoint_excl.plus_1();
}
// range_end is now the last seqnum in the contiguous range.
// Note that when receiving a GAP, the interpreted range is
// (gap_start .. gapList.base -1). But since gapList.base is included in the
// gapList, gapList.base is also always interpred as irrelevant, hence
// completing the range
let list_base = range_end;
// Set gapList.base as this exclusive endpoint of the range
let list_base = range_endpoint_excl;
let list_set = irrelevant_sns.clone().split_off(&list_base);
let gap_list = SequenceNumberSet::from_base_and_set(list_base, &list_set);

Expand Down

0 comments on commit 2cc61da

Please sign in to comment.