Fixing FileSystem for ESP32 #2985
Conversation
josesimoes
added
the
Platform: ESP32
WalkthroughThe changes primarily enhance the ESP32 file system functionalities by enabling support for SD cards and file system drivers (LittleFS and FATFS) across various target configurations. This includes activating previously commented-out code and streamlining volume management logic, significantly improving flexibility and performance in handling storage devices. Changes
Sequence Diagram(s)sequenceDiagram
participant User
participant App
participant FileSystem
participant Storage
User->>App: Start Application
App->>FileSystem: Initialize File Systems
FileSystem->>Storage: Check SD Card Support
Storage-->>FileSystem: Supported
FileSystem->>Storage: Mount SD Card
Storage-->>FileSystem: Mounted Successfully
FileSystem->>App: File System Ready
App-->>User: Ready to Use
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (invoked as PR comments)
Additionally, you can add CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 0
Outside diff range, codebase verification and nitpick comments (2)
targets/ESP32/ESP32_H2/target_FileSystem.cpp (1)
60-60: Document the lack of implementation inFS_MountRemovableVolumes.The
FS_MountRemovableVolumesfunction has no implementation. Ensure that this is intentional and document the reason for clarity.targets/ESP32/ESP32_S2/target_FileSystem.cpp (1)
60-60: Document the lack of implementation inFS_MountRemovableVolumes.The
FS_MountRemovableVolumesfunction has no implementation. Ensure that this is intentional and document the reason for clarity.
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
targets/ESP32/_IDF/esp32c6/partitions_nanoclr_16mb.csvis excluded by!**/*.csvtargets/ESP32/_IDF/esp32c6/partitions_nanoclr_4mb.csvis excluded by!**/*.csvtargets/ESP32/_IDF/esp32c6/partitions_nanoclr_8mb.csvis excluded by!**/*.csv
Files selected for processing (12)
- targets/ESP32/CMakePresets.json (1 hunks)
- targets/ESP32/ESP32/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_C3/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_C6/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_H2/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_S2/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_S3/target_FileSystem.cpp (2 hunks)
- targets/ESP32/_IDF/esp32/app_main.c (1 hunks)
- targets/ESP32/_common/Target_System_IO_FileSystem.c (2 hunks)
- targets/ESP32/_include/Target_System_IO_FileSystem.h (1 hunks)
- targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1 hunks)
- targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp (7 hunks)
Additional comments not posted (50)
targets/ESP32/_include/Target_System_IO_FileSystem.h (1)
16-18: LGTM! Conditional function declaration is correctly added.The conditional compilation directive and the new function declaration are correctly implemented.
targets/ESP32/ESP32/target_FileSystem.cpp (4)
12-14: LGTM! Conditional inclusion of the FATFS driver is correctly implemented.The inclusion of the FATFS driver is conditionally defined based on
HAL_USE_SDC.
19-19: LGTM! Variable renaming is correctly implemented.The variable
g_FATFS_STREAM_DriverInterfaceis correctly renamed tog_FATFS_FILE_SYSTEM_DriverInterface.
21-24: LGTM! Conditional inclusion of the FATFS stream driver is correctly implemented.The inclusion of the FATFS stream driver is conditionally defined based on
HAL_USE_SDC.
28-30: LGTM! Initialization of theg_AvailableFSInterfacesarray is correctly updated.The initialization of the array correctly reflects the conditional inclusion of the FATFS driver interface.
targets/ESP32/_IDF/esp32/app_main.c (1)
55-55: LGTM! Logging configuration is correctly modified.The log level for all components is correctly changed from
ESP_LOG_VERBOSEtoESP_LOG_NONE.targets/ESP32/ESP32_S3/target_FileSystem.cpp (8)
10-10: Inclusion of LittleFS driver enabled.The inclusion of
littlefs_FS_Driver.hhas been uncommented, allowing the use of the LittleFS file system driver.
12-14: Conditional inclusion of FATFS driver.The inclusion of
fatfs_FS_Driver.his within a conditional compilation block, ensuring it is only included when SDC support is enabled.
18-18: External declaration of LittleFS driver interface.The external declaration for
g_LITTLEFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
19-19: External declaration of FATFS driver interface.The external declaration for
g_FATFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
21-24: Conditional external declarations of stream driver interfaces.The external declarations for
g_FATFS_STREAM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfaceare within a conditional compilation block, ensuring they are only included when SDC support is enabled.
26-30: Initialization of available file system interfaces.The
g_AvailableFSInterfacesarray has been updated to include LittleFS interfaces, enhancing the file system interface options available in the system.
41-55: Simplified volume management logic.The logic for adding file system volumes has been simplified to initialize
g_FS_NumVolumesto 1, reflecting a single internal storage volume in the LittleFS partition.
60-60: No operations for removable volumes.The function
FS_MountRemovableVolumesindicates that no operations are necessary for removable volumes, reflecting a design choice.targets/ESP32/ESP32_C3/target_FileSystem.cpp (8)
10-10: Inclusion of LittleFS driver enabled.The inclusion of
littlefs_FS_Driver.hhas been uncommented, allowing the use of the LittleFS file system driver.
12-14: Conditional inclusion of FATFS driver.The inclusion of
fatfs_FS_Driver.his within a conditional compilation block, ensuring it is only included when SDC support is enabled.
18-18: External declaration of LittleFS driver interface.The external declaration for
g_LITTLEFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
19-19: External declaration of FATFS driver interface.The external declaration for
g_FATFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
21-24: Conditional external declarations of stream driver interfaces.The external declarations for
g_FATFS_STREAM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfaceare within a conditional compilation block, ensuring they are only included when SDC support is enabled.
26-31: Initialization of available file system interfaces.The
g_AvailableFSInterfacesarray has been updated to include LittleFS interfaces, enhancing the file system interface options available in the system.
41-55: Simplified volume management logic.The logic for adding file system volumes has been simplified to initialize
g_FS_NumVolumesto 1, reflecting a single internal storage volume in the LittleFS partition.
60-60: No operations for removable volumes.The function
FS_MountRemovableVolumesindicates that no operations are necessary for removable volumes, reflecting a design choice.targets/ESP32/ESP32_C6/target_FileSystem.cpp (8)
10-10: Inclusion of LittleFS driver enabled.The inclusion of
littlefs_FS_Driver.hhas been uncommented, allowing the use of the LittleFS file system driver.
12-14: Conditional inclusion of FATFS driver.The inclusion of
fatfs_FS_Driver.his within a conditional compilation block, ensuring it is only included when SDC support is enabled.
18-18: External declaration of LittleFS driver interface.The external declaration for
g_LITTLEFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
19-19: External declaration of FATFS driver interface.The external declaration for
g_FATFS_FILE_SYSTEM_DriverInterfacehas been uncommented, allowing its use.
21-24: Conditional external declarations of stream driver interfaces.The external declarations for
g_FATFS_STREAM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfaceare within a conditional compilation block, ensuring they are only included when SDC support is enabled.
26-31: Initialization of available file system interfaces.The
g_AvailableFSInterfacesarray has been updated to include LittleFS interfaces, enhancing the file system interface options available in the system.
41-55: Simplified volume management logic.The logic for adding file system volumes has been simplified to initialize
g_FS_NumVolumesto 1, reflecting a single internal storage volume in the LittleFS partition.
60-60: No operations for removable volumes.The function
FS_MountRemovableVolumesindicates that no operations are necessary for removable volumes, reflecting a design choice.targets/ESP32/ESP32_H2/target_FileSystem.cpp (6)
10-10: Inclusion of LittleFS driver approved.The inclusion of
littlefs_FS_Driver.his necessary for integrating the LittleFS driver.
12-14: Conditional inclusion of FATFS driver approved.The conditional inclusion of
fatfs_FS_Driver.hbased onHAL_USE_SDCis correctly implemented and necessary for integrating the FATFS driver when SD card support is enabled.
18-19: External declarations of file system driver interfaces approved.The external declarations for
g_LITTLEFS_FILE_SYSTEM_DriverInterfaceandg_FATFS_FILE_SYSTEM_DriverInterfaceare necessary for integrating these drivers.
21-24: Conditional external declarations of stream driver interfaces approved.The conditional external declarations for
g_FATFS_STREAM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfacebased onHAL_USE_SDCare correctly implemented and necessary for integrating these stream drivers when SD card support is enabled.
26-31: Initialization ofg_AvailableFSInterfacesapproved.The initialization of the
g_AvailableFSInterfacesarray with LittleFS and FATFS interfaces based onHAL_USE_SDCis correctly implemented and necessary for integrating these file systems.
41-55: Implementation ofFS_AddVolumesfunction approved.The
FS_AddVolumesfunction correctly initializes one volume for the LittleFS partition, ensuring proper volume management.targets/ESP32/ESP32_S2/target_FileSystem.cpp (6)
10-10: Inclusion of LittleFS driver approved.The inclusion of
littlefs_FS_Driver.his necessary for integrating the LittleFS driver.
12-14: Conditional inclusion of FATFS driver approved.The conditional inclusion of
fatfs_FS_Driver.hbased onHAL_USE_SDCis correctly implemented and necessary for integrating the FATFS driver when SD card support is enabled.
18-19: External declarations of file system driver interfaces approved.The external declarations for
g_LITTLEFS_FILE_SYSTEM_DriverInterfaceandg_FATFS_FILE_SYSTEM_DriverInterfaceare necessary for integrating these drivers.
21-24: Conditional external declarations of stream driver interfaces approved.The conditional external declarations for
g_FATFS_STREAM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfacebased onHAL_USE_SDCare correctly implemented and necessary for integrating these stream drivers when SD card support is enabled.
26-31: Initialization ofg_AvailableFSInterfacesapproved.The initialization of the
g_AvailableFSInterfacesarray with LittleFS and FATFS interfaces based onHAL_USE_SDCis correctly implemented and necessary for integrating these file systems.
41-55: Implementation ofFS_AddVolumesfunction approved.The
FS_AddVolumesfunction correctly initializes one volume for the LittleFS partition, ensuring proper volume management.targets/ESP32/_common/Target_System_IO_FileSystem.c (2)
Line range hint
86-188:
Conditional compilation directive for SDMMC host support approved.The
#if SOC_SDMMC_HOST_SUPPORTEDdirective is correctly implemented and necessary for conditionally including the SDMMC host support code.
188-188: Implementation ofStorage_MountSpifunction approved.The
Storage_MountSpifunction correctly handles the mounting of an SPI SD card, ensuring proper volume management.targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp (4)
26-27: Conditional Compilation for MMC Pin ReservationThe introduction of conditional compilation for MMC pin reservation is well-implemented.
48-49: End of Conditional Compilation BlockThe end of the conditional compilation block for MMC pin reservation is correctly placed.
Line range hint
104-119:
Conditional Compilation for MMC InitializationThe conditional compilation for MMC initialization within the
InitNativemethod is well-implemented.
Line range hint
191-203:
Conditional Compilation for MMC Pin UnreservationThe conditional compilation for MMC pin unreservation within the
NativeDisposemethod is well-implemented.targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1)
92-94: Verify Impact of Commented-Out Volume Retrieval and Drive SettingThe commented-out lines in the
GetSizeInfomethod indicate a change in how the function interacts with the file system. Verify that this change does not negatively impact the functionality.targets/ESP32/CMakePresets.json (1)
290-290: Verify Impact of Enabling SD Card SupportThe change in the configuration setting for
NF_FEATURE_HAS_SDCARDfrom "OFF" to "ON" enables SD card support. Verify that this change does not introduce any issues and that the system interacts correctly with SD card hardware and software components.
josesimoes
force-pushed
the
filesystem_fixes
branch
from
8a67a69
to
fa1dc87
Compare
There was a problem hiding this comment.
Actionable comments posted: 0
Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
Files ignored due to path filters (3)
targets/ESP32/_IDF/esp32c6/partitions_nanoclr_16mb.csvis excluded by!**/*.csvtargets/ESP32/_IDF/esp32c6/partitions_nanoclr_4mb.csvis excluded by!**/*.csvtargets/ESP32/_IDF/esp32c6/partitions_nanoclr_8mb.csvis excluded by!**/*.csv
Files selected for processing (12)
- targets/ESP32/CMakePresets.json (1 hunks)
- targets/ESP32/ESP32/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_C3/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_C6/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_H2/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_S2/target_FileSystem.cpp (2 hunks)
- targets/ESP32/ESP32_S3/target_FileSystem.cpp (2 hunks)
- targets/ESP32/_IDF/esp32/app_main.c (1 hunks)
- targets/ESP32/_common/Target_System_IO_FileSystem.c (2 hunks)
- targets/ESP32/_include/Target_System_IO_FileSystem.h (1 hunks)
- targets/ESP32/_littlefs/littlefs_FS_Driver.cpp (1 hunks)
- targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp (7 hunks)
Files skipped from review due to trivial changes (1)
- targets/ESP32/_include/Target_System_IO_FileSystem.h
Files skipped from review as they are similar to previous changes (10)
- targets/ESP32/CMakePresets.json
- targets/ESP32/ESP32/target_FileSystem.cpp
- targets/ESP32/ESP32_C3/target_FileSystem.cpp
- targets/ESP32/ESP32_C6/target_FileSystem.cpp
- targets/ESP32/ESP32_H2/target_FileSystem.cpp
- targets/ESP32/ESP32_S2/target_FileSystem.cpp
- targets/ESP32/_IDF/esp32/app_main.c
- targets/ESP32/_common/Target_System_IO_FileSystem.c
- targets/ESP32/_littlefs/littlefs_FS_Driver.cpp
- targets/ESP32/_nanoCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp
Additional comments not posted (6)
targets/ESP32/ESP32_S3/target_FileSystem.cpp (6)
10-10: Inclusion of LittleFS driver approved.Uncommenting the
#include <littlefs_FS_Driver.h>directive enables the use of the LittleFS driver.
12-14: Conditional inclusion of FATFS driver approved.Including the
#include <fatfs_FS_Driver.h>directive within the#if (HAL_USE_SDC == TRUE)block ensures that the FATFS driver is included only when SDC support is enabled.
18-24: External declarations for file system interfaces approved.The external declarations for
g_LITTLEFS_FILE_SYSTEM_DriverInterfaceandg_LITTLEFS_STREAM_DriverInterfacemake these interfaces available for use. The conditional inclusion ofg_FATFS_FILE_SYSTEM_DriverInterfaceandg_FATFS_STREAM_DriverInterfaceensures they are available only when SDC support is enabled.
27-30: Updates tog_AvailableFSInterfacesarray approved.The
g_AvailableFSInterfacesarray now includes the LittleFS interfaces and conditionally includes the FATFS interfaces, expanding the available file system interface options.
41-55: Simplification of volume management logic approved.The
FS_AddVolumesfunction has been simplified to manage a single internal storage volume within the LittleFS partition, enhancing maintainability and focusing on a straightforward volume management approach.
60-60: Placeholder comment inFS_MountRemovableVolumesfunction approved.The placeholder comment indicating no action is required is appropriate given the current scope and potential future use of the function.
Description
(thank you @AdrianSoundy for taking care of this)
Motivation and Context
How Has This Been Tested?
Screenshots
Types of changes
Checklist
Summary by CodeRabbit
New Features
Bug Fixes
Documentation