Skip to content
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
24 changes: 13 additions & 11 deletions crates/dcps/src/runtime.rs
Original file line number Diff line number Diff line change
Expand Up @@ -2564,11 +2564,13 @@ fn build_publication_data(
}

/// The `DataRepresentation` set a **DataReader** announces (PID_DATA_REPRESENTATION
/// in its SEDP subscription). Per OMG XTypes 1.3 §7.6.2, a reader with the
/// default (empty) policy accepts **both** XCDR1 and XCDR2 — and ZeroDDS decodes
/// both (the read path dispatches on the per-sample encapsulation id). So the
/// reader advertises every representation it can decode, not just the writer's
/// preferred one.
/// in its SEDP subscription). Per OMG XTypes 1.3 §7.6.2 the *default* (empty)
/// policy is **XCDR1 only** — a reader left at that default would announce, and
/// accept, only XCDR1. ZeroDDS deliberately does not rely on the default: it
/// advertises **both** XCDR1 and XCDR2 (it can decode either — the read path
/// dispatches on the per-sample encapsulation id), so the reader accepts every
/// representation it can decode rather than a single preferred one. This is
/// ZeroDDS' own interop choice, not what the spec's default policy means.
///
/// This matters cross-vendor: CycloneDDS (and legacy RTI / OpenDDS < 3.16)
/// default their *writers* to **XCDR1** for `@final` types (non-XTypes backward
Expand Down Expand Up @@ -11905,12 +11907,12 @@ mod tests {
assert_ne!(parse_data_repr_offer_str("XCDR1"), Some(vec![dr::XML]));
}

/// A DataReader announces every representation it can decode (XCDR2 + XCDR1)
/// XTypes 1.3 §7.6.2: the default reader policy accepts both. CycloneDDS
/// (and legacy RTI / OpenDDS < 3.16) default their writers to XCDR1 for
/// `@final` types; without XCDR1 in the reader's announced set those writers
/// fail the DataRepresentation RxO check and never deliver. Regression for
/// Bug DR1.
/// A DataReader announces every representation it can decode (XCDR2 + XCDR1).
/// The spec default (empty policy, XTypes 1.3 §7.6.2) is XCDR1 only; ZeroDDS
/// deliberately advertises both instead. CycloneDDS (and legacy RTI /
/// OpenDDS < 3.16) default their writers to XCDR1 for `@final` types; without
/// XCDR1 in the reader's announced set those writers fail the
/// DataRepresentation RxO check and never deliver. Regression for Bug DR1.
#[test]
fn reader_accept_repr_always_includes_both_representations() {
use zerodds_rtps::publication_data::data_representation as dr;
Expand Down
93 changes: 68 additions & 25 deletions crates/dcps/src/subscriber.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

extern crate alloc;
use alloc::boxed::Box;
use alloc::string::ToString;
use alloc::sync::Arc;
use alloc::vec::Vec;
use core::marker::PhantomData;
Expand Down Expand Up @@ -80,6 +79,45 @@ fn decode_for_encap<T: DdsType>(
}
}

/// Builds a diagnostic `WireError` for a failed sample decode. Beyond the inner
/// error it records the received encapsulation (representation + byte order) and
/// this reader type's own extensibility, and — because the most common
/// cross-vendor cause is an extensibility/framing mismatch — names that as a
/// *plausible* cause. It is deliberately not asserted: without the remote type
/// the true cause cannot be confirmed here. See issue #27.
fn decode_wire_error<T: DdsType>(
inner: &crate::dds_type::DecodeError,
representation: u8,
big_endian: bool,
) -> DdsError {
use crate::dds_type::Extensibility;
let repr = if representation == 0 {
"XCDR1"
} else {
"XCDR2"
};
let endian = if big_endian {
"big-endian"
} else {
"little-endian"
};
let ext = match T::EXTENSIBILITY {
Extensibility::Final => "final",
Extensibility::Appendable => "appendable",
Extensibility::Mutable => "mutable",
};
DdsError::WireError {
message: alloc::format!(
"decode error: {inner} (received {repr} {endian}; this reader's type '{}' is @{ext}. \
A plausible cross-vendor cause is an extensibility mismatch — in XCDR2 \
@appendable/@mutable carry a DHEADER length prefix and @final does not, so a peer \
whose type has a different extensibility fails to decode. Not confirmed: the remote \
type is not available here to verify.)",
T::TYPE_NAME,
),
}
}

/// Subscriber — entity group for DataReaders.
#[derive(Debug)]
pub struct Subscriber {
Expand Down Expand Up @@ -748,9 +786,7 @@ impl<T: DdsType> DataReader<T> {
..
} => {
let sample = decode_for_encap::<T>(&bytes, representation, big_endian)
.map_err(|e| DdsError::WireError {
message: e.to_string(),
})?;
.map_err(|e| decode_wire_error::<T>(&e, representation, big_endian))?;
if !self.sample_passes_filter(&sample) {
continue;
}
Expand Down Expand Up @@ -785,9 +821,7 @@ impl<T: DdsType> DataReader<T> {
..
} => {
let sample = decode_for_encap::<T>(&bytes, representation, big_endian)
.map_err(|e| DdsError::WireError {
message: e.to_string(),
})?;
.map_err(|e| decode_wire_error::<T>(&e, representation, big_endian))?;
if !self.sample_passes_filter(&sample) {
continue;
}
Expand Down Expand Up @@ -844,12 +878,8 @@ impl<T: DdsType> DataReader<T> {
else {
continue;
};
let sample =
decode_for_encap::<T>(&bytes, representation, big_endian).map_err(|e| {
DdsError::WireError {
message: e.to_string(),
}
})?;
let sample = decode_for_encap::<T>(&bytes, representation, big_endian)
.map_err(|e| decode_wire_error::<T>(&e, representation, big_endian))?;
if !self.sample_passes_filter(&sample) {
continue;
}
Expand Down Expand Up @@ -983,12 +1013,8 @@ impl<T: DdsType> DataReader<T> {
else {
continue;
};
let sample =
decode_for_encap::<T>(&bytes, representation, big_endian).map_err(|e| {
DdsError::WireError {
message: e.to_string(),
}
})?;
let sample = decode_for_encap::<T>(&bytes, representation, big_endian)
.map_err(|e| decode_wire_error::<T>(&e, representation, big_endian))?;
if !self.sample_passes_filter(&sample) {
continue;
}
Expand Down Expand Up @@ -1739,12 +1765,8 @@ impl<T: DdsType> DataReader<T> {
let sample_source_ts = src_ts.map_or(now, crate::time::he_timestamp_to_time);
// Decode T to (a) evaluate the filter and (b) compute the
// KeyHash.
let sample =
decode_for_encap::<T>(&bytes, representation, big_endian).map_err(|e| {
DdsError::WireError {
message: alloc::string::ToString::to_string(&e),
}
})?;
let sample = decode_for_encap::<T>(&bytes, representation, big_endian)
.map_err(|e| decode_wire_error::<T>(&e, representation, big_endian))?;
if !self.sample_passes_filter(&sample) {
continue;
}
Expand Down Expand Up @@ -2384,6 +2406,27 @@ mod tests {
assert_eq!(decode_for_encap::<Probe>(&[], 0, true).unwrap(), Probe(20));
}

/// A failed decode carries diagnostic context (encapsulation + this reader's
/// extensibility) and flags the extensibility/framing mismatch as a
/// *plausible* — not asserted — cause (issue #27).
#[test]
fn decode_wire_error_carries_diagnostic_context() {
use crate::dds_type::DecodeError;
let inner = DecodeError::Invalid { what: "boom" };
// RawBytes is @final by default; received XCDR2 little-endian.
let msg = match decode_wire_error::<RawBytes>(&inner, 1, false) {
DdsError::WireError { message } => message,
_ => alloc::string::String::new(),
};
assert!(!msg.is_empty(), "expected a WireError variant");
assert!(msg.contains("XCDR2"), "{msg}");
assert!(msg.contains("little-endian"), "{msg}");
assert!(msg.contains("@final"), "{msg}");
assert!(msg.contains("DHEADER"), "{msg}");
assert!(msg.to_lowercase().contains("plausible"), "{msg}");
assert!(msg.contains("Not confirmed"), "{msg}");
}

#[test]
fn subscriber_creates_datareader_for_matching_type() {
let s = Subscriber::new(SubscriberQos::default(), None);
Expand Down
55 changes: 55 additions & 0 deletions interop/cyclone-xtypes-27/README.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,55 @@
# CycloneDDS ↔ ZeroDDS XTypes interop matrix (issue #27)

A reproducible, live DCPS-over-UDP interop matrix between ZeroDDS and
CycloneDDS on domain 100, topic `robot` (`struct Robot { uint32 id; uint32 label; }`).

It pins down issue #27: an un-annotated struct is `@final` under CycloneDDS'
generator default but `@appendable` under ZeroDDS' default. Under XCDR1 that is
invisible (an `@appendable` type emits no DHEADER, so the wire is identical to
`@final`); force XCDR2 and the framing differs (DHEADER present vs not), so a
`@final` writer and an `@appendable` reader stop understanding each other.

## What it checks (separately: match, samples, decode errors)

| Case | Writer (Cyclone) | Reader (ZeroDDS) | Expected |
|---|---|---|---|
| 1 | final, XCDR1 | `@appendable` (default) | match, samples > 0, 0 errors |
| 2 | appendable, XCDR2 | `@appendable` (default) | match, samples > 0, 0 errors |
| 3 | final, XCDR2 | `@appendable` (default) | **match, 0 samples, errors > 0** (the #27 symptom) |
| 4 | final, XCDR2 | `@final` (`--cyclone`) | match, samples > 0, 0 errors (the fix) |
| 5 | — (reverse) | ZeroDDS `@appendable`/XCDR2 writer → Cyclone reader | samples > 0 |

Case 3 is the reporter's failure: the endpoints **match** (no incompatible
QoS), but every sample fails to decode. Crucially the decode error is *not*
silent — the ZeroDDS reader's `take()` returns `WireError`; the counters here
report `errors > 0`. Case 4 shows the fix: generating the ZeroDDS reader type
with `zerodds-idlc --cyclone` (which defaults un-annotated aggregates to
`@final`) makes the same Cyclone writer interoperate.

## Requirements & running

Needs a Python that can `import cyclonedds` plus the CycloneDDS C library.
The script **loud-skips (exit 0)** when they are absent, so it is safe to
invoke unconditionally in CI.

```
PYBIN=/path/to/venv/bin/python3 CYCLONEDDS_HOME=/path/to/cyclone \
interop/cyclone-xtypes-27/run_matrix.sh
```

`PYBIN` must point at a Python that can `import cyclonedds`; `CYCLONEDDS_HOME`
at the matching CycloneDDS C install prefix. The runner exits non-zero if any
case deviates from the table above.

**Reference vendor:** CycloneDDS 11.0.1. CycloneDDS 0.10.5 is a manual
compatibility check (same outcomes observed); it is not the CI reference.

## Layout

- `robot.idl` — the shared type.
- `reader/` — standalone ZeroDDS reader/writer crate (own `[workspace]`, so a
root `cargo build` ignores it). `src/robot.rs` is generated per case by the
runner (git-ignored) — `@appendable` by default, `@final` via `--cyclone`.
- `writers/cyclone_writer.py`, `writers/cyclone_reader.py` — CycloneDDS peers,
parameterized by extensibility and representation.
- `run_matrix.sh` — orchestrator; reports match / sample / error counts per case.
3 changes: 3 additions & 0 deletions interop/cyclone-xtypes-27/reader/.gitignore
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
src/robot.rs
target/
Cargo.lock
24 changes: 24 additions & 0 deletions interop/cyclone-xtypes-27/reader/Cargo.toml
Original file line number Diff line number Diff line change
@@ -0,0 +1,24 @@
# Standalone reproducer crate for the CycloneDDS XTypes interop matrix (#27).
# Own `[workspace]` table so it is excluded from the parent workspace and
# never built by a normal `cargo build` at the repo root — it is driven only
# by `run_matrix.sh`, which regenerates `src/robot.rs` per case.
[package]
name = "cyclone27-reader"
version = "0.0.0"
edition = "2021"
publish = false

[dependencies]
zerodds-dcps = { path = "../../../crates/dcps" }
zerodds-cdr = { path = "../../../crates/cdr" }
zerodds-types = { path = "../../../crates/types" }

[[bin]]
name = "reader"
path = "src/main.rs"

[[bin]]
name = "writer"
path = "src/writer.rs"

[workspace]
57 changes: 57 additions & 0 deletions interop/cyclone-xtypes-27/reader/src/main.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
//! ZeroDDS reader for the CycloneDDS XTypes interop matrix (#27).
//!
//! Reads topic `robot` on domain 100 for a fixed window and reports three
//! counters SEPARATELY, so a match without data (the #27 symptom) is visible:
//! * `matched` — highest matched_publication_count seen (0 or 1)
//! * `samples` — successfully decoded samples (`take()` Ok)
//! * errors — `take()` calls that returned WireError (decode failures)
//!
//! The reader's extensibility is whatever `src/robot.rs` was generated with
//! (`run_matrix.sh` regenerates it per case: default `@appendable`, or `@final`
//! via `zerodds-idlc --cyclone`). Output line: `RESULT matched=.. samples=.. errors=..`.
#![allow(clippy::unwrap_used, clippy::print_stdout, clippy::print_stderr)]

#[path = "robot.rs"]
mod robot;
use robot::Robot;
use zerodds_dcps::{
DataReaderQos, DomainParticipantFactory, DomainParticipantQos, SubscriberQos, TopicQos,
};

fn main() {
let secs: u64 = std::env::args()
.nth(1)
.and_then(|s| s.parse().ok())
.unwrap_or(12);
let f = DomainParticipantFactory::instance();
let p = f
.create_participant(100, DomainParticipantQos::default())
.unwrap();
let t = p
.create_topic::<Robot>("robot", TopicQos::default())
.expect("topic");
let s = p.create_subscriber(SubscriberQos::default());
let r = s
.create_datareader::<Robot>(&t, DataReaderQos::default())
.expect("reader");
eprintln!("[zerodds reader] domain=100 topic=robot window={secs}s");

let start = std::time::Instant::now();
let mut matched = 0usize;
let mut samples = 0u64;
let mut errors = 0u64;
while start.elapsed().as_secs() < secs {
matched = matched.max(r.matched_publication_count());
match r.take() {
Ok(v) => samples += v.len() as u64,
Err(e) => {
errors += 1;
if errors <= 3 {
eprintln!("[zerodds reader] take error: {e:?}");
}
}
}
std::thread::sleep(std::time::Duration::from_millis(100));
}
println!("RESULT matched={matched} samples={samples} errors={errors}");
}
40 changes: 40 additions & 0 deletions interop/cyclone-xtypes-27/reader/src/writer.rs
Original file line number Diff line number Diff line change
@@ -0,0 +1,40 @@
//! ZeroDDS writer for the reverse-direction leg of the #27 interop matrix
//! (ZeroDDS writer -> CycloneDDS reader). Writes topic `robot` on domain 100
//! for a fixed window. Extensibility follows the generated `src/robot.rs`.
#![allow(clippy::unwrap_used, clippy::print_stderr)]

#[path = "robot.rs"]
mod robot;
use robot::Robot;
use zerodds_dcps::{
DataWriterQos, DomainParticipantFactory, DomainParticipantQos, PublisherQos, TopicQos,
};

fn main() {
let secs: u64 = std::env::args()
.nth(1)
.and_then(|s| s.parse().ok())
.unwrap_or(20);
let f = DomainParticipantFactory::instance();
let p = f
.create_participant(100, DomainParticipantQos::default())
.unwrap();
let t = p
.create_topic::<Robot>("robot", TopicQos::default())
.expect("topic");
let pubr = p.create_publisher(PublisherQos::default());
let w = pubr
.create_datawriter::<Robot>(&t, DataWriterQos::default())
.expect("writer");
eprintln!("[zerodds writer] domain=100 topic=robot window={secs}s");
let start = std::time::Instant::now();
let mut c: u32 = 0;
while start.elapsed().as_secs() < secs {
let _ = w.write(&Robot {
id: 1,
label: c % 1000,
});
c += 1;
std::thread::sleep(std::time::Duration::from_millis(300));
}
}
4 changes: 4 additions & 0 deletions interop/cyclone-xtypes-27/robot.idl
Original file line number Diff line number Diff line change
@@ -0,0 +1,4 @@
struct Robot {
uint32 id;
uint32 label;
};
Loading
Loading