refactor: Clean Up and Simplify

Added the block skip into the plan which allowed for a great
simplification in many places.

Author: JamesWiresmith

src/io/data_types/mod.rs

@@ -119,7 +119,7 @@ impl Display for DataType {
 
 type StorageResult<T> = std::result::Result<T, TdmsError>;
 
-pub trait TdmsStorageType: Sized {
+pub trait TdmsStorageType: Sized + 'static {
     /// The [`DataType`] that can be read as this storage type.
     const SUPPORTED_TYPES: &'static [DataType];
     /// The [`DataType`] that this storage type is naturally written as.

src/raw_data/contigious_multi_channel_read.rs

@@ -11,17 +11,17 @@ use std::{
     marker::PhantomData,
 };
 
-/// The multichannel contigious reader will read from an contigous block.
+/// The multichannel contiguous reader will read from a contiguous block.
 ///
 /// We will assume a single datatype as it is unclear if multiple types exist in the wild.
-pub struct MultiChannelContigousReader<R: Read + Seek, T: TdmsReader<R>> {
+pub struct MultiChannelContiguousReader<R: Read + Seek, T: TdmsReader<R>> {
     reader: T,
     _marker: PhantomData<R>,
     block_size: NonZeroU64,
     block_start: u64,
 }
 
-impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
+impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContiguousReader<R, T> {
     pub fn new(reader: T, block_start: u64, block_size: NonZeroU64) -> Self {
         Self {
             reader,
@@ -75,10 +75,28 @@ impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
                 self.reader.move_position(skip_bytes)?;
             } else if sub_block_idx == sub_blocks_to_skip {
                 // First sub-block to read - apply remainder skip
-                length += self.read_sub_block_with_offset(&mut channels, remainder_skip)?;
+                for channel in channels.read_instructions() {
+                    if let RecordEntryPlan::Read {
+                        block_skip: skip_first_samples,
+                        ..
+                    } = &mut channel.plan
+                    {
+                        *skip_first_samples = remainder_skip;
+                    }
+                }
+                length += self.read_sub_block(&mut channels)?;
             } else {
+                for channel in channels.read_instructions() {
+                    if let RecordEntryPlan::Read {
+                        block_skip: skip_first_samples,
+                        ..
+                    } = &mut channel.plan
+                    {
+                        *skip_first_samples = 0;
+                    }
+                }
                 // Subsequent sub-blocks - no skip
-                length += self.read_sub_block_with_offset(&mut channels, 0)?;
+                length += self.read_sub_block(&mut channels)?;
             }
         }
 
@@ -109,7 +127,6 @@ impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
             .collect();
 
         let mut length = 0;
-        let mut sub_block_skips = Vec::with_capacity(skip_amounts.len());
 
         for sub_block_idx in 0..total_sub_blocks {
             // Check if any channel needs to read from this sub-block
@@ -121,40 +138,69 @@ impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
                 self.reader.move_position(skip_bytes)?;
             } else {
                 // Build skip amounts for this sub-block
-                sub_block_skips.clear();
-                for (idx, &blocks_to_skip) in sub_blocks_to_skip.iter().enumerate() {
-                    if sub_block_idx == blocks_to_skip {
-                        // First sub-block to read for this channel - use remainder
-                        sub_block_skips.push(remainder_skips[idx]);
-                    } else if sub_block_idx > blocks_to_skip {
-                        // Subsequent sub-blocks - no skip
-                        sub_block_skips.push(0);
-                    } else {
-                        // Should not happen if any_channel_reads is correct
-                        sub_block_skips.push(0);
-                    }
+                let channel_skip_values =
+                    channels
+                        .read_instructions()
+                        .iter_mut()
+                        .filter_map(|instruction| {
+                            if let RecordEntryPlan::Read {
+                                block_skip: skip_first_samples,
+                                ..
+                            } = &mut instruction.plan
+                            {
+                                Some(skip_first_samples)
+                            } else {
+                                None
+                            }
+                        });
+                for ((blocks_to_skip, remainder_skip), sub_block_skip) in sub_blocks_to_skip
+                    .iter()
+                    .zip(remainder_skips.iter())
+                    .zip(channel_skip_values)
+                {
+                    *sub_block_skip = Self::calculate_skip_for_this_block(
+                        sub_block_idx,
+                        *blocks_to_skip,
+                        *remainder_skip,
+                    );
                 }
-                length +=
-                    self.read_sub_block_with_per_channel_skip(&mut channels, &sub_block_skips)?;
+                length += self.read_sub_block(&mut channels)?;
             }
         }
 
         Ok(length)
     }
 
-    fn read_sub_block_with_per_channel_skip<D: TdmsStorageType>(
+    fn calculate_skip_for_this_block(
+        sub_block_idx: u64,
+        blocks_to_skip: u64,
+        remainder_skip: u64,
+    ) -> u64 {
+        if sub_block_idx == blocks_to_skip {
+            // First sub-block to read for this channel - use remainder
+            remainder_skip
+        } else if sub_block_idx > blocks_to_skip {
+            // Subsequent sub-blocks - no skip
+            0
+        } else {
+            // Should not happen if any_channel_reads is correct
+            0
+        }
+    }
+
+    fn read_sub_block<D: TdmsStorageType>(
         &mut self,
         channels: &mut RecordPlan<'_, D>,
-        skip_amounts: &[u64],
     ) -> Result<usize, TdmsError> {
         let mut length = 0;
-        let mut skip_idx = 0;
 
         for read_instruction in channels.read_instructions().iter_mut() {
             match &mut read_instruction.plan {
-                RecordEntryPlan::Read(output) => {
-                    let skip = skip_amounts[skip_idx].min(read_instruction.length as u64) as usize;
-                    skip_idx += 1;
+                RecordEntryPlan::Read {
+                    output,
+                    block_skip: skip_first_samples,
+                } => {
+                    let skip = (*skip_first_samples).min(read_instruction.length as u64) as usize;
 
                     let samples_to_read = read_instruction.length.saturating_sub(skip);
 
@@ -165,12 +211,7 @@ impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
                     }
 
                     // Read the remaining samples
-                    for _ in 0..samples_to_read {
-                        let read_value = self.reader.read_value()?;
-                        if let Some(value) = output.next() {
-                            *value = read_value;
-                        }
-                    }
+                    self.read_sequential_samples(output, samples_to_read)?;
                     length = samples_to_read;
                 }
                 RecordEntryPlan::Skip(bytes) => {
@@ -183,42 +224,18 @@ impl<R: Read + Seek, T: TdmsReader<R>> MultiChannelContigousReader<R, T> {
         Ok(length)
     }
 
-    fn read_sub_block_with_offset<D: TdmsStorageType>(
+    fn read_sequential_samples<'a, D: TdmsStorageType, I: Iterator<Item = &'a mut D>>(
         &mut self,
-        channels: &mut RecordPlan<'_, D>,
-        start_sample: u64,
-    ) -> Result<usize, TdmsError> {
-        let mut length = 0;
-        for read_instruction in channels.read_instructions().iter_mut() {
-            match &mut read_instruction.plan {
-                RecordEntryPlan::Read(output) => {
-                    // Skip the specified number of samples at the start
-                    let samples_to_skip = start_sample.min(read_instruction.length as u64) as usize;
-                    let samples_to_read = read_instruction.length.saturating_sub(samples_to_skip);
-
-                    // Skip samples by seeking
-                    if samples_to_skip > 0 {
-                        let skip_bytes = samples_to_skip as i64 * D::SIZE_BYTES as i64;
-                        self.reader.move_position(skip_bytes)?;
-                    }
-
-                    // Read the remaining samples
-                    for _ in 0..samples_to_read {
-                        let read_value = self.reader.read_value()?;
-                        if let Some(value) = output.next() {
-                            *value = read_value;
-                        }
-                    }
-                    length = samples_to_read;
-                }
-                RecordEntryPlan::Skip(bytes) => {
-                    let skip_bytes = *bytes * read_instruction.length as i64;
-                    self.reader.move_position(skip_bytes)?;
-                }
-            };
+        output: &mut I,
+        samples_to_read: usize,
+    ) -> Result<(), TdmsError> {
+        for _ in 0..samples_to_read {
+            let read_value = self.reader.read_value()?;
+            if let Some(value) = output.next() {
+                *value = read_value;
+            }
         }
-
-        Ok(length)
+        Ok(())
     }
 }
 
@@ -259,7 +276,7 @@ mod tests {
         let mut buffer = create_test_buffer();
         let meta = create_test_meta_data(2);
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),
@@ -277,7 +294,7 @@ mod tests {
         let meta = create_test_meta_data(4);
         let length = meta.first().unwrap().number_of_values as f64;
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),
@@ -312,7 +329,7 @@ mod tests {
         // ch3: 4, 5, 12, 13
         // ch4: 6, 7, 14, 15
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),
@@ -333,7 +350,7 @@ mod tests {
         let meta = create_test_meta_data(4);
         let length = meta.first().unwrap().number_of_values as f64;
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),
@@ -356,7 +373,7 @@ mod tests {
         let meta = create_test_meta_data(4);
         let length = meta.first().unwrap().number_of_values as f64;
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),
@@ -391,7 +408,7 @@ mod tests {
             channel.number_of_values = 3;
         }
 
-        let mut reader = MultiChannelContigousReader::<_, _>::new(
+        let mut reader = MultiChannelContiguousReader::<_, _>::new(
             BigEndianReader::from_reader(&mut buffer),
             0,
             800.try_into().unwrap(),