Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

SD Filesystem compatible with 8266 File, using latest SdFat #5525

Merged
merged 23 commits into from
Mar 6, 2019

Conversation

earlephilhower
Copy link
Collaborator

@earlephilhower earlephilhower commented Dec 19, 2018

--edit-- Ready for trial, memory error cleaned up, SDWebServer demo and host tests running locally

Arduino forked a copy of SD lib several years ago, put their own wrapper
around it, and it's been languishing in our ESP8266 libraries ever since
as SD. It doesn't support long file names, has class names which
conflict with the ESP8266 internal names, and hasn't been updated in
ages.

The original author of the SD library has continued work in the
meantime, and produced a very feature rich implementation of SdFat. It
unfortunately also conflicts with the class names we use in ESP8266
Arduino and has a different API than the internal SPIFFS or proposed
LittleFS filesystem objects.

This PR puts a wrapper around the latest and greatest SdFat library,
by forking it and wrapping its classes in a private namespace "sdfat,"
and making as thin a wrapper as possible around it to conform to
the ESP8266 FS, File, and Dir classes.

This PR also removes the Arduino SD.h class library and rewrites it
using the new SDFS filesystem to make everything in the ESP8266
Arduino core compatible with each other.

By doing so it lets us use a single interface for anything needing a
file instead of multiple ones (see SDWebServer and how a different
object is needed vs. one serving from SPIFFS even though the logic is
all the same). Same for BearSSL's CertStores and probably a few others
I've missed, cleaning up our code base significantly.

Like LittleFS, silently create directories when a file is created with
a subdirectory specifier ("/path/to/file.txt") if they do not yet exist.

Adds a blacklist of sketches to skip in the CI process (because SdFat
has many examples which do not build properly on the ESP8266).

Now that LittleFS and SDFS have directory support, the FS needs to be
able to communicate whether a name is one or the other. Add a simple
bool FS::isDirectory() and bool FS::isFile() method. SPIFFS doesn't
have directories, so if it's valid it's a file and reported as such.

Add ::mkdir/::rmdir to the FS class to allow users to make and destroy
subdirectories. SPIFFS directory operations will, of course, fail
and return false.

Emulate a 16MB SD card and allow test runner to exercise it by using
a custom SdFat HOST_MOCK-enabled object.

Throw out the original Arduino SD.h class and rewrite from scratch using
only the ESP8266 native SDFS calls. This makes "SD" based applications
compatible with normal ESP8266 "File" and "FS" and "SPIFFS" operations.

The only major visible change for users is that long filenames now are
fully supported and work without any code changes. If there are static
arrays of 11 bytes for old 8.3 names in code, they will need to be
adjusted.

While it is recommended to use the more powerful SDFS class to access SD
cards, this SD.h wrapper allows for use of existing Arduino libraries
which are built to only with with that SD class.

Additional helper functions added to ESP8266 native Filesystem:: classes
to help support this portability.

The rewrite is good enough to run the original SDWebServer and SD
example code without any changes.

@pieman64
Copy link

@earlephilhower what does SdFat offer that is not available with SPIFF's?

@earlephilhower
Copy link
Collaborator Author

It's not for internal flash, only external SD cards. You would not want to run FAT on flash w/o wear levelling, it would destroy your chip in no time!

The idea is this lets you use internal flash w/SPIFFS or LittlleFS or external SD card with no code changes by unifying the underlying objects.

@earlephilhower
Copy link
Collaborator Author

Already running the speed test in the LittleFS PR by simply passing in the SDFS variable instead of SPIFFS or LittleFS and setting the CS pin and SPI speed (10MHz in this instance, my setup is not so nice):

Beginning SDFS test
Creating 512KB file, may take a while...
==> Time to write 512KB in 256b chunks = 3165 milliseconds
==> Created file size = 524288
Reading 512KB file sequentially in 256b chunks
==> Time to read 512KB sequentially in 256b chunks = 1076 milliseconds = 487000 bytes/s
Reading 512KB file MISALIGNED in flash and RAM sequentially in 256b chunks
==> Time to read 512KB sequentially MISALIGNED in flash and RAM in 256b chunks = 1137 milliseconds = 461000 bytes/s
Reading 512KB file in reverse by 256b chunks
==> Time to read 512KB in reverse in 256b chunks = 4248 milliseconds = 123000 bytes/s
Writing 64K file in 1-byte chunks
==> Time to write 64KB in 1b chunks = 468 milliseconds = 140000 bytes/s
Reading 64K file in 1-byte chunks
==> Time to read 64KB in 1b chunks = 2293 milliseconds = 28000 bytes/s

@liebman
Copy link
Contributor

liebman commented Dec 19, 2018

I wonder if its possible to submit a PR to the SdFat to make the namespace a compile time option. That would eliminate the need for a fork.

@earlephilhower
Copy link
Collaborator Author

@liebman, good point. I can wrap my namespace changes (3 lines in every lib source and 1 in each example) in a #ifdef. There are still issues with the base SdFat examples, though, where some don't compile for non-AVR(or other specific HW) and emit #errors. That's what's making the CI fail now.

@devyte , @d-a-v, any issues with me adding a blacklist to the CI test runner to stop trying to build carefully selected examples (AVR-only, etc.)? Then I can see about getting a PR into @greiman with the minor namespace patch and we can use the original lib sources (yay, I do not want to maintain other people's libs!).

@earlephilhower earlephilhower changed the title WIP - SD Filesystem compatible with 8266 File, using latest SdFat SD Filesystem compatible with 8266 File, using latest SdFat Dec 19, 2018
@earlephilhower earlephilhower changed the title SD Filesystem compatible with 8266 File, using latest SdFat WIP - SD Filesystem compatible with 8266 File, using latest SdFat Dec 20, 2018
@earlephilhower
Copy link
Collaborator Author

@liebman, per greiman/SdFat#121, it looks like SdFat is hitting maintenance mode in favor of a new exFAT compatible version. For now I'll just keep my own private fork (since it's going maintenance, don't expect to have to pull many changes from upstream). When SdExFat or whatever it is called appears, it should be a simple process to migrate to that new lib (either again with a fork or a PR to the main repo).

Arduino forked a copy of SD lib several years ago, put their own wrapper
around it, and it's been languishing in our ESP8266 libraries ever since
as SD. It doesn't support long file names, has class names which
conflict with the ESP8266 internal names, and hasn't been updated in
ages.

The original author of the SD library has continued work in the
meantime, and produced a very feature rich implementation of SdFat. It
unfortunately also conflicts with the class names we use in ESP8266
Arduino and has a different API than the internal SPIFFS or proposed
LittleFS filesystem objects.

This PR puts a wrapper around the latest and greatest SdFat library,
by forking it and wrapping its classes in a private namespace "sdfat,"
and making as thin a wrapper as possible around it to conform to
the ESP8266 FS, File, and Dir classes.

This PR also removes the Arduino SD.h class library and rewrites it
using the new SDFS filesystem to make everything in the ESP8266
Arduino core compatible with each other.

By doing so it lets us use a single interface for anything needing a
file instead of multiple ones (see SDWebServer and how a different
object is needed vs. one serving from SPIFFS even though the logic is
all the same). Same for BearSSL's CertStores and probably a few others
I've missed, cleaning up our code base significantly.

Like LittleFS, silently create directories when a file is created with
a subdirectory specifier ("/path/to/file.txt") if they do not yet exist.

Adds a blacklist of sketches to skip in the CI process (because SdFat
has many examples which do not build properly on the ESP8266).

Now that LittleFS and SDFS have directory support, the FS needs to be
able to communicate whether a name is one or the other.  Add a simple
bool FS::isDirectory() and bool FS::isFile() method.  SPIFFS doesn't
have directories, so if it's valid it's a file and reported as such.

Add ::mkdir/::rmdir to the FS class to allow users to make and destroy
subdirectories.  SPIFFS directory operations will, of course, fail
and return false.

Emulate a 16MB SD card and allow test runner to exercise it by using
a custom SdFat HOST_MOCK-enabled object.

Throw out the original Arduino SD.h class and rewrite from scratch using
only the ESP8266 native SDFS calls.  This makes "SD" based applications
compatible with normal ESP8266 "File" and "FS" and "SPIFFS" operations.

The only major visible change for users is that long filenames now are
fully supported and work without any code changes.  If there are static
arrays of 11 bytes for old 8.3 names in code, they will need to be
adjusted.

While it is recommended to use the more powerful SDFS class to access SD
cards, this SD.h wrapper allows for use of existing Arduino libraries
which are built to only with with that SD class.

Additional helper functions added to ESP8266 native Filesystem:: classes
to help support this portability.

The rewrite is good enough to run the original SDWebServer and SD
example code without any changes.
@earlephilhower earlephilhower changed the title WIP - SD Filesystem compatible with 8266 File, using latest SdFat SD Filesystem compatible with 8266 File, using latest SdFat Dec 21, 2018
@earlephilhower
Copy link
Collaborator Author

Wow, looks like this PR deletes a net of over 4,000 lines of code from the repo. Yay!

For brave enough folks, the code now runs pretty stably and well. I'm able to just build and run the existing SDWebServer demo without any changes and it runs even better than before (it now has long file name support, with no user code changes).

Allows for configuration values to be passed into a filesystem via the
begin method.  By default, a FS will receive a nullptr and should so
whatever is appropriate.

The base FSConfig class has one parameter, _autoFormat, set by the
default constructor to true.

For SPIFFS, you can now disable auto formatting on mount failure by
passing in a FSConfig(false) object.

For SDFS a SDFSConfig parameter can be passed into config specifying the
chip select and SPI configuration.  If nothing is passed in, the begin
will fail since there are no safe default values here.
Earle F. Philhower, III and others added 3 commits January 3, 2019 12:07
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.
@earlephilhower earlephilhower added this to the 2.6.0 milestone Feb 1, 2019
@d-a-v d-a-v requested a review from devyte February 17, 2019 22:19
cores/esp8266/FS.cpp Show resolved Hide resolved
cores/esp8266/FS.cpp Show resolved Hide resolved
libraries/SD/src/SD.h Show resolved Hide resolved
libraries/SDFS/src/SDFS.h Show resolved Hide resolved
libraries/SDFS/src/SDFS.h Show resolved Hide resolved
libraries/SDFS/src/SDFS.h Outdated Show resolved Hide resolved
libraries/SDFS/src/SDFSFormatter.h Outdated Show resolved Hide resolved
Use the new polledTimeout class to ensure a yield every 5ms while
formatting.

Add in default case handling and some debug messages when invalid inputs
specified.
cores/esp8266/spiffs_api.h Outdated Show resolved Hide resolved
libraries/SD/src/SD.h Outdated Show resolved Hide resolved
libraries/SDFS/src/SDFS.cpp Outdated Show resolved Hide resolved
setConfig now can take a parameter defined directly in the call by using
a const &ref to it, leading to one less line of code to write and
cleaner reading of the code.

Also clean up SDFS implementation pointer definition.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants