From fae83a0fca90c23a9ae2918ae1d3cef29fd94ea4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 10 Mar 2023 02:29:28 +0100 Subject: [PATCH 1/3] this should avoid lock issues al together --- bsp_q7s/core/CoreController.cpp | 25 +++++---- bsp_q7s/fs/SdCardManager.cpp | 89 ++++++++++++++++----------------- bsp_q7s/fs/SdCardManager.h | 10 ++-- 3 files changed, 65 insertions(+), 59 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index f51f6bf9..d5023111 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -42,6 +42,11 @@ CoreController::CoreController(object_id_t objectId) if (not BLOCKING_SD_INIT) { sdcMan->setBlocking(false); } + Stopwatch watch; + sdcMan->updateSdCardStateFile(true); + sdcMan->updateSdStatePair(); + SdCardManager::SdStatePair sdStates; + sdcMan->getSdCardsStatus(sdStates); auto sdCard = sdcMan->getPreferredSdCard(); if (not sdCard.has_value()) { sif::error << "CoreController::initializeAfterTaskCreation: " @@ -50,7 +55,11 @@ CoreController::CoreController(object_id_t objectId) sdCard = sd::SdCard::SLOT_0; } sdInfo.active = sdCard.value(); - sdcMan->setActiveSdCard(sdInfo.active); + if (sdStates.first == sd::SdState::MOUNTED) { + sdcMan->setActiveSdCard(sd::SdCard::SLOT_0); + } else if (sdStates.second == sd::SdState::MOUNTED) { + sdcMan->setActiveSdCard(sd::SdCard::SLOT_1); + } currMntPrefix = sdcMan->getCurrentMountPrefix(); getCurrentBootCopy(CURRENT_CHIP, CURRENT_COPY); @@ -314,7 +323,7 @@ ReturnValue_t CoreController::checkModeCommand(Mode_t mode, Submode_t submode, ReturnValue_t CoreController::initSdCardBlocking() { // Create update status file - ReturnValue_t result = sdcMan->updateSdCardStateFile(); + ReturnValue_t result = sdcMan->updateSdCardStateFile(true); if (result != returnvalue::OK) { sif::warning << "CoreController::initialize: Updating SD card state file failed" << std::endl; } @@ -334,7 +343,7 @@ ReturnValue_t CoreController::initSdCardBlocking() { << static_cast(sdInfo.active) << std::endl; result = sdColdRedundantBlockingInit(); // Update status file - sdcMan->updateSdCardStateFile(); + sdcMan->updateSdCardStateFile(true); return result; } if (sdInfo.cfgMode == SdCfgMode::HOT_REDUNDANT) { @@ -342,7 +351,7 @@ ReturnValue_t CoreController::initSdCardBlocking() { sdCardSetup(sd::SdCard::SLOT_0, sd::SdState::MOUNTED, "0", false); sdCardSetup(sd::SdCard::SLOT_1, sd::SdState::MOUNTED, "1", false); // Update status file - sdcMan->updateSdCardStateFile(); + sdcMan->updateSdCardStateFile(true); } return returnvalue::OK; } @@ -395,7 +404,7 @@ ReturnValue_t CoreController::sdStateMachine() { if (sdFsmState == SdStates::GET_INFO) { if (not sdInfo.commandExecuted) { // Create updated status file - result = sdcMan->updateSdCardStateFile(); + result = sdcMan->updateSdCardStateFile(false); if (result != returnvalue::OK) { sif::warning << "CoreController::sdStateMachine: Updating SD card state file failed" << std::endl; @@ -543,12 +552,8 @@ ReturnValue_t CoreController::sdStateMachine() { if (sdFsmState == SdStates::SKIP_CYCLE_BEFORE_INFO_UPDATE) { sdFsmState = SdStates::UPDATE_INFO; } else if (sdFsmState == SdStates::UPDATE_INFO) { - // It is assumed that all tasks are running by the point this section is reached. - // Therefore, perform this operation in blocking mode because it does not take long - // and the ready state of the SD card is available sooner - sdcMan->setBlocking(true); // Update status file - result = sdcMan->updateSdCardStateFile(); + result = sdcMan->updateSdCardStateFile(true); if (result != returnvalue::OK) { sif::warning << "CoreController::initialize: Updating SD card state file failed" << std::endl; } diff --git a/bsp_q7s/fs/SdCardManager.cpp b/bsp_q7s/fs/SdCardManager.cpp index fa46f8e0..41569000 100644 --- a/bsp_q7s/fs/SdCardManager.cpp +++ b/bsp_q7s/fs/SdCardManager.cpp @@ -195,29 +195,9 @@ ReturnValue_t SdCardManager::setSdCardState(sd::SdCard sdCard, bool on) { return result; } -ReturnValue_t SdCardManager::getSdCardsStatus(SdStatePair& active) { - using namespace std; +ReturnValue_t SdCardManager::getSdCardsStatus(SdStatePair& sdStates) { MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX); - std::error_code e; - if (not filesystem::exists(SD_STATE_FILE, e)) { - return STATUS_FILE_NEXISTS; - } - - // Now the file should exist in any case. Still check whether it exists. - fstream sdStatus(SD_STATE_FILE); - if (not sdStatus.good()) { - return STATUS_FILE_NEXISTS; - } - string line; - uint8_t idx = 0; - sd::SdCard currentSd = sd::SdCard::SLOT_0; - // Process status file line by line - while (std::getline(sdStatus, line)) { - processSdStatusLine(active, line, idx, currentSd); - } - if (active.first != sd::SdState::MOUNTED && active.second != sd::SdState::MOUNTED) { - sdCardActive = false; - } + sdStates = this->sdStates; return returnvalue::OK; } @@ -309,10 +289,9 @@ ReturnValue_t SdCardManager::sanitizeState(SdStatePair* statusPair, sd::SdCard p resetNonBlockingState = true; } if (statusPair == nullptr) { - sdStatusPtr = std::make_unique(); - statusPair = sdStatusPtr.get(); - getSdCardsStatus(*statusPair); + return returnvalue::FAILED; } + getSdCardsStatus(*statusPair); if (statusPair->first == sd::SdState::ON) { result = mountSdCard(prefSdCard); @@ -330,8 +309,34 @@ void SdCardManager::resetState() { currentOp = Operations::IDLE; } -void SdCardManager::processSdStatusLine(std::pair& active, - std::string& line, uint8_t& idx, sd::SdCard& currentSd) { +ReturnValue_t SdCardManager::updateSdStatePair() { + using namespace std; + + MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX); + std::error_code e; + if (not filesystem::exists(SD_STATE_FILE, e)) { + return STATUS_FILE_NEXISTS; + } + + // Now the file should exist in any case. Still check whether it exists. + fstream sdStatus(SD_STATE_FILE); + if (not sdStatus.good()) { + return STATUS_FILE_NEXISTS; + } + string line; + uint8_t idx = 0; + sd::SdCard currentSd = sd::SdCard::SLOT_0; + // Process status file line by line + while (std::getline(sdStatus, line)) { + processSdStatusLine(line, idx, currentSd); + } + if (sdStates.first != sd::SdState::MOUNTED && sdStates.second != sd::SdState::MOUNTED) { + sdCardActive = false; + } + return returnvalue::OK; +} + +void SdCardManager::processSdStatusLine(std::string& line, uint8_t& idx, sd::SdCard& currentSd) { using namespace std; istringstream iss(line); string word; @@ -352,24 +357,24 @@ void SdCardManager::processSdStatusLine(std::pair& act if (word == "on") { if (currentSd == sd::SdCard::SLOT_0) { - active.first = sd::SdState::ON; + sdStates.first = sd::SdState::ON; } else { - active.second = sd::SdState::ON; + sdStates.second = sd::SdState::ON; } } else if (word == "off") { if (currentSd == sd::SdCard::SLOT_0) { - active.first = sd::SdState::OFF; + sdStates.first = sd::SdState::OFF; } else { - active.second = sd::SdState::OFF; + sdStates.second = sd::SdState::OFF; } } } if (mountLine) { if (currentSd == sd::SdCard::SLOT_0) { - active.first = sd::SdState::MOUNTED; + sdStates.first = sd::SdState::MOUNTED; } else { - active.second = sd::SdState::MOUNTED; + sdStates.second = sd::SdState::MOUNTED; } } @@ -400,7 +405,7 @@ ReturnValue_t SdCardManager::setPreferredSdCard(sd::SdCard sdCard) { return scratch::writeNumber(scratch::PREFERED_SDC_KEY, static_cast(sdCard)); } -ReturnValue_t SdCardManager::updateSdCardStateFile() { +ReturnValue_t SdCardManager::updateSdCardStateFile(bool blocking) { if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING) { return CommandExecutor::COMMAND_PENDING; } @@ -475,35 +480,29 @@ bool SdCardManager::isSdCardUsable(std::optional sdCard) { } } - SdCardManager::SdStatePair active; - ReturnValue_t result = this->getSdCardsStatus(active); - - if (result != returnvalue::OK) { - sif::debug << "SdCardManager::isSdCardMounted: Failed to get SD card active state"; - return false; - } + MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX); if (not sdCard) { - if (active.first == sd::MOUNTED or active.second == sd::MOUNTED) { + if (sdStates.first == sd::MOUNTED or sdStates.second == sd::MOUNTED) { return true; } return false; } if (sdCard == sd::SLOT_0) { - if (active.first == sd::MOUNTED) { + if (sdStates.first == sd::MOUNTED) { return true; } else { return false; } } if (sdCard == sd::SLOT_1) { - if (active.second == sd::MOUNTED) { + if (sdStates.second == sd::MOUNTED) { return true; } else { return false; } } if (sdCard == sd::BOTH) { - if (active.first == sd::MOUNTED && active.second == sd::MOUNTED) { + if (sdStates.first == sd::MOUNTED && sdStates.second == sd::MOUNTED) { return true; } } diff --git a/bsp_q7s/fs/SdCardManager.h b/bsp_q7s/fs/SdCardManager.h index 1cd09d7d..adc44bb8 100644 --- a/bsp_q7s/fs/SdCardManager.h +++ b/bsp_q7s/fs/SdCardManager.h @@ -25,7 +25,7 @@ class MutexIF; * state */ class SdCardManager : public SystemObject, public SdCardMountedIF { - friend class SdCardAccess; + friend class CoreController; public: using mountInitCb = ReturnValue_t (*)(void* args); @@ -125,7 +125,7 @@ class SdCardManager : public SystemObject, public SdCardMountedIF { * - CommandExecutor::COMMAND_PENDING: Non-blocking command is pending * - returnvalue::FAILED: blocking command failed */ - ReturnValue_t updateSdCardStateFile(); + ReturnValue_t updateSdCardStateFile(bool blocking); /** * Get the state of the SD cards. If the state file does not exist, this function will @@ -218,6 +218,7 @@ class SdCardManager : public SystemObject, public SdCardMountedIF { private: CommandExecutor cmdExecutor; + SdStatePair sdStates; Operations currentOp = Operations::IDLE; bool blocking = false; bool sdCardActive = true; @@ -233,10 +234,11 @@ class SdCardManager : public SystemObject, public SdCardMountedIF { SdCardManager(); + ReturnValue_t updateSdStatePair(); + ReturnValue_t setSdCardState(sd::SdCard sdCard, bool on); - void processSdStatusLine(SdStatePair& active, std::string& line, uint8_t& idx, - sd::SdCard& currentSd); + void processSdStatusLine(std::string& line, uint8_t& idx, sd::SdCard& currentSd); std::optional currentPrefix; From 677a3f95bda21e1a04d8662b29338e750dc3505c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 10 Mar 2023 02:45:21 +0100 Subject: [PATCH 2/3] always update sd state file in blocking manner --- bsp_q7s/core/CoreController.cpp | 19 +++++++++++-------- bsp_q7s/fs/SdCardManager.cpp | 4 ++-- bsp_q7s/fs/SdCardManager.h | 2 +- 3 files changed, 14 insertions(+), 11 deletions(-) diff --git a/bsp_q7s/core/CoreController.cpp b/bsp_q7s/core/CoreController.cpp index d5023111..7eb0fe5f 100644 --- a/bsp_q7s/core/CoreController.cpp +++ b/bsp_q7s/core/CoreController.cpp @@ -42,8 +42,9 @@ CoreController::CoreController(object_id_t objectId) if (not BLOCKING_SD_INIT) { sdcMan->setBlocking(false); } - Stopwatch watch; - sdcMan->updateSdCardStateFile(true); + // Set up state of SD card manager and own initial state. + // Stopwatch watch; + sdcMan->updateSdCardStateFile(); sdcMan->updateSdStatePair(); SdCardManager::SdStatePair sdStates; sdcMan->getSdCardsStatus(sdStates); @@ -323,7 +324,7 @@ ReturnValue_t CoreController::checkModeCommand(Mode_t mode, Submode_t submode, ReturnValue_t CoreController::initSdCardBlocking() { // Create update status file - ReturnValue_t result = sdcMan->updateSdCardStateFile(true); + ReturnValue_t result = sdcMan->updateSdCardStateFile(); if (result != returnvalue::OK) { sif::warning << "CoreController::initialize: Updating SD card state file failed" << std::endl; } @@ -343,7 +344,7 @@ ReturnValue_t CoreController::initSdCardBlocking() { << static_cast(sdInfo.active) << std::endl; result = sdColdRedundantBlockingInit(); // Update status file - sdcMan->updateSdCardStateFile(true); + sdcMan->updateSdCardStateFile(); return result; } if (sdInfo.cfgMode == SdCfgMode::HOT_REDUNDANT) { @@ -351,7 +352,7 @@ ReturnValue_t CoreController::initSdCardBlocking() { sdCardSetup(sd::SdCard::SLOT_0, sd::SdState::MOUNTED, "0", false); sdCardSetup(sd::SdCard::SLOT_1, sd::SdState::MOUNTED, "1", false); // Update status file - sdcMan->updateSdCardStateFile(true); + sdcMan->updateSdCardStateFile(); } return returnvalue::OK; } @@ -404,12 +405,14 @@ ReturnValue_t CoreController::sdStateMachine() { if (sdFsmState == SdStates::GET_INFO) { if (not sdInfo.commandExecuted) { // Create updated status file - result = sdcMan->updateSdCardStateFile(false); + result = sdcMan->updateSdCardStateFile(); if (result != returnvalue::OK) { sif::warning << "CoreController::sdStateMachine: Updating SD card state file failed" << std::endl; } - sdInfo.commandExecuted = true; + sdFsmState = SdStates::SET_STATE_SELF; + sdInfo.commandExecuted = false; + sdInfo.cycleCount = 0; } else { nonBlockingOpChecking(SdStates::SET_STATE_SELF, 4, "Updating SDC file"); } @@ -553,7 +556,7 @@ ReturnValue_t CoreController::sdStateMachine() { sdFsmState = SdStates::UPDATE_INFO; } else if (sdFsmState == SdStates::UPDATE_INFO) { // Update status file - result = sdcMan->updateSdCardStateFile(true); + result = sdcMan->updateSdCardStateFile(); if (result != returnvalue::OK) { sif::warning << "CoreController::initialize: Updating SD card state file failed" << std::endl; } diff --git a/bsp_q7s/fs/SdCardManager.cpp b/bsp_q7s/fs/SdCardManager.cpp index 41569000..0ced8a04 100644 --- a/bsp_q7s/fs/SdCardManager.cpp +++ b/bsp_q7s/fs/SdCardManager.cpp @@ -405,14 +405,14 @@ ReturnValue_t SdCardManager::setPreferredSdCard(sd::SdCard sdCard) { return scratch::writeNumber(scratch::PREFERED_SDC_KEY, static_cast(sdCard)); } -ReturnValue_t SdCardManager::updateSdCardStateFile(bool blocking) { +ReturnValue_t SdCardManager::updateSdCardStateFile() { if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING) { return CommandExecutor::COMMAND_PENDING; } MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX); // Use q7hw utility and pipe the command output into the state file std::string updateCmd = "q7hw sd info all > " + std::string(SD_STATE_FILE); - cmdExecutor.load(updateCmd, blocking, printCmdOutput); + cmdExecutor.load(updateCmd, true, printCmdOutput); ReturnValue_t result = cmdExecutor.execute(); if (blocking and result != returnvalue::OK) { utility::handleSystemError(cmdExecutor.getLastError(), "SdCardManager::mountSdCard"); diff --git a/bsp_q7s/fs/SdCardManager.h b/bsp_q7s/fs/SdCardManager.h index adc44bb8..cee06894 100644 --- a/bsp_q7s/fs/SdCardManager.h +++ b/bsp_q7s/fs/SdCardManager.h @@ -125,7 +125,7 @@ class SdCardManager : public SystemObject, public SdCardMountedIF { * - CommandExecutor::COMMAND_PENDING: Non-blocking command is pending * - returnvalue::FAILED: blocking command failed */ - ReturnValue_t updateSdCardStateFile(bool blocking); + ReturnValue_t updateSdCardStateFile(); /** * Get the state of the SD cards. If the state file does not exist, this function will From 3caa9dea75b6de4ef84fc0df63591a006afc7cb2 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 10 Mar 2023 11:18:34 +0100 Subject: [PATCH 3/3] changelog --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 4d2e2cbb..bfcb003c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -20,6 +20,13 @@ will consitute of a breaking change warranting a new major release: - Fix for heater names: HPA heater (index 7) is now the Syrlinks heater. +## Changed + +- More fixes and improvements for SD card handling. Extend SD card setup in core controller to + create full initial state for SD card manager are core controller as early as possible, turn + execution of setup file update blocking. This might solve the issue with the SD card manager + sometimes blocking for a long time. + # [v1.36.0] 2023-03-08 eive-tmtc: v2.17.2