Closed StefanRvO closed 1 year ago
thanks for the PR! I need to think about this for a little bit; I'm a bit worried of cluttering the API, but this looks well implemented and organized.
Would you be up for adding some unit tests? There are instructions in the README about how to run the unit tester.
Yes, i will see if i can get some time to add some unittests, probably next week. I first wanted to test the waters a bit to see if this was something you would be open to merge. :)
I will also see if i can make the filesystem mount read-only (maybe optional). I'm not really sure what happens if you try to write to it atm. Probably nothing good will happen. I think the bootloader normally verifies a checksum of the app partition before booting, which would be invalid if you start to modify a filesystem embedded into it.
@BrianPugh I tried to run the existing unittests (on master and with esp-idf @db4308888), but the "benchmark" tests fails. The other tests seems to pass.
Am i doing something wrong, or are those not working?
[stefan@Mjolner:tools/unit-test-app]$ idf.py -T littlefs monitor (08-13 13:00)
Executing action: monitor
Serial port /dev/ttyUSB0
Connecting....
Detecting chip type... Unsupported detection protocol, switching and trying again...
Connecting....
Detecting chip type... ESP32
Running idf_monitor in directory /home/stefan/littlefs_test/esp-idf/tools/unit-test-app
Executing "/home/stefan/.espressif/python_env/idf5.2_py3.11_env/bin/python /home/stefan/littlefs_test/esp-idf/tools/idf_monitor.py -p /dev/ttyUSB0 -b 115200 --toolchain-prefix xtensa-esp32-elf- --target esp32 --revision 0 /home/stefan/littlefs_test/esp-idf/tools/unit-test-app/build/unit-test-app.elf -m '/home/stefan/.espressif/python_env/idf5.2_py3.11_env/bin/python' '/home/stefan/littlefs_test/esp-idf/tools/idf.py' '-T' 'littlefs'"...
--- idf_monitor on /dev/ttyUSB0 115200 ---
--- Quit: Ctrl+] | Menu: Ctrl+T | Help: Ctrl+T followed by Ctrl+H ---
ets Jul 29 2019 12:21:46
rst:0x1 (POWERON_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff00b0,len:6272
load:0x40078000,len:15312
load:0x40080400,len:4
0x40080400: _init at ??:?
load:0x40080404,len:3504
entry 0x400805e4
W (65) boot.esp32: eFuse virtual mode is enabled. If Secure boot or Flash encryption is enabled then it does not provide any security. FOR TESTING ONLY!
W (98) efuse: [Virtual] Loading virtual efuse blocks from real efuses
I (188) cpu_start: Multicore app
I (189) cpu_start: Pro cpu up.
I (189) cpu_start: Starting app cpu, entry point is 0x4008249c
0x4008249c: call_start_cpu1 at /home/stefan/littlefs_test/esp-idf/components/esp_system/port/cpu_start.c:170
I (0) cpu_start: App cpu up.
I (207) cpu_start: Pro cpu start user code
I (207) cpu_start: cpu freq: 240000000 Hz
I (207) cpu_start: Application information:
I (211) cpu_start: Project name: unit-test-app
I (217) cpu_start: App version: v5.2-dev-2158-gdb4308888d
I (223) cpu_start: Compile time: Aug 13 2023 12:27:01
I (230) cpu_start: ELF file SHA256: 89d16aec2...
I (235) cpu_start: ESP-IDF: v5.2-dev-2158-gdb4308888d
I (241) cpu_start: Min chip rev: v0.0
I (246) cpu_start: Max chip rev: v3.99
I (251) cpu_start: Chip rev: v3.0
I (256) heap_init: Initializing. RAM available for dynamic allocation:
I (263) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (269) heap_init: At 3FFB3160 len 0002CEA0 (179 KiB): DRAM
I (275) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (281) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (289) heap_init: At 4008D1B4 len 00012E4C (75 KiB): IRAM
I (296) spi_flash: detected chip: generic
I (299) spi_flash: flash io: dio
W (303) spi_flash: Detected size(8192k) larger than the size in the binary image header(4096k). Using the size in the binary image header.
W (316) cpu_start: eFuse virtual mode is enabled. If Secure boot or Flash encryption is enabled then it does not provide any security. FOR TESTING ONLY!
W (330) efuse: [Virtual] Loading virtual efuse blocks from real efuses
I (338) app_start: Starting scheduler on CPU0
II (343) app_start: Starting scheduler on CPU1
(343) main_task: Started on CPU0
I (352) main_task: Calling app_main()
I (356) main_task: Returned from app_main()
Press ENTER to see the list of tests.
Here's the test menu, pick your combo:
(1) "Format" [littlefs_benchmark]
(2) "Write 5 files, read 5 files, then delete 5 files" [littlefs_benchmark]
(3) "truncate" [littlefs]
(4) "ftruncate" [littlefs]
(5) "mkdir, rmdir" [littlefs]
(6) "opendir, readdir, rewinddir, seekdir work as expected" [littlefs]
(7) "readdir with large number of files" [littlefs][timeout=30]
(8) "can opendir root directory of FS" [littlefs]
(9) "unlink removes a file" [littlefs]
(10) "rename moves a file" [littlefs]
(11) "can initialize LittleFS in erased partition" [littlefs]
(12) "can format mounted partition" [littlefs]
(13) "can format unmounted partition" [littlefs]
(14) "can create and write file" [littlefs]
(15) "can read file" [littlefs]
(16) "can write to file with offset (pwrite)" [littlefs]
(17) "can read from file with offset (pread)" [littlefs]
(18) "r+ mode read and write file" [littlefs]
(19) "w+ mode read and write file" [littlefs]
(20) "can open maximum number of files" [littlefs]
(21) "overwrite and append file" [littlefs]
(22) "can lseek" [littlefs]
(23) "stat/fstat returns correct values" [littlefs]
(24) "multiple tasks can use same volume" [littlefs]
(25) "esp_littlefs_info" [littlefs]
(26) "mtime support" [littlefs]
(27) "Rewriting file frees space immediately (#7426)" [littlefs]
(28) "esp_littlefs_info returns used_bytes > total_bytes" [littlefs]
(29) "fcntl get flags" [littlefs]
Enter test for running.
1
Running Format...
I (15075) [littlefs_benchmark]: Filling SPIFFS partition with dummy data
Guru Meditation Error: Core 0 panic'ed (LoadProhibited). Exception was unhandled.
Core 0 register dump:
PC : 0x400d1cf3 PS : 0x00060630 A0 : 0x800d1e4a A1 : 0x3ffb8260
0x400d1cf3: fill_partitions at /home/stefan/littlefs_test/esp-idf/tools/unit-test-app/components/littlefs/test/test_benchmark.c:101
A2 : 0xbecf7881 A3 : 0x00000000 A4 : 0x3ffb2f68 A5 : 0x00060023
A6 : 0xb33fffff A7 : 0xb33fffff A8 : 0x800d1cf1 A9 : 0x3ffb8240
A10 : 0x00000000 A11 : 0x00000000 A12 : 0x3f400164 A13 : 0xbecf7881
A14 : 0x3ffb8210 A15 : 0x36353433 SAR : 0x0000001a EXCCAUSE: 0x0000001c
EXCVADDR: 0x00000010 LBEG : 0x4000c46c LEND : 0x4000c477 LCOUNT : 0x00000000
0x4000c46c: memset in ROM
0x4000c477: memset in ROM
Backtrace: 0x400d1cf0:0x3ffb8260 0x400d1e47:0x3ffb8310 0x400da82e:0x3ffb8330 0x400daa65:0x3ffb8360 0x400dab1e:0x3ffb8380 0x400dab51:0x3ffb83a0 0x400dad22:0x3ffb83c0 0x400eb508:0x3ffb84f0 0x40088c35:0x3ffb8510
0x400d1cf0: fill_partitions at /home/stefan/littlefs_test/esp-idf/tools/unit-test-app/components/littlefs/test/test_benchmark.c:99
0x400d1e47: test_func_221 at /home/stefan/littlefs_test/esp-idf/tools/unit-test-app/components/littlefs/test/test_benchmark.c:224
0x400da82e: unity_default_test_run at /home/stefan/littlefs_test/esp-idf/components/unity/unity_runner.c:70
0x400daa65: unity_run_single_test at /home/stefan/littlefs_test/esp-idf/components/unity/unity_runner.c:117
0x400dab1e: unity_run_test_by_index at /home/stefan/littlefs_test/esp-idf/components/unity/unity_runner.c:145
0x400dab51: unity_run_single_test_by_index_parse at /home/stefan/littlefs_test/esp-idf/components/unity/unity_runner.c:154
0x400dad22: unity_run_menu at /home/stefan/littlefs_test/esp-idf/components/unity/unity_runner.c:303
0x400eb508: unity_task at /home/stefan/littlefs_test/esp-idf/tools/unit-test-app/components/test_utils/test_runner.c:27
0x40088c35: vPortTaskWrapper at /home/stefan/littlefs_test/esp-idf/components/freertos/FreeRTOS-Kernel/portable/xtensa/port.c:162
ELF file SHA256: 89d16aec2
Rebooting...
ets Jul 29 2019 12:21:46
rst:0xc (SW_CPU_RESET),boot:0x13 (SPI_FAST_FLASH_BOOT)
configsip: 0, SPIWP:0xee
clk_drv:0x00,q_drv:0x00,d_drv:0x00,cs0_drv:0x00,hd_drv:0x00,wp_drv:0x00
mode:DIO, clock div:2
load:0x3fff00b0,len:6272
load:0x40078000,len:15312
load:0x40080400,len:4
0x40080400: _init at ??:?
load:0x40080404,len:3504
entry 0x400805e4
W (65) boot.esp32: eFuse virtual mode is enabled. If Secure boot or Flash encryption is enabled then it does not provide any security. FOR TESTING ONLY!
W (98) efuse: [Virtual] Loading virtual efuse blocks from real efuses
I (188) cpu_start: Multicore app
I (188) cpu_start: Pro cpu up.
I (188) cpu_start: Starting app cpu, entry point is 0x4008249c
0x4008249c: call_start_cpu1 at /home/stefan/littlefs_test/esp-idf/components/esp_system/port/cpu_start.c:170
I (166) cpu_start: App cpu up.
I (206) cpu_start: Pro cpu start user code
I (206) cpu_start: cpu freq: 240000000 Hz
I (206) cpu_start: Application information:
I (211) cpu_start: Project name: unit-test-app
I (217) cpu_start: App version: v5.2-dev-2158-gdb4308888d
I (223) cpu_start: Compile time: Aug 13 2023 12:27:01
I (229) cpu_start: ELF file SHA256: 89d16aec2...
I (235) cpu_start: ESP-IDF: v5.2-dev-2158-gdb4308888d
I (241) cpu_start: Min chip rev: v0.0
I (246) cpu_start: Max chip rev: v3.99
I (251) cpu_start: Chip rev: v3.0
I (256) heap_init: Initializing. RAM available for dynamic allocation:
I (263) heap_init: At 3FFAE6E0 len 00001920 (6 KiB): DRAM
I (269) heap_init: At 3FFB3160 len 0002CEA0 (179 KiB): DRAM
I (275) heap_init: At 3FFE0440 len 00003AE0 (14 KiB): D/IRAM
I (281) heap_init: At 3FFE4350 len 0001BCB0 (111 KiB): D/IRAM
I (288) heap_init: At 4008D1B4 len 00012E4C (75 KiB): IRAM
I (295) spi_flash: detected chip: generic
I (298) spi_flash: flash io: dio
W (302) spi_flash: Detected size(8192k) larger than the size in the binary image header(4096k). Using the size in the binary image header.
W (315) cpu_start: eFuse virtual mode is enabled. If Secure boot or Flash encryption is enabled then it does not provide any security. FOR TESTING ONLY!
W (330) efuse: [Virtual] Loading virtual efuse blocks from real efuses
I (338) app_start: Starting scheduler on CPU0
II (342) app_start: Starting scheduler on CPU1
(342) main_task: Started on CPU0
I (351) main_task: Calling app_main()
I (355) main_task: Returned from app_main()
Press ENTER to see the list of tests.
2
Running Write 5 files, read 5 files, then delete 5 files...
E (1641) vfs_fat_spiflash: esp_vfs_fat_spiflash_mount_rw_wl(121): Failed to find FATFS partition (type='data', subtype='fat', partition_label='fat_store'). Check the partition table.
E (1648) [littlefs_benchmark]: Failed to mount FATFS (ESP_ERR_NOT_FOUND)
/IDF/tools/unit-test-app/components/littlefs/test/test_benchmark.c:51:Write 5 files, read 5 files, then delete 5 files:FAILMALLOC_CAP_8BIT usage: Free memory delta: 0 Leak threshold: -1024
MALLOC_CAP_32BIT usage: Free memory delta: 0 Leak threshold: -1024
Test ran in 68ms
-----------------------
1 Tests 1 Failures 0 Ignored
FAIL
I have added some initial unittests. I will add some more later :)
@BrianPugh, i have tried to make the mounted filesystem read-only and added some tests for being unable to create, write or delete files. Do you know if this is the correct approach to making the filesystem read-only (by simply making some of the function pointers in the vfs struct NULL), or is there a better way?
And is there more tests that you would like me to add?
the littlefs submodule has options for read-only. However, its a compile-time macro. It might be limiting if the user wants to have a read-only partition embedded in the firmware, as well as a read/write partition elsewhere on the flash.
That said, I think making some of the function points in the vfs struct NULL is the way to go, but I haven't played around with it. It might be good to add an additional read_only
flag to the config struct(s) that then set these to NULL.
I haven't looked at the PR in depth yet, I should be able to tonight. Let me know if you're going to make more changes from above feedback.
I have made it so that it constructs the esp_vfs_t struct with the functions that do writes set to NULL if the register function for raw partitions is called, and constructs it normally if the register function with a partition label is called. I think i will keep it like that unless you prefer that i add a read-only flag to the conf struct :)
Lets add a read_only
flag to both configs, it's just a few extra lines of code now that you've got the rest set up. This might be useful if for some reason someone actually wants to write to their embedded filesystem. A bit questionable of a use-case, but I'd rather not limit the end use-case. I'm sure someone will find it useful. Thanks!
I swear I'll get to this PR 😅 .
From a quick skim, do you think it's possible to combine the config structs, and subsequently not add any new public functions?
I'm thinking like adding const esp_partition_t* partition;
and uint8_t read_only:1;
to esp_vfs_littlefs_conf_t
. If partition
is set, then use that. If partition_label
is set, then use that. If both are set, return ESP_ERR_INVALID_ARG
.
Hi
Yeah, could be a possibility. I'm a bit afraid of checking the partition
field first though. If somebody updates to a new version of esp_littlefs and it is uninitialized, it could be non-null, but just be some random garbage, and we would get a fault. But maybe if i check partition_label
first, and if that is NULL check partition
and use it if it is not NULL, it could work. I will give it a go.
fields are guaranteed to be 0 if they are partially initialized. But I suppose there are scenarios where the user dynamically populates the conf
fields; in that case it wouldn't be guaranteed that partition
is 0.
But yeah, as you mentioned, this should be mitigated by checking partition_label
first. Great idea!
I will give it a go. Might take some time though :)
I have changed the struct to use the suggested fields, so that esp_vfs_littlefs_register can be used for mounting.
I don't see a way to avoid adding the following public functions:
As the existing functions for those just takes a string with the partition label as arguments.
Finally got around to reviewing this; overall excellent PR! I left a few small comments that should be very easy to fix.
However, before merging this, I cannot get your tests to run. Admittedly, I didn't try very hard, but either they should work "out of the box", or we should add some instructions to the README. If you need more info, I can follow up if the errors aren't immediately obvious.
Will look into fixing the comments you had :)
Regarding the test. I think they should work out of the box. What issues did you have? I did have to clean my build folder for it to figure out that i added a new test file. Maybe it is related to that?
Addressed the comments. Tell me if you still have problems running the tests :)
Also, I don't know if i should include the files i have used to make the static file system? They are not used for anything, i constructed the .bin file by running mklittlefs -c testdir/ -p 4096 -s 49152 test/testfs.bin
on the testdir folder. But if one wants to re-build the .bin later for some reason or modify it, they will be useful to have.
Thanks! Looks like you've addressed all my feedback. As for the tests, when running all of them, it passes the first test (can read partition
), and causes a hardfault on can't create file
. Here's the stacktrace:
0x40082d02: panic_abort at /Users/brianpugh/esp-idf/components/esp_system/panic.c:408
0x40085e19: esp_system_abort at /Users/brianpugh/esp-idf/components/esp_system/esp_system.c:137
0x4008a96a: abort at /Users/brianpugh/esp-idf/components/newlib/abort.c:46
0x40081927: esp_flash_erase_region at /Users/brianpugh/esp-idf/components/spi_flash/esp_flash_api.c:561 (discriminator 5)
0x400d4673: esp_partition_erase_range at /Users/brianpugh/esp-idf/components/spi_flash/partition.c:540
0x400e49ad: littlefs_api_erase at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs_api.c:44
0x400e4b43: lfs_bd_erase at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:277
0x400e66c3: lfs_dir_compact at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:1931
0x400e6b33: lfs_dir_splittingcompact at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:2161
0x400e6de6: lfs_dir_relocatingcommit at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:2279
0x400e74b2: lfs_dir_orphaningcommit at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:2360
0x400e79e9: lfs_dir_commit at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:2532
0x400e8c32: lfs_file_rawopencfg at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:3047
0x400e8e09: lfs_file_rawopen at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:3164
0x400e91a1: lfs_file_open at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/littlefs/lfs.c:5870
0x400e3d9e: vfs_littlefs_open at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/src/esp_littlefs.c:1143
0x400d615d: esp_vfs_open at /Users/brianpugh/esp-idf/components/vfs/vfs.c:399 (discriminator 3)
0x400ec557: _fopen_r at /Users/brnomac003/.gitlab-runner/builds/qR2TxTby/0/idf/crosstool-NG/.build/xtensa-esp32-elf/src/newlib/newlib/libc/stdio/fopen.c:129
0x400ec5fc: fopen at /Users/brnomac003/.gitlab-runner/builds/qR2TxTby/0/idf/crosstool-NG/.build/xtensa-esp32-elf/src/newlib/newlib/libc/stdio/fopen.c:168
0x400d41e0: test_func_93 at /Users/brianpugh/esp-idf/tools/unit-test-app/components/littlefs/test/test_littlefs_static_partition.c:98
0x400d9b02: unity_default_test_run at /Users/brianpugh/esp-idf/components/unity/unity_runner.c:78
0x400d9d11: unity_run_single_test at /Users/brianpugh/esp-idf/components/unity/unity_runner.c:125
0x400d9e5b: unity_run_all_tests at /Users/brianpugh/esp-idf/components/unity/unity_runner.c:184 (discriminator 3)
0x400d9f4d: unity_run_menu at /Users/brianpugh/esp-idf/components/unity/unity_runner.c:301
0x400e9358: unity_task at /Users/brianpugh/esp-idf/tools/unit-test-app/components/test_utils/test_runner.c:38
0x400887ed: vPortTaskWrapper at /Users/brianpugh/esp-idf/components/freertos/port/xtensa/port.c:142
This is running esp-idf v4.4 on a vanilla esp32.
Couldn't reproduce that on v4.4 or v5.0, just got some error messages about failing to erase sectors, but no crashes, but that is probably what you are hitting. Made some tweaks:
esp_littlefs_t
to do that.Can you see if that resolves your crash?
added one last comment, but your latest push fixed unit tests on my side. After that I think we're good for a merge and release!
will merge and release soon! Thank you very much!
This is useful for e.g. mounting a littlefs blob embedded in the firmware.
Formatting on failure is not supported as this is intended for static filesystems
I use this to mount a littlefs created using mkfatfs and embedded into my firmware. Example usage:
I have only tested esp_vfs_littlefs_register_partition(), as i don't use the others in my application atm, but i can test the others if this is something you would be interested in merging.
I have tried to move some common code to separate functions, but there may still be some more parts that could be made common.