From bb533746ba663910d05f9da48850bcc31b29f096 Mon Sep 17 00:00:00 2001 From: puddly <32534428+puddly@users.noreply.github.com> Date: Fri, 16 Oct 2026 19:13:36 +0000 Subject: [PATCH] Send back `SendUnicastResult` with the APS seq and allow overrides --- crates/ziggurat-driver/src/zigbee_stack.rs | 3 +++ .../ziggurat-driver/src/zigbee_stack/aps.rs | 27 +++++++++++-------- crates/ziggurat-protocol/src/bridge.rs | 6 +++-- crates/ziggurat-protocol/src/wire.rs | 24 ++++++++++++----- crates/ziggurat-server/src/main.rs | 4 +-- crates/ziggurat-zigbee/src/constants.rs | 6 +---- 6 files changed, 44 insertions(+), 26 deletions(-) diff --git a/crates/ziggurat-driver/src/zigbee_stack.rs b/crates/ziggurat-driver/src/zigbee_stack.rs index 0eb1baf..c6fc531 100644 --- a/crates/ziggurat-driver/src/zigbee_stack.rs +++ b/crates/ziggurat-driver/src/zigbee_stack.rs @@ -230,6 +230,9 @@ pub enum ApsAck { None, } +/// The APS counter of a frame we originate. +pub type ApsCounter = u8; + /// Whether a device is attaching fresh or rejoining. /// /// Selects the link key that protects the transported network key: a fresh joiner only diff --git a/crates/ziggurat-driver/src/zigbee_stack/aps.rs b/crates/ziggurat-driver/src/zigbee_stack/aps.rs index e130d74..d6f933f 100644 --- a/crates/ziggurat-driver/src/zigbee_stack/aps.rs +++ b/crates/ziggurat-driver/src/zigbee_stack/aps.rs @@ -16,9 +16,9 @@ use ziggurat_zigbee::Instant as CoreInstant; use ziggurat_zigbee::flat_map::Entry; use super::{ - ApsAck, ApsAckData, Broadcast, DeliveryError, EnqueueError, NwkSecurityMode, PendingApsAck, - RouteDirective, SendHandle, SendMode, SendSlot, TrackStage, TxOutcome, TxPolicy, TxPriority, - Unicast, ZigbeeStack, + ApsAck, ApsAckData, ApsCounter, Broadcast, DeliveryError, EnqueueError, NwkSecurityMode, + PendingApsAck, RouteDirective, SendHandle, SendMode, SendSlot, TrackStage, TxOutcome, TxPolicy, + TxPriority, Unicast, ZigbeeStack, }; use crate::frame_token::TrafficClass; @@ -189,22 +189,26 @@ impl ZigbeeStack { } } - /// How long to wait for a device's APS ack: longer for a sleepy destination, which - /// only sees (and acks) the frame after polling. + /// How long to wait for a device's APS ack once the next hop accepts the frame. Our + /// own sleepy child accepts it at its poll, after the indirect hold. A sleepy device + /// behind a router is still ahead of that router's hold. fn aps_ack_timeout(&self, destination: Nwk, sleepy_destination: bool) -> Duration { - if sleepy_destination || self.sleepy_child_eui64(destination).is_some() { - self.tunables.aps_ack_timeout_indirect() + if sleepy_destination && self.sleepy_child_eui64(destination).is_none() { + self.tunables.aps_ack_timeout() + self.tunables.transaction_persistence_time() } else { self.tunables.aps_ack_timeout() } } - /// Send a unicast APS data frame, returning a [`SendHandle`] over its two stages. + /// Send a unicast APS data frame, returning a [`SendHandle`] over its two stages and + /// its APS counter. /// `Err` rejects at admission (malformed, rate limited, frame budget) and no handle is /// returned. On `Ok`, `handed_off` resolves on next-hop acceptance and `delivered` on /// the end-to-end APS ack (for an ack send) or on that same acceptance (rule 4, for a /// no-ack send). /// + /// `aps_counter` reuses an earlier send's counter, so a retry is dropped as a duplicate. + /// /// `aps_security` requests APS encryption of the ASDU with the link key shared with /// that device (link keys are pairwise, so this is unicast-only). #[allow(clippy::too_many_arguments)] @@ -216,15 +220,16 @@ impl ZigbeeStack { src_ep: u8, dst_ep: u8, aps_ack: ApsAck, + aps_counter: Option, radius: u8, data: Vec, aps_security: Option, sleepy_destination: bool, priority: TxPriority, route: RouteDirective, - ) -> Result { + ) -> Result<(SendHandle, ApsCounter), EnqueueError> { let asdu = FrameBytes::from_slice(&data).map_err(|_| EnqueueError::PayloadTooLong)?; - let aps_seq = self.next_aps_counter(); + let aps_seq = aps_counter.unwrap_or_else(|| self.next_aps_counter()); let aps_frame = ApsDataFrame { frame_control: ApsFrameControl { @@ -327,7 +332,7 @@ impl ZigbeeStack { return Err(err); } - Ok(handle) + Ok((handle, aps_seq)) } /// Send a broadcast APS data frame to a broadcast sink, returning a [`SendHandle`]. diff --git a/crates/ziggurat-protocol/src/bridge.rs b/crates/ziggurat-protocol/src/bridge.rs index 9ca6613..ddf266a 100644 --- a/crates/ziggurat-protocol/src/bridge.rs +++ b/crates/ziggurat-protocol/src/bridge.rs @@ -268,7 +268,7 @@ impl From<&NetworkBeacon> for BeaconPayload { pub fn send_unicast( stack: &ZigbeeStack, payload: SendUnicastPayload, -) -> Result<(SendHandle, ConfirmKind), Error> { +) -> Result<((SendHandle, ConfirmKind), SendUnicastResultPayload), Error> { let aps_security = (payload.flags.aps_encryption && payload.flags.has_eui64) .then_some(payload.destination_eui64); let aps_ack = if payload.flags.aps_ack { @@ -276,6 +276,7 @@ pub fn send_unicast( } else { ApsAck::None }; + let aps_counter = payload.flags.force_aps_seq.then_some(payload.aps_seq); let route = route_directive(payload.route, payload.next_hop, payload.relays)?; let confirm_kind = if payload.flags.aps_ack { @@ -292,6 +293,7 @@ pub fn send_unicast( payload.src_ep, payload.dst_ep, aps_ack, + aps_counter, payload.radius, payload.asdu, aps_security, @@ -299,7 +301,7 @@ pub fn send_unicast( TxPriority::from_host(payload.priority), route, ) - .map(|handle| (handle, confirm_kind)) + .map(|(handle, aps_seq)| ((handle, confirm_kind), SendUnicastResultPayload { aps_seq })) .map_err(|e| enqueue_error(&e)) } diff --git a/crates/ziggurat-protocol/src/wire.rs b/crates/ziggurat-protocol/src/wire.rs index e09e506..9b6d535 100644 --- a/crates/ziggurat-protocol/src/wire.rs +++ b/crates/ziggurat-protocol/src/wire.rs @@ -414,7 +414,9 @@ pub struct SendUnicastFlags { /// The destination is a sleepy device. It only sees frames by polling its parent, /// so the APS ack wait must cover a poll cycle. pub sleepy_destination: bool, - pub reserved: u4, + /// Send with `aps_seq` instead of a fresh APS counter, to retry an earlier send. + pub force_aps_seq: bool, + pub reserved: u3, } /// How the host wants a unicast routed. @@ -455,9 +457,10 @@ pub struct SendUnicastPayload { pub cluster_id: u16, pub src_ep: u8, pub dst_ep: u8, - /// Ignored: the stack owns the APS counter space so that host sends cannot collide - /// with stack-originated frames (ZDP, APS commands). Kept for wire stability. - pub _aps_seq: u8, + /// The APS counter to send with, used only with `force_aps_seq`. Otherwise the stack + /// assigns one, so host sends cannot collide with stack-originated frames (ZDP, APS + /// commands). + pub aps_seq: u8, pub radius: u8, pub priority: i8, pub route: RouteControl, @@ -474,6 +477,13 @@ pub struct SendUnicastPayload { pub asdu: Vec, } +/// The APS counter an accepted unicast was sent with. +#[abstract_bits] +#[derive(Debug, Clone)] +pub struct SendUnicastResultPayload { + pub aps_seq: u8, +} + #[abstract_bits] #[derive(Debug, Clone)] pub struct SendBroadcastFlags { @@ -491,7 +501,7 @@ pub struct SendBroadcastPayload { pub cluster_id: u16, pub src_ep: u8, pub dst_ep: u8, - /// Ignored, as in [`SendUnicastPayload::_aps_seq`]. + /// Ignored: the stack owns the APS counter space. pub _aps_seq: u8, pub radius: u8, pub priority: i8, @@ -516,7 +526,7 @@ pub struct SendGroupcastPayload { pub profile_id: u16, pub cluster_id: u16, pub src_ep: u8, - /// Ignored, as in [`SendUnicastPayload::_aps_seq`]. + /// Ignored: the stack owns the APS counter space. pub _aps_seq: u8, pub radius: u8, pub priority: i8, @@ -856,6 +866,7 @@ pub enum Response { NetworkInfo(NetworkInfoPayload), ScanCount(ScanCountPayload), CancelResult(CancelResultPayload), + SendUnicastResult(SendUnicastResultPayload), } impl Response { @@ -869,6 +880,7 @@ impl Response { Self::NetworkInfo(payload) => append(&mut bytes, payload), Self::ScanCount(payload) => append(&mut bytes, payload), Self::CancelResult(payload) => append(&mut bytes, payload), + Self::SendUnicastResult(payload) => append(&mut bytes, payload), }; if !fits { return Error::Status(Status::ResponseTooLarge).frame(command, request_id); diff --git a/crates/ziggurat-server/src/main.rs b/crates/ziggurat-server/src/main.rs index e5933a0..4256a98 100644 --- a/crates/ziggurat-server/src/main.rs +++ b/crates/ziggurat-server/src/main.rs @@ -568,9 +568,9 @@ impl ZigguratServer { self.stream_scan(request_id, outbound, events).await } R::SendUnicast(payload) => { - let tracked = proto::send_unicast(&*self.running()?, payload)?; + let (tracked, result) = proto::send_unicast(&*self.running()?, payload)?; self.register_send(request_id, sends, tracked); - Ok(proto::Response::Empty) + Ok(proto::Response::SendUnicastResult(result)) } R::SendBroadcast(payload) => { let tracked = proto::send_broadcast(&*self.running()?, payload)?; diff --git a/crates/ziggurat-zigbee/src/constants.rs b/crates/ziggurat-zigbee/src/constants.rs index 01dadd7..d7c9944 100644 --- a/crates/ziggurat-zigbee/src/constants.rs +++ b/crates/ziggurat-zigbee/src/constants.rs @@ -349,13 +349,9 @@ tunables! { /// duplicate rejection. Must cover the sender's full APS retransmission window. aps_duplicate_rejection_timeout: Duration = Duration::from_secs(9), + /// How long to wait for an APS ack once the next hop has accepted the frame. aps_ack_timeout: Duration = Duration::from_millis(5000), - /// APS acks from a sleepy child arrive only after it polls for the frame, so the - /// wait must cover a full indirect transaction lifetime (7.68s) plus the ack's trip - /// back. - aps_ack_timeout_indirect: Duration = Duration::from_millis(10000), - /// `macMaxCSMABackoffs`: how many times the radio backs off on a busy channel /// before declaring a transmit failed. mac_max_csma_backoffs: u8 = 2,