Send back SendUnicastResult with the APS seq and allow overrides

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

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<P: RadioPhy, R: Runtime> ZigbeeStack<P, R> {
         }
     }
 
-    /// 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<P: RadioPhy, R: Runtime> ZigbeeStack<P, R> {
         src_ep: u8,
         dst_ep: u8,
         aps_ack: ApsAck,
+        aps_counter: Option<ApsCounter>,
         radius: u8,
         data: Vec<u8>,
         aps_security: Option<Eui64>,
         sleepy_destination: bool,
         priority: TxPriority,
         route: RouteDirective,
-    ) -> Result<SendHandle, EnqueueError> {
+    ) -> 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<P: RadioPhy, R: Runtime> ZigbeeStack<P, R> {
             return Err(err);
         }
 
-        Ok(handle)
+        Ok((handle, aps_seq))
     }
 
     /// Send a broadcast APS data frame to a broadcast sink, returning a [`SendHandle`].

crates/ziggurat-protocol/src/bridge.rs

@@ -268,14 +268,15 @@ impl From<&NetworkBeacon> for BeaconPayload {
 pub fn send_unicast<P: RadioPhy, R: Runtime>(
     stack: &ZigbeeStack<P, R>,
     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 {
         ApsAck::Request
     } 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,14 +293,15 @@ pub fn send_unicast<P: RadioPhy, R: Runtime>(
             payload.src_ep,
             payload.dst_ep,
             aps_ack,
+            aps_counter,
             payload.radius,
             payload.asdu,
             aps_security,
             payload.flags.sleepy_destination,
             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))
 }
 

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<u8>,
 }
 
+/// 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);

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)?;

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,