Merge pull request #80 from rust-embedded-community/more-api-cleanups

Author: thejpster

.github/workflows/rust.yml

@@ -14,7 +14,7 @@ jobs:
     runs-on: ubuntu-latest
     strategy:
       matrix:
-        features: ['log', 'defmt-log']
+        features: ['log', 'defmt-log', '""']
     steps:
     - uses: actions/checkout@v1
     - name: Build

CHANGELOG.md

@@ -10,6 +10,8 @@ and this project adheres to [Semantic Versioning](http://semver.org/spec/v2.0.0.
 [Unreleased]: https://github.com/rust-embedded-community/embedded-sdmmc-rs/compare/v0.4.0...develop
 
 - Renamed `Controller` to `VolumeManager`, to better describe what it does.
+- Renamed `SdMmcSpi` to `SdCard`
+- Renamed `BlockSpi` to `AcquiredSdCard`
 
 ## [Version 0.4.0](https://github.com/rust-embedded-community/embedded-sdmmc-rs/releases/tag/v0.4.0)
 

Cargo.toml

@@ -1,25 +1,25 @@
 [package]
-name = "embedded-sdmmc"
-version = "0.4.0"
 authors = ["Jonathan 'theJPster' Pallant <github@thejpster.org.uk>"]
+categories = ["embedded", "no-std"]
 description = "A basic SD/MMC driver for Embedded Rust."
+edition = "2021"
 keywords = ["sdcard", "mmc", "embedded", "fat32"]
-categories = ["embedded", "no-std"]
 license = "MIT OR Apache-2.0"
-repository = "https://github.com/rust-embedded-community/embedded-sdmmc-rs"
-edition = "2021"
+name = "embedded-sdmmc"
 readme = "README.md"
+repository = "https://github.com/rust-embedded-community/embedded-sdmmc-rs"
+version = "0.4.0"
 
 [dependencies]
+byteorder = {version = "1", default-features = false}
+defmt = {version = "0.3", optional = true}
 embedded-hal = "0.2.3"
-byteorder = { version = "1", default-features = false }
-log = { version = "0.4", default-features = false, optional = true }
-defmt = { version = "0.3", optional = true }
+log = {version = "0.4", default-features = false, optional = true}
 
 [dev-dependencies]
-hex-literal = "0.3"
 env_logger = "0.9"
+hex-literal = "0.3"
 
 [features]
-defmt-log = [ "defmt" ]
-default = [ "log" ]
+default = ["log"]
+defmt-log = ["defmt"]

README.md

@@ -11,23 +11,29 @@ designed for readability and simplicity over performance.
 You will need something that implements the `BlockDevice` trait, which can read and write the 512-byte blocks (or sectors) from your card. If you were to implement this over USB Mass Storage, there's no reason this crate couldn't work with a USB Thumb Drive, but we only supply a `BlockDevice` suitable for reading SD and SDHC cards over SPI.
 
 ```rust
-// Build an SD Card interface out of an SPI device
-let mut spi_dev = embedded_sdmmc::SdMmcSpi::new(sdmmc_spi, sdmmc_cs);
-// Try and initialise the SD card
-let block_dev = spi_dev.acquire()?;
-// The SD Card was initialised, and we have a `BlockSpi` object
-// representing the initialised card.
-write!(uart, "Card size is {} bytes", block_dev.card_size_bytes()?)?;
+// Build an SD Card interface out of an SPI device, a chip-select pin and a delay object
+let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, sdmmc_cs, 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.
-let mut cont = embedded_sdmmc::VolumeManager::new(block_dev, time_source);
-// Try and access Volume 0 (i.e. the first partition)
-let mut volume = cont.get_volume(embedded_sdmmc::VolumeIdx(0))?;
-writeln!(uart, "Volume 0: {:?}", v)?;
-// Open the root directory
+// To do this we need a Volume Manager. It will take ownership of the block device.
+let mut volume_mgr = embedded_sdmmc::VolumeManager::new(sdcard, time_source);
+// Try and access Volume 0 (i.e. the first partition).
+// The volume object holds information about the filesystem on that volume.
+// It doesn't hold a reference to the Volume Manager and so must be passed back
+// to every Volume Manager API call. This makes it easier to handle multiple
+// volumes in parallel.
+let mut volume0 = volume_mgr.get_volume(embedded_sdmmc::VolumeIdx(0))?;
+println!("Volume 0: {:?}", volume0);
+// Open the root directory (passing in the volume we're using).
 let root_dir = volume_mgr.open_root_dir(&volume0)?;
 // Open a file called "MY_FILE.TXT" in the root directory
 let mut my_file = volume_mgr.open_file_in_dir(
-    &mut volume0, &root_dir, "MY_FILE.TXT", embedded_sdmmc::Mode::ReadOnly)?;
+    &mut volume0,
+    &root_dir,
+    "MY_FILE.TXT",
+    embedded_sdmmc::Mode::ReadOnly,
+)?;
 // Print the contents of the file
 while !my_file.eof() {
     let mut buffer = [0u8; 32];
@@ -37,7 +43,7 @@ while !my_file.eof() {
     }
 }
 volume_mgr.close_file(&volume0, my_file)?;
-volume_mgr.close_dir(&volume0, root_dir)?;
+volume_mgr.close_dir(&volume0, root_dir);
 ```
 
 ### Open directories and files

examples/readme_test.rs

@@ -0,0 +1,115 @@
+//! This is the code from the README.md file.
+//!
+//! We add enough stuff to make it compile, but it won't run because our fake
+//! SPI doesn't do any replies.
+
+struct FakeSpi();
+
+impl embedded_hal::blocking::spi::Transfer<u8> for FakeSpi {
+    type Error = core::convert::Infallible;
+    fn transfer<'w>(&mut self, words: &'w mut [u8]) -> Result<&'w [u8], Self::Error> {
+        Ok(words)
+    }
+}
+
+impl embedded_hal::blocking::spi::Write<u8> for FakeSpi {
+    type Error = core::convert::Infallible;
+    fn write<'w>(&mut self, _words: &'w [u8]) -> Result<(), Self::Error> {
+        Ok(())
+    }
+}
+
+struct FakeCs();
+
+impl embedded_hal::digital::v2::OutputPin for FakeCs {
+    type Error = core::convert::Infallible;
+    fn set_low(&mut self) -> Result<(), Self::Error> {
+        Ok(())
+    }
+
+    fn set_high(&mut self) -> Result<(), Self::Error> {
+        Ok(())
+    }
+}
+
+struct FakeDelayer();
+
+impl embedded_hal::blocking::delay::DelayUs<u8> for FakeDelayer {
+    fn delay_us(&mut self, us: u8) {
+        std::thread::sleep(std::time::Duration::from_micros(u64::from(us)));
+    }
+}
+
+struct FakeTimesource();
+
+impl embedded_sdmmc::TimeSource for FakeTimesource {
+    fn get_timestamp(&self) -> embedded_sdmmc::Timestamp {
+        embedded_sdmmc::Timestamp {
+            year_since_1970: 0,
+            zero_indexed_month: 0,
+            zero_indexed_day: 0,
+            hours: 0,
+            minutes: 0,
+            seconds: 0,
+        }
+    }
+}
+
+#[derive(Debug, Clone)]
+enum Error {
+    Filesystem(embedded_sdmmc::Error<embedded_sdmmc::SdCardError>),
+    Disk(embedded_sdmmc::SdCardError),
+}
+
+impl From<embedded_sdmmc::Error<embedded_sdmmc::SdCardError>> for Error {
+    fn from(value: embedded_sdmmc::Error<embedded_sdmmc::SdCardError>) -> Error {
+        Error::Filesystem(value)
+    }
+}
+
+impl From<embedded_sdmmc::SdCardError> for Error {
+    fn from(value: embedded_sdmmc::SdCardError) -> Error {
+        Error::Disk(value)
+    }
+}
+
+fn main() -> Result<(), Error> {
+    let sdmmc_spi = FakeSpi();
+    let sdmmc_cs = FakeCs();
+    let delay = FakeDelayer();
+    let time_source = FakeTimesource();
+    // 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);
+    // 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.
+    // To do this we need a Volume Manager. It will take ownership of the block device.
+    let mut volume_mgr = embedded_sdmmc::VolumeManager::new(sdcard, time_source);
+    // Try and access Volume 0 (i.e. the first partition).
+    // The volume object holds information about the filesystem on that volume.
+    // It doesn't hold a reference to the Volume Manager and so must be passed back
+    // to every Volume Manager API call. This makes it easier to handle multiple
+    // volumes in parallel.
+    let mut volume0 = volume_mgr.get_volume(embedded_sdmmc::VolumeIdx(0))?;
+    println!("Volume 0: {:?}", volume0);
+    // Open the root directory (passing in the volume we're using).
+    let root_dir = volume_mgr.open_root_dir(&volume0)?;
+    // Open a file called "MY_FILE.TXT" in the root directory
+    let mut my_file = volume_mgr.open_file_in_dir(
+        &mut volume0,
+        &root_dir,
+        "MY_FILE.TXT",
+        embedded_sdmmc::Mode::ReadOnly,
+    )?;
+    // Print the contents of the file
+    while !my_file.eof() {
+        let mut buffer = [0u8; 32];
+        let num_read = volume_mgr.read(&volume0, &mut my_file, &mut buffer)?;
+        for b in &buffer[0..num_read] {
+            print!("{}", *b as char);
+        }
+    }
+    volume_mgr.close_file(&volume0, my_file)?;
+    volume_mgr.close_dir(&volume0, root_dir);
+    Ok(())
+}

src/fat/volume.rs

@@ -1,18 +1,13 @@
 //! FAT volume
 
-#[cfg(feature = "log")]
-use log::{debug, trace, warn};
-
-#[cfg(feature = "defmt-log")]
-use defmt::{debug, trace, warn};
-
 use crate::{
+    debug,
     fat::{
         Bpb, Fat16Info, Fat32Info, FatSpecificInfo, FatType, InfoSector, OnDiskDirEntry,
         RESERVED_ENTRIES,
     },
-    Attributes, Block, BlockCount, BlockDevice, BlockIdx, Cluster, DirEntry, Directory, Error,
-    ShortFileName, TimeSource, VolumeManager, VolumeType,
+    trace, warn, Attributes, Block, BlockCount, BlockDevice, BlockIdx, Cluster, DirEntry,
+    Directory, Error, ShortFileName, TimeSource, VolumeManager, VolumeType,
 };
 use byteorder::{ByteOrder, LittleEndian};
 use core::convert::TryFrom;
@@ -149,7 +144,7 @@ impl FatVolume {
             }
             FatSpecificInfo::Fat32(_fat32_info) => {
                 // FAT32 => 4 bytes per entry
-                let fat_offset = cluster.0 as u32 * 4;
+                let fat_offset = cluster.0 * 4;
                 this_fat_block_num = self.lba_start + self.fat_start.offset_bytes(fat_offset);
                 let this_fat_ent_offset = (fat_offset % Block::LEN_U32) as usize;
                 volume_mgr

src/filesystem/filename.rs

@@ -81,12 +81,7 @@ impl ShortFileName {
                     }
                 }
                 _ => {
-                    let ch = if (b'a'..=b'z').contains(&ch) {
-                        // Uppercase characters only
-                        ch - 32
-                    } else {
-                        ch
-                    };
+                    let ch = ch.to_ascii_uppercase();
                     if seen_dot {
                         if (Self::FILENAME_BASE_MAX_LEN..Self::FILENAME_MAX_LEN).contains(&idx) {
                             sfn.contents[idx] = ch;

src/filesystem/timestamp.rs

@@ -27,7 +27,7 @@ pub struct Timestamp {
 impl Timestamp {
     /// Create a `Timestamp` from the 16-bit FAT date and time fields.
     pub fn from_fat(date: u16, time: u16) -> Timestamp {
-        let year = (1980 + (date >> 9)) as u16;
+        let year = 1980 + (date >> 9);
         let month = ((date >> 5) & 0x000F) as u8;
         let day = (date & 0x001F) as u8;
         let hours = ((time >> 11) & 0x001F) as u8;

src/lib.rs

@@ -21,9 +21,14 @@
 //! # struct DummyCsPin;
 //! # struct DummyUart;
 //! # struct DummyTimeSource;
+//! # struct DummyDelayer;
 //! # impl embedded_hal::blocking::spi::Transfer<u8> for  DummySpi {
 //! #   type Error = ();
-//! #   fn transfer<'w>(&mut self, data: &'w mut [u8]) -> Result<&'w [u8], ()> { Ok(&[0]) }
+//! #   fn transfer<'w>(&mut self, data: &'w mut [u8]) -> Result<&'w [u8], Self::Error> { Ok(&[0]) }
+//! # }
+//! # impl embedded_hal::blocking::spi::Write<u8> for  DummySpi {
+//! #   type Error = ();
+//! #   fn write(&mut self, data: &[u8]) -> Result<(), Self::Error> { Ok(()) }
 //! # }
 //! # impl embedded_hal::digital::v2::OutputPin for DummyCsPin {
 //! #   type Error = ();
@@ -33,18 +38,21 @@
 //! # impl embedded_sdmmc::TimeSource for DummyTimeSource {
 //! #   fn get_timestamp(&self) -> embedded_sdmmc::Timestamp { embedded_sdmmc::Timestamp::from_fat(0, 0) }
 //! # }
+//! # impl embedded_hal::blocking::delay::DelayUs<u8> for DummyDelayer {
+//! #   fn delay_us(&mut self, us: u8) {}
+//! # }
 //! # impl std::fmt::Write for DummyUart { fn write_str(&mut self, s: &str) -> std::fmt::Result { Ok(()) } }
 //! # use std::fmt::Write;
 //! # use embedded_sdmmc::VolumeManager;
-//! # fn main() -> Result<(), embedded_sdmmc::Error<embedded_sdmmc::SdMmcError>> {
+//! # fn main() -> Result<(), embedded_sdmmc::Error<embedded_sdmmc::SdCardError>> {
 //! # let mut sdmmc_spi = DummySpi;
 //! # let mut sdmmc_cs = DummyCsPin;
 //! # let time_source = DummyTimeSource;
-//! let mut spi_dev = embedded_sdmmc::SdMmcSpi::new(sdmmc_spi, sdmmc_cs);
-//! let block = spi_dev.acquire()?;
-//! println!("Card size {} bytes", block.card_size_bytes()?);
-//! let mut volume_mgr = VolumeManager::new(block, time_source);
-//! println!("Card size is still {} bytes", volume_mgr.device().card_size_bytes()?);
+//! # let delayer = DummyDelayer;
+//! let sdcard = embedded_sdmmc::SdCard::new(sdmmc_spi, sdmmc_cs, delayer);
+//! println!("Card size {} bytes", sdcard.num_bytes()?);
+//! let mut volume_mgr = VolumeManager::new(sdcard, time_source);
+//! println!("Card size is still {} bytes", volume_mgr.device().num_bytes()?);
 //! let mut volume0 = volume_mgr.get_volume(embedded_sdmmc::VolumeIdx(0))?;
 //! println!("Volume 0: {:?}", volume0);
 //! let root_dir = volume_mgr.open_root_dir(&volume0)?;
@@ -91,24 +99,53 @@ mod structure;
 pub mod blockdevice;
 pub mod fat;
 pub mod filesystem;
-pub mod sdmmc;
-pub mod sdmmc_proto;
+pub mod sdcard;
 
 pub use crate::blockdevice::{Block, BlockCount, BlockDevice, BlockIdx};
 pub use crate::fat::FatVolume;
 pub use crate::filesystem::{
     Attributes, Cluster, DirEntry, Directory, File, FilenameError, Mode, ShortFileName, TimeSource,
     Timestamp, MAX_FILE_SIZE,
 };
-pub use crate::sdmmc::Error as SdMmcError;
-pub use crate::sdmmc::{BlockSpi, SdMmcSpi};
+pub use crate::sdcard::Error as SdCardError;
+pub use crate::sdcard::SdCard;
 
 mod volume_mgr;
 pub use volume_mgr::VolumeManager;
 
 #[deprecated]
 pub use volume_mgr::VolumeManager as Controller;
 
+#[cfg(all(feature = "defmt-log", feature = "log"))]
+compile_error!("Cannot enable both log and defmt-log");
+
+#[cfg(feature = "log")]
+use log::{debug, trace, warn};
+
+#[cfg(feature = "defmt-log")]
+use defmt::{debug, trace, warn};
+
+#[cfg(all(not(feature = "defmt-log"), not(feature = "log")))]
+#[macro_export]
+/// Like log::debug! but does nothing at all
+macro_rules! debug {
+    ($($arg:tt)+) => {};
+}
+
+#[cfg(all(not(feature = "defmt-log"), not(feature = "log")))]
+#[macro_export]
+/// Like log::trace! but does nothing at all
+macro_rules! trace {
+    ($($arg:tt)+) => {};
+}
+
+#[cfg(all(not(feature = "defmt-log"), not(feature = "log")))]
+#[macro_export]
+/// Like log::warn! but does nothing at all
+macro_rules! warn {
+    ($($arg:tt)+) => {};
+}
+
 // ****************************************************************************
 //
 // Public Types