From 427e4898e275d5549c70e5fd4a9a1a4beab8f16e Mon Sep 17 00:00:00 2001 From: Sam Clercky Date: Thu, 14 Mar 2024 10:50:26 +0100 Subject: [PATCH 1/5] fix acks --- dot15d4/src/csma/mod.rs | 19 +++++++++++++++---- 1 file changed, 15 insertions(+), 4 deletions(-) diff --git a/dot15d4/src/csma/mod.rs b/dot15d4/src/csma/mod.rs index e395bef..af14518 100644 --- a/dot15d4/src/csma/mod.rs +++ b/dot15d4/src/csma/mod.rs @@ -276,21 +276,32 @@ where RadioFrame::new_checked(buffer).map_err(TransmissionTaskError::InvalidDeviceFrame)?; let mut frame = Frame::new(frame.data_mut()).map_err(|_err| TransmissionTaskError::InvalidIEEEFrame)?; + + // TODO: What about other types of MAC frames if frame.frame_control().frame_type() == FrameType::Data { match frame .addressing() .and_then(|addr| addr.dst_address(&frame.frame_control())) { Some(addr) if addr.is_unicast() && self.config.ack_unicast => { - frame.frame_control_mut().set_ack_request(true) + frame.frame_control_mut().set_ack_request(true); + Ok(frame.sequence_number()) } Some(addr) if addr.is_broadcast() && self.config.ack_broadcast => { - frame.frame_control_mut().set_ack_request(true) + frame.frame_control_mut().set_ack_request(true); + Ok(frame.sequence_number()) + } + Some(_) | None => { + // Make sure that the ack_request field is set to false independent on how the frame was actually created + frame.frame_control_mut().set_ack_request(false); + Ok(None) } - Some(_) | None => {} } + } else { + // We want an ACK, and here is the sequence number + frame.frame_control_mut().set_ack_request(false); + Ok(None) } - Ok(frame.sequence_number()) } async fn wait_for_valid_ack(radio: &mut R, sequence_number: u8, ack_rx: &mut [u8; 128]) { From 105662f8a9a195bedd3668648bc6571c94564116 Mon Sep 17 00:00:00 2001 From: Sam Clercky Date: Thu, 14 Mar 2024 10:57:42 +0100 Subject: [PATCH 2/5] Also allow mac commands to have acks --- dot15d4/src/csma/mod.rs | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/dot15d4/src/csma/mod.rs b/dot15d4/src/csma/mod.rs index af14518..90394f7 100644 --- a/dot15d4/src/csma/mod.rs +++ b/dot15d4/src/csma/mod.rs @@ -277,8 +277,9 @@ where let mut frame = Frame::new(frame.data_mut()).map_err(|_err| TransmissionTaskError::InvalidIEEEFrame)?; - // TODO: What about other types of MAC frames - if frame.frame_control().frame_type() == FrameType::Data { + // Only Data and MAC Commands should be able to get an ACK + let frame_type = frame.frame_control().frame_type(); + if frame_type == FrameType::Data || frame_type == FrameType::MacCommand { match frame .addressing() .and_then(|addr| addr.dst_address(&frame.frame_control())) From b568a7b6e4683d09aa3844307d90b8c05343d848 Mon Sep 17 00:00:00 2001 From: Sam Clercky Date: Thu, 14 Mar 2024 11:45:18 +0100 Subject: [PATCH 3/5] Fix doubble logging --- dot15d4/src/utils/log.rs | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/dot15d4/src/utils/log.rs b/dot15d4/src/utils/log.rs index c71e617..1257912 100644 --- a/dot15d4/src/utils/log.rs +++ b/dot15d4/src/utils/log.rs @@ -3,7 +3,7 @@ #[macro_export] macro_rules! error { ($($arg:tt)*) => { - #[cfg(feature = "defmt")] + #[cfg(all(not(feature="log"), feature = "defmt"))] defmt::error!($($arg)*); #[cfg(feature = "log")] ::log::error!($($arg)*); @@ -13,7 +13,7 @@ macro_rules! error { #[macro_export] macro_rules! warn { ($($arg:tt)*) => { - #[cfg(feature = "defmt")] + #[cfg(all(not(feature="log"), feature = "defmt"))] defmt::warn!($($arg)*); #[cfg(feature = "log")] ::log::warn!($($arg)*); @@ -23,7 +23,7 @@ macro_rules! warn { #[macro_export] macro_rules! info { ($($arg:tt)*) => { - #[cfg(feature = "defmt")] + #[cfg(all(not(feature="log"), feature = "defmt"))] ::defmt::info!($($arg)*); #[cfg(feature = "log")] ::log::info!($($arg)*); @@ -33,7 +33,7 @@ macro_rules! info { #[macro_export] macro_rules! debug { ($($arg:tt)*) => { - #[cfg(feature = "defmt")] + #[cfg(all(not(feature="log"), feature = "defmt"))] ::defmt::debug!($($arg)*); #[cfg(feature = "log")] ::log::debug!($($arg)*); @@ -43,7 +43,7 @@ macro_rules! debug { #[macro_export] macro_rules! trace { ($($arg:tt)*) => { - #[cfg(feature = "defmt")] + #[cfg(all(not(feature="log"), feature = "defmt"))] ::defmt::trace!($($arg)*); #[cfg(feature = "log")] ::log::trace!($($arg)*); From 94bdfc5268e36d5d070799d17480f1692bd7e479 Mon Sep 17 00:00:00 2001 From: Sam Clercky Date: Thu, 14 Mar 2024 17:20:21 +0100 Subject: [PATCH 4/5] Better constants such that ACK's actually arrive in time --- dot15d4/src/csma/constants.rs | 3 ++- dot15d4/src/csma/mod.rs | 22 +++++++++++++------ .../src/csma/user_configurable_constants.rs | 8 +++---- 3 files changed, 21 insertions(+), 12 deletions(-) diff --git a/dot15d4/src/csma/constants.rs b/dot15d4/src/csma/constants.rs index 6b9c450..31563ba 100644 --- a/dot15d4/src/csma/constants.rs +++ b/dot15d4/src/csma/constants.rs @@ -46,7 +46,8 @@ pub const UNIT_BACKOFF_PERIOD: u32 = TURNAROUND_TIME + CCA_TIME; pub const RCCN_BASE_SLOT_DURATION: u32 = 60; /// The symbol rate of IEEE 802.15.4 on 2.5 Ghz (symbols/s) -pub const SYMBOL_RATE: u32 = 250000; +// pub const SYMBOL_RATE: u32 = 250_000; +pub const SYMBOL_RATE: u32 = 62_500; /// The symbol rate of IEEE 802.15.4 on 2.5 Ghz (µs/symbol) pub const SYMBOL_RATE_INV_US: u32 = 1_000_000 / SYMBOL_RATE; diff --git a/dot15d4/src/csma/mod.rs b/dot15d4/src/csma/mod.rs index 90394f7..551c503 100644 --- a/dot15d4/src/csma/mod.rs +++ b/dot15d4/src/csma/mod.rs @@ -265,15 +265,19 @@ where } } + /// If the frame is malformed/invalid -> parsing error will be returned. + /// If the frame is no ack'able -> the sequence number will be None. + /// Second argument in the option is the frame length -> useful to find out how long we should wait for an ACK fn set_ack_request_if_possible<'a, RadioFrame>( &self, buffer: &'a mut [u8], - ) -> Result, TransmissionTaskError> + ) -> Result, TransmissionTaskError> where RadioFrame: RadioFrameMut<&'a mut [u8]>, { let mut frame = RadioFrame::new_checked(buffer).map_err(TransmissionTaskError::InvalidDeviceFrame)?; + let frame_len = frame.data().len() as u8; let mut frame = Frame::new(frame.data_mut()).map_err(|_err| TransmissionTaskError::InvalidIEEEFrame)?; @@ -286,11 +290,11 @@ where { Some(addr) if addr.is_unicast() && self.config.ack_unicast => { frame.frame_control_mut().set_ack_request(true); - Ok(frame.sequence_number()) + Ok(frame.sequence_number().map(|seq| (seq, frame_len))) } Some(addr) if addr.is_broadcast() && self.config.ack_broadcast => { frame.frame_control_mut().set_ack_request(true); - Ok(frame.sequence_number()) + Ok(frame.sequence_number().map(|seq| (seq, frame_len))) } Some(_) | None => { // Make sure that the ack_request field is set to false independent on how the frame was actually created @@ -348,7 +352,7 @@ where // Enable ACK in frame coming from higher layers let mut sequence_number = None; match self.set_ack_request_if_possible::>(&mut tx.buffer) { - Ok(seq_number) => sequence_number = Some(seq_number).flatten(), + Ok(seq_number) => sequence_number = seq_number, Err(TransmissionTaskError::InvalidIEEEFrame) => { // Invalid IEEE frame encountered self.driver.error(driver::Error::InvalidStructure).await; @@ -384,12 +388,14 @@ where } // We now want to try and receive an ACK - if let Some(sequence_number) = sequence_number { + if let Some((sequence_number, frame_length)) = sequence_number { utils::acquire_lock(&self.radio, &wants_to_transmit_signal, &mut radio_guard) .await; let delay = ACKNOWLEDGEMENT_INTERFRAME_SPACING - + MAC_SIFT_PERIOD.max(Duration::from_us(TURNAROUND_TIME as i64)); + + (MAC_SIFT_PERIOD.max(Duration::from_us( + (TURNAROUND_TIME * SYMBOL_RATE_INV_US * frame_length as u32) as i64, + ))); match select::select( Self::wait_for_valid_ack( &mut *radio_guard.unwrap(), @@ -421,7 +427,9 @@ where radio_guard = None; // Wait for SIFS here - let delay = MAC_SIFT_PERIOD.max(Duration::from_us(TURNAROUND_TIME as i64)); + let delay = MAC_SIFT_PERIOD.max(Duration::from_us( + (TURNAROUND_TIME * SYMBOL_RATE_INV_US) as i64, + )); timer.delay_us(delay.as_us() as u32).await; // Was this the last attempt? diff --git a/dot15d4/src/csma/user_configurable_constants.rs b/dot15d4/src/csma/user_configurable_constants.rs index 003f164..9ad7edb 100644 --- a/dot15d4/src/csma/user_configurable_constants.rs +++ b/dot15d4/src/csma/user_configurable_constants.rs @@ -12,8 +12,8 @@ pub const MAC_MAX_CSMA_BACKOFFS: u16 = 16; pub const MAC_UNIT_BACKOFF_DURATION: Duration = Duration::from_us((UNIT_BACKOFF_PERIOD * SYMBOL_RATE_INV_US) as i64); pub const MAC_MAX_FRAME_RETIES: u16 = 16; // TODO: XXX -pub const _MAC_INTER_FRAME_TIME: Duration = Duration::from_us(1); // TODO: XXX +pub const _MAC_INTER_FRAME_TIME: Duration = Duration::from_us(1000); // TODO: XXX /// AIFS=1ms, for SUN PHY, LECIM PHY, TVWS PHY -pub const ACKNOWLEDGEMENT_INTERFRAME_SPACING: Duration = Duration::from_us(1); -pub const MAC_SIFT_PERIOD: Duration = Duration::from_us(1); // TODO: SIFS=XXX -pub const MAC_LIFS_PERIOD: Duration = Duration::from_us(10); // TODO: LIFS=XXX +pub const ACKNOWLEDGEMENT_INTERFRAME_SPACING: Duration = Duration::from_us(1000); +pub const MAC_SIFT_PERIOD: Duration = Duration::from_us(1000); // TODO: SIFS=XXX +pub const MAC_LIFS_PERIOD: Duration = Duration::from_us(10_000); // TODO: LIFS=XXX From f69dfcf302a85a515ba11cf775b5539bf5bca83a Mon Sep 17 00:00:00 2001 From: Sam Clercky Date: Thu, 14 Mar 2024 19:41:56 +0100 Subject: [PATCH 5/5] fix test --- dot15d4/src/csma/constants.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/dot15d4/src/csma/constants.rs b/dot15d4/src/csma/constants.rs index 31563ba..f8c6db6 100644 --- a/dot15d4/src/csma/constants.rs +++ b/dot15d4/src/csma/constants.rs @@ -57,6 +57,6 @@ mod tests { #[test] fn inv_symbol_rate() { - assert_eq!(SYMBOL_RATE_INV_US, 4); + assert_eq!(SYMBOL_RATE_INV_US, 16); } }