Join GitHub today
GitHub is home to over 50 million developers working together to host and review code, manage projects, and build software together.
Sign upAdding LITTLEFS w/o rebuild the IDF. Compatible with IDF v3.3 #4096
Conversation
based on https://github.com/joltwallet/esp_littlefs This will be my first PR, hope will do it right.
…e the PR status here espressif/arduino-esp32#4096
…e the PR status here: espressif/arduino-esp32#4096
since this is embedding https://github.com/joltwallet/esp_littlefs it would be good to have sign-off from @joltwallet / @BrianPugh since some files are showing mixed/missing licensing. This PR will also need to be updated for the esp32s2 branch which is based on IDF v4.2 (master) as there are VFS API changes which may break this functionality. |
} | ||
] | ||
} | ||
{ |
This comment has been minimized.
This comment has been minimized.
atanisoft
Jun 18, 2020
Contributor
reformatting of an existing file (such as this one) is often discouraged and will often lead to the PR sitting for a very long time (if it ever gets merged).
If you do reformat the file, please ensure it is consistent as it doesn't appear to be consist after reformatting (looks like a mix of spaces and tabs)
This comment has been minimized.
This comment has been minimized.
//esp_err_t esp_littlefs_format(const char* partition_label); | ||
//esp_err_t esp_littlefs_info(const char* partition_label, size_t *total_bytes, size_t *used_bytes); | ||
|
||
#define LFS_NAME "spiffs" |
This comment has been minimized.
This comment has been minimized.
atanisoft
Jun 18, 2020
Contributor
instead of a hardcode via DEFINE can you try:
static constexpr const char LFS_NAME[] = "spiffs";
This will prevent global namespace pollution.
This comment has been minimized.
This comment has been minimized.
libraries/LITTLEFS/src/esp_littlefs.c | ||
libraries/LITTLEFS/src/lfs.c | ||
libraries/LITTLEFS/src/lfs_util.c | ||
libraries/LITTLEFS/src/littlefs_api.c |
This comment has been minimized.
This comment has been minimized.
|
||
using namespace fs; | ||
|
||
class LITTLEFSImpl : public VFSImpl |
This comment has been minimized.
This comment has been minimized.
atanisoft
Jun 18, 2020
Contributor
@me-no-dev would it be feasible to provide a default impl for VFSImpl that can be used by the various FS impls rather than each one copying/pasting the same but with a different name?
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
This comment has been minimized.
@@ -0,0 +1,130 @@ | |||
// Copyright 2015-2016 Espressif Systems (Shanghai) PTE LTD |
This comment has been minimized.
This comment has been minimized.
atanisoft
Jun 18, 2020
Contributor
adjust copyright year to 2020 (for all files with similar header)
Asking Brian to review...
Someone else should help here please to adapt the code to match older IDF, maybe @BrianPugh is the best to help with such branch of his library. |
BrianPugh
commented
Jun 18, 2020
•
Hey guys, Some questions/feedback:
My action plans:
edit 1: Added MIT license to my repo |
IDF v4.0 changed a few things: https://github.com/espressif/esp-idf/releases/tag/v4.0, there are likely more changes beyond v4.0 that would need to be handled. You can detect the IDF version via a few compile time defines. |
BrianPugh
commented
Jun 18, 2020
ah gotcha, I've been regularly using my repo off of esp-idf master for esp32 targets, so it should be good to go. I thought there may be even newer or esp32-s2 specific changes. |
Great! I don't know of any changes specific to the ESP32-S2 that will prevent this from working. |
@BrianPugh Hi Brian, It will be difficult for me to re-wrap the LITTLEFS.cpp and .h in such a way that your library is kept as it is and just fork the appropriate revision of it. No problem if you rewrite it entirely around your wrap, especially if it can be brought back to Arduino w/o painful recompiling and IDF syncing. LL |
The best way to avoid that would be have @BrianPugh submit a PR to ESP-IDF to have LittleFS included as a component there. Then it can be wrapped in arduino-esp32 in a clean manner (like other FS types) |
BrianPugh
commented
Jun 18, 2020
I'll make a PR, no harm in trying. I fear that there might be a lot of other requirements (writing up a lot of documentation, etc) |
I wish you a patience and good luck with that :) |
BrianPugh
commented
Jun 18, 2020
Here's a link to my PR for littlefs in esp-idf: |
If you've used the package, you know documentation is not always most thorough... |
BrianPugh
commented
Jun 18, 2020
For registering, is that targeted at me or @lorol ? I don't really use arduino (and am unfamiliar with its package managers, library structures, and all that). I'm just here to help on actions requiring changes in my repo. |
Targeted at OP (@lorol), whom wants it in arduino-esp32. |
|
||
using namespace fs; | ||
|
||
class LITTLEFSImpl : public VFSImpl |
This comment has been minimized.
This comment has been minimized.
return (f == true) && !f.isDirectory(); | ||
} | ||
|
||
LITTLEFSFS::LITTLEFSFS() : FS(FSImplPtr(new LITTLEFSImpl())) |
This comment has been minimized.
This comment has been minimized.
atanisoft
Jun 19, 2020
Contributor
Change this to:
LITTLEFSFS::LITTLEFSFS() : FS(FSImplPtr(new VFSImpl()))
and remove the definition of class LITTLEFSImpl. The customization is not necessary for LITTLEFS as the default behavior from VFSImpl() will work.
This comment has been minimized.
This comment has been minimized.
Thanks but ,,, |
lorol commentedJun 17, 2020
based on https://github.com/joltwallet/esp_littlefs
This will be my first PR, hope will do it right.
Details in README