Implement review suggestions

Co-authored-by: Brad Warren <bmw@users.noreply.github.com>

lib/src/diag/diaglog/ll1.rs

@@ -1,11 +1,11 @@
 use deku::prelude::*;
 
 #[derive(Debug, Clone, PartialEq, DekuRead, DekuWrite)]
-#[deku(bit_order = "lsb", ctx = "_: deku::ctx::Order")]
+#[deku(bit_order = "lsb")]
 pub struct ServingCellTiming {
     #[deku(assert_eq = "1")]
     pub version: u8,
-    #[deku(bits = 5, assert = "*num_records <= 20")]
+    #[deku(bits = 5, assert = "(1..=20).contains(num_records)")]
     pub num_records: u8,
     #[deku(bits = 4, assert = "*starting_sub_fn <= 9")]
     pub starting_sub_fn: u8,
@@ -25,7 +25,7 @@ pub struct ServingCellTiming {
     pub starting_ul_frame_timing_offs: u32, // in Ts units
     #[deku(bits = 11, pad_bits_after = "2")]
     pub starting_ul_timing_advance: u16, // in 16 Ts units
-    #[deku(count = "*num_records")]
+    #[deku(count = "num_records")]
     pub timing_adjustment: Vec<TimingAdjustment>,
 }
 
@@ -39,6 +39,6 @@ pub struct TimingAdjustment {
     pub dl_frame_timing_adjustment: i16, // in Ts units
     #[deku(bits = 5, assert = "(-16..=15).contains(ul_frame_timing_adjustment)")]
     pub ul_frame_timing_adjustment: i8, // in Ts units
-    #[deku(bits = 8, assert = "(-128..=127).contains(timing_advance)")]
+    #[deku(assert = "(-128..=127).contains(timing_advance)")]
     pub timing_advance: i8, // in 16 Ts units
 }

lib/src/diag/diaglog/mac.rs

@@ -1,5 +1,5 @@
 //! Diag MAC RACH serialization/deserialization. As with most of our diag
-//! parsers, these structs were derived SCAT:
+//! parsers, these structs were derived from SCAT:
 //! https://github.com/fgsect/scat/blob/9763cb5b1dcd5ee980f5b0ead9a8d520c8c51a51/src/scat/parsers/qualcomm/diagltelogparser.py#L853
 
 use deku::prelude::*;
@@ -9,7 +9,7 @@ pub struct Packet {
     #[deku(assert_eq = "1")]
     pub version: u8,
     pub num_subpackets: u8,
-    #[deku(pad_bytes_before = "2", count = "*num_subpackets")]
+    #[deku(pad_bytes_before = "2", count = "num_subpackets")]
     pub subpackets: Vec<Subpacket>,
 }
 
@@ -44,11 +44,13 @@ pub mod rach {
     pub struct Attempt {
         #[deku(ctx = "version")]
         pub header: AttemptHeader,
+        // since we may not have meaningful msg fields, they're marked private and the get_msg
+        // functions should be used instead
         #[deku(ctx = "version")]
-        pub msg1: Msg1,
+        msg1: Msg1,
-        pub msg2: Msg2,
+        msg2: Msg2,
         #[deku(ctx = "version")]
-        pub msg3: Msg3,
+        msg3: Msg3,
         #[deku(cond = "version == 0x31 || version == 0x32")]
         pub additional_info: Option<AdditionalInfo>,
     }
@@ -143,27 +145,33 @@ pub mod rach {
 
     impl Msg3 {
         pub fn get_grant(&self) -> u32 {
-            match &self.grant {
+            match self.grant {
-                Msg3Grant::V1 { grant } => *grant & 0xfffff,
+                Msg3Grant::V1 { grant } => grant,
-                Msg3Grant::V32 { grant } => *grant & 0xfffff,
+                Msg3Grant::V32 { grant } => grant,
-            }
+            }   
-        }
+        }   
-    }
+    }   
 
     #[derive(DekuRead, DekuWrite, Debug, Clone, PartialEq)]
     #[deku(ctx = "version: u8", id = "version")]
     pub enum Msg3Grant {
         #[deku(id_pat = "0..0x32")]
         V1 {
-            #[deku(endian = "little")]
+            #[deku(endian = "little", map = "Msg3Grant::map_grant")]
             grant: u32,
-        },
+        },  
         #[deku(id_pat = "0x32..")]
         V32 {
-            #[deku(endian = "big")]
+            #[deku(endian = "big", map = "Msg3Grant::map_grant")]
             grant: u32,
-        },
+        },  
-    }
+    }   
+
+    impl Msg3Grant {
+        fn map_grant(grant: u32) -> Result<u32, DekuError> {
+            Ok(grant & 0xfffff)
+        }   
+    }   
 
     #[derive(DekuRead, DekuWrite, Debug, Clone, PartialEq)]
     #[deku(ctx = "version: u8", id = "version")]

lib/src/diag/diaglog/ml1.rs

@@ -1,5 +1,5 @@
 //! Diag ML1 measurement log serialization/deserialization. As with most of our
-//! diag parsers, these structs were derived SCAT:
+//! diag parsers, these structs were derived from SCAT:
 //! Neighbor cell measurements: https://github.com/fgsect/scat/blob/9763cb5b1dcd5ee980f5b0ead9a8d520c8c51a51/src/scat/parsers/qualcomm/diagltelogparser.py#L192
 //! Serving cell measurements: https://github.com/fgsect/scat/blob/9763cb5b1dcd5ee980f5b0ead9a8d520c8c51a51/src/scat/parsers/qualcomm/diagltelogparser.py#L114
 
@@ -27,7 +27,8 @@ pub mod serving_cell {
         pub header: MeasurementAndEvaluationHeader,
         #[deku(bits = 12, pad_bits_after = "20")]
         meas_rsrp: u16,
-        avg_rsrp: u32,
+        #[deku(bits = 12, pad_bits_after = "20")]
+        avg_rsrp: u16,
         #[deku(bits = 10, pad_bits_after = "22")]
         meas_rsrq: u16,
         #[deku(pad_bits_before = "10", bits = 11, pad_bits_after = "11")]
@@ -170,7 +171,7 @@ pub mod neighbor_cells {
         #[deku(bits = 10, pad_bits_after = "10")]
         avg_rsrq: u16,
         #[deku(bits = 6, pad_bits_after = "6")]
-        s_rxlev: u16,
+        s_rxlev: u8,
         n_freq_offset: u16,
         val5: u16,
         ant0_offset: u32,

lib/src/diag/diaglog/mod.rs

@@ -94,7 +94,6 @@ pub enum LogBody {
     LteMacUl { packet: mac::Packet },
     #[deku(id = "0xb114")]
     LteLl1ServingCellTiming {
-        #[deku(ctx = "deku::ctx::Order::Lsb0")]
         data: ll1::ServingCellTiming,
     },
 }

lib/src/gsmtap/mac.rs

@@ -48,7 +48,7 @@ pub enum RntiType {
     #[deku(id = "3")]
     C,
     #[deku(id = "4")]
-    Ri,
+    Si,
     #[deku(id = "5")]
     Sps,
     #[deku(id = "6")]
@@ -106,13 +106,13 @@ pub fn mac_subpacket_to_gsmtap(
                 ETRAPIDSubheader {
                     extended: false,
                     type_field: true,
-                    rapid: msg1.get_preamble_index(),
+                    rapid: msg1.get_preamble_index() & 0b111111
                 }
                 .to_bytes()?,
             );
             payload.extend(
                 RACHResponse {
-                    tac: msg2.ta,
+                    tac: msg2.ta & 0b11111111111,
                     ul_grant: msg3.get_grant(),
                     tc_rnti: msg2.tc_rnti,
                 }