Add FS::setConfig to set FS-specific options

Add a new call, FS::setConfig(const {SDFS,SPIFFS}Config *cfg), which takes a FS-specific configuration object and copies any special settings on a per-FS basis. The call is only valid on unmounted filesystems, and checks the type of object passed in matches the FS being configured. Updates the docs and tests to utilize this new configuration method.
esp8266 · earlephilhower · Mar 6, 2019 · Dec 19, 2018 · Jan 2, 2019 · Jan 3, 2019
commit 241d0ffdff492e5d309c7df17062cf1754d3126f
diff --git a/cores/esp8266/FS.cpp b/cores/esp8266/FS.cpp
@@ -220,11 +220,19 @@ bool Dir::rewind() {
     return _impl->rewind();
 }
 
-bool FS::begin(const FSConfig *cfg) {
+bool FS::setConfig(const FSConfig *cfg) {
+    if (!_impl || !cfg) {
+        return false;
+    }
+
+    return _impl->setConfig(cfg);
+}
+
+bool FS::begin() {
     if (!_impl) {
         return false;
     }
-    return _impl->begin(cfg);
+    return _impl->begin();
 }
 
 void FS::end() {

diff --git a/cores/esp8266/FS.h b/cores/esp8266/FS.h
@@ -123,18 +123,38 @@ class FSConfig
 {
 public:
     FSConfig(bool autoFormat = true) {
-        _autoFormat = autoFormat;
+        _type = FSConfig::fsid::FSId;
+	_autoFormat = autoFormat;
     }
 
-    bool _autoFormat;
+    enum fsid { FSId = 0x00000000 };
+    FSConfig setAutoFormat(bool val = true) {
+        _autoFormat = val;
+        return *this;
+    }
+
+    uint32_t _type;
+    bool     _autoFormat;
+};
+
+class SPIFFSConfig : public FSConfig
+{
+public:
+    SPIFFSConfig(bool autoFormat = true) {
+        _type = SPIFFSConfig::fsid::FSId;
+	_autoFormat = autoFormat;
+    }
+    enum fsid { FSId = 0x53504946 };
 };
 
 class FS
 {
 public:
     FS(FSImplPtr impl) : _impl(impl) { }
 
-    bool begin(const FSConfig *cfg = nullptr);
+    bool setConfig(const FSConfig *cfg);
+
+    bool begin();
     void end();
 
     bool format();

diff --git a/cores/esp8266/FSImpl.h b/cores/esp8266/FSImpl.h
@@ -69,7 +69,8 @@ class DirImpl {
 class FSImpl {
 public:
     virtual ~FSImpl () { }
-    virtual bool begin(const FSConfig *cfg) = 0;
+    virtual bool setConfig(const FSConfig *cfg) = 0;
+    virtual bool begin() = 0;
     virtual void end() = 0;
     virtual bool format() = 0;
     virtual bool info(FSInfo& info) = 0;

diff --git a/cores/esp8266/spiffs_api.h b/cores/esp8266/spiffs_api.h
@@ -136,7 +136,16 @@ class SPIFFSImpl : public FSImpl
         return false;
     }
 
-    bool begin(const FSConfig *cfg) override
+    bool setConfig(const FSConfig *cfg) override
+    {
+        if ((cfg->_type != SPIFFSConfig::fsid::FSId) || (SPIFFS_mounted(&_fs) != 0)) {
+            return false;
+        }
+        _cfg = *static_cast<const SPIFFSConfig *>(cfg);
+	return true;
+    }
+
+    bool begin() override
     {
 #if defined(ARDUINO) && !defined(CORE_MOCK)
         if (&_SPIFFS_end <= &_SPIFFS_start)
@@ -152,7 +161,7 @@ class SPIFFSImpl : public FSImpl
         if (_tryMount()) {
             return true;
         }
-	if (!cfg || cfg->_autoFormat) {
+	if (_cfg._autoFormat) {
             auto rc = SPIFFS_format(&_fs);
             if (rc != SPIFFS_OK) {
                 DEBUGV("SPIFFS_format: rc=%d, err=%d\r\n", rc, _fs.err_code);
@@ -296,6 +305,8 @@ class SPIFFSImpl : public FSImpl
     std::unique_ptr<uint8_t[]> _workBuf;
     std::unique_ptr<uint8_t[]> _fdsBuf;
     std::unique_ptr<uint8_t[]> _cacheBuf;
+
+    SPIFFSConfig _cfg;
 };
 
 #define CHECKFD() while (_fd == 0) { panic(); }
@@ -531,5 +542,4 @@ class SPIFFSDirImpl : public DirImpl
     bool _valid;
 };
 
-
 #endif//spiffs_api_h
diff --git a/doc/filesystem.rst b/doc/filesystem.rst
@@ -125,6 +125,24 @@ directory into ESP8266 flash file system.
 File system object (SPIFFS)
 ---------------------------
 
+setConfig
+~~~~~~~~~
+
+.. code:: cpp
+
+    SPIFFSConfig cfg;
+    cfg.setAutoFormat(false);
+    SPIFFS.setConfig(&cfg);
+
+This method allows you to configure the parameters of a filesystem
+before mounting.  All filesystems have their own ``*Config`` (i.e.
+``SDFSConfig`` or ``SPIFFSConfig`` with their custom set of options.
+All filesystems allow explicitly enabling/disabling formatting when
+mounts fail.  If you do not call this ``setConfig`` method before
+perforing ``begin()``, you will get the filesystem's default
+behavior and configuration. By default, SPIFFS will autoformat the
+filesystem if it cannot mount it, while SDFS will not.
+
 begin
 ~~~~~
 
@@ -137,15 +155,6 @@ other FS APIs are used. Returns *true* if file system was mounted
 successfully, false otherwise.  With no options it will format SPIFFS
 if it is unable to mount it on the first try.
 
-.. code:: cpp
-
-    auto cfg = FSConfig(false);
-    SPIFFS.begin(&cfg);
-
-If a ``FSConfig`` object is passed in, with the autoFormat parameter
-set to ``false``, then if the filesystem is unable to be mounted it
-will return an error code instead of trying to format it.
-
 end
 ~~~
 

diff --git a/libraries/SD/src/SD.h b/libraries/SD/src/SD.h
@@ -33,7 +33,8 @@ class SDClass {
 public:
     boolean begin(uint8_t csPin, SPISettings cfg = SPI_HALF_SPEED) {
         auto fsCfg = SDFSConfig(csPin, cfg);
-        return (boolean)SDFS.begin(&fsCfg);
+	SDFS.setConfig(&fsCfg);
+        return (boolean)SDFS.begin();
     }
 
     void end(bool endSPI = true) {

diff --git a/libraries/SDFS/src/SDFS.cpp b/libraries/SDFS/src/SDFS.cpp
@@ -146,7 +146,7 @@ bool SDFSImpl::format() {
         return false;
     }
     SDFSFormatter formatter;
-    bool ret = formatter.format(&_fs, _csPin, _spiSettings);
+    bool ret = formatter.format(&_fs, _cfg._csPin, _cfg._spiSettings);
     return ret;
 }
 

diff --git a/libraries/SDFS/src/SDFS.h b/libraries/SDFS/src/SDFS.h
@@ -44,20 +44,50 @@ class SDFSDirImpl;
 class SDFSConfig : public FSConfig
 {
 public:
-    SDFSConfig(uint8_t cs, SPISettings spi) {
+    SDFSConfig() {
+        _type = SDFSConfig::fsid::FSId;
         _autoFormat = false;
-        _cs = cs;
-        _spi = spi;
+        _csPin = 4;
+        _spiSettings = SD_SCK_MHZ(10);
+	_part = 0;
+    }
+    SDFSConfig(uint8_t csPin, SPISettings spi) {
+        _type = SDFSConfig::fsid::FSId;
+        _autoFormat = false;
        _csPin = csPin;
+        _spiSettings = spi;
+	_part = 0;
+    }
+
+    enum fsid { FSId = 0x53444653 };
+
+    SDFSConfig setAutoFormat(bool val = true) {
+        _autoFormat = val;
+        return *this;
+    }
+    SDFSConfig setCSPin(uint8_t pin) {
+        _csPin = pin;
+        return *this;
+    }
+    SDFSConfig setSPI(SPISettings spi) {
+        _spiSettings = spi;
+        return *this;
+    }
+    SDFSConfig setPart(uint8_t part) {
+        _part = part;
+        return *this;
     }
 
-    uint8_t _cs;
-    SPISettings _spi;
+    // Inherit _type and _autoFormat
+    uint8_t     _csPin;
+    uint8_t     _part;
+    SPISettings _spiSettings;
 };
 
 class SDFSImpl : public FSImpl
 {
 public:
-    SDFSImpl() : _part(0), _csPin(-1), _spiSettings(SPI_FULL_SPEED), _mounted(false)
+    SDFSImpl() : _mounted(false)
     {
     }
 
@@ -98,20 +128,23 @@ class SDFSImpl : public FSImpl
         return _mounted ?_fs.rmdir(path) : false;
     }
 
-    bool begin(const FSConfig *cfg) override {
+    bool setConfig(const FSConfig *cfg) override
+    {
+        if ((cfg->_type != SDFSConfig::fsid::FSId) || _mounted) {
+            return false;
+        }
+	_cfg = *static_cast<const SDFSConfig *>(cfg);
+        return true;
+    }
+
+    bool begin() override {
         if (_mounted) {
             end();
         }
-        if (!cfg) {
-            return false; // You need to tell me the CS and SPI setup or I can't do anything!
-        }
-        const SDFSConfig *myCfg = static_cast<const SDFSConfig *>(cfg);
-        _csPin = myCfg->_cs;
-        _spiSettings = myCfg->_spi;
-        _mounted = _fs.begin(_csPin, _spiSettings);
-        if (!_mounted && myCfg->_autoFormat) {
+        _mounted = _fs.begin(_cfg._csPin, _cfg._spiSettings);
+        if (!_mounted && _cfg._autoFormat) {
             format();
-            _mounted = _fs.begin(_csPin, _spiSettings);
+            _mounted = _fs.begin(_cfg._csPin, _cfg._spiSettings);
         }
         return _mounted;
     }
@@ -123,12 +156,6 @@ class SDFSImpl : public FSImpl
 
     bool format() override;
 
-    // SDFS-only configuration calls
-    void setSDFSIOConfig(int8_t csPin, SPISettings spiConfig) {
-        _csPin = csPin;
-        _spiSettings = spiConfig;
-    }
-
 protected:
     friend class SDFileImpl;
     friend class SDFSDirImpl;
@@ -159,9 +186,7 @@ class SDFSImpl : public FSImpl
     }
 
     sdfat::SdFat _fs;
-    uint8_t      _part;
-    int8_t       _csPin;
-    SPISettings  _spiSettings;
+    SDFSConfig   _cfg;
     bool         _mounted;
 };
 

diff --git a/libraries/esp8266/keywords.txt b/libraries/esp8266/keywords.txt
@@ -93,7 +93,7 @@ SPIFFS	KEYWORD1
 SDFS	KEYWORD1
 File	KEYWORD1
 Dir	KEYWORD1
-FSConfig	KEYWORD1
+SPIFFSConfig	KEYWORD1
 
 # Seek enums
 SeekSet	LITERAL1

diff --git a/tests/host/fs/test_fs.cpp b/tests/host/fs/test_fs.cpp
@@ -51,10 +51,41 @@ static std::set<String> listDir (const char* path)
     return result;
 }
 
+TEST_CASE("SPIFFS checks the config object passed in", "[fs]")
+{
+    SPIFFS_MOCK_DECLARE(64, 8, 512, false);
+    FSConfig f;
+    SPIFFSConfig s;
+    SDFSConfig d;
+
+    REQUIRE_FALSE(SPIFFS.setConfig(&f));
+    REQUIRE(SPIFFS.setConfig(&s));
+    REQUIRE_FALSE(SPIFFS.setConfig(&d));
+}
+
+TEST_CASE("SDFS checks the config object passed in", "[fs]")
+{
+    SDFS_MOCK_DECLARE();
+    FSConfig f;
+    SPIFFSConfig s;
+    SDFSConfig d;
+
+    REQUIRE_FALSE(SDFS.setConfig(&f));
+    REQUIRE_FALSE(SDFS.setConfig(&s));
+    REQUIRE(SDFS.setConfig(&d));
+}
+
 TEST_CASE("FS can begin","[fs]")
 {
     SPIFFS_MOCK_DECLARE(64, 8, 512, false);
+    SPIFFSConfig cfg;
+    cfg.setAutoFormat(false);
+    SPIFFS.setConfig(&cfg);
+    REQUIRE_FALSE(SPIFFS.begin());
+    cfg.setAutoFormat(true);
+    SPIFFS.setConfig(&cfg);
     REQUIRE(SPIFFS.begin());
+    REQUIRE_FALSE(SPIFFS.setConfig(&cfg)); // Can't change config of mounted FS
 }
 
 TEST_CASE("FS can't begin with zero size","[fs]")
@@ -191,11 +222,11 @@ TEST_CASE("#1819 Can list all files with openDir(\"\")", "[fs][bugreport]")
 TEST_CASE("SDFS", "[sdfs]")
 {
     SDFS_MOCK_DECLARE();
-    SDFS.end();
     auto cfg = SDFSConfig(0, SD_SCK_MHZ(1));
-    SDFS.begin(&cfg);
+    SDFS.setConfig(&cfg);
     REQUIRE(SDFS.format());
-    REQUIRE(SDFS.begin(&cfg));
+    REQUIRE(SDFS.begin());
+    REQUIRE_FALSE(SDFS.setConfig(&cfg)); // Can't change config of mounted fs
     REQUIRE(SDFS.mkdir("/happy/face"));
     REQUIRE(SDFS.mkdir("/happy/nose"));
     REQUIRE(SDFS.rmdir("/happy/face"));
@@ -210,8 +241,7 @@ TEST_CASE("Files.ino example", "[sd]")
     SDFS_MOCK_DECLARE();
     SDFS.end();
     auto cfg = SDFSConfig(0, SD_SCK_MHZ(1));
-    SDFS.begin(&cfg);
-    SDFS.end();
+    SDFS.setConfig(&cfg);
     REQUIRE(SDFS.format());
     REQUIRE(SD.begin(4));
     REQUIRE_FALSE(SD.exists("example.txt"));
@@ -246,10 +276,8 @@ static String readFileSD(const char* name)
 TEST_CASE("Listfiles.ino example", "[sd]")
 {
     SDFS_MOCK_DECLARE();
-    SDFS.end();
     auto cfg = SDFSConfig(0, SD_SCK_MHZ(1));
-    SDFS.begin(&cfg);
-    SDFS.end();
+    SDFS.setConfig(&cfg);
     REQUIRE(SDFS.format());
     REQUIRE(SD.begin(4));