Merge pull request #112 from rust-embedded-community/enhanced-shell

Enhanced shell example

Author: thejpster

CHANGELOG.md

@@ -9,8 +9,12 @@ The format is based on [Keep a Changelog] and this project adheres to [Semantic
 * `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.
 * Renamed the old types to `RawVolume`, `RawDirectory` and `RawFile`
 * New method `make_dir_in_dir`
-* Fixed long-standing bug that caused an integer overflow when a FAT32 directory
-  was longer than one cluster ([#74])
+* Fixed long-standing bug that caused an integer overflow when a FAT32 directory was longer than one cluster ([#74])
+* Updated 'shell' example to support `mkdir`, `tree` and relative/absolute paths
+* Renamed `Error::FileNotFound` to `Error::NotFound`
+* New API `change_dir` which changes a directory to point to some child directory (or the parent) without opening a new directory.
+* Empty strings and `"."` convert to `ShortFileName::this_dir()`
+* You can now open directories multiple times without error
 
 [#74]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/issues/74
 

examples/shell.rs

@@ -575,7 +575,7 @@ impl FatVolume {
                             match_name,
                             block,
                         ) {
-                            Err(Error::FileNotFound) => continue,
+                            Err(Error::NotFound) => continue,
                             x => return x,
                         }
                     }
@@ -592,7 +592,7 @@ impl FatVolume {
                         current_cluster = None;
                     }
                 }
-                Err(Error::FileNotFound)
+                Err(Error::NotFound)
             }
             FatSpecificInfo::Fat32(fat32_info) => {
                 let mut current_cluster = match dir.cluster {
@@ -609,7 +609,7 @@ impl FatVolume {
                             match_name,
                             block,
                         ) {
-                            Err(Error::FileNotFound) => continue,
+                            Err(Error::NotFound) => continue,
                             x => return x,
                         }
                     }
@@ -619,7 +619,7 @@ impl FatVolume {
                             _ => None,
                         }
                 }
-                Err(Error::FileNotFound)
+                Err(Error::NotFound)
             }
         }
     }
@@ -653,7 +653,7 @@ impl FatVolume {
                 return Ok(dir_entry.get_entry(fat_type, block, start));
             }
         }
-        Err(Error::FileNotFound)
+        Err(Error::NotFound)
     }
 
     /// Delete an entry from the given directory
@@ -691,7 +691,7 @@ impl FatVolume {
                     // Scan the cluster / root dir a block at a time
                     for block in first_dir_block_num.range(dir_size) {
                         match self.delete_entry_in_block(block_device, match_name, block) {
-                            Err(Error::FileNotFound) => {
+                            Err(Error::NotFound) => {
                                 // Carry on
                             }
                             x => {
@@ -731,7 +731,7 @@ impl FatVolume {
                     let block_idx = self.cluster_to_block(cluster);
                     for block in block_idx.range(BlockCount(u32::from(self.blocks_per_cluster))) {
                         match self.delete_entry_in_block(block_device, match_name, block) {
-                            Err(Error::FileNotFound) => {
+                            Err(Error::NotFound) => {
                                 // Carry on
                                 continue;
                             }
@@ -755,7 +755,7 @@ impl FatVolume {
         }
         // If we get here we never found the right entry in any of the
         // blocks that made up the directory
-        Err(Error::FileNotFound)
+        Err(Error::NotFound)
     }
 
     /// Deletes a directory entry from a block of directory entries.
@@ -790,7 +790,7 @@ impl FatVolume {
                     .map_err(Error::DeviceError);
             }
         }
-        Err(Error::FileNotFound)
+        Err(Error::NotFound)
     }
 
     /// Finds the next free cluster after the start_cluster and before end_cluster

src/fat/volume.rs

@@ -121,6 +121,19 @@ where
         Ok(d.to_directory(self.volume_mgr))
     }
 
+    /// Change to a directory, mutating this object.
+    ///
+    /// You can then read the directory entries with `iterate_dir` and `open_file_in_dir`.
+    pub fn change_dir<N>(&mut self, name: N) -> Result<(), Error<D::Error>>
+    where
+        N: ToShortFileName,
+    {
+        let d = self.volume_mgr.open_dir(self.raw_directory, name)?;
+        self.volume_mgr.close_dir(self.raw_directory).unwrap();
+        self.raw_directory = d;
+        Ok(())
+    }
+
     /// Look in a directory for a named file.
     pub fn find_directory_entry<N>(&mut self, name: N) -> Result<DirEntry, Error<D::Error>>
     where
@@ -161,6 +174,14 @@ where
         self.volume_mgr.delete_file_in_dir(self.raw_directory, name)
     }
 
+    /// Make a directory inside this directory
+    pub fn make_dir_in_dir<N>(&mut self, name: N) -> Result<(), Error<D::Error>>
+    where
+        N: ToShortFileName,
+    {
+        self.volume_mgr.make_dir_in_dir(self.raw_directory, name)
+    }
+
     /// Convert back to a raw directory
     pub fn to_raw_directory(self) -> RawDirectory {
         let d = self.raw_directory;

src/filesystem/directory.rs

@@ -91,6 +91,11 @@ impl ShortFileName {
             return Ok(ShortFileName::parent_dir());
         }
 
+        // Special case `.` (or blank), which means "this directory".
+        if name.is_empty() || name == "." {
+            return Ok(ShortFileName::this_dir());
+        }
+
         let mut idx = 0;
         let mut seen_dot = false;
         for ch in name.bytes() {
@@ -318,9 +323,16 @@ mod test {
         assert_eq!(sfn, ShortFileName::create_from_str("1.C").unwrap());
     }
 
+    #[test]
+    fn filename_empty() {
+        assert_eq!(
+            ShortFileName::create_from_str("").unwrap(),
+            ShortFileName::this_dir()
+        );
+    }
+
     #[test]
     fn filename_bad() {
-        assert!(ShortFileName::create_from_str("").is_err());
         assert!(ShortFileName::create_from_str(" ").is_err());
         assert!(ShortFileName::create_from_str("123456789").is_err());
         assert!(ShortFileName::create_from_str("12345678.ABCD").is_err());

src/filesystem/filename.rs

@@ -159,8 +159,8 @@ where
     TooManyOpenFiles,
     /// Bad handle given
     BadHandle,
-    /// That file doesn't exist
-    FileNotFound,
+    /// That file or directory doesn't exist
+    NotFound,
     /// You can't open a file twice or delete an open file
     FileAlreadyOpen,
     /// You can't open a directory twice

src/lib.rs

@@ -12,9 +12,9 @@ use crate::filesystem::{
     SearchIdGenerator, TimeSource, ToShortFileName, MAX_FILE_SIZE,
 };
 use crate::{
-    debug, Block, BlockCount, BlockDevice, BlockIdx, Error, RawVolume, Volume, VolumeIdx,
-    VolumeInfo, VolumeType, PARTITION_ID_FAT16, PARTITION_ID_FAT16_LBA, PARTITION_ID_FAT32_CHS_LBA,
-    PARTITION_ID_FAT32_LBA,
+    debug, 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;
 
@@ -206,11 +206,7 @@ where
     /// You can then read the directory entries with `iterate_dir`, or you can
     /// use `open_file_in_dir`.
     pub fn open_root_dir(&mut self, volume: RawVolume) -> Result<RawDirectory, Error<D::Error>> {
-        for dir in self.open_dirs.iter() {
-            if dir.cluster == ClusterId::ROOT_DIR && dir.volume_id == volume {
-                return Err(Error::DirAlreadyOpen);
-            }
-        }
+        // Opening a root directory twice is OK
 
         let directory_id = RawDirectory(self.id_generator.get());
         let dir_info = DirectoryInfo {
@@ -229,6 +225,8 @@ where
     /// Open a directory.
     ///
     /// You can then read the directory entries with `iterate_dir` and `open_file_in_dir`.
+    ///
+    /// Passing "." as the name results in opening the `parent_dir` a second time.
     pub fn open_dir<N>(
         &mut self,
         parent_dir: RawDirectory,
@@ -245,9 +243,25 @@ where
         let parent_dir_idx = self.get_dir_by_id(parent_dir)?;
         let volume_idx = self.get_volume_by_id(self.open_dirs[parent_dir_idx].volume_id)?;
         let short_file_name = name.to_short_filename().map_err(Error::FilenameError)?;
+        let parent_dir_info = &self.open_dirs[parent_dir_idx];
 
         // Open the directory
-        let parent_dir_info = &self.open_dirs[parent_dir_idx];
+        if short_file_name == ShortFileName::this_dir() {
+            // short-cut (root dir doesn't have ".")
+            let directory_id = RawDirectory(self.id_generator.get());
+            let dir_info = DirectoryInfo {
+                directory_id,
+                volume_id: self.open_volumes[volume_idx].volume_id,
+                cluster: parent_dir_info.cluster,
+            };
+
+            self.open_dirs
+                .push(dir_info)
+                .map_err(|_| Error::TooManyOpenDirs)?;
+
+            return Ok(directory_id);
+        }
+
         let dir_entry = match &self.open_volumes[volume_idx].volume_type {
             VolumeType::Fat(fat) => {
                 fat.find_directory_entry(&self.block_device, parent_dir_info, &short_file_name)?
@@ -260,14 +274,8 @@ where
             return Err(Error::OpenedFileAsDir);
         }
 
-        // Check it's not already open
-        for d in self.open_dirs.iter() {
-            if d.volume_id == self.open_volumes[volume_idx].volume_id
-                && d.cluster == dir_entry.cluster
-            {
-                return Err(Error::DirAlreadyOpen);
-            }
-        }
+        // We don't check if the directory is already open - directories hold
+        // no cached state and so opening a directory twice is allowable.
 
         // Remember this open directory.
         let directory_id = RawDirectory(self.id_generator.get());
@@ -493,7 +501,7 @@ where
             }
             _ => {
                 // We are opening a non-existant file, and that's not OK.
-                return Err(Error::FileNotFound);
+                return Err(Error::NotFound);
             }
         };
 
@@ -905,7 +913,7 @@ where
             Ok(_entry) => {
                 return Err(Error::FileAlreadyExists);
             }
-            Err(Error::FileNotFound) => {
+            Err(Error::NotFound) => {
                 // perfect, let's make it
             }
             Err(e) => {

src/volume_mgr.rs

@@ -237,10 +237,9 @@ fn open_dir_twice() {
         .open_root_dir(fat32_volume)
         .expect("open root dir");
 
-    assert!(matches!(
-        volume_mgr.open_root_dir(fat32_volume),
-        Err(embedded_sdmmc::Error::DirAlreadyOpen)
-    ));
+    let root_dir2 = volume_mgr
+        .open_root_dir(fat32_volume)
+        .expect("open it again");
 
     assert!(matches!(
         volume_mgr.open_dir(root_dir, "README.TXT"),
@@ -251,13 +250,12 @@ fn open_dir_twice() {
         .open_dir(root_dir, "TEST")
         .expect("open test dir");
 
-    assert!(matches!(
-        volume_mgr.open_dir(root_dir, "TEST"),
-        Err(embedded_sdmmc::Error::DirAlreadyOpen)
-    ));
+    let test_dir2 = volume_mgr.open_dir(root_dir, "TEST").unwrap();
 
     volume_mgr.close_dir(root_dir).expect("close root dir");
     volume_mgr.close_dir(test_dir).expect("close test dir");
+    volume_mgr.close_dir(test_dir2).expect("close test dir");
+    volume_mgr.close_dir(root_dir2).expect("close test dir");
 
     assert!(matches!(
         volume_mgr.close_dir(test_dir),
@@ -316,7 +314,7 @@ fn find_dir_entry() {
 
     assert!(matches!(
         volume_mgr.find_directory_entry(root_dir, "README.TXS"),
-        Err(embedded_sdmmc::Error::FileNotFound)
+        Err(embedded_sdmmc::Error::NotFound)
     ));
 }
 
@@ -345,7 +343,7 @@ fn delete_file() {
 
     assert!(matches!(
         volume_mgr.delete_file_in_dir(root_dir, "README2.TXT"),
-        Err(embedded_sdmmc::Error::FileNotFound)
+        Err(embedded_sdmmc::Error::NotFound)
     ));
 
     volume_mgr.close_file(file).unwrap();
@@ -356,12 +354,12 @@ fn delete_file() {
 
     assert!(matches!(
         volume_mgr.delete_file_in_dir(root_dir, "README.TXT"),
-        Err(embedded_sdmmc::Error::FileNotFound)
+        Err(embedded_sdmmc::Error::NotFound)
     ));
 
     assert!(matches!(
         volume_mgr.open_file_in_dir(root_dir, "README.TXT", Mode::ReadOnly),
-        Err(embedded_sdmmc::Error::FileNotFound)
+        Err(embedded_sdmmc::Error::NotFound)
     ));
 }
 

tests/directories.rs

@@ -67,7 +67,7 @@ fn open_files() {
 
     assert!(matches!(
         volume_mgr.open_file_in_dir(root_dir, "README.TXS", Mode::ReadOnly),
-        Err(Error::FileNotFound)
+        Err(Error::NotFound)
     ));
 
     // Create a new file