device-os icon indicating copy to clipboard operation
device-os copied to clipboard

Add support for accessing the SPI flash LittleFS from user-level

Open tve opened this issue 5 years ago • 7 comments

At this point this PR is for discussion purposes. It is clearly incomplete. Is this even going into an interesting direction at all?

Problem

The purpose of this PR is to provide user-level access to the LittleFS filesystem on the SPI flash chip.

Solution

As a first step, this PR exposes all the LittleFS file and directory functions to user-level. It strips the lfs_t first parameter and inserts the filesystem of the SPI flash. A second step would be to provide a "wiring" wrapper to the raw lfs function, perhaps a clone of https://github.com/espressif/arduino-esp32/blob/master/libraries/FS/src/FSImpl.h

Steps to Test

No tests yet.

Example App

The following functions print the directory tree:

void fs_print_dir(lfs_dir_t *d, const char *path, int level) {
    while (true) {
        lfs_info info;
        int err = dir_read(d, &info);
        if (err == 0) return;
        if (err < 0) {
            Log.warn("dir_read->%d", err);
            return;
        }
        if (strcmp(info.name, ".") == 0 || strcmp(info.name, "..") == 0) continue;
        for (int i=0; i<level; i++) Serial.print("  ");
        Serial.printf("%s (%d)\n", info.name, info.size);
        if (info.type == LFS_TYPE_DIR) {
            char child[strlen(path)+strlen(info.name)+2];
            sprintf(child, "%s/%s", path, info.name);
            lfs_dir_t c;
            err = dir_open(&c, child);
            if (err != 0) {
                Log.warn("dir_open(%s)->%d", child, err);
                return;
            }
            fs_print_dir(&c, child, level+1);
        }
    }
}

void fs_print_tree() {
    lfs_dir_t root;
    int err = dir_open(&root, "");
    if (err != 0) {
        Serial.printf("Error opening root: %d\n", err);
        return;
    }
    Serial.println("----- directory tree");
    fs_print_dir(&root, "", 0);
    Serial.println("-----");
}

Output:

----- directory tree
sys (0)
  dct.bin (8391)
  openthread.dat (16)
  cellular_config.bin (21)
-----

Completeness

  • [x] User is totes amazing for contributing!
  • [x] Contributor has signed CLA (Info here)
  • [ ] Problem and Solution clearly stated
  • [ ] Run unit/integration/application tests on device
  • [ ] Added documentation
  • [ ] Added to CHANGELOG.md after merging (add links to docs and issues)

tve avatar Mar 02 '19 03:03 tve

No comment after over two months... I rebased my patch onto the latest develop branch, it still applies fine without changes. I can create a fresh PR for that, but if no-one cares I won't bother...

tve avatar May 17 '19 17:05 tve

Hi @tve ! Thank you for the submission. This somehow slipped through our fingers, so I'm apologizing for taking a long time to reply.

While this PR does in fact what it's supposed to do (exposing the filesystem access to the user applications), it unfortunately relies too much on LittleFS, which is not something we prefer to do unless absolutely necessary. Some pointers on why:

  1. Some future platform might use a completely different filesystem implementation and we'd like to keep our dynalib interface the same. Currently the usage of lfs_file_t and lfs_dir_t will completely prevent that, so we require some form of abstraction from the LittleFS specifics.
  2. ABI-stability/compatibility. In it's current form lfs_file_t and lfs_dir_t will be allocated by the user application which may be built with an older version of DeviceOS, which may be relying on an older version of LittleFS. Our user applications built with older DeviceOS can and do always run with newer DeviceOS system firmware. What happens if the newer DeviceOS uses newer LittleFS, which added additional fields to lfs_file_t and lfs_dir_t? We have ABI-incompatibility, which will result in out-of-bounds reads/writes.

We do have plans to implement such functionality and are currently working on a PoC, although it's not a very high priority item given other big features we are currently working on (e.g. BLE). We are currently exploring the idea of implementing a POSIX-compatible file/filesystem API which will allow us to have a stable interface which can be implemented no matter the underlying filesystem implementation and for example also allow the usage of standard libc or libstdc++ stream APIs, although this is not exactly the reason we are going with this. We're already doing that with socket API on Gen 3 devices, so this sounds like a reasonable course of action.

I cannot tell the exact timeline for this feature, but you may expect a WIP PR in the nearest future.

avtolstoy avatar May 29 '19 08:05 avtolstoy

Thanks for the clarifications. Makes sense. The littlefs interface is already pretty close to POSIX. Other than hiding the file and dir structs and renaming constants, how far do you want to go? I noticed some semantic stuff, for example, you open a file for writing and separately the same file for reading then the two are not in sync, i.e., you can't immediately read what you wrote unless you call lfs_sync (the use-case is writing a log and simultaneously trying to keep up sending it over the network).

tve avatar Jun 03 '19 15:06 tve

Any updates on this @avtolstoy?

pkourany avatar Oct 09 '19 23:10 pkourany

@pkourany Not yet, but this is being prioritized in the nearest perspective.

avtolstoy avatar Oct 10 '19 01:10 avtolstoy

I just tried tweaking the files in my DeviceOS 1.5 installation to add this to my Argon board... It doesn't seem to work... Do I have to downgrade my deviceOS to get this to work?

saewoonam avatar Apr 21 '20 00:04 saewoonam

Are there any news on that? We are currently using rickkas Spiffs library for our flash which is quite a shame given littlefs is already in device os. Is there already a timeline for that feature?

perotom avatar May 15 '20 17:05 perotom