From f880b51c1b9069ea791b20985d8464f70d90e503 Mon Sep 17 00:00:00 2001 From: Michael de Silva Date: Sun, 25 Feb 2024 14:35:28 +0530 Subject: [PATCH 01/17] Upgrade to heapless ^0.8 --- CHANGELOG.md | 10 ++++++---- Cargo.toml | 2 +- 2 files changed, 7 insertions(+), 5 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index edb029f..1c85c56 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -6,11 +6,13 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic ## [Unreleased] -* None +### Changed + +- Updated to `heapless` ^0.8 ## [Version 0.7.0] - 2024-02-04 -## Changed +### Changed - __Breaking Change__: `Volume`, `Directory` and `File` are now smart! They hold references to the thing they were made from, and will clean themselves up when dropped. The trade-off is you can can't open multiple volumes, directories or files at the same time. - __Breaking Change__: Renamed the old types to `RawVolume`, `RawDirectory` and `RawFile` @@ -19,7 +21,7 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic - You can now open directories multiple times without error - Updated to [embedded-hal] 1.0 -## Added +### Added - `RawVolume`, `RawDirectory` and `RawFile` types (like the old `Volume`, `Directory` and `File` types) - New method `make_dir_in_dir` @@ -27,7 +29,7 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic - New API `change_dir` which changes a directory to point to some child directory (or the parent) without opening a new directory. - Updated 'shell' example to support `mkdir`, `tree` and relative/absolute paths -## Removed +### Removed * None diff --git a/Cargo.toml b/Cargo.toml index f64a194..bd6549c 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -14,7 +14,7 @@ version = "0.7.0" byteorder = {version = "1", default-features = false} defmt = {version = "0.3", optional = true} embedded-hal = "1.0.0" -heapless = "0.7" +heapless = "^0.8" log = {version = "0.4", default-features = false, optional = true} [dev-dependencies] From 3cdb6cb3b5dfab590739de5682c4cbab9cd3c5ee Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Mon, 25 Mar 2024 03:27:53 +0100 Subject: [PATCH 02/17] Add ability to flush written data --- src/filesystem/files.rs | 7 ++++- src/volume_mgr.rs | 66 ++++++++++++++++++++++++----------------- tests/write_file.rs | 47 +++++++++++++++++++++++++++++ 3 files changed, 92 insertions(+), 28 deletions(-) diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index 31fc966..a06b2e7 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -1,6 +1,6 @@ use crate::{ filesystem::{ClusterId, DirEntry, SearchId}, - RawVolume, VolumeManager, + Error, RawVolume, VolumeManager, }; /// Represents an open file on disk. @@ -123,6 +123,11 @@ where core::mem::forget(self); f } + + /// Flush any written data by updating the directory entry. + pub fn flush(&mut self) -> Result<(), Error> { + self.volume_mgr.flush_file(self.raw_file) + } } impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index 54d1a5f..43c2dcb 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -773,38 +773,18 @@ where Ok(()) } - /// Close a file with the given full path. + /// Close a file with the given raw file handle. pub fn close_file(&mut self, file: RawFile) -> Result<(), Error> { - let mut found_idx = None; - for (idx, info) in self.open_files.iter().enumerate() { - if file == info.file_id { - found_idx = Some((info, idx)); - break; - } - } - - let (file_info, file_idx) = found_idx.ok_or(Error::BadHandle)?; - - if file_info.dirty { - let volume_idx = self.get_volume_by_id(file_info.volume_id)?; - match self.open_volumes[volume_idx].volume_type { - VolumeType::Fat(ref mut fat) => { - debug!("Updating FAT info sector"); - fat.update_info_sector(&self.block_device)?; - debug!("Updating dir entry {:?}", file_info.entry); - if file_info.entry.size != 0 { - // If you have a length, you must have a cluster - assert!(file_info.entry.cluster.0 != 0); - } - fat.write_entry_to_disk(&self.block_device, &file_info.entry)?; - } - }; - } - + let file_idx = self.flush_file_get_index(file)?; self.open_files.swap_remove(file_idx); Ok(()) } + /// Flush (update the entry) for a file with the given raw file handle. + pub fn flush_file(&mut self, file: RawFile) -> Result<(), Error> { + self.flush_file_get_index(file).map(|_| ()) + } + /// Check if any files or folders are open. pub fn has_open_handles(&self) -> bool { !(self.open_dirs.is_empty() || self.open_files.is_empty()) @@ -1074,6 +1054,38 @@ where let available = Block::LEN - block_offset; Ok((block_idx, block_offset, available)) } + + /// Flush (update the entry) for a file with the given raw file handle and + /// get its index. + fn flush_file_get_index(&mut self, file: RawFile) -> Result> { + let mut found_idx = None; + for (idx, info) in self.open_files.iter().enumerate() { + if file == info.file_id { + found_idx = Some((info, idx)); + break; + } + } + + let (file_info, file_idx) = found_idx.ok_or(Error::BadHandle)?; + + if file_info.dirty { + let volume_idx = self.get_volume_by_id(file_info.volume_id)?; + match self.open_volumes[volume_idx].volume_type { + VolumeType::Fat(ref mut fat) => { + debug!("Updating FAT info sector"); + fat.update_info_sector(&self.block_device)?; + debug!("Updating dir entry {:?}", file_info.entry); + if file_info.entry.size != 0 { + // If you have a length, you must have a cluster + assert!(file_info.entry.cluster.0 != 0); + } + fat.write_entry_to_disk(&self.block_device, &file_info.entry)?; + } + }; + } + + Ok(file_idx) + } } /// Transform mode variants (ReadWriteCreate_Or_Append) to simple modes ReadWriteAppend or diff --git a/tests/write_file.rs b/tests/write_file.rs index 5fc91c6..97e7bcd 100644 --- a/tests/write_file.rs +++ b/tests/write_file.rs @@ -55,6 +55,53 @@ fn append_file() { volume_mgr.close_volume(volume).expect("close volume"); } +#[test] +fn flush_file() { + let time_source = utils::make_time_source(); + let disk = utils::make_block_device(utils::DISK_SOURCE).unwrap(); + let mut volume_mgr: VolumeManager>, utils::TestTimeSource, 4, 2, 1> = + VolumeManager::new_with_limits(disk, time_source, 0xAA00_0000); + let volume = volume_mgr + .open_raw_volume(VolumeIdx(0)) + .expect("open volume"); + let root_dir = volume_mgr.open_root_dir(volume).expect("open root dir"); + + // Open with string + let f = volume_mgr + .open_file_in_dir(root_dir, "README.TXT", Mode::ReadWriteTruncate) + .expect("open file"); + + // Write some data to the file + let test_data = vec![0xCC; 64]; + volume_mgr.write(f, &test_data).expect("file write"); + + // Check that the file length is zero in the directory entry, as we haven't + // flushed yet + let entry = volume_mgr + .find_directory_entry(root_dir, "README.TXT") + .expect("find entry"); + assert_eq!(entry.size, 0); + + volume_mgr.flush_file(f).expect("flush"); + + // Now check the file length again after flushing + let entry = volume_mgr + .find_directory_entry(root_dir, "README.TXT") + .expect("find entry"); + assert_eq!(entry.size, 64); + + // Flush more writes + volume_mgr.write(f, &test_data).expect("file write"); + volume_mgr.write(f, &test_data).expect("file write"); + volume_mgr.flush_file(f).expect("flush"); + + // Now check the file length again, again + let entry = volume_mgr + .find_directory_entry(root_dir, "README.TXT") + .expect("find entry"); + assert_eq!(entry.size, 64 * 3); +} + // **************************************************************************** // // End Of File From 86c20e042385428c299b3eaf6f703293245e9dc1 Mon Sep 17 00:00:00 2001 From: Peter Krull Date: Fri, 29 Mar 2024 19:22:15 +0100 Subject: [PATCH 03/17] Do not panic on errors during drop, add optional manual closing to non-raw --- src/filesystem/directory.rs | 23 ++++++++++---- src/filesystem/files.rs | 23 ++++++++++---- src/lib.rs | 23 ++++++++++---- src/volume_mgr.rs | 60 ++++++++++++++++--------------------- 4 files changed, 76 insertions(+), 53 deletions(-) diff --git a/src/filesystem/directory.rs b/src/filesystem/directory.rs index f0a7bb0..0ed53e8 100644 --- a/src/filesystem/directory.rs +++ b/src/filesystem/directory.rs @@ -72,9 +72,12 @@ impl RawDirectory { /// Represents an open directory on disk. /// -/// If you drop a value of this type, it closes the directory automatically. However, -/// it holds a mutable reference to its parent `VolumeManager`, which restricts -/// which operations you can perform. +/// In contrast to a `RawDirectory`, a `Directory` holds a mutable reference to +/// its parent `VolumeManager`, which restricts which operations you can perform. +/// +/// If you drop a value of this type, it closes the directory automatically, but +/// any error that may occur will be ignored. To handle potential errors, use +/// the [`Directory::close`] method. pub struct Directory< 'a, D, @@ -188,6 +191,16 @@ where core::mem::forget(self); d } + + /// Consume the `Directory` handle and close it. The behavior of this is similar + /// to using [`core::mem::drop`] or letting the `Directory` go out of scope, + /// except this lets the user handle any errors that may occur in the process, + /// whereas when using drop, any errors will be discarded silently. + pub fn close(self) -> Result<(), Error> { + let result = self.volume_mgr.close_dir(self.raw_directory); + core::mem::forget(self); + result + } } impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop @@ -197,9 +210,7 @@ where T: crate::TimeSource, { fn drop(&mut self) { - self.volume_mgr - .close_dir(self.raw_directory) - .expect("Failed to close directory"); + _ = self.volume_mgr.close_dir(self.raw_directory) } } diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index a06b2e7..08a50e8 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -39,9 +39,12 @@ impl RawFile { /// Represents an open file on disk. /// -/// If you drop a value of this type, it closes the file automatically. However, -/// it holds a mutable reference to its parent `VolumeManager`, which restricts -/// which operations you can perform. +/// In contrast to a `RawFile`, a `File` holds a mutable reference to its +/// parent `VolumeManager`, which restricts which operations you can perform. +/// +/// If you drop a value of this type, it closes the file automatically, and but +/// error that may occur will be ignored. To handle potential errors, use +/// the [`File::close`] method. pub struct File<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> where D: crate::BlockDevice, @@ -128,6 +131,16 @@ where pub fn flush(&mut self) -> Result<(), Error> { self.volume_mgr.flush_file(self.raw_file) } + + /// Consume the `File` handle and close it. The behavior of this is similar + /// to using [`core::mem::drop`] or letting the `File` go out of scope, + /// except this lets the user handle any errors that may occur in the process, + /// whereas when using drop, any errors will be discarded silently. + pub fn close(self) -> Result<(), Error> { + let result = self.volume_mgr.close_file(self.raw_file); + core::mem::forget(self); + result + } } impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop @@ -137,9 +150,7 @@ where T: crate::TimeSource, { fn drop(&mut self) { - self.volume_mgr - .close_file(self.raw_file) - .expect("Failed to close file"); + _ = self.volume_mgr.close_file(self.raw_file); } } diff --git a/src/lib.rs b/src/lib.rs index ebf9a4d..1bcd429 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -239,9 +239,12 @@ impl RawVolume { /// Represents an open volume on disk. /// -/// If you drop a value of this type, it closes the volume automatically. However, -/// it holds a mutable reference to its parent `VolumeManager`, which restricts -/// which operations you can perform. +/// In contrast to a `RawVolume`, a `Volume` holds a mutable reference to its +/// parent `VolumeManager`, which restricts which operations you can perform. +/// +/// If you drop a value of this type, it closes the volume automatically, but +/// any error that may occur will be ignored. To handle potential errors, use +/// the [`Volume::close`] method. pub struct Volume<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> where D: crate::BlockDevice, @@ -285,6 +288,16 @@ where core::mem::forget(self); v } + + /// Consume the `Volume` handle and close it. The behavior of this is similar + /// to using [`core::mem::drop`] or letting the `Volume` go out of scope, + /// except this lets the user handle any errors that may occur in the process, + /// whereas when using drop, any errors will be discarded silently. + pub fn close(self) -> Result<(), Error> { + let result = self.volume_mgr.close_volume(self.raw_volume); + core::mem::forget(self); + result + } } impl<'a, D, T, const MAX_DIRS: usize, const MAX_FILES: usize, const MAX_VOLUMES: usize> Drop @@ -294,9 +307,7 @@ where T: crate::TimeSource, { fn drop(&mut self) { - self.volume_mgr - .close_volume(self.raw_volume) - .expect("Failed to close volume"); + _ = self.volume_mgr.close_volume(self.raw_volume) } } diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index 43c2dcb..b5f3940 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -775,14 +775,36 @@ where /// Close a file with the given raw file handle. pub fn close_file(&mut self, file: RawFile) -> Result<(), Error> { - let file_idx = self.flush_file_get_index(file)?; + let flush_result = self.flush_file(file); + let file_idx = self.get_file_by_id(file)?; self.open_files.swap_remove(file_idx); - Ok(()) + flush_result } /// Flush (update the entry) for a file with the given raw file handle. pub fn flush_file(&mut self, file: RawFile) -> Result<(), Error> { - self.flush_file_get_index(file).map(|_| ()) + let file_info = self + .open_files + .iter() + .find(|info| info.file_id == file) + .ok_or(Error::BadHandle)?; + + if file_info.dirty { + let volume_idx = self.get_volume_by_id(file_info.volume_id)?; + match self.open_volumes[volume_idx].volume_type { + VolumeType::Fat(ref mut fat) => { + debug!("Updating FAT info sector"); + fat.update_info_sector(&self.block_device)?; + debug!("Updating dir entry {:?}", file_info.entry); + if file_info.entry.size != 0 { + // If you have a length, you must have a cluster + assert!(file_info.entry.cluster.0 != 0); + } + fat.write_entry_to_disk(&self.block_device, &file_info.entry)?; + } + }; + } + Ok(()) } /// Check if any files or folders are open. @@ -1054,38 +1076,6 @@ where let available = Block::LEN - block_offset; Ok((block_idx, block_offset, available)) } - - /// Flush (update the entry) for a file with the given raw file handle and - /// get its index. - fn flush_file_get_index(&mut self, file: RawFile) -> Result> { - let mut found_idx = None; - for (idx, info) in self.open_files.iter().enumerate() { - if file == info.file_id { - found_idx = Some((info, idx)); - break; - } - } - - let (file_info, file_idx) = found_idx.ok_or(Error::BadHandle)?; - - if file_info.dirty { - let volume_idx = self.get_volume_by_id(file_info.volume_id)?; - match self.open_volumes[volume_idx].volume_type { - VolumeType::Fat(ref mut fat) => { - debug!("Updating FAT info sector"); - fat.update_info_sector(&self.block_device)?; - debug!("Updating dir entry {:?}", file_info.entry); - if file_info.entry.size != 0 { - // If you have a length, you must have a cluster - assert!(file_info.entry.cluster.0 != 0); - } - fat.write_entry_to_disk(&self.block_device, &file_info.entry)?; - } - }; - } - - Ok(file_idx) - } } /// Transform mode variants (ReadWriteCreate_Or_Append) to simple modes ReadWriteAppend or From 1e89779c618a3bca9b47659fdf10d04c2b885964 Mon Sep 17 00:00:00 2001 From: gram Date: Wed, 3 Apr 2024 10:26:32 +0200 Subject: [PATCH 04/17] split FatVolume.iterate_dir into 2 subfunctions --- src/fat/volume.rs | 191 ++++++++++++++++++++++++++-------------------- 1 file changed, 108 insertions(+), 83 deletions(-) diff --git a/src/fat/volume.rs b/src/fat/volume.rs index 05ffc0e..8f44c6d 100644 --- a/src/fat/volume.rs +++ b/src/fat/volume.rs @@ -435,7 +435,7 @@ impl FatVolume { &self, block_device: &D, dir: &DirectoryInfo, - mut func: F, + func: F, ) -> Result<(), Error> where F: FnMut(&DirEntry), @@ -443,97 +443,122 @@ impl FatVolume { { match &self.fat_specific_info { FatSpecificInfo::Fat16(fat16_info) => { - // Root directories on FAT16 have a fixed size, because they use - // a specially reserved space on disk (see - // `first_root_dir_block`). Other directories can have any size - // as they are made of regular clusters. - let mut current_cluster = Some(dir.cluster); - let mut first_dir_block_num = match dir.cluster { - ClusterId::ROOT_DIR => self.lba_start + fat16_info.first_root_dir_block, - _ => self.cluster_to_block(dir.cluster), - }; - let dir_size = match dir.cluster { - ClusterId::ROOT_DIR => { - let len_bytes = - u32::from(fat16_info.root_entries_count) * OnDiskDirEntry::LEN_U32; - BlockCount::from_bytes(len_bytes) - } - _ => BlockCount(u32::from(self.blocks_per_cluster)), - }; + self.iterate_fat16(dir, fat16_info, block_device, func) + } + FatSpecificInfo::Fat32(fat32_info) => { + self.iterate_fat32(dir, fat32_info, block_device, func) + } + } + } - 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")?; - for entry in 0..Block::LEN / OnDiskDirEntry::LEN { - let start = entry * OnDiskDirEntry::LEN; - let end = (entry + 1) * OnDiskDirEntry::LEN; - let dir_entry = OnDiskDirEntry::new(&block[start..end]); - if dir_entry.is_end() { - // Can quit early - return Ok(()); - } else if dir_entry.is_valid() && !dir_entry.is_lfn() { - // Safe, since Block::LEN always fits on a u32 - let start = u32::try_from(start).unwrap(); - let entry = dir_entry.get_entry(FatType::Fat16, block_idx, start); - func(&entry); - } - } - } - if cluster != ClusterId::ROOT_DIR { - current_cluster = - match self.next_cluster(block_device, cluster, &mut block_cache) { - Ok(n) => { - first_dir_block_num = self.cluster_to_block(n); - Some(n) - } - _ => None, - }; - } else { - current_cluster = None; + fn iterate_fat16( + &self, + dir: &DirectoryInfo, + fat16_info: &Fat16Info, + block_device: &D, + mut func: F, + ) -> Result<(), Error> + where + F: FnMut(&DirEntry), + D: BlockDevice, + { + // Root directories on FAT16 have a fixed size, because they use + // a specially reserved space on disk (see + // `first_root_dir_block`). Other directories can have any size + // as they are made of regular clusters. + let mut current_cluster = Some(dir.cluster); + let mut first_dir_block_num = match dir.cluster { + ClusterId::ROOT_DIR => self.lba_start + fat16_info.first_root_dir_block, + _ => self.cluster_to_block(dir.cluster), + }; + let dir_size = match dir.cluster { + ClusterId::ROOT_DIR => { + let len_bytes = u32::from(fat16_info.root_entries_count) * OnDiskDirEntry::LEN_U32; + BlockCount::from_bytes(len_bytes) + } + _ => BlockCount(u32::from(self.blocks_per_cluster)), + }; + + 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")?; + for entry in 0..Block::LEN / OnDiskDirEntry::LEN { + let start = entry * OnDiskDirEntry::LEN; + let end = (entry + 1) * OnDiskDirEntry::LEN; + let dir_entry = OnDiskDirEntry::new(&block[start..end]); + if dir_entry.is_end() { + // Can quit early + return Ok(()); + } else if dir_entry.is_valid() && !dir_entry.is_lfn() { + // Safe, since Block::LEN always fits on a u32 + let start = u32::try_from(start).unwrap(); + let entry = dir_entry.get_entry(FatType::Fat16, block_idx, start); + func(&entry); } } - Ok(()) } - FatSpecificInfo::Fat32(fat32_info) => { - // All directories on FAT32 have a cluster chain but the root - // dir starts in a specified cluster. - let mut current_cluster = match dir.cluster { - ClusterId::ROOT_DIR => Some(fat32_info.first_root_dir_cluster), - _ => Some(dir.cluster), + if cluster != ClusterId::ROOT_DIR { + current_cluster = match self.next_cluster(block_device, cluster, &mut block_cache) { + Ok(n) => { + first_dir_block_num = self.cluster_to_block(n); + Some(n) + } + _ => None, }; - let mut blocks = [Block::new()]; - let mut block_cache = BlockCache::empty(); - 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))) { - block_device - .read(&mut blocks, block, "read_dir") - .map_err(Error::DeviceError)?; - for entry in 0..Block::LEN / OnDiskDirEntry::LEN { - let start = entry * OnDiskDirEntry::LEN; - let end = (entry + 1) * OnDiskDirEntry::LEN; - let dir_entry = OnDiskDirEntry::new(&blocks[0][start..end]); - if dir_entry.is_end() { - // Can quit early - return Ok(()); - } else if dir_entry.is_valid() && !dir_entry.is_lfn() { - // Safe, since Block::LEN always fits on a u32 - let start = u32::try_from(start).unwrap(); - let entry = dir_entry.get_entry(FatType::Fat32, block, start); - func(&entry); - } - } + } else { + current_cluster = None; + } + } + Ok(()) + } + + fn iterate_fat32( + &self, + dir: &DirectoryInfo, + fat32_info: &Fat32Info, + block_device: &D, + mut func: F, + ) -> Result<(), Error> + where + F: FnMut(&DirEntry), + D: BlockDevice, + { + // All directories on FAT32 have a cluster chain but the root + // dir starts in a specified cluster. + let mut current_cluster = match dir.cluster { + ClusterId::ROOT_DIR => Some(fat32_info.first_root_dir_cluster), + _ => Some(dir.cluster), + }; + let mut blocks = [Block::new()]; + let mut block_cache = BlockCache::empty(); + 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))) { + block_device + .read(&mut blocks, block, "read_dir") + .map_err(Error::DeviceError)?; + for entry in 0..Block::LEN / OnDiskDirEntry::LEN { + let start = entry * OnDiskDirEntry::LEN; + let end = (entry + 1) * OnDiskDirEntry::LEN; + let dir_entry = OnDiskDirEntry::new(&blocks[0][start..end]); + if dir_entry.is_end() { + // Can quit early + return Ok(()); + } else if dir_entry.is_valid() && !dir_entry.is_lfn() { + // Safe, since Block::LEN always fits on a u32 + let start = u32::try_from(start).unwrap(); + let entry = dir_entry.get_entry(FatType::Fat32, block, start); + func(&entry); } - current_cluster = - match self.next_cluster(block_device, cluster, &mut block_cache) { - Ok(n) => Some(n), - _ => None, - }; } - Ok(()) } + current_cluster = match self.next_cluster(block_device, cluster, &mut block_cache) { + Ok(n) => Some(n), + _ => None, + }; } + Ok(()) } /// Get an entry from the given directory From f26de604804ce16cdb40f60e6aca5e6da1a4a383 Mon Sep 17 00:00:00 2001 From: gram Date: Wed, 3 Apr 2024 12:14:34 +0200 Subject: [PATCH 05/17] FIx some docs --- src/filesystem/filename.rs | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/src/filesystem/filename.rs b/src/filesystem/filename.rs index a8327ea..4cb763f 100644 --- a/src/filesystem/filename.rs +++ b/src/filesystem/filename.rs @@ -59,19 +59,19 @@ impl ShortFileName { } } - /// Get a short file name containing "..", which means "this directory". + /// Get a short file name containing ".", which means "this directory". pub const fn this_dir() -> Self { Self { contents: *b". ", } } - /// Get base name (name without extension) of file name + /// Get base name (without extension) of the file. pub fn base_name(&self) -> &[u8] { Self::bytes_before_space(&self.contents[..Self::FILENAME_BASE_MAX_LEN]) } - /// Get base name (name without extension) of file name + /// Get extension of the file (without base name). pub fn extension(&self) -> &[u8] { Self::bytes_before_space(&self.contents[Self::FILENAME_BASE_MAX_LEN..]) } From 83128a3057cbfa83213b4688b8a7bab2510ab9b0 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 18:35:36 +0100 Subject: [PATCH 06/17] Add new test. Based on https://github.com/rust-embedded-community/embedded-sdmmc-rs/issues/131 --- tests/read_file_with_seek.rs | 26 ++++++++++++++++++++++++++ 1 file changed, 26 insertions(+) create mode 100644 tests/read_file_with_seek.rs diff --git a/tests/read_file_with_seek.rs b/tests/read_file_with_seek.rs new file mode 100644 index 0000000..35becb1 --- /dev/null +++ b/tests/read_file_with_seek.rs @@ -0,0 +1,26 @@ +mod utils; + +static FILE_TO_READ: &str = "64MB.DAT"; + +#[test] +fn read_file_with_seek() { + let time_source = utils::make_time_source(); + let disk = utils::make_block_device(utils::DISK_SOURCE).unwrap(); + let mut volume_mgr = embedded_sdmmc::VolumeManager::new(disk, time_source); + + let mut volume = volume_mgr + .open_volume(embedded_sdmmc::VolumeIdx(0)) + .unwrap(); + let mut root_dir = volume.open_root_dir().unwrap(); + println!("\nReading file {}...", FILE_TO_READ); + let mut f = root_dir + .open_file_in_dir(FILE_TO_READ, embedded_sdmmc::Mode::ReadOnly) + .unwrap(); + f.seek_from_start(0x2c).unwrap(); + while f.offset() < 1000000 { + let mut buffer = [0u8; 2048]; + f.read(&mut buffer).unwrap(); + f.seek_from_current(-1024).unwrap(); + } + println!("Done!"); +} From 07d7bd2d9b35e0fa02d4824d88e24e1e10c6fe2e Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 18:40:12 +0100 Subject: [PATCH 07/17] Bug fix for #131 Start from beginning of file if current position is *ahead* of desired position. --- src/volume_mgr.rs | 88 ++++++++++++++++++++++++++++------------------- 1 file changed, 52 insertions(+), 36 deletions(-) diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index b5f3940..f06875c 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -624,6 +624,7 @@ where let (block_idx, block_offset, block_avail) = self.find_data_on_disk( volume_idx, &mut current_cluster, + self.open_files[file_idx].entry.cluster, self.open_files[file_idx].current_offset, )?; self.open_files[file_idx].current_cluster = current_cluster; @@ -701,44 +702,45 @@ where written, bytes_to_write, current_cluster ); let current_offset = self.open_files[file_idx].current_offset; - let (block_idx, block_offset, block_avail) = - match self.find_data_on_disk(volume_idx, &mut current_cluster, current_offset) { - Ok(vars) => { - debug!( - "Found block_idx={:?}, block_offset={:?}, block_avail={}", - vars.0, vars.1, vars.2 - ); - vars - } - Err(Error::EndOfFile) => { - debug!("Extending file"); - match self.open_volumes[volume_idx].volume_type { - VolumeType::Fat(ref mut fat) => { - if fat - .alloc_cluster( - &self.block_device, - Some(current_cluster.1), - false, - ) - .is_err() - { - return Err(Error::DiskFull); - } - debug!("Allocated new FAT cluster, finding offsets..."); - let new_offset = self - .find_data_on_disk( - volume_idx, - &mut current_cluster, - self.open_files[file_idx].current_offset, - ) - .map_err(|_| Error::AllocationError)?; - debug!("New offset {:?}", new_offset); - new_offset + let (block_idx, block_offset, block_avail) = match self.find_data_on_disk( + volume_idx, + &mut current_cluster, + self.open_files[file_idx].entry.cluster, + current_offset, + ) { + Ok(vars) => { + debug!( + "Found block_idx={:?}, block_offset={:?}, block_avail={}", + vars.0, vars.1, vars.2 + ); + vars + } + Err(Error::EndOfFile) => { + debug!("Extending file"); + match self.open_volumes[volume_idx].volume_type { + VolumeType::Fat(ref mut fat) => { + if fat + .alloc_cluster(&self.block_device, Some(current_cluster.1), false) + .is_err() + { + return Err(Error::DiskFull); } + debug!("Allocated new FAT cluster, finding offsets..."); + let new_offset = self + .find_data_on_disk( + volume_idx, + &mut current_cluster, + self.open_files[file_idx].entry.cluster, + self.open_files[file_idx].current_offset, + ) + .map_err(|_| Error::AllocationError)?; + debug!("New offset {:?}", new_offset); + new_offset } } - Err(e) => return Err(e), - }; + } + Err(e) => return Err(e), + }; let mut blocks = [Block::new()]; let to_copy = core::cmp::min(block_avail, bytes_to_write - written); if block_offset != 0 { @@ -1044,16 +1046,30 @@ where /// This function turns `desired_offset` into an appropriate block to be /// read. It either calculates this based on the start of the file, or /// from the last cluster we read - whichever is better. + /// + /// Returns: + /// + /// * the index for the block on the disk that contains the data we want, + /// * the byte offset into that block for the data we want, and + /// * how many bytes remain in that block. fn find_data_on_disk( &self, volume_idx: usize, start: &mut (u32, ClusterId), + file_start: ClusterId, desired_offset: u32, ) -> Result<(BlockIdx, usize, usize), Error> { let bytes_per_cluster = match &self.open_volumes[volume_idx].volume_type { VolumeType::Fat(fat) => fat.bytes_per_cluster(), }; // How many clusters forward do we need to go? + if desired_offset < start.0 { + // user wants to go backwards - start from the beginning of the file + // because the FAT is only a singly-linked list. + start.0 = 0; + start.1 = file_start; + } + // walk through the FAT chain let offset_from_cluster = desired_offset - start.0; let num_clusters = offset_from_cluster / bytes_per_cluster; let mut block_cache = BlockCache::empty(); @@ -1065,7 +1081,7 @@ where }; start.0 += bytes_per_cluster; } - // How many blocks in are we? + // How many blocks in are we now? let offset_from_cluster = desired_offset - start.0; assert!(offset_from_cluster < bytes_per_cluster); let num_blocks = BlockCount(offset_from_cluster / Block::LEN_U32); From a3c895ba7030efc94695cb257aa7a54d162b65b7 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 19:51:43 +0100 Subject: [PATCH 08/17] Fix read_file_backwards. We weren't actually reading backwards because we always did "seek from start". Now we do some "seek from current" with a negative offset, and a "seek from end". --- tests/read_file.rs | 16 ++++++++++------ 1 file changed, 10 insertions(+), 6 deletions(-) diff --git a/tests/read_file.rs b/tests/read_file.rs index b0e7628..c4c9305 100644 --- a/tests/read_file.rs +++ b/tests/read_file.rs @@ -150,27 +150,31 @@ fn read_file_backwards() { const CHUNK_SIZE: u32 = 100; let length = volume_mgr.file_length(test_file).expect("file length"); - let mut offset = length - CHUNK_SIZE; let mut read = 0; + // go to end + volume_mgr.file_seek_from_end(test_file, 0).expect("seek"); + // We're going to read the file backwards in chunks of 100 bytes. This // checks we didn't make any assumptions about only going forwards. while read < length { + // go to start of next chunk volume_mgr - .file_seek_from_start(test_file, offset) + .file_seek_from_current(test_file, -(CHUNK_SIZE as i32)) .expect("seek"); + // read chunk let mut buffer = [0u8; CHUNK_SIZE as usize]; let len = volume_mgr.read(test_file, &mut buffer).expect("read"); assert_eq!(len, CHUNK_SIZE as usize); contents.push_front(buffer.to_vec()); read += CHUNK_SIZE; - if offset >= CHUNK_SIZE { - offset -= CHUNK_SIZE; - } + // go to start of chunk we just read + volume_mgr + .file_seek_from_current(test_file, -(CHUNK_SIZE as i32)) + .expect("seek"); } assert_eq!(read, length); - assert_eq!(offset, 0); let flat: Vec = contents.iter().flatten().copied().collect(); From a04e5f015cf6bbf8e23c7b6f24df286296ca837c Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 19:52:04 +0100 Subject: [PATCH 09/17] Fix seek_from_end. --- src/filesystem/files.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index 08a50e8..37f9c6a 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -252,7 +252,7 @@ impl FileInfo { pub fn seek_from_end(&mut self, offset: u32) -> Result<(), FileError> { if offset <= self.entry.size { self.current_offset = self.entry.size - offset; - if offset < self.current_cluster.0 { + if self.current_offset < self.current_cluster.0 { // Back to start self.current_cluster = (0, self.entry.cluster); } From 0de58d3e8a84dd37b3e03d3a596490226ecda6d2 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 19:52:14 +0100 Subject: [PATCH 10/17] Move new test into read_file. --- tests/read_file.rs | 21 +++++++++++++++++++++ tests/read_file_with_seek.rs | 26 -------------------------- 2 files changed, 21 insertions(+), 26 deletions(-) delete mode 100644 tests/read_file_with_seek.rs diff --git a/tests/read_file.rs b/tests/read_file.rs index c4c9305..b7c80c9 100644 --- a/tests/read_file.rs +++ b/tests/read_file.rs @@ -184,6 +184,27 @@ fn read_file_backwards() { assert_eq!(&hash[..], TEST_DAT_SHA256_SUM); } +#[test] +fn read_file_with_odd_seek() { + let time_source = utils::make_time_source(); + let disk = utils::make_block_device(utils::DISK_SOURCE).unwrap(); + let mut volume_mgr = embedded_sdmmc::VolumeManager::new(disk, time_source); + + let mut volume = volume_mgr + .open_volume(embedded_sdmmc::VolumeIdx(0)) + .unwrap(); + let mut root_dir = volume.open_root_dir().unwrap(); + let mut f = root_dir + .open_file_in_dir("64MB.DAT", embedded_sdmmc::Mode::ReadOnly) + .unwrap(); + f.seek_from_start(0x2c).unwrap(); + while f.offset() < 1000000 { + let mut buffer = [0u8; 2048]; + f.read(&mut buffer).unwrap(); + f.seek_from_current(-1024).unwrap(); + } +} + // **************************************************************************** // // End Of File diff --git a/tests/read_file_with_seek.rs b/tests/read_file_with_seek.rs deleted file mode 100644 index 35becb1..0000000 --- a/tests/read_file_with_seek.rs +++ /dev/null @@ -1,26 +0,0 @@ -mod utils; - -static FILE_TO_READ: &str = "64MB.DAT"; - -#[test] -fn read_file_with_seek() { - let time_source = utils::make_time_source(); - let disk = utils::make_block_device(utils::DISK_SOURCE).unwrap(); - let mut volume_mgr = embedded_sdmmc::VolumeManager::new(disk, time_source); - - let mut volume = volume_mgr - .open_volume(embedded_sdmmc::VolumeIdx(0)) - .unwrap(); - let mut root_dir = volume.open_root_dir().unwrap(); - println!("\nReading file {}...", FILE_TO_READ); - let mut f = root_dir - .open_file_in_dir(FILE_TO_READ, embedded_sdmmc::Mode::ReadOnly) - .unwrap(); - f.seek_from_start(0x2c).unwrap(); - while f.offset() < 1000000 { - let mut buffer = [0u8; 2048]; - f.read(&mut buffer).unwrap(); - f.seek_from_current(-1024).unwrap(); - } - println!("Done!"); -} From 59e4c2fc8bea016a7cb9ffa6b4b0a5d850c28df9 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 20:06:09 +0100 Subject: [PATCH 11/17] Code cleanups. --- src/filesystem/files.rs | 41 ++++++++++++++++------------------------- src/volume_mgr.rs | 7 ++++--- 2 files changed, 20 insertions(+), 28 deletions(-) diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index 37f9c6a..462511a 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -211,7 +211,10 @@ pub(crate) struct FileInfo { pub(crate) file_id: RawFile, /// The unique ID for the volume this directory is on pub(crate) volume_id: RawVolume, - /// The current cluster, and how many bytes that short-cuts us + /// The last cluster we accessed, and how many bytes that short-cuts us. + /// + /// This saves us walking from the very start of the FAT chain when we move + /// forward through a file. pub(crate) current_cluster: (u32, ClusterId), /// How far through the file we've read (in bytes). pub(crate) current_offset: u32, @@ -236,41 +239,30 @@ impl FileInfo { /// Seek to a new position in the file, relative to the start of the file. pub fn seek_from_start(&mut self, offset: u32) -> Result<(), FileError> { - if offset <= self.entry.size { - self.current_offset = offset; - if offset < self.current_cluster.0 { - // Back to start - self.current_cluster = (0, self.entry.cluster); - } - Ok(()) - } else { - Err(FileError::InvalidOffset) + if offset > self.entry.size { + return Err(FileError::InvalidOffset); } + self.current_offset = offset; + Ok(()) } /// Seek to a new position in the file, relative to the end of the file. pub fn seek_from_end(&mut self, offset: u32) -> Result<(), FileError> { - if offset <= self.entry.size { - self.current_offset = self.entry.size - offset; - if self.current_offset < self.current_cluster.0 { - // Back to start - self.current_cluster = (0, self.entry.cluster); - } - Ok(()) - } else { - Err(FileError::InvalidOffset) + if offset > self.entry.size { + return Err(FileError::InvalidOffset); } + self.current_offset = self.entry.size - offset; + Ok(()) } /// Seek to a new position in the file, relative to the current position. pub fn seek_from_current(&mut self, offset: i32) -> Result<(), FileError> { let new_offset = i64::from(self.current_offset) + i64::from(offset); - if new_offset >= 0 && new_offset <= i64::from(self.entry.size) { - self.current_offset = new_offset as u32; - Ok(()) - } else { - Err(FileError::InvalidOffset) + if new_offset < 0 || new_offset > i64::from(self.entry.size) { + return Err(FileError::InvalidOffset); } + self.current_offset = new_offset as u32; + Ok(()) } /// Amount of file left to read. @@ -281,7 +273,6 @@ impl FileInfo { /// Update the file's length. pub(crate) fn update_length(&mut self, new: u32) { self.entry.size = new; - self.entry.size = new; } } diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index f06875c..15e4f42 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -1045,7 +1045,7 @@ where /// This function turns `desired_offset` into an appropriate block to be /// read. It either calculates this based on the start of the file, or - /// from the last cluster we read - whichever is better. + /// from the given start point - whichever is better. /// /// Returns: /// @@ -1062,15 +1062,16 @@ where let bytes_per_cluster = match &self.open_volumes[volume_idx].volume_type { VolumeType::Fat(fat) => fat.bytes_per_cluster(), }; - // How many clusters forward do we need to go? + // do we need to be before our start point? if desired_offset < start.0 { // user wants to go backwards - start from the beginning of the file // because the FAT is only a singly-linked list. start.0 = 0; start.1 = file_start; } - // walk through the FAT chain + // How many clusters forward do we need to go? let offset_from_cluster = desired_offset - start.0; + // walk through the FAT chain let num_clusters = offset_from_cluster / bytes_per_cluster; let mut block_cache = BlockCache::empty(); for _ in 0..num_clusters { From 7b5b876afba9b3543373b88ee4e910d7d6b840d7 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 20:32:55 +0100 Subject: [PATCH 12/17] Appease clippy. --- examples/readme_test.rs | 2 + src/sdcard/proto.rs | 88 ++++++++++++++++++++--------------------- tests/open_files.rs | 10 ++--- tests/utils/mod.rs | 6 +-- 4 files changed, 54 insertions(+), 52 deletions(-) diff --git a/examples/readme_test.rs b/examples/readme_test.rs index 153b758..2069694 100644 --- a/examples/readme_test.rs +++ b/examples/readme_test.rs @@ -3,6 +3,8 @@ //! We add enough stuff to make it compile, but it won't run because our fake //! SPI doesn't do any replies. +#![allow(dead_code)] + use core::cell::RefCell; use embedded_sdmmc::sdcard::DummyCsPin; diff --git a/src/sdcard/proto.rs b/src/sdcard/proto.rs index 6418574..68ae248 100644 --- a/src/sdcard/proto.rs +++ b/src/sdcard/proto.rs @@ -298,19 +298,19 @@ mod test { // Partial Blocks for Read Allowed: // 0b1 [Interpreted: Yes] - assert_eq!(EXAMPLE.read_partial_blocks(), true); + assert!(EXAMPLE.read_partial_blocks()); // Write Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.write_block_misalignment(), false); + assert!(!EXAMPLE.write_block_misalignment()); // Read Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.read_block_misalignment(), false); + assert!(!EXAMPLE.read_block_misalignment()); // DSR Implemented: indicates configurable driver stage integrated on // card 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.dsr_implemented(), false); + assert!(!EXAMPLE.dsr_implemented()); // Device Size: to calculate the card capacity excl. security area // ((device size + 1)*device size multiplier*max read data block @@ -339,7 +339,7 @@ mod test { // Erase Single Block Enabled: // 0x1 [Interpreted: Yes] - assert_eq!(EXAMPLE.erase_single_block_enabled(), true); + assert!(EXAMPLE.erase_single_block_enabled()); // Erase Sector Size: size of erasable sector in write blocks // 0x1f [Interpreted: 32 blocks] @@ -351,7 +351,7 @@ mod test { // Write Protect Group Enable: // 0x1 [Interpreted: Yes] - assert_eq!(EXAMPLE.write_protect_group_enable(), true); + assert!(EXAMPLE.write_protect_group_enable()); // Write Speed Factor: block program time as multiple of read access time // 0x4 [Interpreted: x16] @@ -363,23 +363,23 @@ mod test { // Partial Blocks for Write Allowed: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_partial_blocks(), false); + assert!(!EXAMPLE.write_partial_blocks()); // File Format Group: // 0b0 [Interpreted: is either Hard Disk with Partition Table/DOS FAT without Partition Table/Universal File Format/Other/Unknown] - assert_eq!(EXAMPLE.file_format_group_set(), false); + assert!(!EXAMPLE.file_format_group_set()); // Copy Flag: // 0b1 [Interpreted: Non-Original] - assert_eq!(EXAMPLE.copy_flag_set(), true); + assert!(EXAMPLE.copy_flag_set()); // Permanent Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.permanent_write_protection(), false); + assert!(!EXAMPLE.permanent_write_protection()); // Temporary Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.temporary_write_protection(), false); + assert!(!EXAMPLE.temporary_write_protection()); // File Format: // 0x0 [Interpreted: Hard Disk with Partition Table] @@ -424,19 +424,19 @@ mod test { // Partial Blocks for Read Allowed: // 0b1 [Interpreted: Yes] - assert_eq!(EXAMPLE.read_partial_blocks(), true); + assert!(EXAMPLE.read_partial_blocks()); // Write Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.write_block_misalignment(), false); + assert!(!EXAMPLE.write_block_misalignment()); // Read Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.read_block_misalignment(), false); + assert!(!EXAMPLE.read_block_misalignment()); // DSR Implemented: indicates configurable driver stage integrated on card // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.dsr_implemented(), false); + assert!(!EXAMPLE.dsr_implemented()); // Device Size: to calculate the card capacity excl. security area // ((device size + 1)*device size multiplier*max read data block @@ -465,7 +465,7 @@ mod test { // Erase Single Block Enabled: // 0x1 [Interpreted: Yes] - assert_eq!(EXAMPLE.erase_single_block_enabled(), true); + assert!(EXAMPLE.erase_single_block_enabled()); // Erase Sector Size: size of erasable sector in write blocks // 0x1f [Interpreted: 32 blocks] @@ -477,7 +477,7 @@ mod test { // Write Protect Group Enable: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_protect_group_enable(), false); + assert!(!EXAMPLE.write_protect_group_enable()); // Write Speed Factor: block program time as multiple of read access time // 0x5 [Interpreted: x32] @@ -489,23 +489,23 @@ mod test { // Partial Blocks for Write Allowed: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_partial_blocks(), false); + assert!(!EXAMPLE.write_partial_blocks()); // File Format Group: // 0b0 [Interpreted: is either Hard Disk with Partition Table/DOS FAT without Partition Table/Universal File Format/Other/Unknown] - assert_eq!(EXAMPLE.file_format_group_set(), false); + assert!(!EXAMPLE.file_format_group_set()); // Copy Flag: // 0b0 [Interpreted: Original] - assert_eq!(EXAMPLE.copy_flag_set(), false); + assert!(!EXAMPLE.copy_flag_set()); // Permanent Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.permanent_write_protection(), false); + assert!(!EXAMPLE.permanent_write_protection()); // Temporary Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.temporary_write_protection(), false); + assert!(!EXAMPLE.temporary_write_protection()); // File Format: // 0x0 [Interpreted: Hard Disk with Partition Table] @@ -550,19 +550,19 @@ mod test { // Partial Blocks for Read Allowed: // 0b0 [Interpreted: Yes] - assert_eq!(EXAMPLE.read_partial_blocks(), false); + assert!(!EXAMPLE.read_partial_blocks()); // Write Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.write_block_misalignment(), false); + assert!(!EXAMPLE.write_block_misalignment()); // Read Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.read_block_misalignment(), false); + assert!(!EXAMPLE.read_block_misalignment()); // DSR Implemented: indicates configurable driver stage integrated on card // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.dsr_implemented(), false); + assert!(!EXAMPLE.dsr_implemented()); // Device Size: to calculate the card capacity excl. security area // ((device size + 1)* 512kbytes @@ -571,7 +571,7 @@ mod test { // Erase Single Block Enabled: // 0x1 [Interpreted: Yes] - assert_eq!(EXAMPLE.erase_single_block_enabled(), true); + assert!(EXAMPLE.erase_single_block_enabled()); // Erase Sector Size: size of erasable sector in write blocks // 0x7f [Interpreted: 128 blocks] @@ -583,7 +583,7 @@ mod test { // Write Protect Group Enable: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_protect_group_enable(), false); + assert!(!EXAMPLE.write_protect_group_enable()); // Write Speed Factor: block program time as multiple of read access time // 0x2 [Interpreted: x4] @@ -595,23 +595,23 @@ mod test { // Partial Blocks for Write Allowed: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_partial_blocks(), false); + assert!(!EXAMPLE.write_partial_blocks()); // File Format Group: // 0b0 [Interpreted: is either Hard Disk with Partition Table/DOS FAT without Partition Table/Universal File Format/Other/Unknown] - assert_eq!(EXAMPLE.file_format_group_set(), false); + assert!(!EXAMPLE.file_format_group_set()); // Copy Flag: // 0b0 [Interpreted: Original] - assert_eq!(EXAMPLE.copy_flag_set(), false); + assert!(!EXAMPLE.copy_flag_set()); // Permanent Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.permanent_write_protection(), false); + assert!(!EXAMPLE.permanent_write_protection()); // Temporary Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.temporary_write_protection(), false); + assert!(!EXAMPLE.temporary_write_protection()); // File Format: // 0x0 [Interpreted: Hard Disk with Partition Table] @@ -656,19 +656,19 @@ mod test { // Partial Blocks for Read Allowed: // 0b0 [Interpreted: Yes] - assert_eq!(EXAMPLE.read_partial_blocks(), false); + assert!(!EXAMPLE.read_partial_blocks()); // Write Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.write_block_misalignment(), false); + assert!(!EXAMPLE.write_block_misalignment()); // Read Block Misalignment: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.read_block_misalignment(), false); + assert!(!EXAMPLE.read_block_misalignment()); // DSR Implemented: indicates configurable driver stage integrated on card // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.dsr_implemented(), false); + assert!(!EXAMPLE.dsr_implemented()); // Device Size: to calculate the card capacity excl. security area // ((device size + 1)* 512kbytes @@ -677,7 +677,7 @@ mod test { // Erase Single Block Enabled: // 0x1 [Interpreted: Yes] - assert_eq!(EXAMPLE.erase_single_block_enabled(), true); + assert!(EXAMPLE.erase_single_block_enabled()); // Erase Sector Size: size of erasable sector in write blocks // 0x7f [Interpreted: 128 blocks] @@ -689,7 +689,7 @@ mod test { // Write Protect Group Enable: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_protect_group_enable(), false); + assert!(!EXAMPLE.write_protect_group_enable()); // Write Speed Factor: block program time as multiple of read access time // 0x2 [Interpreted: x4] @@ -701,23 +701,23 @@ mod test { // Partial Blocks for Write Allowed: // 0x0 [Interpreted: No] - assert_eq!(EXAMPLE.write_partial_blocks(), false); + assert!(!EXAMPLE.write_partial_blocks()); // File Format Group: // 0b0 [Interpreted: is either Hard Disk with Partition Table/DOS FAT without Partition Table/Universal File Format/Other/Unknown] - assert_eq!(EXAMPLE.file_format_group_set(), false); + assert!(!EXAMPLE.file_format_group_set()); // Copy Flag: // 0b0 [Interpreted: Original] - assert_eq!(EXAMPLE.copy_flag_set(), false); + assert!(!EXAMPLE.copy_flag_set()); // Permanent Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.permanent_write_protection(), false); + assert!(!EXAMPLE.permanent_write_protection()); // Temporary Write Protection: // 0b0 [Interpreted: No] - assert_eq!(EXAMPLE.temporary_write_protection(), false); + assert!(!EXAMPLE.temporary_write_protection()); // File Format: // 0x0 [Interpreted: Hard Disk with Partition Table] diff --git a/tests/open_files.rs b/tests/open_files.rs index dcdaadf..10e2181 100644 --- a/tests/open_files.rs +++ b/tests/open_files.rs @@ -109,24 +109,24 @@ fn open_non_raw() { assert_eq!(len, 258); assert_eq!(f.length(), 258); f.seek_from_current(0).unwrap(); - assert_eq!(f.is_eof(), true); + assert!(f.is_eof()); assert_eq!(f.offset(), 258); assert!(matches!(f.seek_from_current(1), Err(Error::InvalidOffset))); f.seek_from_current(-258).unwrap(); - assert_eq!(f.is_eof(), false); + assert!(!f.is_eof()); assert_eq!(f.offset(), 0); f.seek_from_current(10).unwrap(); - assert_eq!(f.is_eof(), false); + assert!(!f.is_eof()); assert_eq!(f.offset(), 10); f.seek_from_end(0).unwrap(); - assert_eq!(f.is_eof(), true); + assert!(f.is_eof()); assert_eq!(f.offset(), 258); assert!(matches!( f.seek_from_current(-259), Err(Error::InvalidOffset) )); f.seek_from_start(25).unwrap(); - assert_eq!(f.is_eof(), false); + assert!(!f.is_eof()); assert_eq!(f.offset(), 25); drop(f); diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 9976975..9c371a4 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -8,7 +8,7 @@ use embedded_sdmmc::{Block, BlockCount, BlockDevice, BlockIdx}; /// /// ```console /// $ fdisk ./disk.img -/// Disk: ./disk.img geometry: 520/32/63 [1048576 sectors] +/// Disk: ./disk.img geometry: 520/32/63 [1048576 sectors] /// Signature: 0xAA55 /// Starting Ending /// #: id cyl hd sec - cyl hd sec [ start - size] @@ -126,7 +126,7 @@ where fn num_blocks(&self) -> Result { let borrow = self.contents.borrow(); let contents: &[u8] = borrow.as_ref(); - let len_blocks = contents.len() as usize / embedded_sdmmc::Block::LEN; + let len_blocks = contents.len() / embedded_sdmmc::Block::LEN; if len_blocks > u32::MAX as usize { panic!("Test disk too large! Only 2**32 blocks allowed"); } @@ -155,7 +155,7 @@ pub struct TestTimeSource { impl embedded_sdmmc::TimeSource for TestTimeSource { fn get_timestamp(&self) -> embedded_sdmmc::Timestamp { - self.fixed.clone() + self.fixed } } From 6b1ef4c64e415b26dbb13f49add42bbfce45f959 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 1 Jun 2024 20:33:48 +0100 Subject: [PATCH 13/17] Update README example Notes that converting a byte to a char is assuming the the file is ISO-8859-1 encoded. --- README.md | 2 +- examples/readme_test.rs | 2 +- 2 files changed, 2 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index 3ba2918..9b881a2 100644 --- a/README.md +++ b/README.md @@ -27,7 +27,7 @@ let mut root_dir = volume0.open_root_dir()?; // Open a file called "MY_FILE.TXT" in the root directory // This mutably borrows the directory. let mut my_file = root_dir.open_file_in_dir("MY_FILE.TXT", embedded_sdmmc::Mode::ReadOnly)?; -// Print the contents of the file +// Print the contents of the file, assuming it's in ISO-8859-1 encoding while !my_file.is_eof() { let mut buffer = [0u8; 32]; let num_read = my_file.read(&mut buffer)?; diff --git a/examples/readme_test.rs b/examples/readme_test.rs index 2069694..ed4b146 100644 --- a/examples/readme_test.rs +++ b/examples/readme_test.rs @@ -120,7 +120,7 @@ fn main() -> Result<(), Error> { // Open a file called "MY_FILE.TXT" in the root directory // This mutably borrows the directory. let mut my_file = root_dir.open_file_in_dir("MY_FILE.TXT", embedded_sdmmc::Mode::ReadOnly)?; - // Print the contents of the file + // Print the contents of the file, assuming it's in ISO-8859-1 encoding while !my_file.is_eof() { let mut buffer = [0u8; 32]; let num_read = my_file.read(&mut buffer)?; From 510d50ed347beca3d85332b0d04ba2376c361ea1 Mon Sep 17 00:00:00 2001 From: Jonathan 'theJPster' Pallant Date: Sat, 6 Jul 2024 23:37:44 +0100 Subject: [PATCH 14/17] Use SpiDevice to control the chip select. Closes #126 --- Cargo.toml | 2 +- README.md | 2 +- examples/readme_test.rs | 23 +++- src/lib.rs | 5 +- src/sdcard/mod.rs | 234 ++++++++++++++-------------------------- tests/utils/mod.rs | 1 + 6 files changed, 102 insertions(+), 165 deletions(-) diff --git a/Cargo.toml b/Cargo.toml index bd6549c..1fa09ea 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -19,7 +19,7 @@ log = {version = "0.4", default-features = false, optional = true} [dev-dependencies] chrono = "0.4" -embedded-hal-bus = "0.1.0" +embedded-hal-bus = "0.2.0" env_logger = "0.10.0" flate2 = "1.0" hex-literal = "0.4.1" diff --git a/README.md b/README.md index 9b881a2..1e3194c 100644 --- a/README.md +++ b/README.md @@ -12,7 +12,7 @@ You will need something that implements the `BlockDevice` trait, which can read ```rust // Build an SD Card interface out of an SPI device, a chip-select pin and the delay object -let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, sdmmc_cs, delay); +let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, delay); // Get the card size (this also triggers card initialisation because it's not been done yet) println!("Card size is {} bytes", sdcard.num_bytes()?); // Now let's look for volumes (also known as partitions) on our block device. diff --git a/examples/readme_test.rs b/examples/readme_test.rs index ed4b146..7ae7384 100644 --- a/examples/readme_test.rs +++ b/examples/readme_test.rs @@ -7,7 +7,23 @@ use core::cell::RefCell; -use embedded_sdmmc::sdcard::DummyCsPin; +pub struct DummyCsPin; + +impl embedded_hal::digital::ErrorType for DummyCsPin { + type Error = core::convert::Infallible; +} + +impl embedded_hal::digital::OutputPin for DummyCsPin { + #[inline(always)] + fn set_low(&mut self) -> Result<(), Self::Error> { + Ok(()) + } + + #[inline(always)] + fn set_high(&mut self) -> Result<(), Self::Error> { + Ok(()) + } +} struct FakeSpiBus(); @@ -99,13 +115,12 @@ fn main() -> Result<(), Error> { // BEGIN Fake stuff that will be replaced with real peripherals let spi_bus = RefCell::new(FakeSpiBus()); let delay = FakeDelayer(); - let sdmmc_spi = embedded_hal_bus::spi::RefCellDevice::new(&spi_bus, DummyCsPin, delay); - let sdmmc_cs = FakeCs(); + let sdmmc_spi = embedded_hal_bus::spi::RefCellDevice::new(&spi_bus, DummyCsPin, delay).unwrap(); let time_source = FakeTimesource(); // END Fake stuff that will be replaced with real peripherals // Build an SD Card interface out of an SPI device, a chip-select pin and the delay object - let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, sdmmc_cs, delay); + let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, delay); // Get the card size (this also triggers card initialisation because it's not been done yet) println!("Card size is {} bytes", sdcard.num_bytes()?); // Now let's look for volumes (also known as partitions) on our block device. diff --git a/src/lib.rs b/src/lib.rs index 1bcd429..6466102 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -19,14 +19,13 @@ //! ```rust //! use embedded_sdmmc::{Error, Mode, SdCard, SdCardError, TimeSource, VolumeIdx, VolumeManager}; //! -//! fn example(spi: S, cs: CS, delay: D, ts: T) -> Result<(), Error> +//! fn example(spi: S, delay: D, ts: T) -> Result<(), Error> //! where //! S: embedded_hal::spi::SpiDevice, -//! CS: embedded_hal::digital::OutputPin, //! D: embedded_hal::delay::DelayNs, //! T: TimeSource, //! { -//! let sdcard = SdCard::new(spi, cs, delay); +//! let sdcard = SdCard::new(spi, delay); //! println!("Card size is {} bytes", sdcard.num_bytes()?); //! let mut volume_mgr = VolumeManager::new(sdcard, ts); //! let mut volume0 = volume_mgr.open_volume(VolumeIdx(0))?; diff --git a/src/sdcard/mod.rs b/src/sdcard/mod.rs index 8f0dbb9..d53a15a 100644 --- a/src/sdcard/mod.rs +++ b/src/sdcard/mod.rs @@ -21,80 +21,43 @@ use crate::{debug, warn}; // Types and Implementations // **************************************************************************** -/// A dummy "CS pin" that does nothing when set high or low. -/// -/// Should be used when constructing an [`SpiDevice`] implementation for use with [`SdCard`]. -/// -/// Let the [`SpiDevice`] use this dummy CS pin that does not actually do anything, and pass the -/// card's real CS pin to [`SdCard`]'s constructor. This allows the driver to have more -/// fine-grained control of how the CS pin is managed than is allowed by default using the -/// [`SpiDevice`] trait, which is needed to implement the SD/MMC SPI communication spec correctly. -/// -/// If you're not sure how to get a [`SpiDevice`], you may use one of the implementations -/// in the [`embedded-hal-bus`] crate, providing a wrapped version of your platform's HAL-provided -/// [`SpiBus`] and [`DelayNs`] as well as our [`DummyCsPin`] in the constructor. -/// -/// [`SpiDevice`]: embedded_hal::spi::SpiDevice -/// [`SpiBus`]: embedded_hal::spi::SpiBus -/// [`DelayNs`]: embedded_hal::delay::DelayNs -/// [`embedded-hal-bus`]: https://docs.rs/embedded-hal-bus -pub struct DummyCsPin; - -impl embedded_hal::digital::ErrorType for DummyCsPin { - type Error = core::convert::Infallible; -} - -impl embedded_hal::digital::OutputPin for DummyCsPin { - #[inline(always)] - fn set_low(&mut self) -> Result<(), Self::Error> { - Ok(()) - } - - #[inline(always)] - fn set_high(&mut self) -> Result<(), Self::Error> { - Ok(()) - } -} - /// Represents an SD Card on an SPI bus. /// /// Built from an [`SpiDevice`] implementation and a Chip Select pin. -/// Unfortunately, We need control of the chip select pin separately from the [`SpiDevice`] -/// implementation so we can clock out some bytes without Chip Select asserted -/// (which is necessary to make the SD card actually release the Spi bus after performing -/// operations on it, according to the spec). To support this, we provide [`DummyCsPin`] -/// which should be provided to your chosen [`SpiDevice`] implementation rather than the card's -/// actual CS pin. Then provide the actual CS pin to [`SdCard`]'s constructor. +/// +/// Before talking to the SD Card, the caller needs to send 74 clocks cycles on +/// the SPI Clock line, at 400 kHz, with no chip-select asserted (or at least, +/// not the chip-select of the SD Card). +/// +/// This kind of breaks the embedded-hal model, so how to do this is left to +/// the caller. You could drive the SpiBus directly, or use an SpiDevice with +/// a dummy chip-select pin. Or you could try just not doing the 74 clocks and +/// see if your card works anyway - some do, some don't. /// /// All the APIs take `&self` - mutability is handled using an inner `RefCell`. /// /// [`SpiDevice`]: embedded_hal::spi::SpiDevice -pub struct SdCard +pub struct SdCard where SPI: embedded_hal::spi::SpiDevice, - CS: embedded_hal::digital::OutputPin, DELAYER: embedded_hal::delay::DelayNs, { - inner: RefCell>, + inner: RefCell>, } -impl SdCard +impl SdCard where SPI: embedded_hal::spi::SpiDevice, - CS: embedded_hal::digital::OutputPin, DELAYER: embedded_hal::delay::DelayNs, { /// Create a new SD/MMC Card driver using a raw SPI interface. /// - /// See the docs of the [`SdCard`] struct for more information about - /// how to construct the needed `SPI` and `CS` types. - /// /// The card will not be initialised at this time. Initialisation is /// deferred until a method is called on the object. /// /// Uses the default options. - pub fn new(spi: SPI, cs: CS, delayer: DELAYER) -> SdCard { - Self::new_with_options(spi, cs, delayer, AcquireOpts::default()) + pub fn new(spi: SPI, delayer: DELAYER) -> SdCard { + Self::new_with_options(spi, delayer, AcquireOpts::default()) } /// Construct a new SD/MMC Card driver, using a raw SPI interface and the given options. @@ -106,14 +69,12 @@ where /// deferred until a method is called on the object. pub fn new_with_options( spi: SPI, - cs: CS, delayer: DELAYER, options: AcquireOpts, - ) -> SdCard { + ) -> SdCard { SdCard { inner: RefCell::new(SdCardInner { spi, - cs, delayer, card_type: None, options, @@ -193,10 +154,9 @@ where } } -impl BlockDevice for SdCard +impl BlockDevice for SdCard where SPI: embedded_hal::spi::SpiDevice, - CS: embedded_hal::digital::OutputPin, DELAYER: embedded_hal::delay::DelayNs, { type Error = Error; @@ -244,23 +204,20 @@ where /// Represents an SD Card on an SPI bus. /// /// All the APIs required `&mut self`. -struct SdCardInner +struct SdCardInner where SPI: embedded_hal::spi::SpiDevice, - CS: embedded_hal::digital::OutputPin, DELAYER: embedded_hal::delay::DelayNs, { spi: SPI, - cs: CS, delayer: DELAYER, card_type: Option, options: AcquireOpts, } -impl SdCardInner +impl SdCardInner where SPI: embedded_hal::spi::SpiDevice, - CS: embedded_hal::digital::OutputPin, DELAYER: embedded_hal::delay::DelayNs, { /// Read one or more blocks, starting at the given block index. @@ -270,22 +227,21 @@ where Some(CardType::SDHC) => start_block_idx.0, None => return Err(Error::CardNotFound), }; - self.with_chip_select(|s| { - if blocks.len() == 1 { - // Start a single-block read - s.card_command(CMD17, start_idx)?; - s.read_data(&mut blocks[0].contents)?; - } else { - // Start a multi-block read - s.card_command(CMD18, start_idx)?; - for block in blocks.iter_mut() { - s.read_data(&mut block.contents)?; - } - // Stop the read - s.card_command(CMD12, 0)?; + + if blocks.len() == 1 { + // Start a single-block read + self.card_command(CMD17, start_idx)?; + self.read_data(&mut blocks[0].contents)?; + } else { + // Start a multi-block read + self.card_command(CMD18, start_idx)?; + for block in blocks.iter_mut() { + self.read_data(&mut block.contents)?; } - Ok(()) - }) + // Stop the read + self.card_command(CMD12, 0)?; + } + Ok(()) } /// Write one or more blocks, starting at the given block index. @@ -295,74 +251,66 @@ where Some(CardType::SDHC) => start_block_idx.0, None => return Err(Error::CardNotFound), }; - self.with_chip_select(|s| { - if blocks.len() == 1 { - // Start a single-block write - s.card_command(CMD24, start_idx)?; - s.write_data(DATA_START_BLOCK, &blocks[0].contents)?; - s.wait_not_busy(Delay::new_write())?; - if s.card_command(CMD13, 0)? != 0x00 { - return Err(Error::WriteError); - } - if s.read_byte()? != 0x00 { - return Err(Error::WriteError); - } - } else { - // > It is recommended using this command preceding CMD25, some of the cards will be faster for Multiple - // > Write Blocks operation. Note that the host should send ACMD23 just before WRITE command if the host - // > wants to use the pre-erased feature - s.card_acmd(ACMD23, blocks.len() as u32)?; - // wait for card to be ready before sending the next command - s.wait_not_busy(Delay::new_write())?; - - // Start a multi-block write - s.card_command(CMD25, start_idx)?; - for block in blocks.iter() { - s.wait_not_busy(Delay::new_write())?; - s.write_data(WRITE_MULTIPLE_TOKEN, &block.contents)?; - } - // Stop the write - s.wait_not_busy(Delay::new_write())?; - s.write_byte(STOP_TRAN_TOKEN)?; + if blocks.len() == 1 { + // Start a single-block write + self.card_command(CMD24, start_idx)?; + self.write_data(DATA_START_BLOCK, &blocks[0].contents)?; + self.wait_not_busy(Delay::new_write())?; + if self.card_command(CMD13, 0)? != 0x00 { + return Err(Error::WriteError); } - Ok(()) - }) + if self.read_byte()? != 0x00 { + return Err(Error::WriteError); + } + } else { + // > It is recommended using this command preceding CMD25, some of the cards will be faster for Multiple + // > Write Blocks operation. Note that the host should send ACMD23 just before WRITE command if the host + // > wants to use the pre-erased feature + self.card_acmd(ACMD23, blocks.len() as u32)?; + // wait for card to be ready before sending the next command + self.wait_not_busy(Delay::new_write())?; + + // Start a multi-block write + self.card_command(CMD25, start_idx)?; + for block in blocks.iter() { + self.wait_not_busy(Delay::new_write())?; + self.write_data(WRITE_MULTIPLE_TOKEN, &block.contents)?; + } + // Stop the write + self.wait_not_busy(Delay::new_write())?; + self.write_byte(STOP_TRAN_TOKEN)?; + } + Ok(()) } /// Determine how many blocks this device can hold. fn num_blocks(&mut self) -> Result { - let num_blocks = self.with_chip_select(|s| { - let csd = s.read_csd()?; - debug!("CSD: {:?}", csd); - match csd { - Csd::V1(ref contents) => Ok(contents.card_capacity_blocks()), - Csd::V2(ref contents) => Ok(contents.card_capacity_blocks()), - } - })?; + let csd = self.read_csd()?; + debug!("CSD: {:?}", csd); + let num_blocks = match csd { + Csd::V1(ref contents) => contents.card_capacity_blocks(), + Csd::V2(ref contents) => contents.card_capacity_blocks(), + }; Ok(BlockCount(num_blocks)) } /// Return the usable size of this SD card in bytes. fn num_bytes(&mut self) -> Result { - self.with_chip_select(|s| { - let csd = s.read_csd()?; - debug!("CSD: {:?}", csd); - match csd { - Csd::V1(ref contents) => Ok(contents.card_capacity_bytes()), - Csd::V2(ref contents) => Ok(contents.card_capacity_bytes()), - } - }) + let csd = self.read_csd()?; + debug!("CSD: {:?}", csd); + match csd { + Csd::V1(ref contents) => Ok(contents.card_capacity_bytes()), + Csd::V2(ref contents) => Ok(contents.card_capacity_bytes()), + } } /// Can this card erase single blocks? pub fn erase_single_block_enabled(&mut self) -> Result { - self.with_chip_select(|s| { - let csd = s.read_csd()?; - match csd { - Csd::V1(ref contents) => Ok(contents.erase_single_block_enabled()), - Csd::V2(ref contents) => Ok(contents.erase_single_block_enabled()), - } - }) + let csd = self.read_csd()?; + match csd { + Csd::V1(ref contents) => Ok(contents.erase_single_block_enabled()), + Csd::V2(ref contents) => Ok(contents.erase_single_block_enabled()), + } } /// Read the 'card specific data' block. @@ -447,14 +395,6 @@ where } } - fn cs_high(&mut self) -> Result<(), Error> { - self.cs.set_high().map_err(|_| Error::GpioError) - } - - fn cs_low(&mut self) -> Result<(), Error> { - self.cs.set_low().map_err(|_| Error::GpioError) - } - /// Check the card is initialised. fn check_init(&mut self) -> Result<(), Error> { if self.card_type.is_none() { @@ -473,11 +413,6 @@ where // Assume it hasn't worked let mut card_type; trace!("Reset card.."); - // Supply minimum of 74 clock cycles without CS asserted. - s.cs_high()?; - s.write_bytes(&[0xFF; 10])?; - // Assert CS - s.cs_low()?; // Enter SPI mode. let mut delay = Delay::new(s.options.acquire_retries); for _attempts in 1.. { @@ -551,23 +486,10 @@ where Ok(()) }; let result = f(self); - self.cs_high()?; let _ = self.read_byte(); result } - /// Perform a function that might error with the chipselect low. - /// Always releases the chipselect, even if the function errors. - fn with_chip_select(&mut self, func: F) -> Result - where - F: FnOnce(&mut Self) -> Result, - { - self.cs_low()?; - let result = func(self); - self.cs_high()?; - result - } - /// Perform an application-specific command. fn card_acmd(&mut self, command: u8, arg: u32) -> Result { self.card_command(CMD55, 0)?; diff --git a/tests/utils/mod.rs b/tests/utils/mod.rs index 9c371a4..22ed2ea 100644 --- a/tests/utils/mod.rs +++ b/tests/utils/mod.rs @@ -41,6 +41,7 @@ use embedded_sdmmc::{Block, BlockCount, BlockDevice, BlockIdx}; pub static DISK_SOURCE: &[u8] = include_bytes!("../disk.img.gz"); #[derive(Debug)] +#[allow(dead_code)] pub enum Error { /// Failed to read the source image Io(std::io::Error), From c8282505cd6e41dc32f3e5a21711f6e29483d214 Mon Sep 17 00:00:00 2001 From: Jonathan Pallant Date: Fri, 12 Jul 2024 14:03:44 +0100 Subject: [PATCH 15/17] Minor clean-ups of docs. Trying to make the cargo-doc output look good. Also adds an Apache-2.0 NOTICE file. --- LICENSE-MIT | 3 ++- NOTICE | 6 ++++++ README.md | 14 +++++++++----- src/blockdevice.rs | 23 +++++++++++++---------- src/fat/bpb.rs | 7 ++++--- src/fat/ondiskdirentry.rs | 7 ++++--- src/filesystem/directory.rs | 7 +++---- src/filesystem/files.rs | 4 ++-- src/filesystem/timestamp.rs | 7 ++++--- src/lib.rs | 9 ++++----- src/sdcard/mod.rs | 8 +++----- src/volume_mgr.rs | 7 +++++-- 12 files changed, 59 insertions(+), 43 deletions(-) create mode 100644 NOTICE diff --git a/LICENSE-MIT b/LICENSE-MIT index ec19225..b59c97c 100644 --- a/LICENSE-MIT +++ b/LICENSE-MIT @@ -1,4 +1,5 @@ -Copyright (c) 2018-2023 Jonathan 'theJPster' Pallant and the Rust Embedded Community developers +Copyright (c) 2018-2024 Jonathan 'theJPster' Pallant and the Rust Embedded Community developers +Copyright (c) 2011-2018 Bill Greiman Permission is hereby granted, free of charge, to any person obtaining a copy of this software and associated diff --git a/NOTICE b/NOTICE new file mode 100644 index 0000000..5dc1a0e --- /dev/null +++ b/NOTICE @@ -0,0 +1,6 @@ +# Copyright Notices + +This is a copyright notices file, as described by the Apache-2.0 license. + +Copyright (c) 2018-2024 Jonathan 'theJPster' Pallant and the Rust Embedded Community developers +Copyright (c) 2011-2018 Bill Greiman diff --git a/README.md b/README.md index 1e3194c..41985d2 100644 --- a/README.md +++ b/README.md @@ -59,10 +59,12 @@ let mut cont: VolumeManager<_, _, 6, 12, 4> = VolumeManager::new_with_limits(blo * Log over defmt or the common log interface (feature flags). ## No-std usage + This repository houses no examples for no-std usage, however you can check out the following examples: -- [Pi Pico](https://github.com/rp-rs/rp-hal-boards/blob/main/boards/rp-pico/examples/pico_spi_sd_card.rs) -- [STM32H7XX](https://github.com/stm32-rs/stm32h7xx-hal/blob/master/examples/sdmmc_fat.rs) -- [atsamd(pygamer)](https://github.com/atsamd-rs/atsamd/blob/master/boards/pygamer/examples/sd_card.rs) + +* [Pi Pico](https://github.com/rp-rs/rp-hal-boards/blob/main/boards/rp-pico/examples/pico_spi_sd_card.rs) +* [STM32H7XX](https://github.com/stm32-rs/stm32h7xx-hal/blob/master/examples/sdmmc_fat.rs) +* [atsamd(pygamer)](https://github.com/atsamd-rs/atsamd/blob/master/boards/pygamer/examples/sd_card.rs) ## Todo List (PRs welcome!) @@ -79,12 +81,14 @@ The changelog has moved to [CHANGELOG.md](/CHANGELOG.md) Licensed under either of - Apache License, Version 2.0 ([LICENSE-APACHE](LICENSE-APACHE) or - http://www.apache.org/licenses/LICENSE-2.0) + ) -- MIT license ([LICENSE-MIT](LICENSE-MIT) or http://opensource.org/licenses/MIT) +- MIT license ([LICENSE-MIT](LICENSE-MIT) or ) at your option. +Copyright notices are stored in the [NOTICE](./NOTICE) file. + ## Contribution Unless you explicitly state otherwise, any contribution intentionally diff --git a/src/blockdevice.rs b/src/blockdevice.rs index 76bd235..4564457 100644 --- a/src/blockdevice.rs +++ b/src/blockdevice.rs @@ -1,11 +1,12 @@ -//! Block Device support +//! Traits and types for working with Block Devices. //! //! Generic code for handling block devices, such as types for identifying //! a particular block on a block device by its index. -/// Represents a standard 512 byte block (also known as a sector). IBM PC -/// formatted 5.25" and 3.5" floppy disks, SD/MMC cards up to 1 GiB in size -/// and IDE/SATA Hard Drives up to about 2 TiB all have 512 byte blocks. +/// A standard 512 byte block (also known as a sector). +/// +/// IBM PC formatted 5.25" and 3.5" floppy disks, IDE/SATA Hard Drives up to +/// about 2 TiB, and almost all SD/MMC cards have 512 byte blocks. /// /// This library does not support devices with a block size other than 512 /// bytes. @@ -15,15 +16,17 @@ pub struct Block { pub contents: [u8; Block::LEN], } -/// Represents the linear numeric address of a block (or sector). The first -/// block on a disk gets `BlockIdx(0)` (which usually contains the Master Boot -/// Record). +/// The linear numeric address of a block (or sector). +/// +/// The first block on a disk gets `BlockIdx(0)` (which usually contains the +/// Master Boot Record). #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct BlockIdx(pub u32); -/// Represents the a number of blocks (or sectors). Add this to a `BlockIdx` -/// to get an actual address on disk. +/// The a number of blocks (or sectors). +/// +/// Add this to a `BlockIdx` to get an actual address on disk. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Debug, Copy, Clone, PartialEq, Eq, PartialOrd, Ord)] pub struct BlockCount(pub u32); @@ -34,7 +37,7 @@ pub struct BlockIter { current: BlockIdx, } -/// Represents a block device - a device which can read and write blocks (or +/// A block device - a device which can read and write blocks (or /// sectors). Only supports devices which are <= 2 TiB in size. pub trait BlockDevice { /// The errors that the `BlockDevice` can return. Must be debug formattable. diff --git a/src/fat/bpb.rs b/src/fat/bpb.rs index cc77900..f06f23e 100644 --- a/src/fat/bpb.rs +++ b/src/fat/bpb.rs @@ -6,9 +6,10 @@ use crate::{ }; use byteorder::{ByteOrder, LittleEndian}; -/// Represents a Boot Parameter Block. This is the first sector of a FAT -/// formatted partition, and it describes various properties of the FAT -/// filesystem. +/// A Boot Parameter Block. +/// +/// This is the first sector of a FAT formatted partition, and it describes +/// various properties of the FAT filesystem. pub struct Bpb<'a> { data: &'a [u8; 512], pub(crate) fat_type: FatType, diff --git a/src/fat/ondiskdirentry.rs b/src/fat/ondiskdirentry.rs index 14be74c..49b8bb2 100644 --- a/src/fat/ondiskdirentry.rs +++ b/src/fat/ondiskdirentry.rs @@ -3,7 +3,10 @@ use crate::{fat::FatType, Attributes, BlockIdx, ClusterId, DirEntry, ShortFileName, Timestamp}; use byteorder::{ByteOrder, LittleEndian}; -/// Represents a 32-byte directory entry as stored on-disk in a directory file. +/// A 32-byte directory entry as stored on-disk in a directory file. +/// +/// This is the same for FAT16 and FAT32 (except FAT16 doesn't use +/// first_cluster_hi). pub struct OnDiskDirEntry<'a> { data: &'a [u8], } @@ -38,8 +41,6 @@ impl<'a> core::fmt::Debug for OnDiskDirEntry<'a> { } } -/// Represents the 32 byte directory entry. This is the same for FAT16 and -/// FAT32 (except FAT16 doesn't use first_cluster_hi). impl<'a> OnDiskDirEntry<'a> { pub(crate) const LEN: usize = 32; pub(crate) const LEN_U32: u32 = 32; diff --git a/src/filesystem/directory.rs b/src/filesystem/directory.rs index 0ed53e8..b965995 100644 --- a/src/filesystem/directory.rs +++ b/src/filesystem/directory.rs @@ -7,8 +7,7 @@ use crate::{Error, RawVolume, VolumeManager}; use super::ToShortFileName; -/// Represents a directory entry, which tells you about -/// other files and directories. +/// A directory entry, which tells you about other files and directories. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Debug, PartialEq, Eq, Clone)] pub struct DirEntry { @@ -30,7 +29,7 @@ pub struct DirEntry { pub entry_offset: u32, } -/// Represents an open directory on disk. +/// A handle for an open directory on disk. /// /// Do NOT drop this object! It doesn't hold a reference to the Volume Manager /// it was created from and if you drop it, the VolumeManager will think you @@ -70,7 +69,7 @@ impl RawDirectory { } } -/// Represents an open directory on disk. +/// A handle for an open directory on disk, which closes on drop. /// /// In contrast to a `RawDirectory`, a `Directory` holds a mutable reference to /// its parent `VolumeManager`, which restricts which operations you can perform. diff --git a/src/filesystem/files.rs b/src/filesystem/files.rs index 462511a..5e52477 100644 --- a/src/filesystem/files.rs +++ b/src/filesystem/files.rs @@ -3,7 +3,7 @@ use crate::{ Error, RawVolume, VolumeManager, }; -/// Represents an open file on disk. +/// A handle for an open file on disk. /// /// Do NOT drop this object! It doesn't hold a reference to the Volume Manager /// it was created from and cannot update the directory entry if you drop it. @@ -37,7 +37,7 @@ impl RawFile { } } -/// Represents an open file on disk. +/// A handle for an open file on disk, which closes on drop. /// /// In contrast to a `RawFile`, a `File` holds a mutable reference to its /// parent `VolumeManager`, which restricts which operations you can perform. diff --git a/src/filesystem/timestamp.rs b/src/filesystem/timestamp.rs index 3e70cc0..ff2c0be 100644 --- a/src/filesystem/timestamp.rs +++ b/src/filesystem/timestamp.rs @@ -4,9 +4,10 @@ pub trait TimeSource { fn get_timestamp(&self) -> Timestamp; } -/// Represents an instant in time, in the local time zone. TODO: Consider -/// replacing this with POSIX time as a `u32`, which would save two bytes at -/// the expense of some maths. +/// A Gregorian Calendar date/time, in the local time zone. +/// +/// TODO: Consider replacing this with POSIX time as a `u32`, which would save +/// two bytes at the expense of some maths. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Copy, Clone, PartialOrd, Ord, PartialEq, Eq)] pub struct Timestamp { diff --git a/src/lib.rs b/src/lib.rs index 6466102..f1c4e02 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -135,7 +135,7 @@ macro_rules! warn { // // **************************************************************************** -/// Represents all the ways the functions in this crate can fail. +/// All the ways the functions in this crate can fail. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Debug, Clone)] pub enum Error @@ -211,7 +211,7 @@ where } } -/// Represents a partition with a filesystem within it. +/// A partition with a filesystem within it. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] #[derive(Debug, Copy, Clone, PartialEq, Eq)] pub struct RawVolume(SearchId); @@ -236,7 +236,7 @@ impl RawVolume { } } -/// Represents an open volume on disk. +/// An open volume on disk, which closes on drop. /// /// In contrast to a `RawVolume`, a `Volume` holds a mutable reference to its /// parent `VolumeManager`, which restricts which operations you can perform. @@ -354,8 +354,7 @@ pub enum VolumeType { Fat(FatVolume), } -/// A `VolumeIdx` is a number which identifies a volume (or partition) on a -/// disk. +/// A number which identifies a volume (or partition) on a disk. /// /// `VolumeIdx(0)` is the first primary partition on an MBR partitioned disk. #[cfg_attr(feature = "defmt-log", derive(defmt::Format))] diff --git a/src/sdcard/mod.rs b/src/sdcard/mod.rs index d53a15a..6593043 100644 --- a/src/sdcard/mod.rs +++ b/src/sdcard/mod.rs @@ -1,6 +1,4 @@ -//! The SD/MMC Protocol -//! -//! Implements the SD/MMC protocol on some generic SPI interface. +//! Implements the BlockDevice trait for an SD/MMC Protocol over SPI. //! //! This is currently optimised for readability and debugability, not //! performance. @@ -21,7 +19,7 @@ use crate::{debug, warn}; // Types and Implementations // **************************************************************************** -/// Represents an SD Card on an SPI bus. +/// Driver for an SD Card on an SPI bus. /// /// Built from an [`SpiDevice`] implementation and a Chip Select pin. /// @@ -201,7 +199,7 @@ where } } -/// Represents an SD Card on an SPI bus. +/// Inner details for the SD Card driver. /// /// All the APIs required `&mut self`. struct SdCardInner diff --git a/src/volume_mgr.rs b/src/volume_mgr.rs index 15e4f42..98b7781 100644 --- a/src/volume_mgr.rs +++ b/src/volume_mgr.rs @@ -18,8 +18,11 @@ use crate::{ }; use heapless::Vec; -/// A `VolumeManager` wraps a block device and gives access to the FAT-formatted -/// volumes within it. +/// Wraps a block device and gives access to the FAT-formatted volumes within +/// it. +/// +/// Tracks which files and directories are open, to prevent you from deleting +/// a file or directory you currently have open. #[derive(Debug)] pub struct VolumeManager< D, From b178599a55a8f52ebf5344cb8bbc789b20076f7c Mon Sep 17 00:00:00 2001 From: Jonathan Pallant Date: Fri, 12 Jul 2024 13:40:39 +0100 Subject: [PATCH 16/17] Update CHANGELOG. Picked up a bunch of changes we forgot to add as the PRs came in. --- CHANGELOG.md | 36 ++++++++++++++++++++++++++++++++---- 1 file changed, 32 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 1c85c56..ecb40f3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,7 +8,33 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic ### Changed -- Updated to `heapless` ^0.8 +- None + +### Added + +- None + +### Removed + +- None + +## [Version 0.8.0] - 2024-07-12 + +### Changed + +- Fixed a bug when seeking backwards through files. +- Updated to `heapless-0.8` and `embedded-hal-bus-0.2`. +- No longer panics if the close fails when a `Volume` is dropped - the failure is instead ignored. + +### Added + +- `File` now has a `flush()` method. +- `File` now has a `close()` method. + +### Removed + +- __Breaking Change__: Removed `CS` type-param on `SdCard` - now we use the `SpiDevice` chip-select (closing [#126]) +- __Breaking Change__: Removed the 74 clock cycle 'init' sequence - now applications must do this ## [Version 0.7.0] - 2024-02-04 @@ -31,8 +57,9 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic ### Removed -* None +- None +[#126]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/issues/126 [#74]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/issues/74 [embedded-hal]: https://crates.io/crates/embedded-hal @@ -128,7 +155,7 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic - Reduce delay waiting for response. Big speed improvements. -## [Version 0.1.0] - 2018-12-23 +## [Version 0.1.1] - 2018-12-23 ### Changed @@ -139,7 +166,8 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic [Keep a Changelog]: http://keepachangelog.com/en/1.0.0/ [Semantic Versioning]: http://semver.org/spec/v2.0.0.html -[Unreleased]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.7.0...develop +[Unreleased]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.8.0...develop +[Version 0.8.0]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.8.0...v0.7.0 [Version 0.7.0]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.7.0...v0.6.0 [Version 0.6.0]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.6.0...v0.5.0 [Version 0.5.0]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.5.0...v0.4.0 From a1cc45553259625638b09c7ca75865eb8e713bb2 Mon Sep 17 00:00:00 2001 From: Jonathan Pallant Date: Fri, 12 Jul 2024 13:40:49 +0100 Subject: [PATCH 17/17] Update version to 0.8.0 --- Cargo.toml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/Cargo.toml b/Cargo.toml index 1fa09ea..88412f7 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -8,7 +8,7 @@ license = "MIT OR Apache-2.0" name = "embedded-sdmmc" readme = "README.md" repository = "https://github.com/rust-embedded-community/embedded-sdmmc-rs" -version = "0.7.0" +version = "0.8.0" [dependencies] byteorder = {version = "1", default-features = false}