Merge pull request 'Improve SD card manager' (#454) from improve_sd_card_manager into develop

Reviewed-on: #454

CHANGELOG.md

@@ -22,6 +22,10 @@ will consitute of a breaking change warranting a new major release:
 
 ## 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.
 - Request raw MTM measurement twice for IMTQ, might reduce number of times measurement could not
   be retrieved.
 

bsp_q7s/core/CoreController.cpp

@@ -42,6 +42,12 @@ CoreController::CoreController(object_id_t objectId)
     if (not BLOCKING_SD_INIT) {
       sdcMan->setBlocking(false);
     }
+    // Set up state of SD card manager and own initial state.
+    // Stopwatch watch;
+    sdcMan->updateSdCardStateFile();
+    sdcMan->updateSdStatePair();
+    SdCardManager::SdStatePair sdStates;
+    sdcMan->getSdCardsStatus(sdStates);
     auto sdCard = sdcMan->getPreferredSdCard();
     if (not sdCard.has_value()) {
       sif::error << "CoreController::initializeAfterTaskCreation: "
@@ -50,7 +56,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);
@@ -400,7 +410,9 @@ ReturnValue_t CoreController::sdStateMachine() {
         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");
     }
@@ -543,10 +555,6 @@ 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();
     if (result != returnvalue::OK) {

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) {
+ReturnValue_t SdCardManager::getSdCardsStatus(SdStatePair& sdStates) {
-  using namespace std;
   MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX);
-  std::error_code e;
+  sdStates = this->sdStates;
-  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;
-  }
   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<SdStatePair>();
+    return returnvalue::FAILED;
-    statusPair = sdStatusPtr.get();
-    getSdCardsStatus(*statusPair);
   }
+  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<sd::SdState, sd::SdState>& active,
+ReturnValue_t SdCardManager::updateSdStatePair() {
-                                        std::string& line, uint8_t& idx, sd::SdCard& currentSd) {
+  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<sd::SdState, sd::SdState>& 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;
       }
     }
 
@@ -407,7 +412,7 @@ ReturnValue_t SdCardManager::updateSdCardStateFile() {
   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");
@@ -475,35 +480,29 @@ bool SdCardManager::isSdCardUsable(std::optional<sd::SdCard> sdCard) {
     }
   }
 
-  SdCardManager::SdStatePair active;
+  MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX);
-  ReturnValue_t result = this->getSdCardsStatus(active);
-
-  if (result != returnvalue::OK) {
-    sif::debug << "SdCardManager::isSdCardMounted: Failed to get SD card active state";
-    return false;
-  }
   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;
     }
   }

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);
@@ -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,
+  void processSdStatusLine(std::string& line, uint8_t& idx, sd::SdCard& currentSd);
-                           sd::SdCard& currentSd);
 
   std::optional<std::string> currentPrefix;