Add a proper File class
#287
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## master #287 +/- ##
===========================================
- Coverage 97.69% 85.72% -11.97%
===========================================
Files 33 37 +4
Lines 1039 1184 +145
Branches 25 48 +23
===========================================
Hits 1015 1015
- Misses 24 169 +145 ☔ View full report in Codecov by Sentry. |
PatrickKa
changed the title
PatrickKa
force-pushed
the
littlefs-wrapper-273
branch
from
83f4d0d
to
f2bd985
Compare
PatrickKa
force-pushed
the
littlefs-wrapper-273
branch
from
ac96f14
to
d9b457d
Compare
PatrickKa
force-pushed
the
littlefs-wrapper-273
branch
2 times, most recently
from
95a1b61
to
51d8ccd
Compare
PatrickKa
requested changes
Aug 12, 2024
There was a problem hiding this comment.
-
add_compile_definitions(USE_NO_RODOS)in the top-level CML file seems wrong -
Read/Write()should be likefram::ReadFrom/WriteTo(), i.e. they should take byte spans:template<std::size_t extent> [[nodiscard]] auto Read(std::span<Byte, extent> data) const -> Result<int>; template<std::size_t extent> [[nodiscard]] auto Write(std::span<const Byte, extent> data) -> Result<int>;
-
Rename
Read/WriteInternal()to justRead/Write(). The private functions have different parameters, so there is no reason to give them different names for disambiguation (function overloading FTW). -
Use the same order for the public and private
Read/Write()functions
The TEST_CASE()s and other magic from Catch2 made it harder to debug.
This requires addinge the members path_ and openFlags_.
When using designated initializes to initialize a struct, it is perfectly valid not to specify all fields. The unspecified fields are initialized with default member initializers where provided, and empty list-initialization otherwise.
The normal lfs_file_open() uses malloc() to allocate the file buffer. Since we do not want to use the heap, we use lfs_file_opencfg() and provide a statically allocated buffer instead.
This reverts commit 1d0bd2f. In this special case the Swap() function is an abomination, and the previous version with the Move() function is better.
- Rename `Move()` to `MoveConstructFrom()` - Make the class const-correct - Remove `std::cout` and `iostream` - Make cosmetic changes
They were used to test stuff during development but are now obsolete.
The longest path should be "/programs/65536.zip.lock" which is only 25 characters long, so we can save quite a bit of memory when we reduce the max path length from the default 256 to just 30.
PatrickKa
force-pushed
the
littlefs-wrapper-273
branch
from
45988ae
to
62a2058
Compare
- Always pass paths as `Path` aka `etl::string` and use `c_str()` to ensure that the littlefs functions get null-terminated strings - Move member variables which are only used in `CreateLockFile()` to that function
Eventually, the `File` class will need semaphores and therefore require the main function from Rodos. I converted it now, because that allows me to get rid of this `USE_NO_RODOS` nonsense in `Outcome.hpp` too.
PatrickKa
force-pushed
the
littlefs-wrapper-273
branch
from
62a2058
to
8fae95c
Compare
PatrickKa
requested changes
Oct 5, 2024
There was a problem hiding this comment.
- Make
CreateLockFile()andClose()const - In
Open()when callingCreateLockFile()either useOUTCOME_TRY(file.CreateLockFile());or create (and return) a new, specific error code for this, likefileLocked - Instead of removing the lock file in
MoveConstructFrom()by callingClose()and creating it again later, we should add a privateCloseAndKeepLockFile()function that we call instead.Close()should then also call this function and additionally remove the lock file.
flo12356
force-pushed
the
littlefs-wrapper-273
branch
from
22b5b59
to
1624f4b
Compare
PatrickKa
approved these changes
Oct 8, 2024
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Description
Add class
Fileas part of the wrapper for littlefs.Fixes parts of #273
Fixes parts of #186
Fixes parts of #209