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.

Sign up for GitHub

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

Jump to bottom

Add littlefs file system #2911

Open
wants to merge 359 commits into
base: develop-sys.io.filestream
Choose a base branch
from
Open

Add littlefs file system #2911

wants to merge 359 commits into from
+36,167 −37,708

Conversation

josesimoes
Copy link
Member

@josesimoes josesimoes commented Apr 10, 2024 •
edited

Description

  • Add littlefs to build system.
  • Replacing and removing Win.Storage from ChibiOS and Azure RTOS targets.
  • Temporarily removing support for SDCard and USB MSD in ORGPALTHREE.

Motivation and Context

How Has This Been Tested?

Screenshots

Types of changes

  • Improvement (non-breaking change that improves a feature, code or algorithm)
  • Bug fix (non-breaking change which fixes an issue with code or algorithm)
  • New feature (non-breaking change which adds functionality to code)
  • Breaking change (fix or feature that would cause existing functionality to change)
  • Config and build (change in the configuration and build system, has no impact on code or features)
  • Dev Containers (changes related with Dev Containers, has no impact on code or features)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Documentation (changes or updates in the documentation, has no impact on code or features)

Checklist

  • My code follows the code style of this project (only if there are changes in source code).
  • My changes require an update to the documentation (there are changes that require the docs website to be updated).
  • I have updated the documentation accordingly (the changes require an update on the docs in this repo).
  • I have read the CONTRIBUTING document.
  • I have tested everything locally and all new and existing tests passed (only if there are changes in source code).

Ellerbach
Ellerbach reviewed Apr 10, 2024
View reviewed changes
Copy link
Member

@Ellerbach Ellerbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Very nice heavy lifting! Few comments

CMake/Modules/FindLITTLEFS.cmake Show resolved Hide resolved
targets/AzureRTOS/ST/ORGPAL_PALTHREE/CMakePresets.json Show resolved Hide resolved
targets/ChibiOS/CMakeLists.txt Outdated Show resolved Hide resolved
targets/ChibiOS/CMakeLists.txt Show resolved Hide resolved
...oCLR/System.IO.FileSystem/nf_sys_io_filesystem_nanoFramework_System_IO_FileSystem_SDCard.cpp Show resolved Hide resolved
@josesimoes josesimoes force-pushed the develop-add-littlefs branch 5 times, most recently from cfd4dfb to 7f3d6a2 Compare April 10, 2024 23:56
Ellerbach
Ellerbach approved these changes May 2, 2024
View reviewed changes
Copy link
Member

@Ellerbach Ellerbach left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

A lot of work and efforts in the PR and I know a lot of tests. So approving it.

@networkfusion
Copy link
Member

networkfusion commented May 2, 2024 •
edited

Current issue seems to be:
image

and/or undefined ref to lfsFileSystemReady

@networkfusion
Copy link
Member

For ESP32:
image

networkfusion
networkfusion reviewed May 2, 2024
View reviewed changes
.devcontainer/TI/Dockerfile.TI.SRC Outdated
@@ -61,7 +60,7 @@ RUN mkdir -p /usr/local/bin/gcc \
# Clone what is needed for TI
RUN git clone --branch 4.10.00.07 https://github.com/nanoframework/SimpleLink_CC32xx_SDK.git --depth 1 ./sources/SimpleLinkCC32 \
# you can't use the nanoFramework repository as it's Windows only
# && git clone --branch 3.62.01.15 https://github.com/nanoframework/TI_XDCTools.git --depth 1 ./sources/TI_XDCTools \
# && git clone --branch 3.61.00.16 https://github.com/nanoframework/TI_XDCTools.git --depth 1 ./sources/TI_XDCTools \
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any reason in particular for this downgrade in version? perhaps it should be noted as to why.

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

None. I was probably testing something and forgot to revert it.
Actually this line could (should?) be removed as is not doing anything here...

@networkfusion
Copy link
Member

Note: there is no need to continually bump the dockerfile version for smoke tests to succeed. The CI uses the SRC file as default, so the PR should only bump by one version when ready to merge.

networkfusion
networkfusion reviewed May 3, 2024
View reviewed changes
targets/ChibiOS/_littlefs/hal_littlefs.c Outdated

mutex_t lfs_mutex[LITTLEFS_INSTANCES_COUNT];

//bool lfsFileSystemReady;
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Uncommenting this will allow the build to succeed for STM32, but might be commented out for a reason (defined as extern in the header).

alberk8
alberk8 reviewed May 3, 2024
View reviewed changes
CMake/Modules/FindSystem.IO.FileSystem.cmake Outdated
# file system drivers
nanoPAL_FileSystem.cpp

targetPAL_FileSystem.cpp
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These three files are missing in the ESP32 target.

targetPAL_FileSystem.cpp
Target_System_IO_FileSystem.c
target_FileSystem.cpp

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Correct. I'm working on STM32 only. Other platforms will follow.

josesimoes and others added 24 commits June 7, 2024 15:54
@josesimoes
Add implementation of DriveInfo::Refresh
ed5ebb0
@josesimoes
Add back nuget restore for nanoCLR
59a11ba
@josesimoes
Add header file to all platforms
87b4015
@josesimoes
Fix implementation of DriveInfo::Refresh
ffea6bb
@josesimoes
Remove unused files from ChibiOS
45e512d
@josesimoes
First pass migrating ESP32 to new file system
7c5a5af
@josesimoes
Improve comment
33a0646
@josesimoes
Another stab at fixing nanoCLR build in AZDO
d19722d
@josesimoes
Fix NXP build on AZDO
1dc560a
@josesimoes
Comment wrong include
d319f9b 
***WIP***
@josesimoes
More fixes to AZDO build
d3e211f
@AdrianSoundy @nfbot
Fix issue with Wifi AP stopping debug in VS for IDF 5.x (nanoframewor…
2378ea6 
…k#2954)

Co-authored-by: nfbot <nanoframework@outlook.com>
@AdrianSoundy
Fix config for new ESP32_OLIMEX_WROVER target (nanoframework#2955)
d885548
@AdrianSoundy
Add partition table for 32MB ESP32_S3 targets (nanoframework#2956)
5cdd337
@josesimoes
Fix mutex in PAL Events for virtual nanoCLR (nanoframework#2957)
e1e8413 
***NO_CI***
@josesimoes
Merge branch 'main' of https://github.com/nanoframework/nf-interpreter …
0a4e50a 
…into develop-add-littlefs
@josesimoes
Fix forced inclusion of Sys.IO.FileSystem for NXP
9e6dec4
@josesimoes
Comment more fatfs stuff in ESP32 targets
25d5093
@josesimoes
Fix build for NXP
98df075
@josesimoes
Fix declaration for ESP32 build
b4bc9bf
@josesimoes
Include file system header is now dependent of build option
5e7ed27
@josesimoes
Fix dummy declarations
556b68f
@josesimoes
Update declaration of FS_MountVolume
945d9de
@josesimoes
Working on ESP32 FATFS driver
146664e
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Reviewers

@networkfusion networkfusion networkfusion left review comments

@alberk8 alberk8 alberk8 left review comments

@Ellerbach Ellerbach Ellerbach approved these changes

Assignees
No one assigned
Labels
Area: Config-and-Build Area: Dev-Containers Breaking-change Type: enhancement
Projects
None yet
Milestone
No milestone
7 participants
@josesimoes @networkfusion @Ellerbach @alberk8 @nfbot @AdrianSoundy @CoryCharlton