Skip to content

Commit ce9f974

Browse files
committed
test(event): pin the MessageLost wiring, not just its arithmetic
The unit tests in event.rs exercise MessageLossTracker directly, so deleting the observe_loss(..) call from the subscriber receive path leaves every one of them green. This file fails in that case. Loss is induced deterministically instead of by dropping a packet: the test publishes onto the subscriber's own key expression through the node's session with a hand-built Attachment, so the sequence gap is exact and there is no timing to lose.
1 parent 7a03c54 commit ce9f974

1 file changed

Lines changed: 187 additions & 0 deletions

File tree

Lines changed: 187 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,187 @@
1+
//! `RMW_EVENT_MESSAGE_LOST` must actually be raised by a live subscriber.
2+
//!
3+
//! `event.rs`'s unit tests pin `MessageLossTracker`'s arithmetic. They say
4+
//! nothing about whether anything *calls* it: delete the `observe_loss(..)` line
5+
//! from the subscriber's receive path and every one of them still passes. This
6+
//! file is the detector for that wiring.
7+
//!
8+
//! Loss is induced deterministically rather than by trying to drop a packet.
9+
//! The subscriber's own key expression is published to directly, through the
10+
//! node's zenoh session, with a hand-built [`Attachment`] carrying a chosen
11+
//! sequence number — so the gap is exact and there is no timing to lose.
12+
13+
mod common;
14+
15+
use std::{
16+
sync::{Arc, Mutex},
17+
thread,
18+
time::{Duration, Instant},
19+
};
20+
21+
use common::{TestRouter, create_hiroz_context_with_endpoint};
22+
use hiroz::{
23+
Builder, GidArray, TypeHash,
24+
attachment::Attachment,
25+
event::ZenohEventType,
26+
ros_msg::MessageTypeInfo,
27+
};
28+
use serde::{Deserialize, Serialize};
29+
use serial_test::serial;
30+
use zenoh::Wait;
31+
32+
#[derive(Debug, Clone, Serialize, Deserialize, PartialEq)]
33+
struct Tick {
34+
counter: u64,
35+
}
36+
37+
impl MessageTypeInfo for Tick {
38+
fn type_name() -> &'static str {
39+
"test_msgs::msg::dds_::Tick_"
40+
}
41+
fn type_hash() -> TypeHash {
42+
TypeHash::zero()
43+
}
44+
}
45+
46+
impl hiroz::ros_msg::WithTypeInfo for Tick {}
47+
48+
impl hiroz::msg::ZMessage for Tick {
49+
type Serdes = hiroz::msg::SerdeCdrSerdes<Tick>;
50+
}
51+
52+
fn gid(n: u8) -> GidArray {
53+
let mut g = [0u8; 16];
54+
g[0] = n;
55+
g
56+
}
57+
58+
/// Wait until `total_count` for `MessageLost` stops changing, then return it.
59+
fn settled_loss_count(sub_events: &Arc<Mutex<hiroz::event::EventsManager>>) -> i32 {
60+
let mut last = -1;
61+
let mut stable_since = Instant::now();
62+
let deadline = Instant::now() + Duration::from_secs(5);
63+
loop {
64+
let now = sub_events
65+
.lock()
66+
.unwrap()
67+
.take_event_status(ZenohEventType::MessageLost)
68+
.total_count;
69+
if now != last {
70+
last = now;
71+
stable_since = Instant::now();
72+
} else if stable_since.elapsed() >= Duration::from_millis(400) {
73+
return now;
74+
}
75+
assert!(Instant::now() < deadline, "loss count never settled");
76+
thread::sleep(Duration::from_millis(25));
77+
}
78+
}
79+
80+
/// A gap in a publisher's sequence numbers raises `MessageLost` on the
81+
/// subscriber that saw it, with the count of samples that never arrived.
82+
#[test]
83+
#[serial]
84+
fn a_sequence_gap_raises_message_lost() {
85+
const TOPIC: &str = "/message_lost_gap";
86+
87+
let router = TestRouter::new();
88+
let ctx = create_hiroz_context_with_endpoint(router.endpoint()).expect("context");
89+
let node = ctx.create_node("message_lost_node").build().expect("node");
90+
91+
let received = Arc::new(Mutex::new(Vec::<u64>::new()));
92+
let cb_received = received.clone();
93+
let sub = node
94+
.create_sub::<Tick>(TOPIC)
95+
.build_with_callback(move |msg: Tick| {
96+
cb_received.lock().unwrap().push(msg.counter);
97+
})
98+
.expect("subscriber");
99+
100+
// Publish straight onto the subscriber's own key expression, so the
101+
// sequence numbers are ours to choose.
102+
let ke = node
103+
.keyexpr_format()
104+
.topic_key_expr(sub.entity())
105+
.expect("topic key expr");
106+
let session = node.session();
107+
let publisher_gid = gid(42);
108+
109+
let put = |sn: i64, counter: u64| {
110+
let zbuf = <hiroz::msg::SerdeCdrSerdes<Tick> as hiroz::msg::ZSerializer>::serialize_to_zbuf(
111+
&Tick { counter },
112+
);
113+
session
114+
.put((*ke).clone(), zenoh::bytes::ZBytes::from(zbuf))
115+
.attachment(Attachment::new(sn, publisher_gid))
116+
.wait()
117+
.expect("put");
118+
};
119+
120+
thread::sleep(Duration::from_millis(300));
121+
122+
put(0, 0); // baseline — first from this publisher, never counted
123+
put(1, 1); // contiguous
124+
put(5, 5); // 2, 3 and 4 never arrived
125+
126+
let lost = settled_loss_count(sub.events_mgr());
127+
128+
assert_eq!(
129+
lost, 3,
130+
"expected the three skipped sequence numbers to be reported as lost; \
131+
got {lost}. Zero means the receive path never fed the loss tracker"
132+
);
133+
assert_eq!(
134+
received.lock().unwrap().len(),
135+
3,
136+
"all three published samples should still have been delivered — \
137+
detecting loss must not drop anything"
138+
);
139+
}
140+
141+
/// A subscriber that joins late has not "lost" the history it was never sent.
142+
///
143+
/// Without the first-sample exemption this reports the publisher's sequence
144+
/// number as the loss count, so every late joiner looks catastrophically lossy.
145+
#[test]
146+
#[serial]
147+
fn joining_late_reports_no_loss() {
148+
const TOPIC: &str = "/message_lost_late_join";
149+
150+
let router = TestRouter::new();
151+
let ctx = create_hiroz_context_with_endpoint(router.endpoint()).expect("context");
152+
let node = ctx
153+
.create_node("message_lost_late_node")
154+
.build()
155+
.expect("node");
156+
157+
let sub = node
158+
.create_sub::<Tick>(TOPIC)
159+
.build_with_callback(|_msg: Tick| {})
160+
.expect("subscriber");
161+
162+
let ke = node
163+
.keyexpr_format()
164+
.topic_key_expr(sub.entity())
165+
.expect("topic key expr");
166+
let session = node.session();
167+
168+
thread::sleep(Duration::from_millis(300));
169+
170+
// First sample this subscriber ever sees from this publisher, and it is
171+
// already well into the publisher's stream.
172+
let zbuf = <hiroz::msg::SerdeCdrSerdes<Tick> as hiroz::msg::ZSerializer>::serialize_to_zbuf(
173+
&Tick { counter: 9000 },
174+
);
175+
session
176+
.put((*ke).clone(), zenoh::bytes::ZBytes::from(zbuf))
177+
.attachment(Attachment::new(9000, gid(7)))
178+
.wait()
179+
.expect("put");
180+
181+
let lost = settled_loss_count(sub.events_mgr());
182+
183+
assert_eq!(
184+
lost, 0,
185+
"a late joiner must not be charged for history it was never sent"
186+
);
187+
}

0 commit comments

Comments
 (0)