fix(event): raise MessageLost from sequence gaps

[YuanYuYuan]

Summary

hiroz plumbs RMW_EVENT_MESSAGE_LOST fully, but raises it nowhere. A subscriber that registers a callback for it gets one that cannot fire, plus a status that stays zero. In-transit message loss is invisible to every ROS 2 application on hiroz.

This PR adds MessageLossTracker: a per-publisher sequence-number baseline on the subscriber. It raises the event when an arrival skips past that baseline.

Fixes #292.

The defect

What already existed, and the one piece that was missing:

layer	state before this PR
ZenohEventType::MessageLost	defined in event.rs
rmw_event_type 3 → MessageLost	mapped, so rmw_event_type_is_supported returns true
rmw_message_lost_status_t fill-in on rmw_take_event	present
Attachment::sequence_number and source_gid on every sample	serialised already
anything that raises the event	nothing, outside #[cfg(test)]

ZenohEventType has 11 variants. Four map to None and are honestly declared unsupported: both LIVELINESS_* and both DEADLINE_MISSED. Four are raised from rmw.rs. Three are declared supported and never raised.

This PR closes the one where hiroz trails rmw_zenoh_cpp. The other two are SUBSCRIPTION_INCOMPATIBLE_TYPE and PUBLISHER_INCOMPATIBLE_TYPE, tracked as #293, which rmw_zenoh_cpp does not raise either.

What this PR does

change	where
MessageLossTracker — per-GID baseline, raises MessageLost on a forward skip	crates/hiroz/src/event.rs
observe_loss(..) on the subscriber receive path, inside build_internal so every build variant is covered	crates/hiroz/src/pubsub.rs
events_mgr() and entity() widened from the queue-mode impl to all ZSub variants	crates/hiroz/src/pubsub.rs
six unit tests on the arithmetic, two integration tests on the wiring	event.rs, crates/hiroz-tests/tests/message_lost.rs

The tracker ignores a sample whose attachment is missing or undecodable. A plain zenoh peer sends no sequence number. hiroz must not treat such a peer as lossy just because it looks unfamiliar.

Alignment with rmw_zenoh_cpp

The reference is SubscriptionData::add_new_message, upstream commit e3159856e3816e726f2c5f1f9b4858ad6b7ee63e. This section compares against its gap logic.

Matched

behaviour	rmw_zenoh_cpp	this PR
signal	gap in per-publisher sequence numbers	same
first sample from a publisher	no event	same
a gap of n reports	n - 1	same
saturates to i32	std::clamp	min(i32::MAX)
queue-depth drops counted	no — debug log only	no — debug log only, in DataHandler::handle in common.rs
raised from	the receive path	same

The depth-drop row is easy to get wrong. A sample dropped because the subscriber's queue is full arrived. It advanced the baseline, so it produces no gap. Neither implementation reports it as MESSAGE_LOST. This PR does not change that.

Divergences, deliberate

DV1 — an out-of-order arrival no longer reports phantom loss. Upstream computes std::abs(sn - last) and rewrites the baseline unconditionally. This PR advances the baseline only forward, and reports nothing for an arrival at or below it.

arrivals 5, 3, 6	reported lost
rmw_zenoh_cpp	abs(3-5)=2 → 1, then abs(6-3)=3 → 2. Total 3 phantom
this PR	0

This is the recovery path, not a contrived input. Both implementations enable heartbeat-based miss detection on reliable subscribers: recovery->last_sample_miss_detection = RecoveryOptions::Heartbeat{} upstream, and RecoveryConfig::default().heartbeat() here. Recovery exists precisely to deliver a missed sample after newer ones have arrived.

Note

Upstream behaviour here comes from reading rmw_subscription_data.cpp, not from executing it. Both on_sample closures feed add_new_message, and both are installed via declare_advanced_subscriber. Live and recovered samples therefore share the same gap logic. The hiroz side is pinned by message_loss_survives_a_transient_local_replay.

DV2 — no hash collisions between publishers. Upstream keys its map on hash_gid(...), a size_t. Two publishers whose GIDs collide would interleave their sequence numbers into one baseline, producing continuous phantom loss on both. This PR keys on the full 16-byte GidArray.

DV3 — the callout happens with no subscriber lock held. Upstream calls update_event_status from inside add_new_message. That method holds SubscriptionData::mutex_ for its whole body — the shape #259 is about. EventsManager::update_event_status does release event_mutex_ before trigger_event_callback; the subscription mutex is the one still held. In this PR the per-GID map has its own lock, and the guard drops before the call to update_shared_event_status.

DV4 — std::clamp's lower bound is not copied. Upstream clamps to [i32::MIN, i32::MAX]. The value is abs(..) - 1 guarded by abs(..) > 1, so it cannot be negative. This PR saturates at i32::MAX only.

Evidence

direction	measurement	commit
implementation	all 26 GitHub checks completed successfully; license/cla green	604b0cd54bbb1165683a26a80ed8c15cbedc0b96
wiring removed	delete the observe_loss(..) call from the receive path: the six unit tests still pass, a_sequence_gap_raises_message_lost fails	measured locally by reverting that one line; not re-run since

The second row is the point. The unit tests exercise MessageLossTracker directly, so they cannot see whether anything calls it. Without message_lost.rs, deleting the wiring would be a silent, green regression.

crates/hiroz/src/event.rs has 20 #[test] functions at the head commit, six of them new: ordinary gap, first sample, per-publisher isolation, reorder/republish, transient-local replay, and i32 saturation.

The two integration tests induce loss deterministically. Each publishes onto the subscriber's own key expression through the node's session with a hand-built Attachment, so the gap is exact. Only one of the two is a detector:

test	role
a_sequence_gap_raises_message_lost	detector — fails when the wiring is removed
joining_late_reports_no_loss	guard, not a detector: it asserts zero, and no wiring also produces zero

Breaking changes

None. Two API surfaces change shape, both additive:

change	effect
MessageLossTracker in hiroz::event	new public type; nothing existing changes shape
ZSub::events_mgr() and ZSub::entity() moved to the generic impl<T: ZMessage, Q, S: ZDeserializer>	widening only — both were already available on ZSub<T, Sample, S>, and are now also reachable from callback-mode subscribers, which the rmw layer needs

Important

A subscriber that previously saw total_count == 0 forever now sees real counts. That is the purpose of this PR. It changes behaviour for anything asserting on that status.

Coverage this does not have

None of these block the fix. They bound what green CI proves.

tag	gap
G1	Loss is detected between publisher and subscriber only. A gap proves a sample did not arrive; it says nothing about why.
G2	Nothing covers a subscriber's own queue. See the depth-drop row above.
G3	A publisher that restarts its sequence numbering while keeping its GID goes quiet until it passes its previous high-water mark.
G4	No test drives real network loss. Loss is synthesised with hand-built attachments.

G3 is a deliberate trade. In ROS a restarted endpoint normally gets a fresh GID, so it lands in the first-sample path instead. The alternative is upstream's behaviour, which mis-reports every replay (DV1).