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

Add nativefilestream and several new methods to File and FileStream #98

Merged
merged 73 commits into from
Jul 23, 2024

Conversation

josesimoes
Copy link
Member

@josesimoes josesimoes commented Apr 11, 2024

Description

  • Add native file stream.
  • Add new methods to File and FileStream.
  • Major rework on existing code to use these new APIs.
  • Remove a few methods that now exist on .NET aligned API.
  • Remove several properties and methods that are unlikely to have any use (or hard to implement) in embedded systems context.
  • Improve/fix Intellisense comments.
  • Bump native assembly version.

Motivation and Context

  • Need to improve file access performance.
  • Need implementation to offer file access control/share between threads.
  • Need some APIs that are handy for basic file operations.
  • Improve alignment with .NET API.

How Has This Been Tested?

  • Unit tests being added and integration tests with real hardware. Testing in ORGPAL targets. FatFS and littlefs file systems in 2 different flash (Q)SPI chips, SD card and USB MSD.

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)
  • Dependencies (update dependencies and changes associated, has no impact on code or features)
  • Unit Tests (add new Unit Test(s) or improved existing one(s), 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).
  • I have added new tests to cover my changes.

@gligorov

Summary by CodeRabbit

  • Chores
    • Updated testing framework version to ensure compatibility and reliability in unit tests.

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.

nice heavy lifting, couple of quick comments

System.IO.FileSystem/Directory.cs Outdated Show resolved Hide resolved
System.IO.FileSystem/File.cs Outdated Show resolved Hide resolved
System.IO.FileSystem/FileSystemManager.cs Show resolved Hide resolved
System.IO.FileSystem/DirectoryInfo.cs Outdated Show resolved Hide resolved
System.IO.FileSystem/File.cs Show resolved Hide resolved
System.IO.FileSystem/FileSystemManager.cs Show resolved Hide resolved
System.IO.FileSystem/FileSystemManager.cs Show resolved Hide resolved
@MrCSharp22
Copy link

Looks good to me. A few comments but nothing major. Thanks :)

@networkfusion
Copy link
Member

networkfusion commented Apr 13, 2024

I have added a couple of comments regarding the native methods (as yet unimplemented). I feel that it might be advantagous to change the types of at least offset and count to better check that they "will" be valid as should always be a positive value. We also need to check whether they are in a valid range (offset + count cannot be greater than the file size).

The type being unsigned should also improve RAM allocation.

@networkfusion networkfusion mentioned this pull request Apr 13, 2024
14 tasks
- Add unit tests.
- Fix unit tests.
- All unit test now derive from it.
- Add code to setup to wait/mount removable drives.
- RemovableDeviceEventArgs now takes a DriveInfo parameter.
- Update DriveInfo to use volume index instead of handler.
- Update event manager accordingly.
- Fix construction of StorageEvent.
- Add array of drives to properly handle insert/remove events and update files system manager.
- Code optiization on a couple of methods in File system manager.
- Fix namespace to align with rest of the class lib.
- Add helper methods and update code accordingly.
Copy link

@coderabbitai coderabbitai bot left a comment

Choose a reason for hiding this comment

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

Actionable comments posted: 0

Review details

Configuration used: CodeRabbit UI
Review profile: CHILL

Commits

Files that changed from the base of the PR and between fdf142e and c475575.

Files ignored due to path filters (26)
  • System.IO.FileSystem.UnitTests/DirectoryUnitTests.cs is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/FileSystemUnitTestsBase.cs is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/FileUnitTests.cs is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/PathInternalUnitTests.cs is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/PathUnitTests.cs is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/System.IO.FileSystem.UnitTests.nfproj is excluded by none and included by none
  • System.IO.FileSystem.UnitTests/packages.config is excluded by none and included by none
  • System.IO.FileSystem/Directory.cs is excluded by none and included by none
  • System.IO.FileSystem/DirectoryInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/DriveInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/File.cs is excluded by none and included by none
  • System.IO.FileSystem/FileAttributes.cs is excluded by none and included by none
  • System.IO.FileSystem/FileInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/FileStream.cs is excluded by none and included by none
  • System.IO.FileSystem/FileSystemInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/FileSystemManager.cs is excluded by none and included by none
  • System.IO.FileSystem/NativeFileInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/NativeFileStream.cs is excluded by none and included by none
  • System.IO.FileSystem/NativeFindFile.cs is excluded by none and included by none
  • System.IO.FileSystem/NativeIO.cs is excluded by none and included by none
  • System.IO.FileSystem/Path.cs is excluded by none and included by none
  • System.IO.FileSystem/PathInternal.cs is excluded by none and included by none
  • System.IO.FileSystem/Properties/AssemblyInfo.cs is excluded by none and included by none
  • System.IO.FileSystem/System.IO.FileSystem.nfproj is excluded by none and included by none
  • System.IO.FileSystem/nanoFramework/RemovableDriveEventArgs.cs is excluded by none and included by none
  • System.IO.FileSystem/nanoFramework/StorageEventManager.cs is excluded by none and included by none
Files selected for processing (1)
  • System.IO.FileSystem.UnitTests/packages.lock.json (1 hunks)
Files skipped from review due to trivial changes (1)
  • System.IO.FileSystem.UnitTests/packages.lock.json

Copy link

sonarcloud bot commented Jul 23, 2024

@josesimoes josesimoes merged commit aaf1815 into main Jul 23, 2024
5 checks passed
@josesimoes josesimoes deleted the add-nativefilestream branch July 23, 2024 13:03
@josesimoes josesimoes mentioned this pull request Jul 31, 2024
14 tasks
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Development

Successfully merging this pull request may close these issues.

7 participants