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

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

Open
wants to merge 3 commits into
base: mesh-develop
from

Conversation

@tve
Copy link

commented Mar 2, 2019

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

  • User is totes amazing for contributing!
  • 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 added 3 commits Mar 2, 2019
@tve

This comment has been minimized.

Copy link
Author

commented May 17, 2019

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...

@avtolstoy

This comment has been minimized.

Copy link
Member

commented May 29, 2019

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.

@tve

This comment has been minimized.

Copy link
Author

commented Jun 3, 2019

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).

@pkourany

This comment has been minimized.

Copy link
Contributor

commented Oct 9, 2019

Any updates on this @avtolstoy?

@avtolstoy

This comment has been minimized.

Copy link
Member

commented Oct 10, 2019

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

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
3 participants
You can’t perform that action at this time.