From 83f4d0d086161b2f7aac09846c9fd00326305994 Mon Sep 17 00:00:00 2001 From: Patrick Kappl Date: Thu, 20 Jun 2024 13:02:24 +0000 Subject: [PATCH] Use statically allocated file buffer 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. --- Sts1CobcSw/FileSystem/CMakeLists.txt | 5 +++-- Sts1CobcSw/FileSystem/LfsFlash.cpp | 10 ++++++++-- Sts1CobcSw/FileSystem/LfsRam.cpp | 9 +++++++-- Sts1CobcSw/FileSystem/LfsStorageDevice.hpp | 1 + Sts1CobcSw/FileSystem/LfsWrapper.cpp | 12 +++++------- Sts1CobcSw/FileSystem/LfsWrapper.hpp | 13 ++++++++++++- 6 files changed, 36 insertions(+), 14 deletions(-) diff --git a/Sts1CobcSw/FileSystem/CMakeLists.txt b/Sts1CobcSw/FileSystem/CMakeLists.txt index 402b985c..8d495952 100644 --- a/Sts1CobcSw/FileSystem/CMakeLists.txt +++ b/Sts1CobcSw/FileSystem/CMakeLists.txt @@ -1,6 +1,7 @@ target_sources(Sts1CobcSw_FileSystem PRIVATE LfsWrapper.cpp) -target_link_libraries(Sts1CobcSw_FileSystem PUBLIC littlefs::littlefs etl::etl Sts1CobcSw_Outcome) -target_link_libraries(Sts1CobcSw_FileSystem PRIVATE Sts1CobcSw_Serial) +target_link_libraries( + Sts1CobcSw_FileSystem PUBLIC littlefs::littlefs etl::etl Sts1CobcSw_Outcome Sts1CobcSw_Serial +) if(CMAKE_SYSTEM_NAME STREQUAL Generic) target_sources(Sts1CobcSw_FileSystem PRIVATE FileSystem.cpp LfsFlash.cpp) diff --git a/Sts1CobcSw/FileSystem/LfsFlash.cpp b/Sts1CobcSw/FileSystem/LfsFlash.cpp index cef3b205..39b0f98e 100644 --- a/Sts1CobcSw/FileSystem/LfsFlash.cpp +++ b/Sts1CobcSw/FileSystem/LfsFlash.cpp @@ -8,6 +8,7 @@ #include #include +#include #include @@ -27,9 +28,14 @@ auto Erase(lfs_config const * config, lfs_block_t blockNo) -> int; auto Sync(lfs_config const * config) -> int; -auto readBuffer = flash::Page{}; +auto readBuffer = std::array{}; auto programBuffer = decltype(readBuffer){}; -auto lookaheadBuffer = flash::Page{}; +auto lookaheadBuffer = std::array{}; // NOLINT(*magic-numbers) + +// littlefs requires the lookaheadBuffer size to be a multiple of 8 +static_assert(lookaheadBuffer.size() % 8 == 0); // NOLINT(*magic-numbers) +// littlefs requires the cacheSize to be a multiple of the read_size and prog_size, i.e., pageSize +static_assert(lfsCacheSize % flash::pageSize == 0); lfs_config const lfsConfig = lfs_config{.context = nullptr, .read = &Read, diff --git a/Sts1CobcSw/FileSystem/LfsRam.cpp b/Sts1CobcSw/FileSystem/LfsRam.cpp index d20c110a..2ed0012c 100644 --- a/Sts1CobcSw/FileSystem/LfsRam.cpp +++ b/Sts1CobcSw/FileSystem/LfsRam.cpp @@ -33,9 +33,14 @@ constexpr auto sectorSize = 4 * 1024; constexpr auto storageSize = 128 * 1024 * 1024; auto storage = std::vector(); -auto readBuffer = std::array{}; +auto readBuffer = std::array{}; auto programBuffer = decltype(readBuffer){}; -auto lookaheadBuffer = std::array{}; +auto lookaheadBuffer = std::array{}; // NOLINT(*magic-numbers) + +// littlefs requires the lookaheadBuffer size to be a multiple of 8 +static_assert(lookaheadBuffer.size() % 8 == 0); // NOLINT(*magic-numbers) +// littlefs requires the cacheSize to be a multiple of the read_size and prog_size, i.e., pageSize +static_assert(lfsCacheSize % pageSize == 0); lfs_config const lfsConfig = lfs_config{.context = nullptr, .read = &Read, diff --git a/Sts1CobcSw/FileSystem/LfsStorageDevice.hpp b/Sts1CobcSw/FileSystem/LfsStorageDevice.hpp index 96b6ecc6..d3fcf18f 100644 --- a/Sts1CobcSw/FileSystem/LfsStorageDevice.hpp +++ b/Sts1CobcSw/FileSystem/LfsStorageDevice.hpp @@ -5,6 +5,7 @@ namespace sts1cobcsw::fs { +inline constexpr auto lfsCacheSize = 256; extern lfs_config const lfsConfig; diff --git a/Sts1CobcSw/FileSystem/LfsWrapper.cpp b/Sts1CobcSw/FileSystem/LfsWrapper.cpp index 55b4f05e..186aae41 100644 --- a/Sts1CobcSw/FileSystem/LfsWrapper.cpp +++ b/Sts1CobcSw/FileSystem/LfsWrapper.cpp @@ -1,4 +1,3 @@ -#include #include #include @@ -47,8 +46,7 @@ lfs_t lfs{}; auto Open(std::string_view path, int flags) -> Result { auto file = File(); - // TODO: Use lfs_file_opencfg() instead - auto error = lfs_file_open(&lfs, &file.lfsFile_, path.data(), flags); + auto error = lfs_file_opencfg(&lfs, &file.lfsFile_, path.data(), flags, &file.lfsFileConfig_); if(error == 0) { file.path_ = Path(path.data(), path.size()); @@ -66,8 +64,8 @@ File::File(File && other) noexcept { return; } - // TODO: Use lfs_file_opencfg() instead - auto error = lfs_file_open(&lfs, &lfsFile_, other.path_.c_str(), other.openFlags_); + auto error = + lfs_file_opencfg(&lfs, &lfsFile_, other.path_.c_str(), other.openFlags_, &lfsFileConfig_); if(error == 0) { path_ = other.path_; @@ -86,8 +84,8 @@ auto File::operator=(File && other) noexcept -> File & // TODO: Use copy and swap idiom to prevent code duplication if(this != &other and not other.path_.empty()) { - // TODO: Use lfs_file_opencfg() instead - auto error = lfs_file_open(&lfs, &lfsFile_, other.path_.c_str(), other.openFlags_); + auto error = lfs_file_opencfg( + &lfs, &lfsFile_, other.path_.c_str(), other.openFlags_, &lfsFileConfig_); if(error == 0) { path_ = other.path_; diff --git a/Sts1CobcSw/FileSystem/LfsWrapper.hpp b/Sts1CobcSw/FileSystem/LfsWrapper.hpp index ae02caa3..29343941 100644 --- a/Sts1CobcSw/FileSystem/LfsWrapper.hpp +++ b/Sts1CobcSw/FileSystem/LfsWrapper.hpp @@ -2,21 +2,27 @@ #include +#include +#include #include #include +#include #include namespace sts1cobcsw::fs { +// TODO: Consider reducing this to a bit more than 20 = strlen("/programs/65536.zip") to save RAM using Path = etl::string; +// TODO: Get rid of this global variable or at least hide it in an internal namespace extern lfs_t lfs; + class File; [[nodiscard]] auto Mount() -> Result; @@ -33,6 +39,9 @@ class File auto operator=(File && other) noexcept -> File &; ~File(); + // TODO: Read() and Write() should be implemented like ReadFrom() and WriteTo() in Fram.hpp, + // including forwarding to functions in an internal namespace. This way we can move lfs back + // into the .cpp file. template [[nodiscard]] auto Read(T * t) -> Result; template @@ -49,9 +58,11 @@ class File Path path_ = ""; int openFlags_ = 0; bool isOpen_ = false; + std::array buffer_ = {}; lfs_file_t lfsFile_ = {}; + lfs_file_config lfsFileConfig_ = {.buffer = buffer_.data()}; }; } -#include // IWYU pragma: keep \ No newline at end of file +#include // IWYU pragma: keep