From 1d047f3c9238693486398a59e1f79db918b697e7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Jun 2023 14:55:51 +0200 Subject: [PATCH 01/10] implement ordered sets --- mission/tmtc/PersistentTmStore.cpp | 188 +++++++++++++++++------------ mission/tmtc/PersistentTmStore.h | 17 ++- 2 files changed, 130 insertions(+), 75 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index 08a7ea45..8f695aa4 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -32,6 +32,57 @@ ReturnValue_t PersistentTmStore::cancelDump() { return returnvalue::OK; } +ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t upToUnixSeconds) { + using namespace std::filesystem; + std::error_code e; + dumpParams.orderedDumpFilestamps.clear(); + for (auto const& fileOrDir : directory_iterator(basePath)) { + if (not fileOrDir.is_regular_file(e)) { + continue; + } + dumpParams.fileSize = std::filesystem::file_size(dumpParams.dirEntry.path(), e); + if (e) { + sif::error << "PersistentTmStore: Could not retrieve file size: " << e.message() << std::endl; + continue; + } + // sif::debug << "Path: " << dumpParams.dirEntry.path() << std::endl; + + // File empty or can't even read CCSDS header. + if (dumpParams.fileSize <= 6) { + continue; + } + if (dumpParams.fileSize > fileBuf.size()) { + sif::error << "PersistentTmStore: File too large, is deleted" << std::endl; + triggerEvent(persTmStore::FILE_TOO_LARGE, dumpParams.fileSize, fileBuf.size()); + std::filesystem::remove(dumpParams.dirEntry.path(), e); + continue; + } + const path& file = fileOrDir.path(); + struct tm fileTime {}; + if (pathToTime(file, fileTime) != returnvalue::OK) { + sif::error << "Time extraction for file " << file << "failed" << std::endl; + continue; + } + auto fileEpoch = static_cast(timegm(&fileTime)); + if ((fileEpoch > dumpParams.fromUnixTime) and + (fileEpoch + rolloverDiffSeconds <= dumpParams.untilUnixTime)) { + std::ifstream ifile(file, std::ios::binary); + if (ifile.bad()) { + sif::error << "PersistentTmStore: File is bad" << std::endl; + // TODO: Consider deleting file here? + continue; + } + + // TODO: Check whether this is a suffixed file. + DumpIndex dumpIndex; + dumpIndex.epoch = fileEpoch; + dumpParams.orderedDumpFilestamps.emplace(dumpIndex); + return returnvalue::OK; + } + } + return returnvalue::OK; +} + ReturnValue_t PersistentTmStore::assignAndOrCreateMostRecentFile() { if (not activeFile.has_value()) { return createMostRecentFile(std::nullopt); @@ -159,6 +210,10 @@ bool PersistentTmStore::updateBaseDir() { if (not exists(basePath, e)) { create_directories(basePath); } + // Each file will have the base name as a prefix again + path preparedFullFilePath = basePath / baseName; + basePathSize = preparedFullFilePath.string().length(); + std::memcpy(filePathBuf.data(), preparedFullFilePath.c_str(), basePathSize); baseDirUninitialized = false; return true; } @@ -189,12 +244,19 @@ ReturnValue_t PersistentTmStore::startDumpFromUpTo(uint32_t fromUnixSeconds, if (state == State::DUMPING) { return returnvalue::FAILED; } - dumpParams.dirIter = directory_iterator(basePath); - if (dumpParams.dirIter == directory_iterator()) { + auto dirIter = directory_iterator(basePath); + // Directory empty case. + if (dirIter == directory_iterator()) { return returnvalue::FAILED; } dumpParams.fromUnixTime = fromUnixSeconds; dumpParams.untilUnixTime = upToUnixSeconds; + buildDumpSet(fromUnixSeconds, upToUnixSeconds); + // No files in time window found. + if (dumpParams.orderedDumpFilestamps.empty()) { + return DUMP_DONE; + } + dumpParams.dumpIter = dumpParams.orderedDumpFilestamps.begin(); state = State::DUMPING; return loadNextDumpFile(); } @@ -203,49 +265,23 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { using namespace std::filesystem; dumpParams.currentSize = 0; std::error_code e; - for (; dumpParams.dirIter != directory_iterator(); dumpParams.dirIter++) { - dumpParams.dirEntry = *dumpParams.dirIter; - if (dumpParams.dirEntry.is_directory(e)) { + for (; dumpParams.dumpIter != dumpParams.orderedDumpFilestamps.end(); dumpParams.dumpIter++) { + DumpIndex dumpIndex = *dumpParams.dumpIter; + timeval tv{}; + tv.tv_sec = dumpIndex.epoch; + size_t fullPathLength = 0; + createFileName(tv, dumpIndex.suffixIdx, fullPathLength); + path filePath(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); + std::ifstream ifile(filePath, std::ios::binary); + if (ifile.bad()) { + sif::error << "PersistentTmStore: File is bad" << std::endl; continue; } - dumpParams.fileSize = std::filesystem::file_size(dumpParams.dirEntry.path(), e); - if (e) { - sif::error << "PersistentTmStore: Could not retrieve file size: " << e.message() << std::endl; - continue; - } - // sif::debug << "Path: " << dumpParams.dirEntry.path() << std::endl; - - // File empty or can't even read CCSDS header. - if (dumpParams.fileSize <= 6) { - continue; - } - if (dumpParams.fileSize > fileBuf.size()) { - sif::error << "PersistentTmStore: File too large, is deleted" << std::endl; - triggerEvent(persTmStore::FILE_TOO_LARGE, dumpParams.fileSize, fileBuf.size()); - std::filesystem::remove(dumpParams.dirEntry.path(), e); - continue; - } - const path& file = dumpParams.dirEntry.path(); - struct tm fileTime {}; - if (pathToTime(file, fileTime) != returnvalue::OK) { - sif::error << "Time extraction for file " << file << "failed" << std::endl; - continue; - } - auto fileEpoch = static_cast(timegm(&fileTime)); - if ((fileEpoch > dumpParams.fromUnixTime) and - (fileEpoch + rolloverDiffSeconds <= dumpParams.untilUnixTime)) { - dumpParams.currentFileUnixStamp = fileEpoch; - std::ifstream ifile(file, std::ios::binary); - if (ifile.bad()) { - sif::error << "PersistentTmStore: File is bad" << std::endl; - continue; - } - ifile.read(reinterpret_cast(fileBuf.data()), - static_cast(dumpParams.fileSize)); - // Increment iterator for next cycle. - dumpParams.dirIter++; - return returnvalue::OK; - } + ifile.read(reinterpret_cast(fileBuf.data()), + static_cast(dumpParams.fileSize)); + // Increment iterator for next cycle. + dumpParams.dumpIter++; + return returnvalue::OK; } // Directory iterator was consumed and we are done. state = State::IDLE; @@ -302,37 +338,9 @@ ReturnValue_t PersistentTmStore::pathToTime(const std::filesystem::path& path, s ReturnValue_t PersistentTmStore::createMostRecentFile(std::optional suffix) { using namespace std::filesystem; - unsigned currentIdx = 0; - path pathStart = basePath / baseName; - memcpy(fileBuf.data() + currentIdx, pathStart.c_str(), pathStart.string().length()); - currentIdx += pathStart.string().length(); - fileBuf[currentIdx] = '_'; - currentIdx += 1; - time_t epoch = currentTv.tv_sec; - struct tm* time = gmtime(&epoch); - size_t writtenBytes = strftime(reinterpret_cast(fileBuf.data() + currentIdx), - fileBuf.size(), config::FILE_DATE_FORMAT, time); - if (writtenBytes == 0) { - sif::error << "PersistentTmStore::createMostRecentFile: Could not create file timestamp" - << std::endl; - return returnvalue::FAILED; - } - currentIdx += writtenBytes; - char* res = strcpy(reinterpret_cast(fileBuf.data() + currentIdx), ".bin"); - if (res == nullptr) { - return returnvalue::FAILED; - } - currentIdx += 4; - if (suffix.has_value()) { - std::string fullSuffix = "." + std::to_string(suffix.value()); - res = strcpy(reinterpret_cast(fileBuf.data() + currentIdx), fullSuffix.c_str()); - if (res == nullptr) { - return returnvalue::FAILED; - } - currentIdx += fullSuffix.size(); - } - - path newPath(std::string(reinterpret_cast(fileBuf.data()), currentIdx)); + size_t currentIdx; + createFileName(currentTv, suffix, currentIdx); + path newPath(std::string(reinterpret_cast(filePathBuf.data()), currentIdx)); std::ofstream of(newPath, std::ios::binary); activeFile = newPath; activeFileTv = currentTv; @@ -354,3 +362,35 @@ void PersistentTmStore::getStartAndEndTimeCurrentOrLastDump(uint32_t& startTime, startTime = dumpParams.fromUnixTime; endTime = dumpParams.untilUnixTime; } + +ReturnValue_t PersistentTmStore::createFileName(timeval& tv, std::optional suffix, + size_t& fullPathLength) { + unsigned currentIdx = basePathSize; + fileBuf[currentIdx] = '_'; + currentIdx += 1; + time_t epoch = tv.tv_sec; + struct tm* time = gmtime(&epoch); + size_t writtenBytes = strftime(reinterpret_cast(filePathBuf.data() + currentIdx), + filePathBuf.size(), config::FILE_DATE_FORMAT, time); + if (writtenBytes == 0) { + sif::error << "PersistentTmStore::createMostRecentFile: Could not create file timestamp" + << std::endl; + return returnvalue::FAILED; + } + currentIdx += writtenBytes; + char* res = strcpy(reinterpret_cast(filePathBuf.data() + currentIdx), ".bin"); + if (res == nullptr) { + return returnvalue::FAILED; + } + currentIdx += 4; + if (suffix.has_value()) { + std::string fullSuffix = "." + std::to_string(suffix.value()); + res = strcpy(reinterpret_cast(filePathBuf.data() + currentIdx), fullSuffix.c_str()); + if (res == nullptr) { + return returnvalue::FAILED; + } + currentIdx += fullSuffix.size(); + } + fullPathLength = currentIdx; + return returnvalue::OK; +} diff --git a/mission/tmtc/PersistentTmStore.h b/mission/tmtc/PersistentTmStore.h index 17d2c9e7..8684d89a 100644 --- a/mission/tmtc/PersistentTmStore.h +++ b/mission/tmtc/PersistentTmStore.h @@ -10,6 +10,7 @@ #include #include +#include #include "eive/eventSubsystemIds.h" #include "eive/resultClassIds.h" @@ -37,6 +38,13 @@ struct PersistentTmStoreArgs { SdCardMountedIF& sdcMan; }; +struct DumpIndex { + uint32_t epoch; + uint8_t suffixIdx; + // Define a custom comparison function based on the epoch variable + bool operator<(const DumpIndex& other) const { return epoch < other.epoch; } +}; + class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { public: enum class State { IDLE, DUMPING }; @@ -96,7 +104,10 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { std::string baseName; uint8_t currentSameSecNumber = 0; std::filesystem::path basePath; + // std::filesystem::path pathStart = basePath / baseName; uint32_t rolloverDiffSeconds = 0; + std::array filePathBuf{}; + size_t basePathSize; std::array fileBuf{}; timeval currentTv; timeval activeFileTv{}; @@ -106,8 +117,10 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { uint32_t fromUnixTime = 0; uint32_t untilUnixTime = 0; uint32_t currentFileUnixStamp = 0; - std::filesystem::directory_iterator dirIter; + // std::filesystem::directory_iterator dirIter; std::filesystem::directory_entry dirEntry; + std::set orderedDumpFilestamps{}; + std::set::iterator dumpIter; size_t fileSize = 0; size_t currentSize = 0; }; @@ -122,10 +135,12 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { [[nodiscard]] MessageQueueId_t getCommandQueue() const override; void calcDiffSeconds(RolloverInterval intervalUnit, uint32_t intervalCount); + ReturnValue_t buildDumpSet(uint32_t fromUnixSeconds, uint32_t upToUnixSeconds); ReturnValue_t createMostRecentFile(std::optional suffix); static ReturnValue_t pathToTime(const std::filesystem::path& path, struct tm& time); void fileToPackets(const std::filesystem::path& path, uint32_t unixStamp); ReturnValue_t loadNextDumpFile(); + ReturnValue_t createFileName(timeval& tv, std::optional suffix, size_t& fullPathLength); bool updateBaseDir(); ReturnValue_t assignAndOrCreateMostRecentFile(); }; From 9023405b960a1a36d8f417d883916099f314066f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Jun 2023 16:05:38 +0200 Subject: [PATCH 02/10] some fxes --- mission/tmtc/PersistentTmStore.cpp | 31 ++++++++++++++++----- mission/tmtc/PersistentTmStore.h | 4 +-- unittest/CMakeLists.txt | 2 +- unittest/testGenericFilesystem.cpp | 43 ++++++++++++++++++++++++++++++ unittest/testStampInFilename.cpp | 21 --------------- 5 files changed, 70 insertions(+), 31 deletions(-) create mode 100644 unittest/testGenericFilesystem.cpp delete mode 100644 unittest/testStampInFilename.cpp diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index 8f695aa4..db953c4b 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -40,7 +40,7 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t if (not fileOrDir.is_regular_file(e)) { continue; } - dumpParams.fileSize = std::filesystem::file_size(dumpParams.dirEntry.path(), e); + dumpParams.fileSize = std::filesystem::file_size(fileOrDir.path(), e); if (e) { sif::error << "PersistentTmStore: Could not retrieve file size: " << e.message() << std::endl; continue; @@ -54,7 +54,7 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t if (dumpParams.fileSize > fileBuf.size()) { sif::error << "PersistentTmStore: File too large, is deleted" << std::endl; triggerEvent(persTmStore::FILE_TOO_LARGE, dumpParams.fileSize, fileBuf.size()); - std::filesystem::remove(dumpParams.dirEntry.path(), e); + std::filesystem::remove(fileOrDir.path(), e); continue; } const path& file = fileOrDir.path(); @@ -73,9 +73,9 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t continue; } - // TODO: Check whether this is a suffixed file. DumpIndex dumpIndex; dumpIndex.epoch = fileEpoch; + dumpIndex.suffixIdx = extractSuffix(file.string()); dumpParams.orderedDumpFilestamps.emplace(dumpIndex); return returnvalue::OK; } @@ -83,6 +83,21 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t return returnvalue::OK; } +uint8_t PersistentTmStore::extractSuffix(const std::string& pathStr) { + std::string numberStr; + // Find the position of the dot at the end of the file path + size_t dotPos = pathStr.find_last_of('.'); + if (dotPos != std::string::npos && dotPos < pathStr.length() - 1) { + // Extract the substring after the dot + numberStr = pathStr.substr(dotPos + 1); + } + int number = std::stoi(numberStr); + if (number < 0 or number > std::numeric_limits::max()) { + return 0; + } + return static_cast(number); +} + ReturnValue_t PersistentTmStore::assignAndOrCreateMostRecentFile() { if (not activeFile.has_value()) { return createMostRecentFile(std::nullopt); @@ -271,12 +286,14 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { tv.tv_sec = dumpIndex.epoch; size_t fullPathLength = 0; createFileName(tv, dumpIndex.suffixIdx, fullPathLength); - path filePath(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); - std::ifstream ifile(filePath, std::ios::binary); + dumpParams.currentFile = + path(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); + std::ifstream ifile(dumpParams.currentFile, std::ios::binary); if (ifile.bad()) { sif::error << "PersistentTmStore: File is bad" << std::endl; continue; } + sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); // Increment iterator for next cycle. @@ -303,8 +320,8 @@ ReturnValue_t PersistentTmStore::getNextDumpPacket(PusTmReader& reader, bool& fi // Delete the file and load next. Could use better algorithm to partially // restore the file dump, but for now do not trust the file. std::error_code e; - std::filesystem::remove(dumpParams.dirEntry.path().c_str(), e); - if (dumpParams.dirEntry.path() == activeFile) { + std::filesystem::remove(dumpParams.currentFile.c_str(), e); + if (dumpParams.currentFile == activeFile) { activeFile == std::nullopt; assignAndOrCreateMostRecentFile(); } diff --git a/mission/tmtc/PersistentTmStore.h b/mission/tmtc/PersistentTmStore.h index 8684d89a..e2ad3cd0 100644 --- a/mission/tmtc/PersistentTmStore.h +++ b/mission/tmtc/PersistentTmStore.h @@ -117,8 +117,7 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { uint32_t fromUnixTime = 0; uint32_t untilUnixTime = 0; uint32_t currentFileUnixStamp = 0; - // std::filesystem::directory_iterator dirIter; - std::filesystem::directory_entry dirEntry; + std::filesystem::path currentFile; std::set orderedDumpFilestamps{}; std::set::iterator dumpIter; size_t fileSize = 0; @@ -141,6 +140,7 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { void fileToPackets(const std::filesystem::path& path, uint32_t unixStamp); ReturnValue_t loadNextDumpFile(); ReturnValue_t createFileName(timeval& tv, std::optional suffix, size_t& fullPathLength); + uint8_t extractSuffix(const std::string& pathStr); bool updateBaseDir(); ReturnValue_t assignAndOrCreateMostRecentFile(); }; diff --git a/unittest/CMakeLists.txt b/unittest/CMakeLists.txt index 20b98979..e9a01891 100644 --- a/unittest/CMakeLists.txt +++ b/unittest/CMakeLists.txt @@ -4,7 +4,7 @@ add_subdirectory(mocks) target_sources(${UNITTEST_NAME} PRIVATE main.cpp testEnvironment.cpp - testStampInFilename.cpp + testGenericFilesystem.cpp hdlcEncodingRw.cpp printChar.cpp ) \ No newline at end of file diff --git a/unittest/testGenericFilesystem.cpp b/unittest/testGenericFilesystem.cpp new file mode 100644 index 00000000..e5279bab --- /dev/null +++ b/unittest/testGenericFilesystem.cpp @@ -0,0 +1,43 @@ + +#include +#include + +#include "fsfw/timemanager/Clock.h" + +uint8_t extractSuffix(const std::string& pathStr) { + std::string numberStr; + // Find the position of the dot at the end of the file path + size_t dotPos = pathStr.find_last_of('.'); + if (dotPos != std::string::npos && dotPos < pathStr.length() - 1) { + // Extract the substring after the dot + numberStr = pathStr.substr(dotPos + 1); + } + int number = std::stoi(numberStr); + if (number < 0 or number > std::numeric_limits::max()) { + return 0; + } + return static_cast(number); +} + +TEST_CASE("Stamp in Filename", "[Stamp In Filename]") { + Clock::TimeOfDay_t tod; + std::string baseName = "verif"; + std::string pathStr = "verif_2022-05-25T16:55:23Z.bin"; + unsigned int underscorePos = pathStr.find_last_of('_'); + std::string stampStr = pathStr.substr(underscorePos + 1); + float seconds = 0.0; + char* prefix = nullptr; + int count = + sscanf(stampStr.c_str(), + "%4" SCNu32 "-%2" SCNu32 "-%2" SCNu32 "T%2" SCNu32 ":%2" SCNu32 ":%2" SCNu32 "Z", + &tod.year, &tod.month, &tod.day, &tod.hour, &tod.minute, &tod.second); + static_cast(count); + CHECK(count == 6); +} + +TEST_CASE("Suffix Extraction") { + std::string pathStr = "/mnt/sd0/tm/hk/hk-some-stamp.bin.0"; + CHECK(extractSuffix(pathStr) == 0); + pathStr = "/mnt/sd0/tm/hk/hk-some-stamp.bin.2"; + CHECK(extractSuffix(pathStr) == 2); +} diff --git a/unittest/testStampInFilename.cpp b/unittest/testStampInFilename.cpp deleted file mode 100644 index c66bdce6..00000000 --- a/unittest/testStampInFilename.cpp +++ /dev/null @@ -1,21 +0,0 @@ - -#include -#include - -#include "fsfw/timemanager/Clock.h" - -TEST_CASE("Stamp in Filename", "[Stamp In Filename]") { - Clock::TimeOfDay_t tod; - std::string baseName = "verif"; - std::string pathStr = "verif_2022-05-25T16:55:23Z.bin"; - unsigned int underscorePos = pathStr.find_last_of('_'); - std::string stampStr = pathStr.substr(underscorePos + 1); - float seconds = 0.0; - char* prefix = nullptr; - int count = - sscanf(stampStr.c_str(), - "%4" SCNu32 "-%2" SCNu32 "-%2" SCNu32 "T%2" SCNu32 ":%2" SCNu32 ":%2" SCNu32 "Z", - &tod.year, &tod.month, &tod.day, &tod.hour, &tod.minute, &tod.second); - static_cast(count); - CHECK(count == 6); -} From 2e041c3013549b27940942640c9817c4f55109bb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Jun 2023 17:07:07 +0200 Subject: [PATCH 03/10] some more fixes --- mission/tmtc/PersistentTmStore.cpp | 25 +++++++++++++++++-------- mission/tmtc/PersistentTmStore.h | 4 ++-- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index db953c4b..f4ce65ef 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -40,12 +40,12 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t if (not fileOrDir.is_regular_file(e)) { continue; } + sif::debug << "Path: " << fileOrDir.path() << std::endl; dumpParams.fileSize = std::filesystem::file_size(fileOrDir.path(), e); if (e) { sif::error << "PersistentTmStore: Could not retrieve file size: " << e.message() << std::endl; continue; } - // sif::debug << "Path: " << dumpParams.dirEntry.path() << std::endl; // File empty or can't even read CCSDS header. if (dumpParams.fileSize <= 6) { @@ -83,7 +83,7 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t return returnvalue::OK; } -uint8_t PersistentTmStore::extractSuffix(const std::string& pathStr) { +std::optional PersistentTmStore::extractSuffix(const std::string& pathStr) { std::string numberStr; // Find the position of the dot at the end of the file path size_t dotPos = pathStr.find_last_of('.'); @@ -91,11 +91,18 @@ uint8_t PersistentTmStore::extractSuffix(const std::string& pathStr) { // Extract the substring after the dot numberStr = pathStr.substr(dotPos + 1); } - int number = std::stoi(numberStr); - if (number < 0 or number > std::numeric_limits::max()) { - return 0; + std::optional number; + try { + number = std::stoi(numberStr); + if (number < 0 or number > std::numeric_limits::max()) { + return number; + } + + } catch (std::invalid_argument& exception) { + sif::error << "PersistentTmStore::extractSuffix: Exception " << exception.what() + << ", invald input string: " << numberStr << std::endl; } - return static_cast(number); + return number; } ReturnValue_t PersistentTmStore::assignAndOrCreateMostRecentFile() { @@ -195,6 +202,7 @@ ReturnValue_t PersistentTmStore::storePacket(PusTmReader& reader) { createMostRecentFile(suffix); } + sif::debug << "active file " << activeFile.value() << std::endl; // Rollover conditions were handled, write to file now std::ofstream of(activeFile.value(), std::ios::app | std::ios::binary); of.write(reinterpret_cast(reader.getFullData()), @@ -229,6 +237,8 @@ bool PersistentTmStore::updateBaseDir() { path preparedFullFilePath = basePath / baseName; basePathSize = preparedFullFilePath.string().length(); std::memcpy(filePathBuf.data(), preparedFullFilePath.c_str(), basePathSize); + filePathBuf[basePathSize] = '_'; + basePathSize += 1; baseDirUninitialized = false; return true; } @@ -358,6 +368,7 @@ ReturnValue_t PersistentTmStore::createMostRecentFile(std::optional suf size_t currentIdx; createFileName(currentTv, suffix, currentIdx); path newPath(std::string(reinterpret_cast(filePathBuf.data()), currentIdx)); + sif::debug << "creating new file: " << newPath << std::endl; std::ofstream of(newPath, std::ios::binary); activeFile = newPath; activeFileTv = currentTv; @@ -383,8 +394,6 @@ void PersistentTmStore::getStartAndEndTimeCurrentOrLastDump(uint32_t& startTime, ReturnValue_t PersistentTmStore::createFileName(timeval& tv, std::optional suffix, size_t& fullPathLength) { unsigned currentIdx = basePathSize; - fileBuf[currentIdx] = '_'; - currentIdx += 1; time_t epoch = tv.tv_sec; struct tm* time = gmtime(&epoch); size_t writtenBytes = strftime(reinterpret_cast(filePathBuf.data() + currentIdx), diff --git a/mission/tmtc/PersistentTmStore.h b/mission/tmtc/PersistentTmStore.h index e2ad3cd0..1694a8c9 100644 --- a/mission/tmtc/PersistentTmStore.h +++ b/mission/tmtc/PersistentTmStore.h @@ -40,7 +40,7 @@ struct PersistentTmStoreArgs { struct DumpIndex { uint32_t epoch; - uint8_t suffixIdx; + std::optional suffixIdx; // Define a custom comparison function based on the epoch variable bool operator<(const DumpIndex& other) const { return epoch < other.epoch; } }; @@ -140,7 +140,7 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { void fileToPackets(const std::filesystem::path& path, uint32_t unixStamp); ReturnValue_t loadNextDumpFile(); ReturnValue_t createFileName(timeval& tv, std::optional suffix, size_t& fullPathLength); - uint8_t extractSuffix(const std::string& pathStr); + std::optional extractSuffix(const std::string& pathStr); bool updateBaseDir(); ReturnValue_t assignAndOrCreateMostRecentFile(); }; From 18b67d18a72a4c224c0bd0bff3ca57a9305ed77f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 02:07:11 +0200 Subject: [PATCH 04/10] seems to work now --- mission/tmtc/PersistentTmStore.cpp | 27 ++++++++++++++++----------- 1 file changed, 16 insertions(+), 11 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index f4ce65ef..6a92cdf1 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -40,7 +40,6 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t if (not fileOrDir.is_regular_file(e)) { continue; } - sif::debug << "Path: " << fileOrDir.path() << std::endl; dumpParams.fileSize = std::filesystem::file_size(fileOrDir.path(), e); if (e) { sif::error << "PersistentTmStore: Could not retrieve file size: " << e.message() << std::endl; @@ -76,8 +75,8 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t DumpIndex dumpIndex; dumpIndex.epoch = fileEpoch; dumpIndex.suffixIdx = extractSuffix(file.string()); + // sif::debug << "Inserting file " << fileOrDir.path() << std::endl; dumpParams.orderedDumpFilestamps.emplace(dumpIndex); - return returnvalue::OK; } } return returnvalue::OK; @@ -87,10 +86,11 @@ std::optional PersistentTmStore::extractSuffix(const std::string& pathS std::string numberStr; // Find the position of the dot at the end of the file path size_t dotPos = pathStr.find_last_of('.'); - if (dotPos != std::string::npos && dotPos < pathStr.length() - 1) { - // Extract the substring after the dot - numberStr = pathStr.substr(dotPos + 1); + if ((dotPos < pathStr.length()) and not std::isdigit(pathStr[dotPos + 1])) { + return std::nullopt; } + // Extract the substring after the dot + numberStr = pathStr.substr(dotPos + 1); std::optional number; try { number = std::stoi(numberStr); @@ -202,7 +202,6 @@ ReturnValue_t PersistentTmStore::storePacket(PusTmReader& reader) { createMostRecentFile(suffix); } - sif::debug << "active file " << activeFile.value() << std::endl; // Rollover conditions were handled, write to file now std::ofstream of(activeFile.value(), std::ios::app | std::ios::binary); of.write(reinterpret_cast(reader.getFullData()), @@ -298,12 +297,19 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { createFileName(tv, dumpIndex.suffixIdx, fullPathLength); dumpParams.currentFile = path(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); - std::ifstream ifile(dumpParams.currentFile, std::ios::binary); - if (ifile.bad()) { - sif::error << "PersistentTmStore: File is bad" << std::endl; + dumpParams.fileSize = std::filesystem::file_size(dumpParams.currentFile, e); + if (e) { + // TODO: Event? + sif::error << "PersistentTmStore: Could not load next dump file: " << e.message() + << std::endl; continue; } - sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; + std::ifstream ifile(dumpParams.currentFile, std::ios::binary); + if (ifile.bad()) { + sif::error << "PersistentTmStore: File is bad. Loading next file" << std::endl; + continue; + } + // sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); // Increment iterator for next cycle. @@ -368,7 +374,6 @@ ReturnValue_t PersistentTmStore::createMostRecentFile(std::optional suf size_t currentIdx; createFileName(currentTv, suffix, currentIdx); path newPath(std::string(reinterpret_cast(filePathBuf.data()), currentIdx)); - sif::debug << "creating new file: " << newPath << std::endl; std::ofstream of(newPath, std::ios::binary); activeFile = newPath; activeFileTv = currentTv; From a8ab40674fcae17e8e33e3aef43f6838ad9de287 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 11:08:50 +0200 Subject: [PATCH 05/10] seems to work well --- mission/tmtc/PersistentTmStore.cpp | 17 +++++++++++++---- 1 file changed, 13 insertions(+), 4 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index 6a92cdf1..f8830170 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -17,6 +17,8 @@ using namespace returnvalue; +static constexpr bool DEBUG_DUMPS = false; + PersistentTmStore::PersistentTmStore(PersistentTmStoreArgs args) : SystemObject(args.objectId), tmStore(args.tmStore), @@ -75,7 +77,12 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t DumpIndex dumpIndex; dumpIndex.epoch = fileEpoch; dumpIndex.suffixIdx = extractSuffix(file.string()); - // sif::debug << "Inserting file " << fileOrDir.path() << std::endl; + if (DEBUG_DUMPS) { + sif::debug << "Inserting file " << fileOrDir.path() << std::endl; + if (dumpIndex.suffixIdx.has_value()) { + sif::debug << "Suffix: " << static_cast(dumpIndex.suffixIdx.value()) << std::endl; + } + } dumpParams.orderedDumpFilestamps.emplace(dumpIndex); } } @@ -94,8 +101,8 @@ std::optional PersistentTmStore::extractSuffix(const std::string& pathS std::optional number; try { number = std::stoi(numberStr); - if (number < 0 or number > std::numeric_limits::max()) { - return number; + if (number.value() > std::numeric_limits::max()) { + return std::nullopt; } } catch (std::invalid_argument& exception) { @@ -309,7 +316,9 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { sif::error << "PersistentTmStore: File is bad. Loading next file" << std::endl; continue; } - // sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; + if (DEBUG_DUMPS) { + sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; + } ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); // Increment iterator for next cycle. From 53b48ad99bfea6014721d8d5487e4354cf87b0f4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 11:46:56 +0200 Subject: [PATCH 06/10] this is tricky --- mission/tmtc/PersistentTmStore.cpp | 39 +++++++++++++++++++++++------- mission/tmtc/PersistentTmStore.h | 4 ++- 2 files changed, 33 insertions(+), 10 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index f8830170..6ee2f3a6 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -17,7 +17,7 @@ using namespace returnvalue; -static constexpr bool DEBUG_DUMPS = false; +static constexpr bool DEBUG_DUMPS = true; PersistentTmStore::PersistentTmStore(PersistentTmStoreArgs args) : SystemObject(args.objectId), @@ -74,16 +74,21 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t continue; } - DumpIndex dumpIndex; - dumpIndex.epoch = fileEpoch; - dumpIndex.suffixIdx = extractSuffix(file.string()); if (DEBUG_DUMPS) { sif::debug << "Inserting file " << fileOrDir.path() << std::endl; - if (dumpIndex.suffixIdx.has_value()) { - sif::debug << "Suffix: " << static_cast(dumpIndex.suffixIdx.value()) << std::endl; - } } - dumpParams.orderedDumpFilestamps.emplace(dumpIndex); + DumpIndex dumpIndex; + dumpIndex.epoch = fileEpoch; + // Multiple files for the same time are supported via a special suffix. We smply count the + // number of copies and later try to dump the same number of files with the additonal + // suffixes + auto& iter = dumpParams.orderedDumpFilestamps.find(dumpIndex); + if (iter != dumpParams.orderedDumpFilestamps.end()) { + iter->additionalFiles++; + } else { + dumpIndex.additionalFiles = 0; + dumpParams.orderedDumpFilestamps.emplace(dumpIndex); + } } } return returnvalue::OK; @@ -301,7 +306,7 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { timeval tv{}; tv.tv_sec = dumpIndex.epoch; size_t fullPathLength = 0; - createFileName(tv, dumpIndex.suffixIdx, fullPathLength); + createFileName(tv, dumpParams.currentSameFileIdx, fullPathLength); dumpParams.currentFile = path(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); dumpParams.fileSize = std::filesystem::file_size(dumpParams.currentFile, e); @@ -321,6 +326,22 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { } ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); + if (dumpIndex.additionalFiles > 0) { + dumpParams.currentSameFileIdx++; + } + if (dumpIndex.additionalFiles > 0 and not dumpParams.currentSameFileIdx.has_value()) { + if (not dumpParams.currentSameFileIdx.has_value()) { + // Initialze the file index and stay on same file + dumpParams.currentSameFileIdx = 0; + continue; + + } else if (dumpParams.currentSameFileIdx.value() < dumpIndex.additionalFiles) { + dumpParams.currentSameFileIdx += 1; + continue; + } + } + // File will change, reset this field for correct state-keeping. + dumpParams.currentSameFileIdx = std::nullopt; // Increment iterator for next cycle. dumpParams.dumpIter++; return returnvalue::OK; diff --git a/mission/tmtc/PersistentTmStore.h b/mission/tmtc/PersistentTmStore.h index 1694a8c9..54b87a42 100644 --- a/mission/tmtc/PersistentTmStore.h +++ b/mission/tmtc/PersistentTmStore.h @@ -40,7 +40,8 @@ struct PersistentTmStoreArgs { struct DumpIndex { uint32_t epoch; - std::optional suffixIdx; + // Number of additional files with a suffix like .0, .1 etc. + uint8_t additionalFiles; // Define a custom comparison function based on the epoch variable bool operator<(const DumpIndex& other) const { return epoch < other.epoch; } }; @@ -120,6 +121,7 @@ class PersistentTmStore : public TmStoreFrontendSimpleIF, public SystemObject { std::filesystem::path currentFile; std::set orderedDumpFilestamps{}; std::set::iterator dumpIter; + std::optional currentSameFileIdx = 0; size_t fileSize = 0; size_t currentSize = 0; }; From 04f4eedb78e35cfee982ce0741b08872151000f5 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 12:01:12 +0200 Subject: [PATCH 07/10] that should make it work --- mission/tmtc/PersistentTmStore.cpp | 13 ++++++------- 1 file changed, 6 insertions(+), 7 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index 6ee2f3a6..f68299a0 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -82,13 +82,15 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t // Multiple files for the same time are supported via a special suffix. We smply count the // number of copies and later try to dump the same number of files with the additonal // suffixes - auto& iter = dumpParams.orderedDumpFilestamps.find(dumpIndex); + auto iter = dumpParams.orderedDumpFilestamps.find(dumpIndex); if (iter != dumpParams.orderedDumpFilestamps.end()) { - iter->additionalFiles++; + dumpIndex.epoch = iter->epoch; + dumpIndex.additionalFiles = iter->additionalFiles + 1; + dumpParams.orderedDumpFilestamps.erase(dumpIndex); } else { dumpIndex.additionalFiles = 0; - dumpParams.orderedDumpFilestamps.emplace(dumpIndex); } + dumpParams.orderedDumpFilestamps.emplace(dumpIndex); } } return returnvalue::OK; @@ -326,9 +328,6 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { } ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); - if (dumpIndex.additionalFiles > 0) { - dumpParams.currentSameFileIdx++; - } if (dumpIndex.additionalFiles > 0 and not dumpParams.currentSameFileIdx.has_value()) { if (not dumpParams.currentSameFileIdx.has_value()) { // Initialze the file index and stay on same file @@ -336,7 +335,7 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { continue; } else if (dumpParams.currentSameFileIdx.value() < dumpIndex.additionalFiles) { - dumpParams.currentSameFileIdx += 1; + dumpParams.currentSameFileIdx = dumpParams.currentSameFileIdx.value() + 1; continue; } } From 25d10e3877fe85cc9d3ee0f5596337797b0a0505 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 20:57:54 +0200 Subject: [PATCH 08/10] that was all the fixes (hopefully) --- mission/tmtc/PersistentTmStore.cpp | 50 +++++++++++++++++------------- mission/tmtc/PersistentTmStore.h | 4 +-- 2 files changed, 31 insertions(+), 23 deletions(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index f68299a0..825f9d73 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -79,8 +79,8 @@ ReturnValue_t PersistentTmStore::buildDumpSet(uint32_t fromUnixSeconds, uint32_t } DumpIndex dumpIndex; dumpIndex.epoch = fileEpoch; - // Multiple files for the same time are supported via a special suffix. We smply count the - // number of copies and later try to dump the same number of files with the additonal + // Multiple files for the same time are supported via a special suffix. We simply count the + // number of copies and later try to dump the same number of files with the additional // suffixes auto iter = dumpParams.orderedDumpFilestamps.find(dumpIndex); if (iter != dumpParams.orderedDumpFilestamps.end()) { @@ -295,6 +295,7 @@ ReturnValue_t PersistentTmStore::startDumpFromUpTo(uint32_t fromUnixSeconds, return DUMP_DONE; } dumpParams.dumpIter = dumpParams.orderedDumpFilestamps.begin(); + dumpParams.currentSameFileIdx = std::nullopt; state = State::DUMPING; return loadNextDumpFile(); } @@ -303,7 +304,25 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { using namespace std::filesystem; dumpParams.currentSize = 0; std::error_code e; - for (; dumpParams.dumpIter != dumpParams.orderedDumpFilestamps.end(); dumpParams.dumpIter++) { + // Handle iteration, which does not necessarily have to increment the set iterator + // if there are multiple files for a given timestamp. + auto handleIteration = [&](DumpIndex& dumpIndex) { + if (dumpIndex.additionalFiles > 0) { + if (not dumpParams.currentSameFileIdx.has_value()) { + // Initialize the file index and stay on same file + dumpParams.currentSameFileIdx = 0; + return; + } else if (dumpParams.currentSameFileIdx.value() < dumpIndex.additionalFiles - 1) { + dumpParams.currentSameFileIdx = dumpParams.currentSameFileIdx.value() + 1; + return; + } + } + // File will change, reset this field for correct state-keeping. + dumpParams.currentSameFileIdx = std::nullopt; + // Increment iterator for next cycle. + dumpParams.dumpIter++; + }; + while (dumpParams.dumpIter != dumpParams.orderedDumpFilestamps.end()) { DumpIndex dumpIndex = *dumpParams.dumpIter; timeval tv{}; tv.tv_sec = dumpIndex.epoch; @@ -311,38 +330,27 @@ ReturnValue_t PersistentTmStore::loadNextDumpFile() { createFileName(tv, dumpParams.currentSameFileIdx, fullPathLength); dumpParams.currentFile = path(std::string(reinterpret_cast(filePathBuf.data()), fullPathLength)); + if (DEBUG_DUMPS) { + sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; + } dumpParams.fileSize = std::filesystem::file_size(dumpParams.currentFile, e); if (e) { // TODO: Event? sif::error << "PersistentTmStore: Could not load next dump file: " << e.message() << std::endl; + handleIteration(dumpIndex); continue; } std::ifstream ifile(dumpParams.currentFile, std::ios::binary); if (ifile.bad()) { sif::error << "PersistentTmStore: File is bad. Loading next file" << std::endl; + handleIteration(dumpIndex); continue; } - if (DEBUG_DUMPS) { - sif::debug << baseName << " dump: Loading " << dumpParams.currentFile << std::endl; - } ifile.read(reinterpret_cast(fileBuf.data()), static_cast(dumpParams.fileSize)); - if (dumpIndex.additionalFiles > 0 and not dumpParams.currentSameFileIdx.has_value()) { - if (not dumpParams.currentSameFileIdx.has_value()) { - // Initialze the file index and stay on same file - dumpParams.currentSameFileIdx = 0; - continue; - - } else if (dumpParams.currentSameFileIdx.value() < dumpIndex.additionalFiles) { - dumpParams.currentSameFileIdx = dumpParams.currentSameFileIdx.value() + 1; - continue; - } - } - // File will change, reset this field for correct state-keeping. - dumpParams.currentSameFileIdx = std::nullopt; - // Increment iterator for next cycle. - dumpParams.dumpIter++; + // Next file is loaded, exit the loop. + handleIteration(dumpIndex); return returnvalue::OK; } // Directory iterator was consumed and we are done. diff --git a/mission/tmtc/PersistentTmStore.h b/mission/tmtc/PersistentTmStore.h index 54b87a42..e86acaaf 100644 --- a/mission/tmtc/PersistentTmStore.h +++ b/mission/tmtc/PersistentTmStore.h @@ -39,9 +39,9 @@ struct PersistentTmStoreArgs { }; struct DumpIndex { - uint32_t epoch; + uint32_t epoch = 0; // Number of additional files with a suffix like .0, .1 etc. - uint8_t additionalFiles; + uint8_t additionalFiles = 0; // Define a custom comparison function based on the epoch variable bool operator<(const DumpIndex& other) const { return epoch < other.epoch; } }; From c50a74d11719c2558b7ccc2f2fcc6088b9734a69 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 21:00:57 +0200 Subject: [PATCH 09/10] delete ordering --- mission/tmtc/PersistentTmStore.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/mission/tmtc/PersistentTmStore.cpp b/mission/tmtc/PersistentTmStore.cpp index 825f9d73..acef0b3a 100644 --- a/mission/tmtc/PersistentTmStore.cpp +++ b/mission/tmtc/PersistentTmStore.cpp @@ -17,7 +17,7 @@ using namespace returnvalue; -static constexpr bool DEBUG_DUMPS = true; +static constexpr bool DEBUG_DUMPS = false; PersistentTmStore::PersistentTmStore(PersistentTmStoreArgs args) : SystemObject(args.objectId), From 76933671a6321746af4ced168a6c04bc09f7b4e4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 25 Jun 2023 11:07:41 +0200 Subject: [PATCH 10/10] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index fb62b8fc..38c7cd51 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,10 @@ will consitute of a breaking change warranting a new major release: # [unreleased] +## Changed + +- Persistent TM store dumps are now performed in chronological order + # [v4.0.1] 2023-06-24 ## Fixed