From a3b140b680f1c2dad52d41f216252d6c3e1b6ef1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 22 Jun 2023 16:09:24 +0200 Subject: [PATCH 01/40] internal error reporter extension --- bsp_q7s/em/emObjectFactory.cpp | 4 ++-- bsp_q7s/fmObjectFactory.cpp | 2 +- fsfw | 2 +- mission/genericFactory.cpp | 4 ++-- mission/genericFactory.h | 3 ++- tmtc | 2 +- 6 files changed, 9 insertions(+), 8 deletions(-) diff --git a/bsp_q7s/em/emObjectFactory.cpp b/bsp_q7s/em/emObjectFactory.cpp index 464b3398..d068bed0 100644 --- a/bsp_q7s/em/emObjectFactory.cpp +++ b/bsp_q7s/em/emObjectFactory.cpp @@ -36,8 +36,8 @@ void ObjectFactory::produce(void* args) { PersistentTmStores stores; ObjectFactory::produceGenericObjects(&healthTable, &pusFunnel, &cfdpFunnel, - *SdCardManager::instance(), &ipcStore, &tmStore, stores, - 200); + *SdCardManager::instance(), &ipcStore, &tmStore, stores, 200, + enableHkSets); LinuxLibgpioIF* gpioComIF = nullptr; SerialComIF* uartComIF = nullptr; diff --git a/bsp_q7s/fmObjectFactory.cpp b/bsp_q7s/fmObjectFactory.cpp index 413e5648..da14f58c 100644 --- a/bsp_q7s/fmObjectFactory.cpp +++ b/bsp_q7s/fmObjectFactory.cpp @@ -34,7 +34,7 @@ void ObjectFactory::produce(void* args) { PersistentTmStores stores; ObjectFactory::produceGenericObjects(&healthTable, &pusFunnel, &cfdpFunnel, *SdCardManager::instance(), &ipcStore, &tmStore, stores, - 200); + 200, true); LinuxLibgpioIF* gpioComIF = nullptr; SerialComIF* uartComIF = nullptr; diff --git a/fsfw b/fsfw index 0f76cdb3..a85a38c8 160000 --- a/fsfw +++ b/fsfw @@ -1 +1 @@ -Subproject commit 0f76cdb3ba54f5e90a8eee4316c49cf0f581f996 +Subproject commit a85a38c882af968200a0dd54ded9fd99b3961e7a diff --git a/mission/genericFactory.cpp b/mission/genericFactory.cpp index cbe78c7f..5edee640 100644 --- a/mission/genericFactory.cpp +++ b/mission/genericFactory.cpp @@ -96,14 +96,14 @@ void ObjectFactory::produceGenericObjects(HealthTableIF** healthTable_, PusTmFun CfdpTmFunnel** cfdpFunnel, SdCardMountedIF& sdcMan, StorageManagerIF** ipcStore, StorageManagerIF** tmStore, PersistentTmStores& stores, - uint32_t eventManagerQueueDepth) { + uint32_t eventManagerQueueDepth, bool enableHkSets) { // Framework objects new EventManager(objects::EVENT_MANAGER, eventManagerQueueDepth); auto healthTable = new HealthTable(objects::HEALTH_TABLE); if (healthTable_ != nullptr) { *healthTable_ = healthTable; } - new InternalErrorReporter(objects::INTERNAL_ERROR_REPORTER); + new InternalErrorReporter(objects::INTERNAL_ERROR_REPORTER, 5, enableHkSets, 120); new VerificationReporter(); auto* timeStamper = new CdsShortTimeStamper(objects::TIME_STAMPER); StorageManagerIF* tcStore; diff --git a/mission/genericFactory.h b/mission/genericFactory.h index a3a52704..03bbfaa5 100644 --- a/mission/genericFactory.h +++ b/mission/genericFactory.h @@ -45,7 +45,8 @@ namespace ObjectFactory { void produceGenericObjects(HealthTableIF** healthTable, PusTmFunnel** pusFunnel, CfdpTmFunnel** cfdpFunnel, SdCardMountedIF& sdcMan, StorageManagerIF** ipcStore, StorageManagerIF** tmStore, - PersistentTmStores& stores, uint32_t eventManagerQueueDepth); + PersistentTmStores& stores, uint32_t eventManagerQueueDepth, + bool enableHkSets); void createGenericHeaterComponents(GpioIF& gpioIF, PowerSwitchIF& pwrSwitcher, HeaterHandler*& heaterHandler); diff --git a/tmtc b/tmtc index 1bb8bea8..e0457831 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit 1bb8bea8d92fef2c9ec58ea657b04b56635c16dd +Subproject commit e0457831d72e0de8611c5f56221a30f19aea7e2f From 74e945ddffe101653955c75ea0849293c3c2383c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 22 Jun 2023 16:10:07 +0200 Subject: [PATCH 02/40] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index c642b897..4ea3ac85 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,12 @@ will consitute of a breaking change warranting a new major release: # [unreleased] +# [v4.0.1] to be released + +## Changed + +- Internal error reporter set is now enabled by defalt and generated every 120 seconds. + # [v4.0.0] to be released - `eive-tmtc` version v4.0.0 From 32b074b86e4636408a3b3353f6608f5d4be3c4b6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 22 Jun 2023 16:29:52 +0200 Subject: [PATCH 03/40] that should fix the build --- bsp_hosted/ObjectFactory.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bsp_hosted/ObjectFactory.cpp b/bsp_hosted/ObjectFactory.cpp index dd88d552..23f1a79f 100644 --- a/bsp_hosted/ObjectFactory.cpp +++ b/bsp_hosted/ObjectFactory.cpp @@ -68,7 +68,7 @@ void ObjectFactory::produce(void* args) { #endif auto sdcMan = new DummySdCardManager("/tmp"); ObjectFactory::produceGenericObjects(nullptr, &pusFunnel, &cfdpFunnel, *sdcMan, &ipcStore, - &tmStore, persistentStores, 120); + &tmStore, persistentStores, 120, enableHkSets); new TmFunnelHandler(objects::LIVE_TM_TASK, *pusFunnel, *cfdpFunnel); auto* dummyGpioIF = new DummyGpioIF(); From 253af8193fe4160fee19a4041aabad763d6aac20 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 22 Jun 2023 18:43:04 +0200 Subject: [PATCH 04/40] fix unittest --- unittest/testEnvironment.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/unittest/testEnvironment.cpp b/unittest/testEnvironment.cpp index 72726f39..4c19c4cf 100644 --- a/unittest/testEnvironment.cpp +++ b/unittest/testEnvironment.cpp @@ -27,7 +27,7 @@ void factory(void* args) { new HouseKeepingMock(); eventManager = new EventManagerMock(); new HealthTable(objects::HEALTH_TABLE); - new InternalErrorReporter(objects::INTERNAL_ERROR_REPORTER); + new InternalErrorReporter(objects::INTERNAL_ERROR_REPORTER, 5, false, 60.0); new CdsShortTimeStamper(objects::TIME_STAMPER); { From 1d047f3c9238693486398a59e1f79db918b697e7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 23 Jun 2023 14:55:51 +0200 Subject: [PATCH 05/40] 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 06/40] 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 07/40] 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 08/40] 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 09/40] 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 10/40] 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 11/40] 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 88e8268c26c141c9a6ff586f4c2658021215aac2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 17:14:56 +0200 Subject: [PATCH 12/40] start refactoring reboot wd handling --- bsp_q7s/core/CoreController.cpp | 14 +++++++------- bsp_q7s/core/CoreController.h | 6 ++++-- mission/sysDefs.h | 3 ++- unittest/rebootLogic/src/CoreController.cpp | 12 ++++++------ unittest/rebootLogic/src/CoreController.h | 2 +- 5 files changed, 20 insertions(+), 17 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 4e006298..0db3d414 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -321,7 +321,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ if (size < 1) { return HasActionsIF::INVALID_PARAMETERS; } - std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; parseRebootFile(path, rebootFile); if (data[0] == 0) { rebootFile.enabled = false; @@ -335,7 +335,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ return HasActionsIF::EXECUTION_FINISHED; } case (READ_REBOOT_MECHANISM_INFO): { - std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; parseRebootFile(path, rebootFile); RebootMechanismPacket packet(rebootFile); ReturnValue_t result = actionHelper.reportData(commandedBy, actionId, &packet); @@ -446,7 +446,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ if (size < 1) { return HasActionsIF::INVALID_PARAMETERS; } - std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); rebootFile.maxCount = data[0]; @@ -1629,7 +1629,7 @@ ReturnValue_t CoreController::performSdCardCheck() { void CoreController::performRebootFileHandling(bool recreateFile) { using namespace std; - std::string path = currMntPrefix + REBOOT_FILE; + std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::error_code e; if (not std::filesystem::exists(path, e) or recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 @@ -2009,7 +2009,7 @@ bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { } void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { - std::string path = currMntPrefix + REBOOT_FILE; + std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; parseRebootFile(path, rebootFile); if (tgtChip == xsc::ALL_CHIP and tgtCopy == xsc::ALL_COPY) { rebootFile.img00Cnt = 0; @@ -2035,7 +2035,7 @@ void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { } void CoreController::rewriteRebootFile(RebootFile file) { - std::string path = currMntPrefix + REBOOT_FILE; + std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { // Initiate reboot file first. Reboot handling will be on on initialization @@ -2052,7 +2052,7 @@ void CoreController::rewriteRebootFile(RebootFile file) { } void CoreController::setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy) { - std::string path = currMntPrefix + REBOOT_FILE; + std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); if (tgtChip == xsc::CHIP_0) { diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index 7ab80aa7..8d5c16c5 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -113,10 +113,12 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe const std::string VERSION_FILE = "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::VERSION_FILE_NAME); - const std::string REBOOT_FILE = - "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::REBOOT_FILE_NAME); + const std::string REBOOT_WATCHDOG_FILE = + "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::REBOOT_WATCHDOG_FILE_NAME); const std::string BACKUP_TIME_FILE = "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::TIME_FILE_NAME); + const std::string REBOOT_COUNTERS_FILE = + "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::REBOOT_COUNTER_FILE_NAME); static constexpr char CHIP_0_COPY_0_MOUNT_DIR[] = "/tmp/mntupdate-xdi-qspi0-nom-rootfs"; static constexpr char CHIP_0_COPY_1_MOUNT_DIR[] = "/tmp/mntupdate-xdi-qspi0-gold-rootfs"; diff --git a/mission/sysDefs.h b/mission/sysDefs.h index 32ce9642..234b1ff3 100644 --- a/mission/sysDefs.h +++ b/mission/sysDefs.h @@ -41,7 +41,8 @@ enum SystemctlCmd : uint8_t { START = 0, STOP = 1, RESTART = 2, NUM_CMDS = 3 }; static constexpr char CONF_FOLDER[] = "conf"; static constexpr char VERSION_FILE_NAME[] = "version.txt"; -static constexpr char REBOOT_FILE_NAME[] = "reboot.txt"; +static constexpr char REBOOT_WATCHDOG_FILE_NAME[] = "reboot_watchdog.txt"; +static constexpr char REBOOT_COUNTER_FILE_NAME[] = "reboot_counters.txt"; static constexpr char TIME_FILE_NAME[] = "time_backup.txt"; static constexpr uint32_t SYS_ROM_BASE_ADDR = 0x80000000; diff --git a/unittest/rebootLogic/src/CoreController.cpp b/unittest/rebootLogic/src/CoreController.cpp index 2bc8bbef..b99c0114 100644 --- a/unittest/rebootLogic/src/CoreController.cpp +++ b/unittest/rebootLogic/src/CoreController.cpp @@ -21,7 +21,7 @@ CoreController::CoreController() { void CoreController::performRebootFileHandling(bool recreateFile) { using namespace std; - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; if (not std::filesystem::exists(path) or recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 sif::info << "CoreController::performRebootFileHandling: Recreating reboot file" << std::endl; @@ -400,7 +400,7 @@ bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { } void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); if (tgtChip == xsc::ALL_CHIP and tgtCopy == xsc::ALL_COPY) { @@ -427,7 +427,7 @@ void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { } void CoreController::rewriteRebootFile(RebootFile file) { - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { // Initiate reboot file first. Reboot handling will be on on initialization @@ -450,7 +450,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ if (size < 1) { return HasActionsIF::INVALID_PARAMETERS; } - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); if (data[0] == 0) { @@ -490,7 +490,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ if (size < 1) { return HasActionsIF::INVALID_PARAMETERS; } - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); rebootFile.maxCount = data[0]; @@ -504,7 +504,7 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } void CoreController::setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy) { - std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_FILE; + std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism parseRebootFile(path, rebootFile); if (tgtChip == xsc::CHIP_0) { diff --git a/unittest/rebootLogic/src/CoreController.h b/unittest/rebootLogic/src/CoreController.h index 1846c27f..4c3f0e31 100644 --- a/unittest/rebootLogic/src/CoreController.h +++ b/unittest/rebootLogic/src/CoreController.h @@ -39,7 +39,7 @@ class SdCardManager; class CoreController { public: - static constexpr char REBOOT_FILE[] = "/conf/reboot.txt"; + static constexpr char REBOOT_WATCHDOG_FILE[] = "/conf/reboot.txt"; //! [EXPORT] : [COMMENT] The reboot mechanism was triggered. //! P1: First 16 bits: Last Chip, Last 16 bits: Last Copy, //! P2: Each byte is the respective reboot count for the slots From 07df6eb95f3392d737bdfbe1fb4a1eebb4afc394 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 17:29:23 +0200 Subject: [PATCH 13/40] continuing --- bsp_q7s/core/CoreController.cpp | 169 ++++++++++---------- bsp_q7s/core/CoreController.h | 49 +++++- unittest/rebootLogic/src/CoreController.cpp | 157 +++++++++--------- unittest/rebootLogic/src/CoreController.h | 12 +- 4 files changed, 212 insertions(+), 175 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 0db3d414..61ec8659 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -322,13 +322,13 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ return HasActionsIF::INVALID_PARAMETERS; } std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (data[0] == 0) { - rebootFile.enabled = false; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = false; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else if (data[0] == 1) { - rebootFile.enabled = true; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = true; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else { return HasActionsIF::INVALID_PARAMETERS; } @@ -336,8 +336,8 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } case (READ_REBOOT_MECHANISM_INFO): { std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; - parseRebootFile(path, rebootFile); - RebootMechanismPacket packet(rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); + RebootWatchdogPacket packet(rebootWatchdogFile); ReturnValue_t result = actionHelper.reportData(commandedBy, actionId, &packet); if (result != returnvalue::OK) { return result; @@ -448,9 +448,9 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } std::string path = sdcMan->getCurrentMountPrefix() + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); - rebootFile.maxCount = data[0]; - rewriteRebootFile(rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); + rebootWatchdogFile.maxCount = data[0]; + rewriteRebootWatchdogFile(rebootWatchdogFile); return HasActionsIF::EXECUTION_FINISHED; } case (XSC_REBOOT_OBC): { @@ -1555,7 +1555,7 @@ void CoreController::performMountedSdCardOperations() { if (not timeFileInitDone) { initClockFromTimeFile(); } - performRebootFileHandling(false); + performRebootWatchdogHandling(false); } backupTimeFileHandler(); }; @@ -1627,7 +1627,7 @@ ReturnValue_t CoreController::performSdCardCheck() { return returnvalue::OK; } -void CoreController::performRebootFileHandling(bool recreateFile) { +void CoreController::performRebootWatchdogHandling(bool recreateFile) { using namespace std; std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::error_code e; @@ -1635,109 +1635,110 @@ void CoreController::performRebootFileHandling(bool recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 sif::info << "CoreController::performRebootFileHandling: Recreating reboot file" << std::endl; #endif - rebootFile.enabled = false; - rebootFile.img00Cnt = 0; - rebootFile.img01Cnt = 0; - rebootFile.img10Cnt = 0; - rebootFile.img11Cnt = 0; - rebootFile.lastChip = xsc::Chip::CHIP_0; - rebootFile.lastCopy = xsc::Copy::COPY_0; - rebootFile.img00Lock = false; - rebootFile.img01Lock = false; - rebootFile.img10Lock = false; - rebootFile.img11Lock = false; - rebootFile.mechanismNextChip = xsc::Chip::NO_CHIP; - rebootFile.mechanismNextCopy = xsc::Copy::NO_COPY; - rebootFile.bootFlag = false; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = false; + rebootWatchdogFile.img00Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; + rebootWatchdogFile.lastChip = xsc::Chip::CHIP_0; + rebootWatchdogFile.lastCopy = xsc::Copy::COPY_0; + rebootWatchdogFile.img00Lock = false; + rebootWatchdogFile.img01Lock = false; + rebootWatchdogFile.img10Lock = false; + rebootWatchdogFile.img11Lock = false; + rebootWatchdogFile.mechanismNextChip = xsc::Chip::NO_CHIP; + rebootWatchdogFile.mechanismNextCopy = xsc::Copy::NO_COPY; + rebootWatchdogFile.bootFlag = false; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else { - if (not parseRebootFile(path, rebootFile)) { - performRebootFileHandling(true); + if (not parseRebootWatchdogFile(path, rebootWatchdogFile)) { + performRebootWatchdogHandling(true); } } if (CURRENT_CHIP == xsc::CHIP_0) { if (CURRENT_COPY == xsc::COPY_0) { - rebootFile.img00Cnt++; + rebootWatchdogFile.img00Cnt++; } else { - rebootFile.img01Cnt++; + rebootWatchdogFile.img01Cnt++; } } else { if (CURRENT_COPY == xsc::COPY_0) { - rebootFile.img10Cnt++; + rebootWatchdogFile.img10Cnt++; } else { - rebootFile.img11Cnt++; + rebootWatchdogFile.img11Cnt++; } } - if (rebootFile.bootFlag) { + if (rebootWatchdogFile.bootFlag) { // Trigger event to inform ground that a reboot was triggered - uint32_t p1 = rebootFile.lastChip << 16 | rebootFile.lastCopy; + uint32_t p1 = rebootWatchdogFile.lastChip << 16 | rebootWatchdogFile.lastCopy; triggerEvent(core::REBOOT_MECHANISM_TRIGGERED, p1, 0); // Clear the boot flag - rebootFile.bootFlag = false; + rebootWatchdogFile.bootFlag = false; } announceBootCounts(); - if (rebootFile.mechanismNextChip != xsc::NO_CHIP and - rebootFile.mechanismNextCopy != xsc::NO_COPY) { - if (CURRENT_CHIP != rebootFile.mechanismNextChip or - CURRENT_COPY != rebootFile.mechanismNextCopy) { - std::string infoString = std::to_string(rebootFile.mechanismNextChip) + " " + - std::to_string(rebootFile.mechanismNextCopy); + if (rebootWatchdogFile.mechanismNextChip != xsc::NO_CHIP and + rebootWatchdogFile.mechanismNextCopy != xsc::NO_COPY) { + if (CURRENT_CHIP != rebootWatchdogFile.mechanismNextChip or + CURRENT_COPY != rebootWatchdogFile.mechanismNextCopy) { + std::string infoString = std::to_string(rebootWatchdogFile.mechanismNextChip) + " " + + std::to_string(rebootWatchdogFile.mechanismNextCopy); sif::warning << "CoreController::performRebootFileHandling: Expected to be on image " << infoString << " but currently on other image. Locking " << infoString << std::endl; // Firmware or other component might be corrupt and we are on another image then the target // image specified by the mechanism. We can't really trust the target image anymore. // Lock it for now - if (rebootFile.mechanismNextChip == xsc::CHIP_0) { - if (rebootFile.mechanismNextCopy == xsc::COPY_0) { - rebootFile.img00Lock = true; + if (rebootWatchdogFile.mechanismNextChip == xsc::CHIP_0) { + if (rebootWatchdogFile.mechanismNextCopy == xsc::COPY_0) { + rebootWatchdogFile.img00Lock = true; } else { - rebootFile.img01Lock = true; + rebootWatchdogFile.img01Lock = true; } } else { - if (rebootFile.mechanismNextCopy == xsc::COPY_0) { - rebootFile.img10Lock = true; + if (rebootWatchdogFile.mechanismNextCopy == xsc::COPY_0) { + rebootWatchdogFile.img10Lock = true; } else { - rebootFile.img11Lock = true; + rebootWatchdogFile.img11Lock = true; } } } } - rebootFile.lastChip = CURRENT_CHIP; - rebootFile.lastCopy = CURRENT_COPY; + rebootWatchdogFile.lastChip = CURRENT_CHIP; + rebootWatchdogFile.lastCopy = CURRENT_COPY; // Only reboot if the reboot functionality is enabled. // The handler will still increment the boot counts - if (rebootFile.enabled and (*rebootFile.relevantBootCnt >= rebootFile.maxCount)) { + if (rebootWatchdogFile.enabled and + (*rebootWatchdogFile.relevantBootCnt >= rebootWatchdogFile.maxCount)) { // Reboot to other image bool doReboot = false; xsc::Chip tgtChip = xsc::NO_CHIP; xsc::Copy tgtCopy = xsc::NO_COPY; - determineAndExecuteReboot(rebootFile, doReboot, tgtChip, tgtCopy); + determineAndExecuteReboot(rebootWatchdogFile, doReboot, tgtChip, tgtCopy); if (doReboot) { - rebootFile.bootFlag = true; + rebootWatchdogFile.bootFlag = true; #if OBSW_VERBOSE_LEVEL >= 1 sif::info << "Boot counter for image " << CURRENT_CHIP << " " << CURRENT_COPY << " too high. Rebooting to " << tgtChip << " " << tgtCopy << std::endl; #endif - rebootFile.mechanismNextChip = tgtChip; - rebootFile.mechanismNextCopy = tgtCopy; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.mechanismNextChip = tgtChip; + rebootWatchdogFile.mechanismNextCopy = tgtCopy; + rewriteRebootWatchdogFile(rebootWatchdogFile); xsc_boot_copy(static_cast(tgtChip), static_cast(tgtCopy)); } } else { - rebootFile.mechanismNextChip = xsc::NO_CHIP; - rebootFile.mechanismNextCopy = xsc::NO_COPY; + rebootWatchdogFile.mechanismNextChip = xsc::NO_CHIP; + rebootWatchdogFile.mechanismNextCopy = xsc::NO_COPY; } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::determineAndExecuteReboot(RebootFile &rf, bool &needsReboot, +void CoreController::determineAndExecuteReboot(RebootWatchdogFile &rf, bool &needsReboot, xsc::Chip &tgtChip, xsc::Copy &tgtCopy) { tgtChip = xsc::CHIP_0; tgtCopy = xsc::COPY_0; @@ -1826,7 +1827,7 @@ void CoreController::determineAndExecuteReboot(RebootFile &rf, bool &needsReboot } } -bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { +bool CoreController::parseRebootWatchdogFile(std::string path, RebootWatchdogFile &rf) { using namespace std; std::string selfMatch; if (CURRENT_CHIP == xsc::CHIP_0) { @@ -2010,31 +2011,31 @@ bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::ALL_CHIP and tgtCopy == xsc::ALL_COPY) { - rebootFile.img00Cnt = 0; - rebootFile.img01Cnt = 0; - rebootFile.img10Cnt = 0; - rebootFile.img11Cnt = 0; + rebootWatchdogFile.img00Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; } else { if (tgtChip == xsc::CHIP_0) { if (tgtCopy == xsc::COPY_0) { - rebootFile.img00Cnt = 0; + rebootWatchdogFile.img00Cnt = 0; } else { - rebootFile.img01Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; } } else { if (tgtCopy == xsc::COPY_0) { - rebootFile.img10Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; } else { - rebootFile.img11Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; } } } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::rewriteRebootFile(RebootFile file) { +void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { @@ -2054,21 +2055,21 @@ void CoreController::rewriteRebootFile(RebootFile file) { void CoreController::setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::CHIP_0) { if (tgtCopy == xsc::COPY_0) { - rebootFile.img00Lock = lock; + rebootWatchdogFile.img00Lock = lock; } else { - rebootFile.img01Lock = lock; + rebootWatchdogFile.img01Lock = lock; } } else { if (tgtCopy == xsc::COPY_0) { - rebootFile.img10Lock = lock; + rebootWatchdogFile.img10Lock = lock; } else { - rebootFile.img11Lock = lock; + rebootWatchdogFile.img11Lock = lock; } } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } ReturnValue_t CoreController::backupTimeFileHandler() { @@ -2355,10 +2356,12 @@ bool CoreController::startSdStateMachine(sd::SdCard targetActiveSd, SdCfgMode mo } void CoreController::announceBootCounts() { - uint64_t totalBootCount = - rebootFile.img00Cnt + rebootFile.img01Cnt + rebootFile.img10Cnt + rebootFile.img11Cnt; - uint32_t individualBootCountsP1 = (rebootFile.img00Cnt << 16) | rebootFile.img01Cnt; - uint32_t individualBootCountsP2 = (rebootFile.img10Cnt << 16) | rebootFile.img11Cnt; + uint64_t totalBootCount = rebootWatchdogFile.img00Cnt + rebootWatchdogFile.img01Cnt + + rebootWatchdogFile.img10Cnt + rebootWatchdogFile.img11Cnt; + uint32_t individualBootCountsP1 = + (rebootWatchdogFile.img00Cnt << 16) | rebootWatchdogFile.img01Cnt; + uint32_t individualBootCountsP2 = + (rebootWatchdogFile.img10Cnt << 16) | rebootWatchdogFile.img11Cnt; triggerEvent(core::INDIVIDUAL_BOOT_COUNTS, individualBootCountsP1, individualBootCountsP2); triggerEvent(core::REBOOT_COUNTER, (totalBootCount >> 32) & 0xffffffff, totalBootCount & 0xffffffff); diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index 8d5c16c5..778d3fa0 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -24,7 +24,7 @@ class Timer; class SdCardManager; -struct RebootFile { +struct RebootWatchdogFile { static constexpr uint8_t DEFAULT_MAX_BOOT_CNT = 10; bool enabled = true; @@ -45,9 +45,9 @@ struct RebootFile { xsc::Copy mechanismNextCopy = xsc::Copy::NO_COPY; }; -class RebootMechanismPacket : public SerialLinkedListAdapter { +class RebootWatchdogPacket : public SerialLinkedListAdapter { public: - RebootMechanismPacket(RebootFile& rf) { + RebootWatchdogPacket(RebootWatchdogFile& rf) { enabled = rf.enabled; maxCount = rf.maxCount; img00Count = rf.img00Cnt; @@ -100,6 +100,38 @@ class RebootMechanismPacket : public SerialLinkedListAdapter { SerializeElement nextCopy = 0; }; +struct RebootCountersFile { + // 16 bit values so all boot counters fit into one event. + uint16_t img00Cnt = 0; + uint16_t img01Cnt = 0; + uint16_t img10Cnt = 0; + uint16_t img11Cnt = 0; +}; + +class RebootCountersPacket : public SerialLinkedListAdapter { + RebootCountersPacket(RebootCountersFile& rf) { + img00Count = rf.img00Cnt; + img01Count = rf.img01Cnt; + img10Count = rf.img10Cnt; + img11Count = rf.img11Cnt; + setLinks(); + } + + private: + void setLinks() { + setStart(&img00Count); + img00Count.setNext(&img01Count); + img01Count.setNext(&img10Count); + img10Count.setNext(&img11Count); + setLast(&img11Count); + } + + SerializeElement img00Count = 0; + SerializeElement img01Count = 0; + SerializeElement img10Count = 0; + SerializeElement img11Count = 0; +}; + class CoreController : public ExtendedControllerBase, public ReceivesParameterMessagesIF { public: enum ParamId : uint8_t { PREF_SD = 0, NUM_IDS }; @@ -250,7 +282,8 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe std::array dirListingBuf{}; DirListingDumpContext dumpContext{}; - RebootFile rebootFile = {}; + RebootWatchdogFile rebootWatchdogFile = {}; + RebootCountersFile rebootCountersFile = {}; CommandExecutor cmdExecutor; SimpleRingBuffer cmdReplyBuf; @@ -320,7 +353,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe void currentStateSetter(sd::SdCard sdCard, sd::SdState newState); void executeNextExternalSdCommand(); void checkExternalSdCommandStatus(); - void performRebootFileHandling(bool recreateFile); + void performRebootWatchdogHandling(bool recreateFile); ReturnValue_t actionListDirectoryIntoFile(ActionId_t actionId, MessageQueueId_t commandedBy, const uint8_t* data, size_t size); @@ -339,12 +372,12 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe int handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect, bool& protOperationPerformed, bool selfChip, bool selfCopy, bool allChips, bool allCopies, uint8_t arrIdx); - void determineAndExecuteReboot(RebootFile& rf, bool& needsReboot, xsc::Chip& tgtChip, + void determineAndExecuteReboot(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, xsc::Copy& tgtCopy); void resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy); void setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy); - bool parseRebootFile(std::string path, RebootFile& file); - void rewriteRebootFile(RebootFile file); + bool parseRebootWatchdogFile(std::string path, RebootWatchdogFile& file); + void rewriteRebootWatchdogFile(RebootWatchdogFile file); void announceBootCounts(); void announceVersionInfo(); void announceCurrentImageInfo(); diff --git a/unittest/rebootLogic/src/CoreController.cpp b/unittest/rebootLogic/src/CoreController.cpp index b99c0114..23461ea1 100644 --- a/unittest/rebootLogic/src/CoreController.cpp +++ b/unittest/rebootLogic/src/CoreController.cpp @@ -19,116 +19,117 @@ CoreController::CoreController() { setCurrentBootCopy(xsc::CHIP_0, xsc::COPY_0); } -void CoreController::performRebootFileHandling(bool recreateFile) { +void CoreController::performRebootWatchdogHandling(bool recreateFile) { using namespace std; std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; if (not std::filesystem::exists(path) or recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 sif::info << "CoreController::performRebootFileHandling: Recreating reboot file" << std::endl; #endif - rebootFile.enabled = true; - rebootFile.img00Cnt = 0; - rebootFile.img01Cnt = 0; - rebootFile.img10Cnt = 0; - rebootFile.img11Cnt = 0; - rebootFile.lastChip = xsc::Chip::CHIP_0; - rebootFile.lastCopy = xsc::Copy::COPY_0; - rebootFile.img00Lock = false; - rebootFile.img01Lock = false; - rebootFile.img10Lock = false; - rebootFile.img11Lock = false; - rebootFile.mechanismNextChip = xsc::Chip::NO_CHIP; - rebootFile.mechanismNextCopy = xsc::Copy::NO_COPY; - rebootFile.bootFlag = false; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = true; + rebootWatchdogFile.img00Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; + rebootWatchdogFile.lastChip = xsc::Chip::CHIP_0; + rebootWatchdogFile.lastCopy = xsc::Copy::COPY_0; + rebootWatchdogFile.img00Lock = false; + rebootWatchdogFile.img01Lock = false; + rebootWatchdogFile.img10Lock = false; + rebootWatchdogFile.img11Lock = false; + rebootWatchdogFile.mechanismNextChip = xsc::Chip::NO_CHIP; + rebootWatchdogFile.mechanismNextCopy = xsc::Copy::NO_COPY; + rebootWatchdogFile.bootFlag = false; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else { - if (not parseRebootFile(path, rebootFile)) { - performRebootFileHandling(true); + if (not parseRebootWatchdogFile(path, rebootWatchdogFile)) { + performRebootWatchdogHandling(true); } } if (CURRENT_CHIP == xsc::CHIP_0) { if (CURRENT_COPY == xsc::COPY_0) { - rebootFile.img00Cnt++; + rebootWatchdogFile.img00Cnt++; } else { - rebootFile.img01Cnt++; + rebootWatchdogFile.img01Cnt++; } } else { if (CURRENT_COPY == xsc::COPY_0) { - rebootFile.img10Cnt++; + rebootWatchdogFile.img10Cnt++; } else { - rebootFile.img11Cnt++; + rebootWatchdogFile.img11Cnt++; } } - if (rebootFile.bootFlag) { + if (rebootWatchdogFile.bootFlag) { // Trigger event to inform ground that a reboot was triggered - uint32_t p1 = rebootFile.lastChip << 16 | rebootFile.lastCopy; - uint32_t p2 = rebootFile.img00Cnt << 24 | rebootFile.img01Cnt << 16 | rebootFile.img10Cnt << 8 | - rebootFile.img11Cnt; + uint32_t p1 = rebootWatchdogFile.lastChip << 16 | rebootWatchdogFile.lastCopy; + uint32_t p2 = rebootWatchdogFile.img00Cnt << 24 | rebootWatchdogFile.img01Cnt << 16 | + rebootWatchdogFile.img10Cnt << 8 | rebootWatchdogFile.img11Cnt; triggerEvent(REBOOT_MECHANISM_TRIGGERED, p1, p2); // Clear the boot flag - rebootFile.bootFlag = false; + rebootWatchdogFile.bootFlag = false; } - if (rebootFile.mechanismNextChip != xsc::NO_CHIP and - rebootFile.mechanismNextCopy != xsc::NO_COPY) { - if (CURRENT_CHIP != rebootFile.mechanismNextChip or - CURRENT_COPY != rebootFile.mechanismNextCopy) { - std::string infoString = std::to_string(rebootFile.mechanismNextChip) + " " + - std::to_string(rebootFile.mechanismNextCopy); + if (rebootWatchdogFile.mechanismNextChip != xsc::NO_CHIP and + rebootWatchdogFile.mechanismNextCopy != xsc::NO_COPY) { + if (CURRENT_CHIP != rebootWatchdogFile.mechanismNextChip or + CURRENT_COPY != rebootWatchdogFile.mechanismNextCopy) { + std::string infoString = std::to_string(rebootWatchdogFile.mechanismNextChip) + " " + + std::to_string(rebootWatchdogFile.mechanismNextCopy); sif::warning << "CoreController::performRebootFileHandling: Expected to be on image " << infoString << " but currently on other image. Locking " << infoString << std::endl; // Firmware or other component might be corrupt and we are on another image then the target // image specified by the mechanism. We can't really trust the target image anymore. // Lock it for now - if (rebootFile.mechanismNextChip == xsc::CHIP_0) { - if (rebootFile.mechanismNextCopy == xsc::COPY_0) { - rebootFile.img00Lock = true; + if (rebootWatchdogFile.mechanismNextChip == xsc::CHIP_0) { + if (rebootWatchdogFile.mechanismNextCopy == xsc::COPY_0) { + rebootWatchdogFile.img00Lock = true; } else { - rebootFile.img01Lock = true; + rebootWatchdogFile.img01Lock = true; } } else { - if (rebootFile.mechanismNextCopy == xsc::COPY_0) { - rebootFile.img10Lock = true; + if (rebootWatchdogFile.mechanismNextCopy == xsc::COPY_0) { + rebootWatchdogFile.img10Lock = true; } else { - rebootFile.img11Lock = true; + rebootWatchdogFile.img11Lock = true; } } } } - rebootFile.lastChip = CURRENT_CHIP; - rebootFile.lastCopy = CURRENT_COPY; + rebootWatchdogFile.lastChip = CURRENT_CHIP; + rebootWatchdogFile.lastCopy = CURRENT_COPY; // Only reboot if the reboot functionality is enabled. // The handler will still increment the boot counts - if (rebootFile.enabled and (*rebootFile.relevantBootCnt >= rebootFile.maxCount)) { + if (rebootWatchdogFile.enabled and + (*rebootWatchdogFile.relevantBootCnt >= rebootWatchdogFile.maxCount)) { // Reboot to other image bool doReboot = false; xsc::Chip tgtChip = xsc::NO_CHIP; xsc::Copy tgtCopy = xsc::NO_COPY; - determineAndExecuteReboot(rebootFile, doReboot, tgtChip, tgtCopy); + determineAndExecuteReboot(rebootWatchdogFile, doReboot, tgtChip, tgtCopy); if (doReboot) { - rebootFile.bootFlag = true; + rebootWatchdogFile.bootFlag = true; #if OBSW_VERBOSE_LEVEL >= 1 sif::info << "Boot counter for image " << CURRENT_CHIP << " " << CURRENT_COPY << " too high. Rebooting to " << tgtChip << " " << tgtCopy << std::endl; #endif - rebootFile.mechanismNextChip = tgtChip; - rebootFile.mechanismNextCopy = tgtCopy; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.mechanismNextChip = tgtChip; + rebootWatchdogFile.mechanismNextCopy = tgtCopy; + rewriteRebootWatchdogFile(rebootWatchdogFile); xsc_boot_copy(static_cast(tgtChip), static_cast(tgtCopy)); } } else { - rebootFile.mechanismNextChip = xsc::NO_CHIP; - rebootFile.mechanismNextCopy = xsc::NO_COPY; + rebootWatchdogFile.mechanismNextChip = xsc::NO_CHIP; + rebootWatchdogFile.mechanismNextCopy = xsc::NO_COPY; } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::determineAndExecuteReboot(RebootFile &rf, bool &needsReboot, +void CoreController::determineAndExecuteReboot(RebootWatchdogFile &rf, bool &needsReboot, xsc::Chip &tgtChip, xsc::Copy &tgtCopy) { tgtChip = xsc::CHIP_0; tgtCopy = xsc::COPY_0; @@ -217,7 +218,7 @@ void CoreController::determineAndExecuteReboot(RebootFile &rf, bool &needsReboot } } -bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { +bool CoreController::parseRebootWatchdogFile(std::string path, RebootWatchdogFile &rf) { using namespace std; std::string selfMatch; if (CURRENT_CHIP == xsc::CHIP_0) { @@ -402,31 +403,31 @@ bool CoreController::parseRebootFile(std::string path, RebootFile &rf) { void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::ALL_CHIP and tgtCopy == xsc::ALL_COPY) { - rebootFile.img00Cnt = 0; - rebootFile.img01Cnt = 0; - rebootFile.img10Cnt = 0; - rebootFile.img11Cnt = 0; + rebootWatchdogFile.img00Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; } else { if (tgtChip == xsc::CHIP_0) { if (tgtCopy == xsc::COPY_0) { - rebootFile.img00Cnt = 0; + rebootWatchdogFile.img00Cnt = 0; } else { - rebootFile.img01Cnt = 0; + rebootWatchdogFile.img01Cnt = 0; } } else { if (tgtCopy == xsc::COPY_0) { - rebootFile.img10Cnt = 0; + rebootWatchdogFile.img10Cnt = 0; } else { - rebootFile.img11Cnt = 0; + rebootWatchdogFile.img11Cnt = 0; } } } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::rewriteRebootFile(RebootFile file) { +void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { @@ -452,13 +453,13 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (data[0] == 0) { - rebootFile.enabled = false; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = false; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else if (data[0] == 1) { - rebootFile.enabled = true; - rewriteRebootFile(rebootFile); + rebootWatchdogFile.enabled = true; + rewriteRebootWatchdogFile(rebootWatchdogFile); } else { return HasActionsIF::INVALID_PARAMETERS; } @@ -492,9 +493,9 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); - rebootFile.maxCount = data[0]; - rewriteRebootFile(rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); + rebootWatchdogFile.maxCount = data[0]; + rewriteRebootWatchdogFile(rebootWatchdogFile); return HasActionsIF::EXECUTION_FINISHED; } default: { @@ -506,21 +507,21 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ void CoreController::setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = sdcMan->getCurrentMountPrefix(sdInfo.active) + REBOOT_WATCHDOG_FILE; // Disable the reboot file mechanism - parseRebootFile(path, rebootFile); + parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::CHIP_0) { if (tgtCopy == xsc::COPY_0) { - rebootFile.img00Lock = lock; + rebootWatchdogFile.img00Lock = lock; } else { - rebootFile.img01Lock = lock; + rebootWatchdogFile.img01Lock = lock; } } else { if (tgtCopy == xsc::COPY_0) { - rebootFile.img10Lock = lock; + rebootWatchdogFile.img10Lock = lock; } else { - rebootFile.img11Lock = lock; + rebootWatchdogFile.img11Lock = lock; } } - rewriteRebootFile(rebootFile); + rewriteRebootWatchdogFile(rebootWatchdogFile); } void CoreController::setCurrentBootCopy(xsc::Chip chip, xsc::Copy copy) { diff --git a/unittest/rebootLogic/src/CoreController.h b/unittest/rebootLogic/src/CoreController.h index 4c3f0e31..34d2a5f7 100644 --- a/unittest/rebootLogic/src/CoreController.h +++ b/unittest/rebootLogic/src/CoreController.h @@ -14,7 +14,7 @@ enum Copy : int { COPY_0, COPY_1, NO_COPY, SELF_COPY, ALL_COPY }; } // namespace xsc -struct RebootFile { +struct RebootWatchdogFile { static constexpr uint8_t DEFAULT_MAX_BOOT_CNT = 10; bool enabled = true; @@ -57,13 +57,13 @@ class CoreController { static void setCurrentBootCopy(xsc::Chip chip, xsc::Copy copy); ReturnValue_t executeAction(ActionId_t actionId, MessageQueueId_t commandedBy, const uint8_t* data, size_t size); - void performRebootFileHandling(bool recreateFile); - void determineAndExecuteReboot(RebootFile& rf, bool& needsReboot, xsc::Chip& tgtChip, + void performRebootWatchdogHandling(bool recreateFile); + void determineAndExecuteReboot(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, xsc::Copy& tgtCopy); void setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy); void resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy); - bool parseRebootFile(std::string path, RebootFile& file); - void rewriteRebootFile(RebootFile file); + bool parseRebootWatchdogFile(std::string path, RebootWatchdogFile& file); + void rewriteRebootWatchdogFile(RebootWatchdogFile file); private: struct SdFsmParams { @@ -74,6 +74,6 @@ class CoreController { } sdInfo; SdCardManager* sdcMan = nullptr; - RebootFile rebootFile = {}; + RebootWatchdogFile rebootWatchdogFile = {}; bool doPerformRebootFileHandling = true; }; \ No newline at end of file From 7f3645585728286e1f0e84aa1d4dad989166d900 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 18:06:58 +0200 Subject: [PATCH 14/40] fix project file, continue core ctrl --- bsp_q7s/core/CoreController.cpp | 58 +++++++++++++++++++++++++++++++-- bsp_q7s/core/CoreController.h | 5 ++- misc/eclipse/.cproject | 39 ++++++++++++++-------- 3 files changed, 85 insertions(+), 17 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 61ec8659..25ce04bb 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -346,12 +346,13 @@ ReturnValue_t CoreController::executeAction(ActionId_t actionId, MessageQueueId_ } case (RESET_REBOOT_COUNTERS): { if (size == 0) { - resetRebootCount(xsc::ALL_CHIP, xsc::ALL_COPY); + resetRebootWatchdogCounters(xsc::ALL_CHIP, xsc::ALL_COPY); } else if (size == 2) { if (data[0] > 1 or data[1] > 1) { return HasActionsIF::INVALID_PARAMETERS; } - resetRebootCount(static_cast(data[0]), static_cast(data[1])); + resetRebootWatchdogCounters(static_cast(data[0]), + static_cast(data[1])); } return HasActionsIF::EXECUTION_FINISHED; } @@ -2009,7 +2010,56 @@ bool CoreController::parseRebootWatchdogFile(std::string path, RebootWatchdogFil return true; } -void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { +bool CoreController::parseRebootCountersFile(std::string path, RebootCountersFile &rf) { + using namespace std; + ifstream file(path); + string word; + string line; + uint8_t lineIdx = 0; + while (std::getline(file, line)) { + istringstream iss(line); + switch (lineIdx) { + case 0: { + iss >> word; + if (word.find("img00:") == string::npos) { + return false; + } + iss >> rf.img00Cnt; + + break; + } + case 1: { + iss >> word; + if (word.find("img01:") == string::npos) { + return false; + } + iss >> rf.img01Cnt; + + break; + } + case 2: { + iss >> word; + if (word.find("img10:") == string::npos) { + return false; + } + iss >> rf.img10Cnt; + + break; + } + case 3: { + iss >> word; + if (word.find("img11:") == string::npos) { + return false; + } + iss >> rf.img11Cnt; + break; + } + } + } + return true; +} + +void CoreController::resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::ALL_CHIP and tgtCopy == xsc::ALL_COPY) { @@ -2035,6 +2085,8 @@ void CoreController::resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy) { rewriteRebootWatchdogFile(rebootWatchdogFile); } +void CoreController::performRebootCountersHandling(bool recreateFile) { +} void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index 778d3fa0..bfca7f27 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -354,6 +354,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe void executeNextExternalSdCommand(); void checkExternalSdCommandStatus(); void performRebootWatchdogHandling(bool recreateFile); + void performRebootCountersHandling(bool recreateFile); ReturnValue_t actionListDirectoryIntoFile(ActionId_t actionId, MessageQueueId_t commandedBy, const uint8_t* data, size_t size); @@ -374,10 +375,12 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe bool allChips, bool allCopies, uint8_t arrIdx); void determineAndExecuteReboot(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, xsc::Copy& tgtCopy); - void resetRebootCount(xsc::Chip tgtChip, xsc::Copy tgtCopy); + void resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy); void setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy); bool parseRebootWatchdogFile(std::string path, RebootWatchdogFile& file); + bool parseRebootCountersFile(std::string path, RebootCountersFile& file); void rewriteRebootWatchdogFile(RebootWatchdogFile file); + void rewriteRebootCountersFile(RebootCountersFile file); void announceBootCounts(); void announceVersionInfo(); void announceCurrentImageInfo(); diff --git a/misc/eclipse/.cproject b/misc/eclipse/.cproject index 4cfe3fa1..154cb27e 100644 --- a/misc/eclipse/.cproject +++ b/misc/eclipse/.cproject @@ -57,7 +57,8 @@ - + + @@ -119,7 +120,8 @@ - + + @@ -187,7 +189,8 @@ - + + @@ -255,7 +258,8 @@ - + + @@ -418,7 +422,8 @@ - + + @@ -580,7 +585,8 @@ - + + @@ -750,7 +756,8 @@ - + + @@ -917,7 +924,8 @@ - + + @@ -1084,7 +1092,8 @@ - + + @@ -1149,7 +1158,8 @@ - + + @@ -1172,7 +1182,7 @@ - + - + + + @@ -1386,7 +1398,8 @@ - + + From 4e7b774d32b93bdf305f5bf252f15bd1febbd753 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 18:37:58 +0200 Subject: [PATCH 15/40] some legacy/backword compatiblity additions --- bsp_q7s/core/CoreController.cpp | 72 +++++++++++++++++++++++++++++---- bsp_q7s/core/CoreController.h | 3 ++ mission/sysDefs.h | 1 + tmtc | 2 +- 4 files changed, 69 insertions(+), 9 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 25ce04bb..b3f91506 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1557,6 +1557,7 @@ void CoreController::performMountedSdCardOperations() { initClockFromTimeFile(); } performRebootWatchdogHandling(false); + performRebootCountersHandling(false); } backupTimeFileHandler(); }; @@ -1631,10 +1632,17 @@ ReturnValue_t CoreController::performSdCardCheck() { void CoreController::performRebootWatchdogHandling(bool recreateFile) { using namespace std; std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; + std::string legacyPath = currMntPrefix + LEGACY_REBOOT_WATCHDOG_FILE; std::error_code e; + // TODO: Remove at some point in the future. + if (std::filesystem::exists(legacyPath)) { + // Old file might still exist, so copy it to new path + std::filesystem::copy(legacyPath, path); + } if (not std::filesystem::exists(path, e) or recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 - sif::info << "CoreController::performRebootFileHandling: Recreating reboot file" << std::endl; + sif::info << "CoreController::performRebootFileHandling: Recreating reboot watchdog file" + << std::endl; #endif rebootWatchdogFile.enabled = false; rebootWatchdogFile.img00Cnt = 0; @@ -1654,6 +1662,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { } else { if (not parseRebootWatchdogFile(path, rebootWatchdogFile)) { performRebootWatchdogHandling(true); + return; } } @@ -1679,8 +1688,6 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { rebootWatchdogFile.bootFlag = false; } - announceBootCounts(); - if (rebootWatchdogFile.mechanismNextChip != xsc::NO_CHIP and rebootWatchdogFile.mechanismNextCopy != xsc::NO_COPY) { if (CURRENT_CHIP != rebootWatchdogFile.mechanismNextChip or @@ -2086,9 +2093,45 @@ void CoreController::resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tg } void CoreController::performRebootCountersHandling(bool recreateFile) { + std::string path = currMntPrefix + REBOOT_COUNTERS_FILE; + std::error_code e; + if (not std::filesystem::exists(path, e) or recreateFile) { +#if OBSW_VERBOSE_LEVEL >= 1 + sif::info << "CoreController::performRebootFileHandling: Recreating reboot counters file" + << std::endl; +#endif + rebootCountersFile.img00Cnt = 0; + rebootCountersFile.img01Cnt = 0; + rebootCountersFile.img10Cnt = 0; + rebootCountersFile.img11Cnt = 0; + rewriteRebootCountersFile(rebootCountersFile); + } else { + if (not parseRebootCountersFile(path, rebootCountersFile)) { + performRebootCountersHandling(true); + return; + } + } + + if (CURRENT_CHIP == xsc::CHIP_0) { + if (CURRENT_COPY == xsc::COPY_0) { + rebootCountersFile.img00Cnt++; + } else { + rebootCountersFile.img01Cnt++; + } + } else { + if (CURRENT_COPY == xsc::COPY_0) { + rebootCountersFile.img10Cnt++; + } else { + rebootCountersFile.img11Cnt++; + } + } + announceBootCounts(); + rewriteRebootCountersFile(rebootCountersFile); } void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { + using namespace std::filesystem; std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; + std::string legacyPath = currMntPrefix + LEGACY_REBOOT_WATCHDOG_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { // Initiate reboot file first. Reboot handling will be on on initialization @@ -2102,11 +2145,24 @@ void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { << "\nnext: " << static_cast(file.mechanismNextChip) << " " << static_cast(file.mechanismNextCopy) << "\n"; } + // TODO: Remove at some point in the future when all images have been updated. + if (std::filesystem::exists(legacyPath)) { + // Keep those two files in sync + std::filesystem::copy(path, legacyPath); + } +} + +void CoreController::rewriteRebootCountersFile(RebootCountersFile file) { + std::string path = currMntPrefix + REBOOT_COUNTERS_FILE; + std::ofstream rebootFile(path); + if (rebootFile.is_open()) { + rebootFile << "\nimg00: " << file.img00Cnt << "\nimg01: " << file.img01Cnt + << "\nimg10: " << file.img10Cnt << "\nimg11: " << file.img11Cnt << "\n"; + } } void CoreController::setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy) { std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; - // Disable the reboot file mechanism parseRebootWatchdogFile(path, rebootWatchdogFile); if (tgtChip == xsc::CHIP_0) { if (tgtCopy == xsc::COPY_0) { @@ -2408,12 +2464,12 @@ bool CoreController::startSdStateMachine(sd::SdCard targetActiveSd, SdCfgMode mo } void CoreController::announceBootCounts() { - uint64_t totalBootCount = rebootWatchdogFile.img00Cnt + rebootWatchdogFile.img01Cnt + - rebootWatchdogFile.img10Cnt + rebootWatchdogFile.img11Cnt; + uint64_t totalBootCount = rebootCountersFile.img00Cnt + rebootCountersFile.img01Cnt + + rebootCountersFile.img10Cnt + rebootCountersFile.img11Cnt; uint32_t individualBootCountsP1 = - (rebootWatchdogFile.img00Cnt << 16) | rebootWatchdogFile.img01Cnt; + (rebootCountersFile.img00Cnt << 16) | rebootCountersFile.img01Cnt; uint32_t individualBootCountsP2 = - (rebootWatchdogFile.img10Cnt << 16) | rebootWatchdogFile.img11Cnt; + (rebootCountersFile.img10Cnt << 16) | rebootCountersFile.img11Cnt; triggerEvent(core::INDIVIDUAL_BOOT_COUNTS, individualBootCountsP1, individualBootCountsP2); triggerEvent(core::REBOOT_COUNTER, (totalBootCount >> 32) & 0xffffffff, totalBootCount & 0xffffffff); diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index bfca7f27..e3f514d9 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -145,6 +145,9 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe const std::string VERSION_FILE = "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::VERSION_FILE_NAME); + const std::string LEGACY_REBOOT_WATCHDOG_FILE = + "/" + std::string(core::CONF_FOLDER) + "/" + + std::string(core::LEGACY_REBOOT_WATCHDOG_FILE_NAME); const std::string REBOOT_WATCHDOG_FILE = "/" + std::string(core::CONF_FOLDER) + "/" + std::string(core::REBOOT_WATCHDOG_FILE_NAME); const std::string BACKUP_TIME_FILE = diff --git a/mission/sysDefs.h b/mission/sysDefs.h index 234b1ff3..fe572144 100644 --- a/mission/sysDefs.h +++ b/mission/sysDefs.h @@ -41,6 +41,7 @@ enum SystemctlCmd : uint8_t { START = 0, STOP = 1, RESTART = 2, NUM_CMDS = 3 }; static constexpr char CONF_FOLDER[] = "conf"; static constexpr char VERSION_FILE_NAME[] = "version.txt"; +static constexpr char LEGACY_REBOOT_WATCHDOG_FILE_NAME[] = "reboot.txt"; static constexpr char REBOOT_WATCHDOG_FILE_NAME[] = "reboot_watchdog.txt"; static constexpr char REBOOT_COUNTER_FILE_NAME[] = "reboot_counters.txt"; static constexpr char TIME_FILE_NAME[] = "time_backup.txt"; diff --git a/tmtc b/tmtc index c9f4a807..deb0275b 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit c9f4a8070d20bc659809d5b822ac5a17548f57a4 +Subproject commit deb0275bb5603394122e26f74760d2051685f324 From 6040cd2d1ed3de7e98c45e2a26e4641d3fa8b62e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 18:48:12 +0200 Subject: [PATCH 16/40] some important tweaks --- bsp_q7s/core/CoreController.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index b3f91506..9bd323a0 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1637,7 +1637,10 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { // TODO: Remove at some point in the future. if (std::filesystem::exists(legacyPath)) { // Old file might still exist, so copy it to new path - std::filesystem::copy(legacyPath, path); + std::filesystem::copy(legacyPath, path, std::filesystem::copy_options::overwrite_existing, e); + if (e) { + sif::error << "File copy has failed: " << e.message() << std::endl; + } } if (not std::filesystem::exists(path, e) or recreateFile) { #if OBSW_VERBOSE_LEVEL >= 1 @@ -2145,10 +2148,14 @@ void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { << "\nnext: " << static_cast(file.mechanismNextChip) << " " << static_cast(file.mechanismNextCopy) << "\n"; } + std::error_code e; // TODO: Remove at some point in the future when all images have been updated. if (std::filesystem::exists(legacyPath)) { // Keep those two files in sync - std::filesystem::copy(path, legacyPath); + std::filesystem::copy(path, legacyPath, std::filesystem::copy_options::overwrite_existing, e); + if (e) { + sif::error << "File copy has failed: " << e.message() << std::endl; + } } } From 5420e322f715e0c28cb67cd8ad0c4529e2738f48 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 18:52:29 +0200 Subject: [PATCH 17/40] this is annoying to test.. --- bsp_q7s/core/CoreController.cpp | 26 ++++++++++++++------------ 1 file changed, 14 insertions(+), 12 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 9bd323a0..4257d008 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -2135,18 +2135,20 @@ void CoreController::rewriteRebootWatchdogFile(RebootWatchdogFile file) { using namespace std::filesystem; std::string path = currMntPrefix + REBOOT_WATCHDOG_FILE; std::string legacyPath = currMntPrefix + LEGACY_REBOOT_WATCHDOG_FILE; - std::ofstream rebootFile(path); - if (rebootFile.is_open()) { - // Initiate reboot file first. Reboot handling will be on on initialization - rebootFile << "on: " << file.enabled << "\nmaxcnt: " << file.maxCount - << "\nimg00: " << file.img00Cnt << "\nimg01: " << file.img01Cnt - << "\nimg10: " << file.img10Cnt << "\nimg11: " << file.img11Cnt - << "\nimg00lock: " << file.img00Lock << "\nimg01lock: " << file.img01Lock - << "\nimg10lock: " << file.img10Lock << "\nimg11lock: " << file.img11Lock - << "\nbootflag: " << file.bootFlag << "\nlast: " << static_cast(file.lastChip) - << " " << static_cast(file.lastCopy) - << "\nnext: " << static_cast(file.mechanismNextChip) << " " - << static_cast(file.mechanismNextCopy) << "\n"; + { + std::ofstream rebootFile(path); + if (rebootFile.is_open()) { + // Initiate reboot file first. Reboot handling will be on on initialization + rebootFile << "on: " << file.enabled << "\nmaxcnt: " << file.maxCount + << "\nimg00: " << file.img00Cnt << "\nimg01: " << file.img01Cnt + << "\nimg10: " << file.img10Cnt << "\nimg11: " << file.img11Cnt + << "\nimg00lock: " << file.img00Lock << "\nimg01lock: " << file.img01Lock + << "\nimg10lock: " << file.img10Lock << "\nimg11lock: " << file.img11Lock + << "\nbootflag: " << file.bootFlag << "\nlast: " << static_cast(file.lastChip) + << " " << static_cast(file.lastCopy) + << "\nnext: " << static_cast(file.mechanismNextChip) << " " + << static_cast(file.mechanismNextCopy) << "\n"; + } } std::error_code e; // TODO: Remove at some point in the future when all images have been updated. From 4465170747ce9f488266e2b49457e318d5c86c1f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 19:00:09 +0200 Subject: [PATCH 18/40] i hope that is the last bug.. --- bsp_q7s/core/CoreController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 4257d008..6a21156e 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -2165,7 +2165,7 @@ void CoreController::rewriteRebootCountersFile(RebootCountersFile file) { std::string path = currMntPrefix + REBOOT_COUNTERS_FILE; std::ofstream rebootFile(path); if (rebootFile.is_open()) { - rebootFile << "\nimg00: " << file.img00Cnt << "\nimg01: " << file.img01Cnt + rebootFile << "img00: " << file.img00Cnt << "\nimg01: " << file.img01Cnt << "\nimg10: " << file.img10Cnt << "\nimg11: " << file.img11Cnt << "\n"; } } From 5c9d7a43ab7da8eac0d665aaa6e711bd9c08c8a8 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 19:22:25 +0200 Subject: [PATCH 19/40] that reboot was not graceful.. --- bsp_q7s/core/CoreController.cpp | 90 ++++++++++++++++++--------------- bsp_q7s/core/CoreController.h | 1 + 2 files changed, 49 insertions(+), 42 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 6a21156e..51c04fcc 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1209,45 +1209,7 @@ ReturnValue_t CoreController::actionXscReboot(const uint8_t *data, size_t size) auto tgtChip = static_cast(data[1]); auto tgtCopy = static_cast(data[2]); - // This function can not really fail - gracefulShutdownTasks(tgtChip, tgtCopy, protOpPerformed); - - switch (tgtChip) { - case (xsc::Chip::CHIP_0): { - switch (tgtCopy) { - case (xsc::Copy::COPY_0): { - xsc_boot_copy(XSC_LIBNOR_CHIP_0, XSC_LIBNOR_COPY_NOMINAL); - break; - } - case (xsc::Copy::COPY_1): { - xsc_boot_copy(XSC_LIBNOR_CHIP_0, XSC_LIBNOR_COPY_GOLD); - break; - } - default: { - break; - } - } - break; - } - case (xsc::Chip::CHIP_1): { - switch (tgtCopy) { - case (xsc::Copy::COPY_0): { - xsc_boot_copy(XSC_LIBNOR_CHIP_1, XSC_LIBNOR_COPY_NOMINAL); - break; - } - case (xsc::Copy::COPY_1): { - xsc_boot_copy(XSC_LIBNOR_CHIP_1, XSC_LIBNOR_COPY_GOLD); - break; - } - default: { - break; - } - } - break; - } - default: - break; - } + performGracefulShutdown(tgtChip, tgtCopy); return returnvalue::FAILED; } @@ -1267,7 +1229,8 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co // Attempt graceful shutdown by unmounting and switching off SD cards sdcMan->switchOffSdCard(sd::SdCard::SLOT_0); sdcMan->switchOffSdCard(sd::SdCard::SLOT_1); - // If any boot copies are unprotected + // If any boot copies are unprotected. + // Actually this function only ensures that reboots to the own image are protected.. ReturnValue_t result = setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true, protOpPerformed, false); if (result == returnvalue::OK and protOpPerformed) { @@ -1739,8 +1702,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { rebootWatchdogFile.mechanismNextChip = tgtChip; rebootWatchdogFile.mechanismNextCopy = tgtCopy; rewriteRebootWatchdogFile(rebootWatchdogFile); - xsc_boot_copy(static_cast(tgtChip), - static_cast(tgtCopy)); + performGracefulShutdown(tgtChip, tgtCopy); } } else { rebootWatchdogFile.mechanismNextChip = xsc::NO_CHIP; @@ -2563,6 +2525,50 @@ void CoreController::announceCurrentImageInfo() { triggerEvent(CURRENT_IMAGE_INFO, CURRENT_CHIP, CURRENT_COPY); } +ReturnValue_t CoreController::performGracefulShutdown(xsc::Chip tgtChip, xsc::Copy tgtCopy) { + bool protOpPerformed = false; + // This function can not really fail + gracefulShutdownTasks(tgtChip, tgtCopy, protOpPerformed); + + switch (tgtChip) { + case (xsc::Chip::CHIP_0): { + switch (tgtCopy) { + case (xsc::Copy::COPY_0): { + xsc_boot_copy(XSC_LIBNOR_CHIP_0, XSC_LIBNOR_COPY_NOMINAL); + break; + } + case (xsc::Copy::COPY_1): { + xsc_boot_copy(XSC_LIBNOR_CHIP_0, XSC_LIBNOR_COPY_GOLD); + break; + } + default: { + break; + } + } + break; + } + case (xsc::Chip::CHIP_1): { + switch (tgtCopy) { + case (xsc::Copy::COPY_0): { + xsc_boot_copy(XSC_LIBNOR_CHIP_1, XSC_LIBNOR_COPY_NOMINAL); + break; + } + case (xsc::Copy::COPY_1): { + xsc_boot_copy(XSC_LIBNOR_CHIP_1, XSC_LIBNOR_COPY_GOLD); + break; + } + default: { + break; + } + } + break; + } + default: + break; + } + return returnvalue::OK; +} + bool CoreController::isNumber(const std::string &s) { return !s.empty() && std::find_if(s.begin(), s.end(), [](unsigned char c) { return !std::isdigit(c); }) == s.end(); diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index e3f514d9..4bce79da 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -363,6 +363,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe const uint8_t* data, size_t size); ReturnValue_t actionListDirectoryDumpDirectly(ActionId_t actionId, MessageQueueId_t commandedBy, const uint8_t* data, size_t size); + ReturnValue_t performGracefulShutdown(xsc::Chip targetChip, xsc::Copy targetCopy); ReturnValue_t actionListDirectoryCommonCommandCreator(const uint8_t* data, size_t size, std::ostringstream& oss); From 966faf51b5345878911e534caffa451c096f8677 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 19:55:56 +0200 Subject: [PATCH 20/40] last important bugfixes --- bsp_q7s/core/CoreController.cpp | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 51c04fcc..85093fa3 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1226,6 +1226,10 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co sdcMan->markUnusable(); // Wait two seconds to ensure no one uses the SD cards TaskFactory::delayTask(2000); + + // Ensure that all writes/reads do finish. + sync(); + // Attempt graceful shutdown by unmounting and switching off SD cards sdcMan->switchOffSdCard(sd::SdCard::SLOT_0); sdcMan->switchOffSdCard(sd::SdCard::SLOT_1); @@ -2027,6 +2031,7 @@ bool CoreController::parseRebootCountersFile(std::string path, RebootCountersFil break; } } + lineIdx++; } return true; } From d95ecc36781eae2e4e20df3cc4485f4498185c25 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 19:57:16 +0200 Subject: [PATCH 21/40] changelog --- CHANGELOG.md | 9 +++++++++ 1 file changed, 9 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a4eac1f..443ea5bc 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,15 @@ will consitute of a breaking change warranting a new major release: # [unreleased] +# [v4.1.0] + +## Changed + +- Added `sync` syscall in graceful shutdown handler +- Graceful shutdown is now performed by the reboot watchdog +- There is now a separate file for the total reboot counter. The reboot watchdog has its own local + counters to determine whether a reboot is necessary. + # [v4.0.1] 2023-06-24 ## Fixed From 25d10e3877fe85cc9d3ee0f5596337797b0a0505 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sat, 24 Jun 2023 20:57:54 +0200 Subject: [PATCH 22/40] 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 23/40] 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 d88ffb17341e5ec56ec715c868e76dc2f9581e7c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 25 Jun 2023 10:24:27 +0200 Subject: [PATCH 24/40] small fix --- bsp_q7s/core/CoreController.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 85093fa3..9157c59b 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1602,7 +1602,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { std::string legacyPath = currMntPrefix + LEGACY_REBOOT_WATCHDOG_FILE; std::error_code e; // TODO: Remove at some point in the future. - if (std::filesystem::exists(legacyPath)) { + if (std::filesystem::exists(legacyPath, e)) { // Old file might still exist, so copy it to new path std::filesystem::copy(legacyPath, path, std::filesystem::copy_options::overwrite_existing, e); if (e) { From 3ee2e72652262743089022c87b78003a28b2047d Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 25 Jun 2023 10:45:04 +0200 Subject: [PATCH 25/40] better name for function --- bsp_q7s/core/CoreController.cpp | 4 ++-- bsp_q7s/core/CoreController.h | 2 +- 2 files changed, 3 insertions(+), 3 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index 9157c59b..a71f455a 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1696,7 +1696,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { bool doReboot = false; xsc::Chip tgtChip = xsc::NO_CHIP; xsc::Copy tgtCopy = xsc::NO_COPY; - determineAndExecuteReboot(rebootWatchdogFile, doReboot, tgtChip, tgtCopy); + rebootWatchdogAlgorithm((rebootFile, doReboot, tgtChip, tgtCopy); if (doReboot) { rebootWatchdogFile.bootFlag = true; #if OBSW_VERBOSE_LEVEL >= 1 @@ -1715,7 +1715,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::determineAndExecuteReboot(RebootWatchdogFile &rf, bool &needsReboot, +void CoreController::rebootWatchdogAlgorithm(RebootFile &rf, bool &needsReboot, xsc::Chip &tgtChip, xsc::Copy &tgtCopy) { tgtChip = xsc::CHIP_0; tgtCopy = xsc::COPY_0; diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index 4bce79da..fb7becf6 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -377,7 +377,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe int handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect, bool& protOperationPerformed, bool selfChip, bool selfCopy, bool allChips, bool allCopies, uint8_t arrIdx); - void determineAndExecuteReboot(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, + void rebootWatchdogAlgorithm(RebootFile& rf, bool& needsReboot, xsc::Chip& tgtChip, xsc::Copy& tgtCopy); void resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy); void setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy); From 5a642b7d2f1264225355f4c690ef120a0ed336d6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 25 Jun 2023 11:04:13 +0200 Subject: [PATCH 26/40] update core ctrl --- bsp_q7s/core/CoreController.cpp | 6 +++--- bsp_q7s/core/CoreController.h | 4 ++-- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index a71f455a..f9242da3 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -1696,7 +1696,7 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { bool doReboot = false; xsc::Chip tgtChip = xsc::NO_CHIP; xsc::Copy tgtCopy = xsc::NO_COPY; - rebootWatchdogAlgorithm((rebootFile, doReboot, tgtChip, tgtCopy); + rebootWatchdogAlgorithm(rebootWatchdogFile, doReboot, tgtChip, tgtCopy); if (doReboot) { rebootWatchdogFile.bootFlag = true; #if OBSW_VERBOSE_LEVEL >= 1 @@ -1715,8 +1715,8 @@ void CoreController::performRebootWatchdogHandling(bool recreateFile) { rewriteRebootWatchdogFile(rebootWatchdogFile); } -void CoreController::rebootWatchdogAlgorithm(RebootFile &rf, bool &needsReboot, - xsc::Chip &tgtChip, xsc::Copy &tgtCopy) { +void CoreController::rebootWatchdogAlgorithm(RebootWatchdogFile &rf, bool &needsReboot, + xsc::Chip &tgtChip, xsc::Copy &tgtCopy) { tgtChip = xsc::CHIP_0; tgtCopy = xsc::COPY_0; needsReboot = false; diff --git a/bsp_q7s/core/CoreController.h b/bsp_q7s/core/CoreController.h index fb7becf6..b68ae158 100644 --- a/bsp_q7s/core/CoreController.h +++ b/bsp_q7s/core/CoreController.h @@ -377,8 +377,8 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe int handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect, bool& protOperationPerformed, bool selfChip, bool selfCopy, bool allChips, bool allCopies, uint8_t arrIdx); - void rebootWatchdogAlgorithm(RebootFile& rf, bool& needsReboot, xsc::Chip& tgtChip, - xsc::Copy& tgtCopy); + void rebootWatchdogAlgorithm(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip, + xsc::Copy& tgtCopy); void resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy); void setRebootMechanismLock(bool lock, xsc::Chip tgtChip, xsc::Copy tgtCopy); bool parseRebootWatchdogFile(std::string path, RebootWatchdogFile& file); From 76933671a6321746af4ced168a6c04bc09f7b4e4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Sun, 25 Jun 2023 11:07:41 +0200 Subject: [PATCH 27/40] 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 From 05a5c63765f59a3f0f2f975258125610b9df822a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 11:20:33 +0200 Subject: [PATCH 28/40] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 57355ad0..128aa034 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,7 +16,7 @@ will consitute of a breaking change warranting a new major release: # [unreleased] -- Internal error reporter set is now enabled by defalt and generated every 120 seconds. +- Internal error reporter set is now enabled by default and generated every 120 seconds. # [v4.0.1] 2023-06-24 From 1f1890481067c57d850a4866dad0a53fd6506de6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 15:38:37 +0200 Subject: [PATCH 29/40] bump tmtc --- tmtc | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tmtc b/tmtc index deb0275b..ec0ebc36 160000 --- a/tmtc +++ b/tmtc @@ -1 +1 @@ -Subproject commit deb0275bb5603394122e26f74760d2051685f324 +Subproject commit ec0ebc365308198046addc94909b1bca8678aa5a From 73498ac9afc306c8aedbc51378b67a1e8204ca43 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 16:53:54 +0200 Subject: [PATCH 30/40] prepare v5.0.0, revert firmware interfacing changes --- CHANGELOG.md | 14 ++- bsp_q7s/boardconfig/busConf.h | 4 + bsp_q7s/core/ObjectFactory.cpp | 29 +++-- common/config/devices/gpioIds.h | 4 + linux/ipcore/PapbVcInterface.cpp | 120 ++++++++++++++++---- linux/ipcore/PapbVcInterface.h | 10 +- mission/com/PersistentLogTmStoreTask.cpp | 6 + mission/com/PersistentSingleTmStoreTask.cpp | 6 + 8 files changed, 160 insertions(+), 33 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 8e4bf09f..b62369e0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,10 +16,22 @@ will consitute of a breaking change warranting a new major release: # [unreleased] -# [v4.1.0] +# [v6.0.0] to be released + +- Important bugfixes for PTME. See `q7s-package` CHANGELOG. + +# [v5.0.0] to be released + +v3.3.1 and all following version will now be moved to v5.0.0 with the additional changes listed +here. This was done because the firmware update (v4.0.0) is not working right now and it is not +known when and how it will be fixed. Because of that, all updates to make the SW work with the new +firmware, which are limited to a few files will be moved to a dev branch so regular development +compatible to the old firmware can continue. ## Changed +- This version is compatible to the old firmware and some changes which only work with the new + firmware have been reverted. - Added `sync` syscall in graceful shutdown handler - Graceful shutdown is now performed by the reboot watchdog - There is now a separate file for the total reboot counter. The reboot watchdog has its own local diff --git a/bsp_q7s/boardconfig/busConf.h b/bsp_q7s/boardconfig/busConf.h index 146386c4..6a3e0d9e 100644 --- a/bsp_q7s/boardconfig/busConf.h +++ b/bsp_q7s/boardconfig/busConf.h @@ -85,9 +85,13 @@ static constexpr char EN_RW_4[] = "enable_rw_4"; static constexpr char RAD_SENSOR_CHIP_SELECT[] = "rad_sensor_chip_select"; static constexpr char ENABLE_RADFET[] = "enable_radfet"; +static constexpr char PAPB_BUSY_SIGNAL_VC0[] = "papb_busy_signal_vc0"; static constexpr char PAPB_EMPTY_SIGNAL_VC0[] = "papb_empty_signal_vc0"; +static constexpr char PAPB_BUSY_SIGNAL_VC1[] = "papb_busy_signal_vc1"; static constexpr char PAPB_EMPTY_SIGNAL_VC1[] = "papb_empty_signal_vc1"; +static constexpr char PAPB_BUSY_SIGNAL_VC2[] = "papb_busy_signal_vc2"; static constexpr char PAPB_EMPTY_SIGNAL_VC2[] = "papb_empty_signal_vc2"; +static constexpr char PAPB_BUSY_SIGNAL_VC3[] = "papb_busy_signal_vc3"; static constexpr char PAPB_EMPTY_SIGNAL_VC3[] = "papb_empty_signal_vc3"; static constexpr char PTME_RESETN[] = "ptme_resetn"; diff --git a/bsp_q7s/core/ObjectFactory.cpp b/bsp_q7s/core/ObjectFactory.cpp index 35d30c1a..af568553 100644 --- a/bsp_q7s/core/ObjectFactory.cpp +++ b/bsp_q7s/core/ObjectFactory.cpp @@ -731,12 +731,20 @@ ReturnValue_t ObjectFactory::createCcsdsComponents(CcsdsComponentArgs& args) { // GPIO definitions of signals connected to the virtual channel interfaces of the PTME IP Core GpioCookie* gpioCookiePtmeIp = new GpioCookie; GpiodRegularByLineName* gpio = nullptr; + gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC0, "PAPB VC0"); + gpioCookiePtmeIp->addGpio(gpioIds::VC0_PAPB_BUSY, gpio); gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC0, "PAPB VC0"); gpioCookiePtmeIp->addGpio(gpioIds::VC0_PAPB_EMPTY, gpio); + gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC1, "PAPB VC1"); + gpioCookiePtmeIp->addGpio(gpioIds::VC1_PAPB_BUSY, gpio); gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC1, "PAPB VC1"); gpioCookiePtmeIp->addGpio(gpioIds::VC1_PAPB_EMPTY, gpio); + gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC2, "PAPB VC2"); + gpioCookiePtmeIp->addGpio(gpioIds::VC2_PAPB_BUSY, gpio); gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC2, "PAPB VC2"); gpioCookiePtmeIp->addGpio(gpioIds::VC2_PAPB_EMPTY, gpio); + gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_BUSY_SIGNAL_VC3, "PAPB VC3"); + gpioCookiePtmeIp->addGpio(gpioIds::VC3_PAPB_BUSY, gpio); gpio = new GpiodRegularByLineName(q7s::gpioNames::PAPB_EMPTY_SIGNAL_VC3, "PAPB VC3"); gpioCookiePtmeIp->addGpio(gpioIds::VC3_PAPB_EMPTY, gpio); gpio = new GpiodRegularByLineName(q7s::gpioNames::PTME_RESETN, "PTME RESETN", @@ -745,14 +753,19 @@ ReturnValue_t ObjectFactory::createCcsdsComponents(CcsdsComponentArgs& args) { gpioChecker(args.gpioComIF.addGpios(gpioCookiePtmeIp), "PTME PAPB VCs"); // Creating virtual channel interfaces - VirtualChannelIF* vc0 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC0); - VirtualChannelIF* vc1 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC1); - VirtualChannelIF* vc2 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC2); - VirtualChannelIF* vc3 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC3); + VirtualChannelIF* vc0 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_BUSY, gpioIds::VC0_PAPB_EMPTY, + q7s::UIO_PTME, q7s::uiomapids::PTME_VC0); + VirtualChannelIF* vc1 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_BUSY, gpioIds::VC1_PAPB_EMPTY, + q7s::UIO_PTME, q7s::uiomapids::PTME_VC1); + VirtualChannelIF* vc2 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_BUSY, gpioIds::VC2_PAPB_EMPTY, + q7s::UIO_PTME, q7s::uiomapids::PTME_VC2); + VirtualChannelIF* vc3 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_BUSY, gpioIds::VC3_PAPB_EMPTY, + q7s::UIO_PTME, q7s::uiomapids::PTME_VC3); + // Creating ptme object and adding virtual channel interfaces Ptme* ptme = new Ptme(objects::PTME); ptme->addVcInterface(ccsds::VC0, vc0); diff --git a/common/config/devices/gpioIds.h b/common/config/devices/gpioIds.h index bed82142..ac193bd2 100644 --- a/common/config/devices/gpioIds.h +++ b/common/config/devices/gpioIds.h @@ -96,9 +96,13 @@ enum gpioId_t { SPI_MUX, VC0_PAPB_EMPTY, + VC0_PAPB_BUSY, VC1_PAPB_EMPTY, + VC1_PAPB_BUSY, VC2_PAPB_EMPTY, + VC2_PAPB_BUSY, VC3_PAPB_EMPTY, + VC3_PAPB_BUSY, PTME_RESETN, PDEC_RESET, diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index 404f3653..60968cc6 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -7,16 +7,20 @@ #include "fsfw/serviceinterface/ServiceInterface.h" -PapbVcInterface::PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, - std::string uioFile, int mapNum) - : gpioComIF(gpioComIF), papbEmptyId(papbEmptyId), uioFile(std::move(uioFile)), mapNum(mapNum) {} +PapbVcInterface::PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbBusyId, + gpioId_t papbEmptyId, std::string uioFile, int mapNum) + : gpioComIF(gpioComIF), + papbBusyId(papbBusyId), + papbEmptyId(papbEmptyId), + uioFile(std::move(uioFile)), + mapNum(mapNum) {} PapbVcInterface::~PapbVcInterface() {} ReturnValue_t PapbVcInterface::initialize() { UioMapper uioMapper(uioFile, mapNum); ReturnValue_t result = uioMapper.getMappedAdress(const_cast(&vcBaseReg), - UioMapper::Permissions::READ_WRITE); + UioMapper::Permissions::WRITE_ONLY); if (result != returnvalue::OK) { return result; } @@ -28,16 +32,63 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) { if (size < 4) { return returnvalue::FAILED; } - if (pollReadyForPacket()) { + if (pollInterfaceReadiness(0, true) == returnvalue::OK) { startPacketTransfer(ByteWidthCfg::ONE); } else { return DirectTmSinkIF::IS_BUSY; } + // TODO: This should work but does not.. :( + // size_t idx = 0; + // while (idx < size) { + // + // nanosleep(&BETWEEN_POLL_DELAY, &remDelay); + // if ((size - idx) < 4) { + // *vcBaseReg = CONFIG_DATA_INPUT | (size - idx - 1); + // usleep(1); + // } + // if (pollPapbBusySignal(2) == returnvalue::OK) { + // // vcBaseReg + DATA_REG_OFFSET + 3 = static_cast(data + idx); + // // vcBaseReg + DATA_REG_OFFSET + 2 = static_cast(data + idx + 1); + // // vcBaseReg + DATA_REG_OFFSET + 1 = static_cast(data + idx + 2); + // // vcBaseReg + DATA_REG_OFFSET = static_cast(data + idx + 3); + // + // // std::memcpy((vcBaseReg + DATA_REG_OFFSET), data + idx , nextWriteSize); + // *(vcBaseReg + DATA_REG_OFFSET) = *reinterpret_cast(data + idx); + // //uint8_t* byteReg = reinterpret_cast(vcBaseReg + DATA_REG_OFFSET); + // + // //byteReg[0] = data[idx]; + // //byteReg[1] = data[idx]; + // } else { + // abortPacketTransfer(); + // return returnvalue::FAILED; + // } + // // TODO: Change this after the bugfix. Right now, the PAPB ignores the content of the byte + // // width configuration.5 + // // It's okay to increment by a larger amount for the last segment here, loop will be over + // // in any case. + // idx += 4; + // } for (size_t idx = 0; idx < size; idx++) { - // if (pollInterfaceReadiness(2, false) == returnvalue::OK) { - *(vcBaseReg + DATA_REG_OFFSET) = static_cast(data[idx]); + // This delay is super-important, DO NOT REMOVE! + // Polling the GPIO or the config register too often messes up the scheduler. + // TODO: Maybe this should not be done like this. It would be better if there was a custom + // FPGA module which can accept packets and then takes care of dumping that packet into + // the PTME. DMA would be an ideal solution for this. + nanosleep(&BETWEEN_POLL_DELAY, &remDelay); + if (pollInterfaceReadiness(2, false) == returnvalue::OK) { + *(vcBaseReg + DATA_REG_OFFSET) = static_cast(data[idx]); + } else { + abortPacketTransfer(); + return returnvalue::FAILED; + } + } + nanosleep(&BETWEEN_POLL_DELAY, &remDelay); + if (pollInterfaceReadiness(2, false) == returnvalue::OK) { + completePacketTransfer(); + } else { + abortPacketTransfer(); + return returnvalue::FAILED; } - completePacketTransfer(); return returnvalue::OK; } @@ -47,33 +98,60 @@ void PapbVcInterface::startPacketTransfer(ByteWidthCfg initWidth) { void PapbVcInterface::completePacketTransfer() { *vcBaseReg = CONFIG_END; } -bool PapbVcInterface::pollReadyForPacket() const { - // Check if PAPB interface is ready to receive data. Use the configuration register for this. - // Bit 5, see PTME ptme_001_01-0-7-r2 Table 31. - uint32_t reg = *vcBaseReg; - // bool busy = (reg >> 5) & 0b1; - return (reg >> 6) & 0b1; +ReturnValue_t PapbVcInterface::pollInterfaceReadiness(uint32_t maxPollRetries, + bool checkReadyState) const { + uint32_t busyIdx = 0; + nextDelay.tv_nsec = FIRST_DELAY_PAPB_POLLING_NS; + + while (true) { + // Check if PAPB interface is ready to receive data. Use the configuration register for this. + // Bit 5, see PTME ptme_001_01-0-7-r2 Table 31. + uint32_t reg = *vcBaseReg; + bool busy = (reg >> 5) & 0b1; + bool ready = (reg >> 6) & 0b1; + if (not busy) { + return returnvalue::OK; + } + if (checkReadyState and not ready) { + return PAPB_BUSY; + } + + busyIdx++; + if (busyIdx >= maxPollRetries) { + return PAPB_BUSY; + } + + // Ignore signal handling here for now. + nanosleep(&nextDelay, &remDelay); + // Adaptive delay. + if (nextDelay.tv_nsec * 2 <= MAX_DELAY_PAPB_POLLING_NS) { + nextDelay.tv_nsec *= 2; + } + } + return returnvalue::OK; } -bool PapbVcInterface::isVcInterfaceBufferEmpty() { +void PapbVcInterface::isVcInterfaceBufferEmpty() { ReturnValue_t result = returnvalue::OK; gpio::Levels papbEmptyState = gpio::Levels::HIGH; result = gpioComIF->readGpio(papbEmptyId, papbEmptyState); if (result != returnvalue::OK) { - sif::error << "PapbVcInterface::isVcInterfaceBufferEmpty: Failed to read papb empty signal" - << std::endl; - return true; + sif::warning << "PapbVcInterface::isVcInterfaceBufferEmpty: Failed to read papb empty signal" + << std::endl; + return; } if (papbEmptyState == gpio::Levels::HIGH) { - return true; + sif::debug << "PapbVcInterface::isVcInterfaceBufferEmpty: Buffer is empty" << std::endl; + } else { + sif::debug << "PapbVcInterface::isVcInterfaceBufferEmpty: Buffer is not empty" << std::endl; } - return false; + return; } -bool PapbVcInterface::isBusy() const { return not pollReadyForPacket(); } +bool PapbVcInterface::isBusy() const { return pollInterfaceReadiness(0, true) == PAPB_BUSY; } void PapbVcInterface::cancelTransfer() { abortPacketTransfer(); } diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index ba6063b5..e54def5d 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -30,7 +30,8 @@ class PapbVcInterface : public VirtualChannelIF { * @param uioFile UIO file providing access to the PAPB bus * @param mapNum Map number of UIO map associated with this virtual channel */ - PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, std::string uioFile, int mapNum); + PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbBusyId, gpioId_t papbEmptyId, + std::string uioFile, int mapNum); virtual ~PapbVcInterface(); bool isBusy() const override; @@ -82,6 +83,9 @@ class PapbVcInterface : public VirtualChannelIF { static constexpr long int MAX_DELAY_PAPB_POLLING_NS = 40; LinuxLibgpioIF* gpioComIF = nullptr; + + /** Pulled to low when virtual channel not ready to receive data */ + gpioId_t papbBusyId = gpio::NO_GPIO; /** High when external buffer memory of virtual channel is empty */ gpioId_t papbEmptyId = gpio::NO_GPIO; @@ -116,13 +120,13 @@ class PapbVcInterface : public VirtualChannelIF { * * @return returnvalue::OK when ready to receive data else PAPB_BUSY. */ - inline bool pollReadyForPacket() const; + inline ReturnValue_t pollInterfaceReadiness(uint32_t maxPollRetries, bool checkReadyState) const; /** * @brief This function can be used for debugging to check whether there are packets in * the packet buffer of the virtual channel or not. */ - bool isVcInterfaceBufferEmpty(); + void isVcInterfaceBufferEmpty(); /** * @brief This function sends a complete telemetry transfer frame data field (1105 bytes) diff --git a/mission/com/PersistentLogTmStoreTask.cpp b/mission/com/PersistentLogTmStoreTask.cpp index 28545457..77f2bb7d 100644 --- a/mission/com/PersistentLogTmStoreTask.cpp +++ b/mission/com/PersistentLogTmStoreTask.cpp @@ -42,7 +42,13 @@ ReturnValue_t PersistentLogTmStoreTask::performOperation(uint8_t opCode) { if (not someonesBusy) { TaskFactory::delayTask(100); } else if (vcBusyDuringDump) { + // TODO: Might not be necessary + sif::debug << "VC busy, delaying" << std::endl; TaskFactory::delayTask(10); + } else { + // TODO: Would be best to remove this, but not delaying here can lead to evil issues. + // Polling the PAPB of the PTME core too often leads to scheuduling issues. + TaskFactory::delayTask(2); } } } diff --git a/mission/com/PersistentSingleTmStoreTask.cpp b/mission/com/PersistentSingleTmStoreTask.cpp index d6f43289..1b77365b 100644 --- a/mission/com/PersistentSingleTmStoreTask.cpp +++ b/mission/com/PersistentSingleTmStoreTask.cpp @@ -24,7 +24,13 @@ ReturnValue_t PersistentSingleTmStoreTask::performOperation(uint8_t opCode) { if (not busy) { TaskFactory::delayTask(100); } else if (dumpContext.vcBusyDuringDump) { + sif::debug << "VC busy, delaying" << std::endl; + // TODO: Might not be necessary TaskFactory::delayTask(10); + } else { + // TODO: Would be best to remove this, but not delaying here can lead to evil issues. + // Polling the PAPB of the PTME core too often leads to scheuduling issues. + TaskFactory::delayTask(2); } } } From 52f5b088bfdbf3dbaaac93bdce33c624b5be34be Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 17:21:57 +0200 Subject: [PATCH 31/40] add version constraints for FW version read --- bsp_q7s/core/CoreController.cpp | 25 +++++++++++++++---------- 1 file changed, 15 insertions(+), 10 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index f9242da3..9e285c4f 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -190,12 +190,14 @@ ReturnValue_t CoreController::initialize() { sif::warning << "Subscribing for GPS GPS_FIX_CHANGE event failed" << std::endl; } - UioMapper sysRomMapper(q7s::UIO_SYS_ROM); - result = sysRomMapper.getMappedAdress(&mappedSysRomAddr, UioMapper::Permissions::READ_ONLY); - if (result != returnvalue::OK) { - // TODO: This might be a reason to switch to another image.. - sif::error << "Getting mapped SYS ROM UIO address failed" << std::endl; - return ObjectManager::CHILD_INIT_FAILED; + if (common::OBSW_VERSION_MAJOR >= 6 or common::OBSW_VERSION_MAJOR == 4) { + UioMapper sysRomMapper(q7s::UIO_SYS_ROM); + result = sysRomMapper.getMappedAdress(&mappedSysRomAddr, UioMapper::Permissions::READ_ONLY); + if (result != returnvalue::OK) { + // TODO: This might be a reason to switch to another image.. + sif::error << "Getting mapped SYS ROM UIO address failed" << std::endl; + return ObjectManager::CHILD_INIT_FAILED; + } } return returnvalue::OK; } @@ -2518,10 +2520,13 @@ void CoreController::announceVersionInfo() { } triggerEvent(VERSION_INFO, p1, p2); - if (mappedSysRomAddr != nullptr) { - uint32_t p1Firmware = *(reinterpret_cast(mappedSysRomAddr)); - uint32_t p2Firmware = *(reinterpret_cast(mappedSysRomAddr) + 1); - triggerEvent(FIRMWARE_INFO, p1Firmware, p2Firmware); + + if (common::OBSW_VERSION_MAJOR >= 6 or common::OBSW_VERSION_MAJOR == 4) { + if (mappedSysRomAddr != nullptr) { + uint32_t p1Firmware = *(reinterpret_cast(mappedSysRomAddr)); + uint32_t p2Firmware = *(reinterpret_cast(mappedSysRomAddr) + 1); + triggerEvent(FIRMWARE_INFO, p1Firmware, p2Firmware); + } } } From 72ffa365d48432c324aeea2cc1355113988162b1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 17:29:53 +0200 Subject: [PATCH 32/40] version constraints for PL I2C reset pin --- bsp_q7s/core/ObjectFactory.cpp | 24 +++++++++++++----------- 1 file changed, 13 insertions(+), 11 deletions(-) diff --git a/bsp_q7s/core/ObjectFactory.cpp b/bsp_q7s/core/ObjectFactory.cpp index af568553..0e55661e 100644 --- a/bsp_q7s/core/ObjectFactory.cpp +++ b/bsp_q7s/core/ObjectFactory.cpp @@ -1030,16 +1030,18 @@ void ObjectFactory::createRadSensorChipSelect(LinuxLibgpioIF* gpioIF) { void ObjectFactory::createPlI2cResetGpio(LinuxLibgpioIF* gpioIF) { using namespace gpio; - if (gpioIF == nullptr) { - return; + if (common::OBSW_VERSION_MAJOR >= 6 or common::OBSW_VERSION_MAJOR == 4) { + if (gpioIF == nullptr) { + return; + } + GpioCookie* gpioI2cResetnCookie = new GpioCookie; + GpiodRegularByLineName* gpioI2cResetn = new GpiodRegularByLineName( + q7s::gpioNames::PL_I2C_ARESETN, "PL_I2C_ARESETN", Direction::OUT, Levels::HIGH); + gpioI2cResetnCookie->addGpio(gpioIds::PL_I2C_ARESETN, gpioI2cResetn); + gpioChecker(gpioIF->addGpios(gpioI2cResetnCookie), "PL I2C ARESETN"); + // Reset I2C explicitely again. + gpioIF->pullLow(gpioIds::PL_I2C_ARESETN); + TaskFactory::delayTask(1); + gpioIF->pullHigh(gpioIds::PL_I2C_ARESETN); } - GpioCookie* gpioI2cResetnCookie = new GpioCookie; - GpiodRegularByLineName* gpioI2cResetn = new GpiodRegularByLineName( - q7s::gpioNames::PL_I2C_ARESETN, "PL_I2C_ARESETN", Direction::OUT, Levels::HIGH); - gpioI2cResetnCookie->addGpio(gpioIds::PL_I2C_ARESETN, gpioI2cResetn); - gpioChecker(gpioIF->addGpios(gpioI2cResetnCookie), "PL I2C ARESETN"); - // Reset I2C explicitely again. - gpioIF->pullLow(gpioIds::PL_I2C_ARESETN); - TaskFactory::delayTask(1); - gpioIF->pullHigh(gpioIds::PL_I2C_ARESETN); } From bd4e05c944eb25c58a2578172b174847b8078ecd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 17:30:30 +0200 Subject: [PATCH 33/40] bump version specifier --- CMakeLists.txt | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 9e234cae..d391877c 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -9,9 +9,9 @@ # ############################################################################## cmake_minimum_required(VERSION 3.13) -set(OBSW_VERSION_MAJOR 4) +set(OBSW_VERSION_MAJOR 5) set(OBSW_VERSION_MINOR 0) -set(OBSW_VERSION_REVISION 1) +set(OBSW_VERSION_REVISION 0) # set(CMAKE_VERBOSE TRUE) From 3a20748ff636f3466465bb5db379b7176f92ae37 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 17:36:47 +0200 Subject: [PATCH 34/40] create a v5.0.0 snapshot --- CHANGELOG.md | 21 ++++++++++++--------- 1 file changed, 12 insertions(+), 9 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index b62369e0..7a89e679 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,7 +20,16 @@ will consitute of a breaking change warranting a new major release: - Important bugfixes for PTME. See `q7s-package` CHANGELOG. -# [v5.0.0] to be released +# [v5.1.0] to be released + +## Changed + +- Added `sync` syscall in graceful shutdown handler +- Graceful shutdown is now performed by the reboot watchdog +- There is now a separate file for the total reboot counter. The reboot watchdog has its own local + counters to determine whether a reboot is necessary. + +# [v5.0.0] 2023-06-26 v3.3.1 and all following version will now be moved to v5.0.0 with the additional changes listed here. This was done because the firmware update (v4.0.0) is not working right now and it is not @@ -28,14 +37,8 @@ known when and how it will be fixed. Because of that, all updates to make the SW firmware, which are limited to a few files will be moved to a dev branch so regular development compatible to the old firmware can continue. -## Changed - -- This version is compatible to the old firmware and some changes which only work with the new - firmware have been reverted. -- Added `sync` syscall in graceful shutdown handler -- Graceful shutdown is now performed by the reboot watchdog -- There is now a separate file for the total reboot counter. The reboot watchdog has its own local - counters to determine whether a reboot is necessary. +TLDR: This version is compatible to the old firmware and some changes which only work with the new +firmware have been reverted. # [v4.0.1] 2023-06-24 From 67024ec933db2e12d3c3352712048f382d64a040 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 17:39:41 +0200 Subject: [PATCH 35/40] that was already pulled in --- CHANGELOG.md | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7a89e679..9b0d0e5b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -22,13 +22,6 @@ will consitute of a breaking change warranting a new major release: # [v5.1.0] to be released -## Changed - -- Added `sync` syscall in graceful shutdown handler -- Graceful shutdown is now performed by the reboot watchdog -- There is now a separate file for the total reboot counter. The reboot watchdog has its own local - counters to determine whether a reboot is necessary. - # [v5.0.0] 2023-06-26 v3.3.1 and all following version will now be moved to v5.0.0 with the additional changes listed @@ -40,6 +33,13 @@ compatible to the old firmware can continue. TLDR: This version is compatible to the old firmware and some changes which only work with the new firmware have been reverted. +## Changed + +- Added `sync` syscall in graceful shutdown handler +- Graceful shutdown is now performed by the reboot watchdog +- There is now a separate file for the total reboot counter. The reboot watchdog has its own local + counters to determine whether a reboot is necessary. + # [v4.0.1] 2023-06-24 ## Fixed From e752cdcc67701a05085339c188bf2248e4eb39ca Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 26 Jun 2023 18:10:41 +0200 Subject: [PATCH 36/40] bump fsfw --- fsfw | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/fsfw b/fsfw index a85a38c8..6aff3250 160000 --- a/fsfw +++ b/fsfw @@ -1 +1 @@ -Subproject commit a85a38c882af968200a0dd54ded9fd99b3961e7a +Subproject commit 6aff3250c29f5243eb4a6111ba0a5c0cc1a3033c From 15a67b81f98b9037c8fc06d260b8e94bd462bcfc Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 27 Jun 2023 17:36:52 +0200 Subject: [PATCH 37/40] various improvements --- CHANGELOG.md | 6 ++ bsp_q7s/fmObjectFactory.cpp | 4 +- dummies/TemperatureSensorInserter.cpp | 21 ++++- dummies/TemperatureSensorInserter.h | 1 + mission/tcs/HeaterHandler.cpp | 107 +++++++++++++++----------- mission/tcs/HeaterHandler.h | 2 + 6 files changed, 92 insertions(+), 49 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index dbe08848..d446bd49 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,6 +27,12 @@ will consitute of a breaking change warranting a new major release: - Internal error reporter set is now enabled by default and generated every 120 seconds. - Persistent TM store dumps are now performed in chronological order. +## Fixed + +- Various robustness improvements for the heater handler. The heater handler will now only + process the command queue if it is not busy with switch commanding which reduced the amount + of possible bugs. + # [v5.0.0] 2023-06-26 v3.3.1 and all following version will now be moved to v5.0.0 with the additional changes listed diff --git a/bsp_q7s/fmObjectFactory.cpp b/bsp_q7s/fmObjectFactory.cpp index da14f58c..d9329fc0 100644 --- a/bsp_q7s/fmObjectFactory.cpp +++ b/bsp_q7s/fmObjectFactory.cpp @@ -33,8 +33,8 @@ void ObjectFactory::produce(void* args) { PersistentTmStores stores; ObjectFactory::produceGenericObjects(&healthTable, &pusFunnel, &cfdpFunnel, - *SdCardManager::instance(), &ipcStore, &tmStore, stores, - 200, true); + *SdCardManager::instance(), &ipcStore, &tmStore, stores, 200, + true); LinuxLibgpioIF* gpioComIF = nullptr; SerialComIF* uartComIF = nullptr; diff --git a/dummies/TemperatureSensorInserter.cpp b/dummies/TemperatureSensorInserter.cpp index 14a005aa..c58416f7 100644 --- a/dummies/TemperatureSensorInserter.cpp +++ b/dummies/TemperatureSensorInserter.cpp @@ -15,7 +15,7 @@ TemperatureSensorInserter::TemperatureSensorInserter(object_id_t objectId, tmp1075DummyMap(std::move(tempTmpSensorDummies_)) {} ReturnValue_t TemperatureSensorInserter::initialize() { - testCase = TestCase::NONE; + testCase = TestCase::COLD_PLOC_CONSECUTIVE; return returnvalue::OK; } @@ -96,6 +96,25 @@ ReturnValue_t TemperatureSensorInserter::performOperation(uint8_t opCode) { } break; } + case (TestCase::COLD_PLOC_CONSECUTIVE): { + if (cycles == 15) { + sif::debug << "Setting cold PLOC temperature" << std::endl; + max31865DummyMap[objects::RTD_0_IC3_PLOC_HEATSPREADER]->setTemperature(-15, true); + } + if (cycles == 30) { + sif::debug << "Setting warmer PLOC temperature" << std::endl; + max31865DummyMap[objects::RTD_0_IC3_PLOC_HEATSPREADER]->setTemperature(0, true); + } + if (cycles == 45) { + sif::debug << "Setting cold PLOC temperature again" << std::endl; + max31865DummyMap[objects::RTD_0_IC3_PLOC_HEATSPREADER]->setTemperature(-15, true); + } + if (cycles == 60) { + sif::debug << "Setting warmer PLOC temperature again" << std::endl; + max31865DummyMap[objects::RTD_0_IC3_PLOC_HEATSPREADER]->setTemperature(0, true); + } + break; + } case (TestCase::COLD_CAMERA): { if (cycles == 15) { sif::debug << "Setting cold CAM temperature" << std::endl; diff --git a/dummies/TemperatureSensorInserter.h b/dummies/TemperatureSensorInserter.h index eb6cc1ba..009f5b7d 100644 --- a/dummies/TemperatureSensorInserter.h +++ b/dummies/TemperatureSensorInserter.h @@ -32,6 +32,7 @@ class TemperatureSensorInserter : public ExecutableObjectIF, public SystemObject COLD_STR = 4, COLD_STR_CONSECUTIVE = 5, COLD_CAMERA = 6, + COLD_PLOC_CONSECUTIVE = 7, }; int iteration = 0; uint32_t cycles = 0; diff --git a/mission/tcs/HeaterHandler.cpp b/mission/tcs/HeaterHandler.cpp index de4b600d..9f7ba43c 100644 --- a/mission/tcs/HeaterHandler.cpp +++ b/mission/tcs/HeaterHandler.cpp @@ -51,9 +51,13 @@ ReturnValue_t HeaterHandler::performOperation(uint8_t operationCode) { if (mainLineSwitcher->getSwitchState(mainLineSwitch) == SWITCH_OFF) { waitForSwitchOff = false; mode = MODE_OFF; + busyWithSwitchCommanding = false; modeHelper.modeChanged(mode, submode); } } + if (busyWithSwitchCommanding and heaterCmdBusyCd.hasTimedOut()) { + busyWithSwitchCommanding = false; + } } catch (const std::out_of_range& e) { sif::warning << "HeaterHandler::performOperation: " "Out of range error | " @@ -102,20 +106,22 @@ void HeaterHandler::readCommandQueue() { ReturnValue_t result = returnvalue::OK; CommandMessage command; do { - result = commandQueue->receiveMessage(&command); - if (result == MessageQueueIF::EMPTY) { - break; - } else if (result != returnvalue::OK) { - sif::warning << "HeaterHandler::readCommandQueue: Message reception error" << std::endl; - break; - } - result = actionHelper.handleActionMessage(&command); - if (result == returnvalue::OK) { - continue; - } - result = modeHelper.handleModeCommand(&command); - if (result == returnvalue::OK) { - continue; + if (not busyWithSwitchCommanding) { + result = commandQueue->receiveMessage(&command); + if (result == MessageQueueIF::EMPTY) { + break; + } else if (result != returnvalue::OK) { + sif::warning << "HeaterHandler::readCommandQueue: Message reception error" << std::endl; + break; + } + result = modeHelper.handleModeCommand(&command); + if (result == returnvalue::OK) { + continue; + } + result = actionHelper.handleActionMessage(&command); + if (result == returnvalue::OK) { + continue; + } } } while (result == returnvalue::OK); } @@ -167,6 +173,8 @@ ReturnValue_t HeaterHandler::executeAction(ActionId_t actionId, MessageQueueId_t heater.action = action; heater.cmdActive = true; heater.replyQueue = commandedBy; + busyWithSwitchCommanding = true; + heaterCmdBusyCd.resetTimer(); return returnvalue::OK; } @@ -249,6 +257,7 @@ void HeaterHandler::handleSwitchOnCommand(heater::Switch heaterIdx) { sif::error << "HeaterHandler::handleSwitchOnCommand: Main switch setting on timeout" << std::endl; heater.cmdActive = false; + busyWithSwitchCommanding = false; heater.waitMainSwitchOn = false; if (heater.replyQueue != commandQueue->getId()) { actionHelper.finish(false, heater.replyQueue, heater.action, MAIN_SWITCH_SET_TIMEOUT); @@ -259,27 +268,30 @@ void HeaterHandler::handleSwitchOnCommand(heater::Switch heaterIdx) { // Check state of main line switch ReturnValue_t mainSwitchState = mainLineSwitcher->getSwitchState(mainLineSwitch); if (mainSwitchState == PowerSwitchIF::SWITCH_ON) { - if (getSwitchState(heaterIdx) == SwitchState::OFF) { - gpioId_t gpioId = heater.gpioId; - result = gpioInterface->pullHigh(gpioId); - if (result != returnvalue::OK) { - sif::error << "HeaterHandler::handleSwitchOnCommand: Failed to pull gpio with id " << gpioId - << " high" << std::endl; - triggerEvent(GPIO_PULL_HIGH_FAILED, result); - } else { + gpioId_t gpioId = heater.gpioId; + result = gpioInterface->pullHigh(gpioId); + if (result != returnvalue::OK) { + sif::error << "HeaterHandler::handleSwitchOnCommand: Failed to pull GPIO with ID " << gpioId + << " high" << std::endl; + triggerEvent(GPIO_PULL_HIGH_FAILED, result); + } + if (result == returnvalue::OK) { + if (getSwitchState(heaterIdx) == SwitchState::OFF) { triggerEvent(HEATER_WENT_ON, heaterIdx, 0); - EventManagerIF::triggerEvent(helper.heaters[heaterIdx].first->getObjectId(), MODE_INFO, - MODE_ON, 0); { MutexGuard mg(handlerLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); heater.switchState = ON; } + + } else { + triggerEvent(SWITCH_ALREADY_ON, heaterIdx); } - } else { - triggerEvent(SWITCH_ALREADY_ON, heaterIdx); + EventManagerIF::triggerEvent(helper.heaters[heaterIdx].first->getObjectId(), MODE_INFO, + MODE_ON, 0); + busyWithSwitchCommanding = false; + mode = HasModesIF::MODE_ON; + modeHelper.modeChanged(mode, submode); } - mode = HasModesIF::MODE_ON; - modeHelper.modeChanged(mode, submode); // There is no need to send action finish replies if the sender was the // HeaterHandler itself if (heater.replyQueue != commandQueue->getId()) { @@ -312,30 +324,33 @@ void HeaterHandler::handleSwitchOnCommand(heater::Switch heaterIdx) { void HeaterHandler::handleSwitchOffCommand(heater::Switch heaterIdx) { ReturnValue_t result = returnvalue::OK; auto& heater = heaterVec.at(heaterIdx); - // Check whether switch is already off - if (getSwitchState(heaterIdx)) { - gpioId_t gpioId = heater.gpioId; - result = gpioInterface->pullLow(gpioId); - if (result != returnvalue::OK) { - sif::error << "HeaterHandler::handleSwitchOffCommand: Failed to pull gpio with id" << gpioId - << " low" << std::endl; - triggerEvent(GPIO_PULL_LOW_FAILED, result); - } else { + gpioId_t gpioId = heater.gpioId; + result = gpioInterface->pullLow(gpioId); + if (result != returnvalue::OK) { + sif::error << "HeaterHandler::handleSwitchOffCommand: Failed to pull gpio with id" << gpioId + << " low" << std::endl; + triggerEvent(GPIO_PULL_LOW_FAILED, result); + } + if (result == returnvalue::OK) { + // Check whether switch is already off + if (getSwitchState(heaterIdx) == SwitchState::ON) { { MutexGuard mg(handlerLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); heater.switchState = OFF; } triggerEvent(HEATER_WENT_OFF, heaterIdx, 0); - EventManagerIF::triggerEvent(helper.heaters[heaterIdx].first->getObjectId(), MODE_INFO, - MODE_OFF, 0); - // When all switches are off, also main line switch will be turned off - if (allSwitchesOff()) { - mainLineSwitcher->sendSwitchCommand(mainLineSwitch, PowerSwitchIF::SWITCH_OFF); - waitForSwitchOff = true; - } + } else { + triggerEvent(SWITCH_ALREADY_OFF, heaterIdx); + } + EventManagerIF::triggerEvent(helper.heaters[heaterIdx].first->getObjectId(), MODE_INFO, + MODE_OFF, 0); + // When all switches are off, also main line switch will be turned off + if (allSwitchesOff()) { + mainLineSwitcher->sendSwitchCommand(mainLineSwitch, PowerSwitchIF::SWITCH_OFF); + waitForSwitchOff = true; + } else { + busyWithSwitchCommanding = false; } - } else { - triggerEvent(SWITCH_ALREADY_OFF, heaterIdx); } if (heater.replyQueue != NO_COMMANDER) { // Report back switch command reply if necessary diff --git a/mission/tcs/HeaterHandler.h b/mission/tcs/HeaterHandler.h index 609ac725..0198ab04 100644 --- a/mission/tcs/HeaterHandler.h +++ b/mission/tcs/HeaterHandler.h @@ -148,6 +148,7 @@ class HeaterHandler : public ExecutableObjectIF, /** Size of command queue */ size_t cmdQueueSize = 20; bool waitForSwitchOff = true; + bool busyWithSwitchCommanding = false; GpioIF* gpioInterface = nullptr; @@ -163,6 +164,7 @@ class HeaterHandler : public ExecutableObjectIF, power::Switch_t mainLineSwitch; ActionHelper actionHelper; + Countdown heaterCmdBusyCd = Countdown(2000); StorageManagerIF* ipcStore = nullptr; From 1e767afa118c19ef884b11ca8d93130645782b9b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 27 Jun 2023 17:55:49 +0200 Subject: [PATCH 38/40] only handle one message per cycle --- mission/tcs/HeaterHandler.cpp | 47 +++++++++++++++-------------------- 1 file changed, 20 insertions(+), 27 deletions(-) diff --git a/mission/tcs/HeaterHandler.cpp b/mission/tcs/HeaterHandler.cpp index 9f7ba43c..8416a7f7 100644 --- a/mission/tcs/HeaterHandler.cpp +++ b/mission/tcs/HeaterHandler.cpp @@ -105,25 +105,23 @@ ReturnValue_t HeaterHandler::initializeHeaterMap() { void HeaterHandler::readCommandQueue() { ReturnValue_t result = returnvalue::OK; CommandMessage command; - do { - if (not busyWithSwitchCommanding) { - result = commandQueue->receiveMessage(&command); - if (result == MessageQueueIF::EMPTY) { - break; - } else if (result != returnvalue::OK) { - sif::warning << "HeaterHandler::readCommandQueue: Message reception error" << std::endl; - break; - } - result = modeHelper.handleModeCommand(&command); - if (result == returnvalue::OK) { - continue; - } - result = actionHelper.handleActionMessage(&command); - if (result == returnvalue::OK) { - continue; - } + if (not busyWithSwitchCommanding) { + result = commandQueue->receiveMessage(&command); + if (result == MessageQueueIF::EMPTY) { + return; + } else if (result != returnvalue::OK) { + sif::warning << "HeaterHandler::readCommandQueue: Message reception error" << std::endl; + return; } - } while (result == returnvalue::OK); + result = modeHelper.handleModeCommand(&command); + if (result == returnvalue::OK) { + return; + } + result = actionHelper.handleActionMessage(&command); + if (result == returnvalue::OK) { + return; + } + } } ReturnValue_t HeaterHandler::executeAction(ActionId_t actionId, MessageQueueId_t commandedBy, @@ -276,15 +274,10 @@ void HeaterHandler::handleSwitchOnCommand(heater::Switch heaterIdx) { triggerEvent(GPIO_PULL_HIGH_FAILED, result); } if (result == returnvalue::OK) { - if (getSwitchState(heaterIdx) == SwitchState::OFF) { - triggerEvent(HEATER_WENT_ON, heaterIdx, 0); - { - MutexGuard mg(handlerLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); - heater.switchState = ON; - } - - } else { - triggerEvent(SWITCH_ALREADY_ON, heaterIdx); + triggerEvent(HEATER_WENT_ON, heaterIdx, 0); + { + MutexGuard mg(handlerLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX); + heater.switchState = ON; } EventManagerIF::triggerEvent(helper.heaters[heaterIdx].first->getObjectId(), MODE_INFO, MODE_ON, 0); From 50a8a6fffe974e0cb2ba9288f265f2c5ff35b505 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 27 Jun 2023 18:02:49 +0200 Subject: [PATCH 39/40] more changelog --- CHANGELOG.md | 6 +++--- bsp_q7s/core/scheduling.cpp | 4 ++++ dummies/TemperatureSensorInserter.cpp | 2 +- 3 files changed, 8 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index d446bd49..0974f124 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -26,12 +26,12 @@ will consitute of a breaking change warranting a new major release: - Internal error reporter set is now enabled by default and generated every 120 seconds. - Persistent TM store dumps are now performed in chronological order. - -## Fixed - - Various robustness improvements for the heater handler. The heater handler will now only process the command queue if it is not busy with switch commanding which reduced the amount of possible bugs. +- The heater handler is only able to process messages stricly sequentially now but is scheduled + twice in a 0.5 second slot to something like a consecutive heater ON or OFF command can still + be handled relatively quickly. # [v5.0.0] 2023-06-26 diff --git a/bsp_q7s/core/scheduling.cpp b/bsp_q7s/core/scheduling.cpp index 71da5bdc..90918cf3 100644 --- a/bsp_q7s/core/scheduling.cpp +++ b/bsp_q7s/core/scheduling.cpp @@ -324,6 +324,10 @@ void scheduling::initTasks() { if (result != returnvalue::OK) { scheduling::printAddObjectError("HEATER_HANDLER", objects::HEATER_HANDLER); } + result = tcsSystemTask->addComponent(objects::HEATER_HANDLER); + if (result != returnvalue::OK) { + scheduling::printAddObjectError("HEATER_HANDLER", objects::HEATER_HANDLER); + } #if OBSW_ADD_SYRLINKS == 1 PeriodicTaskIF* syrlinksCom = factory->createPeriodicTask( diff --git a/dummies/TemperatureSensorInserter.cpp b/dummies/TemperatureSensorInserter.cpp index c58416f7..5c89d258 100644 --- a/dummies/TemperatureSensorInserter.cpp +++ b/dummies/TemperatureSensorInserter.cpp @@ -15,7 +15,7 @@ TemperatureSensorInserter::TemperatureSensorInserter(object_id_t objectId, tmp1075DummyMap(std::move(tempTmpSensorDummies_)) {} ReturnValue_t TemperatureSensorInserter::initialize() { - testCase = TestCase::COLD_PLOC_CONSECUTIVE; + testCase = TestCase::NONE; return returnvalue::OK; } From fba87cc0e8cb067ef25e3b9fd6b032df97a8161f Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 27 Jun 2023 18:09:38 +0200 Subject: [PATCH 40/40] changelog typos --- CHANGELOG.md | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0974f124..0675eea0 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -27,10 +27,10 @@ will consitute of a breaking change warranting a new major release: - Internal error reporter set is now enabled by default and generated every 120 seconds. - Persistent TM store dumps are now performed in chronological order. - Various robustness improvements for the heater handler. The heater handler will now only - process the command queue if it is not busy with switch commanding which reduced the amount + process the command queue if it is not busy with switch commanding which reduces the amount of possible bugs. - The heater handler is only able to process messages stricly sequentially now but is scheduled - twice in a 0.5 second slot to something like a consecutive heater ON or OFF command can still + twice in a 0.5 second slot so something like a consecutive heater ON or OFF command can still be handled relatively quickly. # [v5.0.0] 2023-06-26