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

File handling commands #741

Open
FreeBear-nc opened this issue May 23, 2024 · 12 comments
Open

File handling commands #741

FreeBear-nc opened this issue May 23, 2024 · 12 comments
Labels
enhancement New feature or request

Comments

@FreeBear-nc
Copy link
Contributor

FreeBear-nc commented May 23, 2024

As part of adding SD support, it occurred to me that it would be useful to have some basic file handling commands. e.g.:
list - Returning a json object with file names & sizes on either SD card or internal storage (spiffs/littlefs) - Already implemented as a dir command and used by the file editor, but doesn't display file size.
copy - Copy file to/from SD card and returns a json object of file name (including new path ?) and size
move - As per copy, but deleting source file if copy is successful.
delete - Already available within the file editor, but no callable from mqtt/telnet/terminal (unless I've missed something)

copy and move commands would only be of use for those using an SD card. list (or an enhanced dir) and copy have a wider appeal to other users.

@FreeBear-nc FreeBear-nc added the enhancement New feature or request label May 23, 2024
@fvanroie
Copy link
Collaborator

fvanroie commented May 23, 2024

There wasn't any need for cross-filesystem operations until now. Even with an SD-card, it's a nice to have, but not as critical imho.

To support universal file handling, I think these operations (and subsequent commands) should be handled through LVGL filesystem APIs. This will prevent a lot of #IFDEFs and branching between different source and destination filesystems.

Alternatively, we could use POSIX operations (fopen etc...) and the ESP32 Virtual Filesystem, which also unifies multiple filesystems under single root / folder.

If it is possible to mount the SD/MMC with ESP32 VFS APIs, then this should be relatively easy to implement. The LittleFS partition is already mounted to /littlefs/ and mapped to LVGL L: drive-letter. I think having a similar /sdcard/ with e.g. S: drive-letter is the way to go.

This will increase the flexibility greatly and should be easier to migrate to a different LVGL version, add new subsystems or even support across operation systems.

@FreeBear-nc
Copy link
Contributor Author

Whilst I agree file system operations fall in to the "nice to have" category, having a copy enables a user to load up an SD card with files and then deploy them to multiple plates. The core of a list command is already in place with filesystem_list(), so spitting out an mqtt message should be trivial - Just a couple of lines were needed to print to the terminal (will commit the changes later).

Will certainly look at the LVGL api if it simplifies handling multiple filesystems and copying files between drives.

@fvanroie
Copy link
Collaborator

fvanroie commented May 23, 2024

There are different routes, each with its own pro and cons. In the short term, filesystem_list can be easily repurposed/extended for SD cards, for sure. I guess I'm a little more focused on the longer term...

On Arduino HASP_FS is currently used for all file related operations. On PCs POSIX is used. ESP32 can do POSIX as well and with VFS already supports multiple filesystems. So, the long term goal is to go in that direction.

The idea is to phase out HASP_FS (and fs::FS classes) as much a possible and consolidate to either LVGL or POSIX or a mix of both. Your request just happens to re-ignite this plan.

SD_FS can certainly be used as proof-of-concept and custom builds. For a long-term solution on all boards, I think the VFS route is the most sensible one, since that's Espressifs native API and an LVGL filesystem driver is already implemented for it.

I think the deeper you dive into this, a universal solution quickly becomes a necessity because there are config files, images, fonts, ... being accessed from LVGL, HTTP, FTP, ... Using #ifdef to account for each case will be very cumbersome.

Of course, this is just my opinion and more discussion is needed. I don't have a clear view on the work needed, but if we combine the effort it should be doable.

@Martinson50
Copy link

I agree my solution works but it's only for a custom build, anyway I will explore for a universal solution.
By the way LVGL v9 will be implemented, I mention this because also for the filesystem solution could be upgraded too to this version

@fvanroie
Copy link
Collaborator

fvanroie commented May 23, 2024

@Martinson50 Thanks for sharing your code. There is more interest in SD card support, so we are seriously looking seriously into adding this feature. The usage of the Arduino SD libraries are a good way to start using the SD card. However, it doesn't integrate well with other filesystems. I would like to avoid using SD.h and FS.h, but use ESP32 VFS instead.

I think if we can mount the card to the Virtual File System using esp_vfs_fat_sdspi_mount then it will be much easier to integrate. I found an example of it here https://github.com/espressif/esp-idf/blob/master/examples/storage/sd_card/sdspi/main/sd_card_example_main.c

This makes sdcardSetup(); is bit more complicated, but once the card is mounted the ESP32 can access LittleFS and the FAT SD card in exacly the same way.

@FreeBear-nc
Copy link
Contributor Author

The Arduino libraries are, for the main part, wrappers around lower level espessif system calls. Well documented, and will run on non-ESP boards. Whilst not portable to say a POSIX environment, SD.h & FS.h allows for "proof of concept" to be thrown together fairly quickly.
SD and LittleFS both inherit from FS, so file access remains the same for all three. So there shouldn't be much need to use loads if ifdef statements. The only issue I can see is how to handle card insertion/removal cleanly without causing fatal system crashes. But that would need to be considered regardless of which library is used.

@fvanroie
Copy link
Collaborator

fvanroie commented May 24, 2024

OK, I see SD.begin includes a mountpoint parameter, which presumably adds it to the VFS on ESP32...
That would suffice to build other functions on-top, as it probably uses the ESP32 VFS api's already; so my comment above is mute

@Martinson50
Copy link

Martinson50 commented May 24, 2024 via email

@FreeBear-nc
Copy link
Contributor Author

FreeBear-nc commented May 24, 2024

So, I'm going to try to code a couple of commands - copy [from Z:] [to L:] - unzip [from Z:],[to L:]

An unzip from Z: to L: (or the other way round) isn't needed in my opinion. You can achieve the same by using the existing unzip (on either Z: or L:) and then copying the files.
Keep it simple and bear in mind, memory is limited. So implementing a full set of shell commands and using/abusing RAM isn't going to fly.

If you wanted to checkout my fork, we can do some of the collaboration there and push changes back here once they pass approval.

@FreeBear-nc
Copy link
Contributor Author

Pushed a rough draft of a "list" command that returns an mqtt message containing files & sizes.
https://github.com/FreeBear-nc/openHASP/tree/commands - This fork doesn't contain any SD card handling, so you'd need to checkout master and cherry pick commit 9c22c0c

@fvanroie
Copy link
Collaborator

fvanroie commented May 24, 2024

Thanks for the demo. It makes the proposal a lot clearer, as I thought the commands would be consumed by the commandline and webserver.

That result can potentially get huge, especially on a big SD card. Maybe we don't return all recursive results by default? Like the http://plate/api/files/?dir=/ endpoint, this command should behave similarly imho.

Instead of using state/list, I'm inclined to propose a separate files topic or so... In my mind state represents GUI state.
I'm really curious on how you will consume the result from MQTT? What's the actual use-case. Do you need all recursive files in one go?

@FreeBear-nc
Copy link
Contributor Author

Agreed, the mqtt message has the potential to consume a LOT of memory if there are shed loads of small files to list. Gets even worse if you descend in to subdirectories. Perhaps place a size limit on the payload string - Both the filesystem_list() and filesystem_list_path() suffer from the same problem. The latter could also overrun the end of the fn[] array if hit with a particularly long file name - There will always be some twisted individual that will try (like me 😜 )

Use case was nothing more than extracting information for curiosity. If the SD card was used to store logs, an end user could monitor the size and rotate/delete if they got too large. Give them the tools, and you might be surprised what gets done.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
enhancement New feature or request
Projects
None yet
Development

No branches or pull requests

3 participants