Merge pull request #17 from SamClercky/csma-ca-fixes

CSMA: fixes

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;
 
@@ -56,6 +57,6 @@ mod tests {
 
     #[test]
     fn inv_symbol_rate() {
-        assert_eq!(SYMBOL_RATE_INV_US, 4);
+        assert_eq!(SYMBOL_RATE_INV_US, 16);
     }
 }

dot15d4/src/csma/mod.rs

@@ -259,29 +259,45 @@ 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<Option<u8>, TransmissionTaskError<RadioFrame::Error>>
+    ) -> Result<Option<(u8, u8)>, TransmissionTaskError<RadioFrame::Error>>
     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)?;
-        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()) {
                 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().map(|seq| (seq, frame_len)))
                 }
                 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().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
+                    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]) {
@@ -327,7 +343,7 @@ where
             // Enable ACK in frame coming from higher layers
             let mut sequence_number = None;
             match self.set_ack_request_if_possible::<R::RadioFrame<_>>(&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;
@@ -363,12 +379,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(),
@@ -400,7 +418,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?
@@ -934,4 +954,4 @@ pub mod tests {
         })
         .await;
     }
-}
+}

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

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