FsVolume begin() with explicit range of sectors

[PaulStoffregen]

Hi Bill. I'm sure you've heard about challenges on Teensy using SdFat for USB drives which have a variety of partition table formats. Rather than go down a path were we add massive patches to SdFat, I've been searching for a cleaner solution.

The main issue is FsVolume has partition table parsing built in, rather than as a separate layer.

bool begin(FsBlockDevice* blockDev, bool setCwv = true, uint8_t part = 1);

To allow us to build more complex partition table parsing as a separate layer, I needed to add another begin() method to FsVolume and other init functions where we tell it the range of sectors to access.

bool begin(FsBlockDevice* blockDev, bool setCwv, uint32_t firstSector, uint32_t numSectors);

This alternative is the same begin/init code but without MBR parsing, which allows us to create partition table parsing external to SdFat. This is the patch:

https://github.com/PaulStoffregen/SdFat/commit/1dd7b5436e1589caa8f51f47ecc8a14c5a683e87

The only other way I've been able to imagine is code within our FsBlockDevice instance which basically lies with a fake MBR when FsVolume begin() calls init functions which read sector 0. While that would allow us to avoid patching of SdFat, but it kinda feels like a bad idea. Or maybe not?

Anyway, just wanted to share where we're headed with SdFat on Teensy for non-SD media, and also ask if you'll consider this or any other API where partition table parsing is a separate layer from volume initialization? I really do appreciate all the incredible work you've put into SdFat. My hope is to publish it for Teensy users for SD and non-SD media in the best way.

[greiman]

Classes that begin with Sd like SdFs are for SD cards. The SD standard is only defined for a single partition with a specific layout.

I use the underlying classes for USB drives, flash and even hard drives.

Here is a test sketch that uses a USB driver.

// Edit SdFatConfig.h and enable generic block devices.
// #define USE_BLOCK_DEVICE_INTERFACE 1
#include "UsbMscDriver.h"

USB usb;
BulkOnly bulk(&usb);
UsbMscDriver usbKey(&bulk);

FsVolume key;
FsFile file;

void setup() {
  Serial.begin(9600);
  while (!Serial) {}
  Serial.println(F("Type any character to start"));
  while (!Serial.available()) {}
 
  if (!initUSB(&usb)) {
    Serial.println("initUSB failed");
    while(1){}
  }
  // Must set USE_BLOCK_DEVICE_INTERFACE non-zero in SdFatConfig.h
  if (!key.begin(&usbKey)) {
    Serial.println("key.begin failed");
    while(1) {}
  }
  if (!file.open("usbtest.txt", FILE_WRITE)) {
    Serial.println("file.open failed");
    while(1) {}    
  }
  file.println("test line");
  file.close();
  key.ls(LS_DATE | LS_SIZE);
}
void loop() {}

You can have partitions since the begin call for FsVolume is:

bool FsVolume::begin(FsBlockDevice* blockDev, bool setCwv = true, uint8_t part = 1);

Here is the driver wrapper:

#ifndef UsbMscDriver_h
#define UsbMscDriver_h
#include "SdFat.h"

// If the next include fails, install the USB_HOST_SHIELD library.
#include <masstorage.h>
//------------------------------------------------------------------------------
/** Simple USB init function.
 * \param[in] usb USB host object to be initialized.
 * \return true for success or false for failure.
 */
bool initUSB(USB* usb);
//------------------------------------------------------------------------------
/** Print debug messages if USB_FAT_DBG_MODE is nonzero */
#define USB_FAT_DBG_MODE 1
//------------------------------------------------------------------------------
/** Maximum time to initialize the USB bus */
#define TIMEOUT_MILLIS 40000L
class UsbMscDriver : public FsBlockDeviceInterface {
 public:
  UsbMscDriver(BulkOnly* bulk)  : m_lun(0), m_bulk(bulk) {}
  bool isBusy() {return false;}
  bool readSector(uint32_t sector, uint8_t* dst) {
    uint8_t rc = m_bulk->Read(m_lun, sector, 512, 1, dst);
    return 0 == rc;
  }
  uint32_t sectorCount() {
    return m_bulk->GetCapacity(m_lun);
  }
  bool syncDevice() {return true;}
  bool writeSector(uint32_t sector, const uint8_t* src) {
    return  0 == m_bulk->Write(m_lun, sector, 512, 1, src);    
  }
 
  bool readSectors(uint32_t sector, uint8_t* dst, size_t ns) {
    return 0 == m_bulk->Read(m_lun, sector, 512, ns, dst);   
  }
  bool writeSectors(uint32_t sector, const uint8_t* src, size_t ns) {
    return  0 == m_bulk->Write(m_lun, sector, 512, ns, src);      
  }

 private:
  uint8_t m_lun;
  BulkOnly* m_bulk;   
};
#endif  // UsbMscDriver_h

I don't plan to extend SdFat. SdFat has evolve from a version in 2008 for ATMEGA168-20PU with 16KB flash and 1KB RAM. It is not practical to support everything from UNO to modern multi-core embedded processors.

SdFat is really a toy and was never intended to be part of a professional level embedded system.

Arduino has been a huge disappointment. Over 35 years ago I was involved with early development of VxWorks. I hoped Arduino would evolve to at least the level of a 35 year old embedded OS.

[PaulStoffregen]

Yes, indeed that's how we've done it on Teensy. There's a class which inherits FsBlockDeviceInterface for the USB communication to read/write blocks, just like you said.

The limitation we're hitting is with this part.

You can have partitions since the begin call for FsVolume is:

You're right about this, you can have partitions. But the issue is you can only have partitions as FsVolume is able to understand. Currently FsVolume only understands basic MBR.

I completely understand your frustration with Arduino. If you're done with SdFat, I'm not here to pressure or persuade. Likewise if you never want to change the code. I just wanted to communicate about what we're doing on Teensy and if there's a better way or a path where Teensy's patches could converge with future SdFat, I'd like to move Teensy's changes in that direction.

[greiman]

FatVolume, ExFatVolume and FsVolume do not read or parse the MBR if the partition number is zero. It then assumes the device has a volume starting at sector zero. If I add volumeStart to the begin calls like this:

bool begin(FsBlockDevice* blockDev, bool setCwv = true, uint8_t part = 1, uint32_t volumeStart = 0);

The call vol.begin(&vol, true, 0, startSector) would look for a volume beginning at startSector. You don't need the sector count since that is determined by the BIOS parameter block in the startSector.

You could also use this call with part == 0.

bool init(FsBlockDevice* dev, uint8_t part, uint32_t volumeStart);

You can use whatever scheme you wish to layout the device.

[PaulStoffregen]

Yes, if you add volumeStart to future SdFat, I will delete my FsVolume begin() patches and use volumeStart. :)

[greiman]

I have done a first cut and the slight rearrangement of code actually saves a few bytes for UNO users who try to use SdFs.

I hear from UNO users every time I add a few bytes to SdFat.

I will put the mod in a new beta for a month or two before a stable release.

[PaulStoffregen]

Thanks, and no hurry. My hope is to eventually have Teensy's copy of SdFat differ as little as possible from your design.