Remove the reason from the Block Device API.

The caller of the API should do their own logging. Also, we didn't have it on write and the asymmetry was displeasing.

examples/linux/mod.rs

@@ -34,22 +34,14 @@ impl LinuxBlockDevice {
 impl BlockDevice for LinuxBlockDevice {
     type Error = std::io::Error;
 
-    fn read(
-        &self,
-        blocks: &mut [Block],
-        start_block_idx: BlockIdx,
-        reason: &str,
-    ) -> Result<(), Self::Error> {
+    fn read(&self, blocks: &mut [Block], start_block_idx: BlockIdx) -> Result<(), Self::Error> {
         self.file
             .borrow_mut()
             .seek(SeekFrom::Start(start_block_idx.into_bytes()))?;
         for block in blocks.iter_mut() {
             self.file.borrow_mut().read_exact(&mut block.contents)?;
             if self.print_blocks {
-                println!(
-                    "Read block ({}) {:?}: {:?}",
-                    reason, start_block_idx, &block
-                );
+                println!("Read block {:?}: {:?}", start_block_idx, &block);
             }
         }
         Ok(())

src/blockdevice.rs

@@ -43,12 +43,7 @@ pub trait BlockDevice {
     /// The errors that the `BlockDevice` can return. Must be debug formattable.
     type Error: core::fmt::Debug;
     /// Read one or more blocks, starting at the given block index.
-    fn read(
-        &self,
-        blocks: &mut [Block],
-        start_block_idx: BlockIdx,
-        reason: &str,
-    ) -> Result<(), Self::Error>;
+    fn read(&self, blocks: &mut [Block], start_block_idx: BlockIdx) -> Result<(), Self::Error>;
     /// Write one or more blocks, starting at the given block index.
     fn write(&self, blocks: &[Block], start_block_idx: BlockIdx) -> Result<(), Self::Error>;
     /// Determine how many blocks this device can hold.

src/fat/mod.rs

@@ -29,15 +29,14 @@ impl BlockCache {
         &mut self,
         block_device: &D,
         block_idx: BlockIdx,
-        reason: &str,
     ) -> Result<&Block, Error<D::Error>>
     where
         D: BlockDevice,
     {
         if Some(block_idx) != self.idx {
             self.idx = Some(block_idx);
             block_device
-                .read(core::slice::from_mut(&mut self.block), block_idx, reason)
+                .read(core::slice::from_mut(&mut self.block), block_idx)
                 .map_err(Error::DeviceError)?;
         }
         Ok(&self.block)

src/fat/volume.rs

@@ -80,8 +80,9 @@ impl FatVolume {
                     return Ok(());
                 }
                 let mut blocks = [Block::new()];
+                trace!("Reading info sector");
                 block_device
-                    .read(&mut blocks, fat32_info.info_location, "read_info_sector")
+                    .read(&mut blocks, fat32_info.info_location)
                     .map_err(Error::DeviceError)?;
                 let block = &mut blocks[0];
                 if let Some(count) = self.free_clusters_count {
@@ -90,6 +91,7 @@ impl FatVolume {
                 if let Some(next_free_cluster) = self.next_free_cluster {
                     block[492..496].copy_from_slice(&next_free_cluster.0.to_le_bytes());
                 }
+                trace!("Writing info sector");
                 block_device
                     .write(&blocks, fat32_info.info_location)
                     .map_err(Error::DeviceError)?;
@@ -123,8 +125,9 @@ impl FatVolume {
                 let fat_offset = cluster.0 * 2;
                 this_fat_block_num = self.lba_start + self.fat_start.offset_bytes(fat_offset);
                 let this_fat_ent_offset = (fat_offset % Block::LEN_U32) as usize;
+                trace!("Reading FAT");
                 block_device
-                    .read(&mut blocks, this_fat_block_num, "read_fat")
+                    .read(&mut blocks, this_fat_block_num)
                     .map_err(Error::DeviceError)?;
                 // See <https://en.wikipedia.org/wiki/Design_of_the_FAT_file_system>
                 let entry = match new_value {
@@ -144,8 +147,9 @@ impl FatVolume {
                 let fat_offset = cluster.0 * 4;
                 this_fat_block_num = self.lba_start + self.fat_start.offset_bytes(fat_offset);
                 let this_fat_ent_offset = (fat_offset % Block::LEN_U32) as usize;
+                trace!("Reading FAT");
                 block_device
-                    .read(&mut blocks, this_fat_block_num, "read_fat")
+                    .read(&mut blocks, this_fat_block_num)
                     .map_err(Error::DeviceError)?;
                 let entry = match new_value {
                     ClusterId::INVALID => 0x0FFF_FFF6,
@@ -163,6 +167,7 @@ impl FatVolume {
                 );
             }
         }
+        trace!("Writing FAT");
         block_device
             .write(&blocks, this_fat_block_num)
             .map_err(Error::DeviceError)?;
@@ -187,8 +192,8 @@ impl FatVolume {
                 let fat_offset = cluster.0 * 2;
                 let this_fat_block_num = self.lba_start + self.fat_start.offset_bytes(fat_offset);
                 let this_fat_ent_offset = (fat_offset % Block::LEN_U32) as usize;
-                let block =
-                    fat_block_cache.read(block_device, this_fat_block_num, "next_cluster")?;
+                trace!("Reading FAT");
+                let block = fat_block_cache.read(block_device, this_fat_block_num)?;
                 let fat_entry =
                     LittleEndian::read_u16(&block[this_fat_ent_offset..=this_fat_ent_offset + 1]);
                 match fat_entry {
@@ -210,8 +215,8 @@ impl FatVolume {
                 let fat_offset = cluster.0 * 4;
                 let this_fat_block_num = self.lba_start + self.fat_start.offset_bytes(fat_offset);
                 let this_fat_ent_offset = (fat_offset % Block::LEN_U32) as usize;
-                let block =
-                    fat_block_cache.read(block_device, this_fat_block_num, "next_cluster")?;
+                trace!("Reading FAT");
+                let block = fat_block_cache.read(block_device, this_fat_block_num)?;
                 let fat_entry =
                     LittleEndian::read_u32(&block[this_fat_ent_offset..=this_fat_ent_offset + 3])
                         & 0x0FFF_FFFF;
@@ -310,8 +315,9 @@ impl FatVolume {
                 let mut blocks = [Block::new()];
                 while let Some(cluster) = current_cluster {
                     for block in first_dir_block_num.range(dir_size) {
+                        trace!("Reading directory");
                         block_device
-                            .read(&mut blocks, block, "read_dir")
+                            .read(&mut blocks, block)
                             .map_err(Error::DeviceError)?;
                         for (i, dir_entry_bytes) in
                             blocks[0].chunks_exact_mut(OnDiskDirEntry::LEN).enumerate()
@@ -330,6 +336,7 @@ impl FatVolume {
                                 );
                                 dir_entry_bytes
                                     .copy_from_slice(&entry.serialize(FatType::Fat16)[..]);
+                                trace!("Updating directory");
                                 block_device
                                     .write(&blocks, block)
                                     .map_err(Error::DeviceError)?;
@@ -375,8 +382,9 @@ impl FatVolume {
                     // Loop through the blocks in the cluster
                     for block in first_dir_block_num.range(dir_size) {
                         // Read a block of directory entries
+                        trace!("Reading directory");
                         block_device
-                            .read(&mut blocks, block, "read_dir")
+                            .read(&mut blocks, block)
                             .map_err(Error::DeviceError)?;
                         // Are any entries in the block we just loaded blank? If so
                         // we can use them.
@@ -397,6 +405,7 @@ impl FatVolume {
                                 );
                                 dir_entry_bytes
                                     .copy_from_slice(&entry.serialize(FatType::Fat32)[..]);
+                                trace!("Updating directory");
                                 block_device
                                     .write(&blocks, block)
                                     .map_err(Error::DeviceError)?;
@@ -481,7 +490,8 @@ impl FatVolume {
         let mut block_cache = BlockCache::empty();
         while let Some(cluster) = current_cluster {
             for block_idx in first_dir_block_num.range(dir_size) {
-                let block = block_cache.read(block_device, block_idx, "read_dir")?;
+                trace!("Reading FAT");
+                let block = block_cache.read(block_device, block_idx)?;
                 for (i, dir_entry_bytes) in block.chunks_exact(OnDiskDirEntry::LEN).enumerate() {
                     let dir_entry = OnDiskDirEntry::new(dir_entry_bytes);
                     if dir_entry.is_end() {
@@ -532,8 +542,9 @@ impl FatVolume {
         while let Some(cluster) = current_cluster {
             let block_idx = self.cluster_to_block(cluster);
             for block in block_idx.range(BlockCount(u32::from(self.blocks_per_cluster))) {
+                trace!("Reading FAT");
                 block_device
-                    .read(&mut blocks, block, "read_dir")
+                    .read(&mut blocks, block)
                     .map_err(Error::DeviceError)?;
                 for (i, dir_entry_bytes) in
                     blocks[0].chunks_exact_mut(OnDiskDirEntry::LEN).enumerate()
@@ -658,8 +669,9 @@ impl FatVolume {
         D: BlockDevice,
     {
         let mut blocks = [Block::new()];
+        trace!("Reading directory");
         block_device
-            .read(&mut blocks, block, "read_dir")
+            .read(&mut blocks, block)
             .map_err(Error::DeviceError)?;
         for (i, dir_entry_bytes) in blocks[0].chunks_exact_mut(OnDiskDirEntry::LEN).enumerate() {
             let dir_entry = OnDiskDirEntry::new(dir_entry_bytes);
@@ -792,8 +804,9 @@ impl FatVolume {
         D: BlockDevice,
     {
         let mut blocks = [Block::new()];
+        trace!("Reading directory");
         block_device
-            .read(&mut blocks, block, "read_dir")
+            .read(&mut blocks, block)
             .map_err(Error::DeviceError)?;
         for (i, dir_entry_bytes) in blocks[0].chunks_exact_mut(OnDiskDirEntry::LEN).enumerate() {
             let dir_entry = OnDiskDirEntry::new(dir_entry_bytes);
@@ -805,6 +818,7 @@ impl FatVolume {
                 let start = i * OnDiskDirEntry::LEN;
                 // set first byte to the 'unused' marker
                 blocks[0].contents[start] = 0xE5;
+                trace!("Updating directory");
                 return block_device
                     .write(&blocks, block)
                     .map_err(Error::DeviceError);
@@ -842,7 +856,7 @@ impl FatVolume {
                         .map_err(|_| Error::ConversionError)?;
                     trace!("Reading block {:?}", this_fat_block_num);
                     block_device
-                        .read(&mut blocks, this_fat_block_num, "next_cluster")
+                        .read(&mut blocks, this_fat_block_num)
                         .map_err(Error::DeviceError)?;
 
                     while this_fat_ent_offset <= Block::LEN - 2 {
@@ -873,7 +887,7 @@ impl FatVolume {
                         .map_err(|_| Error::ConversionError)?;
                     trace!("Reading block {:?}", this_fat_block_num);
                     block_device
-                        .read(&mut blocks, this_fat_block_num, "next_cluster")
+                        .read(&mut blocks, this_fat_block_num)
                         .map_err(Error::DeviceError)?;
 
                     while this_fat_ent_offset <= Block::LEN - 4 {
@@ -969,6 +983,7 @@ impl FatVolume {
             let first_block = self.cluster_to_block(new_cluster);
             let num_blocks = BlockCount(u32::from(self.blocks_per_cluster));
             for block in first_block.range(num_blocks) {
+                trace!("Zeroing cluster");
                 block_device
                     .write(&blocks, block)
                     .map_err(Error::DeviceError)?;
@@ -1041,14 +1056,16 @@ impl FatVolume {
             FatSpecificInfo::Fat32(_) => FatType::Fat32,
         };
         let mut blocks = [Block::new()];
+        trace!("Reading directory for update");
         block_device
-            .read(&mut blocks, entry.entry_block, "read")
+            .read(&mut blocks, entry.entry_block)
             .map_err(Error::DeviceError)?;
         let block = &mut blocks[0];
 
         let start = usize::try_from(entry.entry_offset).map_err(|_| Error::ConversionError)?;
         block[start..start + 32].copy_from_slice(&entry.serialize(fat_type)[..]);
 
+        trace!("Updating directory");
         block_device
             .write(&blocks, entry.entry_block)
             .map_err(Error::DeviceError)?;
@@ -1068,8 +1085,9 @@ where
     D::Error: core::fmt::Debug,
 {
     let mut blocks = [Block::new()];
+    trace!("Reading BPB");
     block_device
-        .read(&mut blocks, lba_start, "read_bpb")
+        .read(&mut blocks, lba_start)
         .map_err(Error::DeviceError)?;
     let block = &blocks[0];
     let bpb = Bpb::create_from_bytes(block).map_err(Error::FormatError)?;
@@ -1112,12 +1130,9 @@ where
             // Safe to unwrap since this is a Fat32 Type
             let info_location = bpb.fs_info_block().unwrap();
             let mut info_blocks = [Block::new()];
+            trace!("Reading info block");
             block_device
-                .read(
-                    &mut info_blocks,
-                    lba_start + info_location,
-                    "read_info_sector",
-                )
+                .read(&mut info_blocks, lba_start + info_location)
                 .map_err(Error::DeviceError)?;
             let info_block = &info_blocks[0];
             let info_sector =

src/sdcard/mod.rs

@@ -162,19 +162,9 @@ where
     /// Read one or more blocks, starting at the given block index.
     ///
     /// This will trigger card (re-)initialisation.
-    fn read(
-        &self,
-        blocks: &mut [Block],
-        start_block_idx: BlockIdx,
-        _reason: &str,
-    ) -> Result<(), Self::Error> {
+    fn read(&self, blocks: &mut [Block], start_block_idx: BlockIdx) -> Result<(), Self::Error> {
         let mut inner = self.inner.borrow_mut();
-        debug!(
-            "Read {} blocks @ {} for {}",
-            blocks.len(),
-            start_block_idx.0,
-            _reason
-        );
+        debug!("Read {} blocks @ {}", blocks.len(), start_block_idx.0,);
         inner.check_init()?;
         inner.read(blocks, start_block_idx)
     }

src/volume_mgr.rs

@@ -2,21 +2,22 @@
 //!
 //! The volume manager handles partitions and open files on a block device.
 
-use byteorder::{ByteOrder, LittleEndian};
 use core::convert::TryFrom;
 
+use byteorder::{ByteOrder, LittleEndian};
+use heapless::Vec;
+
 use crate::fat::{self, BlockCache, FatType, OnDiskDirEntry, RESERVED_ENTRIES};
 
 use crate::filesystem::{
     Attributes, ClusterId, DirEntry, DirectoryInfo, FileInfo, Mode, RawDirectory, RawFile,
     SearchIdGenerator, TimeSource, ToShortFileName, MAX_FILE_SIZE,
 };
 use crate::{
-    debug, Block, BlockCount, BlockDevice, BlockIdx, Error, RawVolume, ShortFileName, Volume,
-    VolumeIdx, VolumeInfo, VolumeType, PARTITION_ID_FAT16, PARTITION_ID_FAT16_LBA,
+    debug, trace, Block, BlockCount, BlockDevice, BlockIdx, Error, RawVolume, ShortFileName,
+    Volume, VolumeIdx, VolumeInfo, VolumeType, PARTITION_ID_FAT16, PARTITION_ID_FAT16_LBA,
     PARTITION_ID_FAT32_CHS_LBA, PARTITION_ID_FAT32_LBA,
 };
-use heapless::Vec;
 
 /// Wraps a block device and gives access to the FAT-formatted volumes within
 /// it.
@@ -142,8 +143,9 @@ where
 
         let (part_type, lba_start, num_blocks) = {
             let mut blocks = [Block::new()];
+            trace!("Reading partition table");
             self.block_device
-                .read(&mut blocks, BlockIdx(0), "read_mbr")
+                .read(&mut blocks, BlockIdx(0))
                 .map_err(Error::DeviceError)?;
             let block = &blocks[0];
             // We only support Master Boot Record (MBR) partitioned cards, not
@@ -632,8 +634,9 @@ where
             )?;
             self.open_files[file_idx].current_cluster = current_cluster;
             let mut blocks = [Block::new()];
+            trace!("Reading file ID {:?}", file);
             self.block_device
-                .read(&mut blocks, block_idx, "read")
+                .read(&mut blocks, block_idx)
                 .map_err(Error::DeviceError)?;
             let block = &blocks[0];
             let to_copy = block_avail
@@ -747,9 +750,9 @@ where
             let mut blocks = [Block::new()];
             let to_copy = core::cmp::min(block_avail, bytes_to_write - written);
             if block_offset != 0 {
-                debug!("Partial block write");
+                debug!("Reading for partial block write");
                 self.block_device
-                    .read(&mut blocks, block_idx, "read")
+                    .read(&mut blocks, block_idx)
                     .map_err(Error::DeviceError)?;
             }
             let block = &mut blocks[0];
@@ -1157,12 +1160,7 @@ mod tests {
         type Error = Error;
 
         /// Read one or more blocks, starting at the given block index.
-        fn read(
-            &self,
-            blocks: &mut [Block],
-            start_block_idx: BlockIdx,
-            _reason: &str,
-        ) -> Result<(), Self::Error> {
+        fn read(&self, blocks: &mut [Block], start_block_idx: BlockIdx) -> Result<(), Self::Error> {
             // Actual blocks taken from an SD card, except I've changed the start and length of partition 0.
             static BLOCKS: [Block; 3] = [
                 Block {

tests/utils/mod.rs

@@ -85,12 +85,7 @@ where
 {
     type Error = Error;
 
-    fn read(
-        &self,
-        blocks: &mut [Block],
-        start_block_idx: BlockIdx,
-        _reason: &str,
-    ) -> Result<(), Self::Error> {
+    fn read(&self, blocks: &mut [Block], start_block_idx: BlockIdx) -> Result<(), Self::Error> {
         let borrow = self.contents.borrow();
         let contents: &[u8] = borrow.as_ref();
         let mut block_idx = start_block_idx;