CHANGELOG.md

@@ -24,6 +24,8 @@ will consitute of a breaking change warranting a new major release:
 - Re-ordered some functions of the core controller in the initialize function.
 - Rad sensor is now only polled every 30 minutes instead of every device cycle to reduce wear of
   the RADFET electronics.
+- The SD cards will still be switched OFF on a reboot, but this is done in a non-blocking manner
+  now with a timeout of 10 seconds where the reboot will be performed n any case.
 
 ## Added
 
@@ -41,6 +43,9 @@ will consitute of a breaking change warranting a new major release:
 
 ## Fixed
 
+- General bugs in the SD card state machine. This might fix some other known bugs for certain
+  combinations of switching ON and OFF SD cards and also makes the whole state machine a lot more
+  robust against hanging up.
 - SUS dummy handler went to `MODE_NORMAL` for ON commands.
 - PL PCDU dummy went to `MODE_NORMAL` for ON commands.
 

bsp_q7s/core/CoreController.cpp

@@ -554,21 +554,23 @@ ReturnValue_t CoreController::sdStateMachine() {
   }
 
   // This lambda checks the non-blocking operation of the SD card manager and assigns the new
-  // state on success. It returns true for an operation success and false otherwise
+  // state on success. It returns 0 for an operation success, -1 for failed operations, and 1
+  // for pending operations
   auto nonBlockingSdcOpChecking = [&](SdStates newStateOnSuccess, uint16_t maxCycleCount,
                                       std::string opPrintout) {
     SdCardManager::OpStatus status = sdcMan->checkCurrentOp(operation);
-    if (status == SdCardManager::OpStatus::SUCCESS) {
+    if (status == SdCardManager::OpStatus::SUCCESS or sdInfo.cycleCount > 4) {
       sdFsmState = newStateOnSuccess;
       sdInfo.commandPending = false;
       sdInfo.cycleCount = 0;
-      return true;
-    } else if (sdInfo.cycleCount > 4) {
-      sif::warning << "CoreController::sdStateMachine: " << opPrintout << " takes too long"
-                   << std::endl;
-      return false;
-    }
-    return false;
+      if (sdInfo.cycleCount > 4) {
+        sif::warning << "CoreController::sdStateMachine: " << opPrintout << " takes too long"
+                     << std::endl;
+        return -1;
+      }
+      return 0;
+    };
+    return 1;
   };
 
   if (sdFsmState == SdStates::UPDATE_SD_INFO_START) {
@@ -644,7 +646,7 @@ ReturnValue_t CoreController::sdStateMachine() {
         sdFsmState = tgtState;
       }
     } else {
-      if (nonBlockingSdcOpChecking(SdStates::MOUNT_SELF, 10, "Setting SDC state")) {
+      if (nonBlockingSdcOpChecking(SdStates::MOUNT_SELF, 10, "Setting SDC state") <= 0) {
         sdInfo.activeState = sd::SdState::ON;
         currentStateSetter(sdInfo.active, sd::SdState::ON);
         // Skip the two cycles now.
@@ -672,7 +674,7 @@ ReturnValue_t CoreController::sdStateMachine() {
       result = sdCardSetup(sdInfo.active, sd::SdState::MOUNTED, sdInfo.activeChar);
       sdInfo.commandPending = true;
     } else {
-      if (nonBlockingSdcOpChecking(SdStates::DETERMINE_OTHER, 5, "Mounting SD card")) {
+      if (nonBlockingSdcOpChecking(SdStates::DETERMINE_OTHER, 5, "Mounting SD card") <= 0) {
         sdcMan->setActiveSdCard(sdInfo.active);
         currMntPrefix = sdcMan->getCurrentMountPrefix();
         sdInfo.activeState = sd::SdState::MOUNTED;
@@ -714,12 +716,7 @@ ReturnValue_t CoreController::sdStateMachine() {
         sdInfo.commandPending = true;
       } else {
         if (nonBlockingSdcOpChecking(SdStates::SKIP_CYCLE_BEFORE_INFO_UPDATE, 10,
-                                     "Switching off other SD card")) {
-          sdInfo.otherState = sd::SdState::OFF;
-          currentStateSetter(sdInfo.other, sd::SdState::OFF);
-        } else {
-          // Continue.. avoid being stuck here..
-          sdFsmState = SdStates::SKIP_CYCLE_BEFORE_INFO_UPDATE;
+                                     "Switching off other SD card") <= 0) {
           sdInfo.otherState = sd::SdState::OFF;
           currentStateSetter(sdInfo.other, sd::SdState::OFF);
         }
@@ -730,12 +727,7 @@ ReturnValue_t CoreController::sdStateMachine() {
         sdInfo.commandPending = true;
       } else {
         if (nonBlockingSdcOpChecking(SdStates::MOUNT_UNMOUNT_OTHER, 10,
-                                     "Switching on other SD card")) {
-          sdInfo.otherState = sd::SdState::ON;
-          currentStateSetter(sdInfo.other, sd::SdState::ON);
-        } else {
-          // Contnue.. avoid being stuck here.
-          sdFsmState = SdStates::MOUNT_UNMOUNT_OTHER;
+                                     "Switching on other SD card") <= 0) {
           sdInfo.otherState = sd::SdState::ON;
           currentStateSetter(sdInfo.other, sd::SdState::ON);
         }
@@ -750,7 +742,8 @@ ReturnValue_t CoreController::sdStateMachine() {
         result = sdCardSetup(sdInfo.other, sd::SdState::ON, sdInfo.otherChar);
         sdInfo.commandPending = true;
       } else {
-        if (nonBlockingSdcOpChecking(SdStates::SET_STATE_OTHER, 10, "Unmounting other SD card")) {
+        if (nonBlockingSdcOpChecking(SdStates::SET_STATE_OTHER, 10, "Unmounting other SD card") <=
+            0) {
           sdInfo.otherState = sd::SdState::ON;
           currentStateSetter(sdInfo.other, sd::SdState::ON);
         } else {
@@ -764,7 +757,8 @@ ReturnValue_t CoreController::sdStateMachine() {
         result = sdCardSetup(sdInfo.other, sd::SdState::MOUNTED, sdInfo.otherChar);
         sdInfo.commandPending = true;
       } else {
-        if (nonBlockingSdcOpChecking(SdStates::UPDATE_SD_INFO_END, 4, "Mounting other SD card")) {
+        if (nonBlockingSdcOpChecking(SdStates::UPDATE_SD_INFO_END, 4, "Mounting other SD card") <=
+            0) {
           sdInfo.otherState = sd::SdState::MOUNTED;
           currentStateSetter(sdInfo.other, sd::SdState::MOUNTED);
         }
@@ -840,7 +834,7 @@ ReturnValue_t CoreController::sdCardSetup(sd::SdCard sdCard, sd::SdState targetS
   if (state == sd::SdState::MOUNTED) {
     if (targetState == sd::SdState::OFF) {
       sif::info << "Switching off SD card " << sdChar << std::endl;
-      return sdcMan->switchOffSdCard(sdCard, true, &sdInfo.currentState);
+      return sdcMan->switchOffSdCard(sdCard, sdInfo.currentState, true);
     } else if (targetState == sd::SdState::ON) {
       sif::info << "Unmounting SD card " << sdChar << std::endl;
       return sdcMan->unmountSdCard(sdCard);
@@ -874,7 +868,7 @@ ReturnValue_t CoreController::sdCardSetup(sd::SdCard sdCard, sd::SdState targetS
       return sdcMan->mountSdCard(sdCard);
     } else if (targetState == sd::SdState::OFF) {
       sif::info << "Switching off SD card " << sdChar << std::endl;
-      return sdcMan->switchOffSdCard(sdCard, false, &sdInfo.currentState);
+      return sdcMan->switchOffSdCard(sdCard, sdInfo.currentState, false);
     }
   } else {
     sif::warning << "CoreController::sdCardSetup: Invalid state for this call" << std::endl;
@@ -898,8 +892,7 @@ ReturnValue_t CoreController::sdColdRedundantBlockingInit() {
     sif::info << "Switching off secondary SD card " << sdInfo.otherChar << std::endl;
     // Switch off other SD card in cold redundant mode if setting up preferred one worked
     // without issues
-    ReturnValue_t result2 =
-        sdcMan->switchOffSdCard(sdInfo.other, sdInfo.otherState, &sdInfo.currentState);
+    ReturnValue_t result2 = sdcMan->switchOffSdCard(sdInfo.other, sdInfo.currentState, true);
     if (result2 != returnvalue::OK and result2 != SdCardManager::ALREADY_OFF) {
       sif::warning << "Switching off secondary SD card " << sdInfo.otherChar
                    << " in cold redundant mode failed" << std::endl;
@@ -1230,7 +1223,7 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co
   sync();
 
   // Unmount and switch off SD cards. This could possibly fix issues with the SD card and is
-  // the more graceful way to reboot the system.
+  // the more graceful way to reboot the system. This function takes around 400 ms.
   ReturnValue_t result = handleSwitchingSdCardsOffNonBlocking();
   if (result != returnvalue::OK) {
     sif::error
@@ -1247,6 +1240,8 @@ ReturnValue_t CoreController::gracefulShutdownTasks(xsc::Chip chip, xsc::Copy co
     // and a reboot is imminent. Use scratch buffer?
     sif::info << "Running slot was writeprotected before reboot" << std::endl;
   }
+  // Ensure that all diagnostic prinouts arrive.
+  TaskFactory::delayTask(50);
   return result;
 }
 
@@ -2590,9 +2585,6 @@ void CoreController::announceSdInfo(SdCardManager::SdStatePair sdStates) {
 }
 
 ReturnValue_t CoreController::handleSwitchingSdCardsOffNonBlocking() {
-  Countdown maxWaitTimeCd(10000);
-  Stopwatch watch;
-
   sdcMan->setBlocking(false);
   SdCardManager::Operations op;
   std::pair<sd::SdState, sd::SdState> sdStatus;
@@ -2600,42 +2592,46 @@ ReturnValue_t CoreController::handleSwitchingSdCardsOffNonBlocking() {
   if (result != returnvalue::OK) {
     return result;
   }
+  Countdown maxWaitTimeCd(10000);
+  // Stopwatch watch;
   auto waitingForFinish = [&]() {
-    while (sdcMan->checkCurrentOp(op) == SdCardManager::OpStatus::ONGOING) {
-      TaskFactory::delayTask(50);
+    auto currentState = sdcMan->checkCurrentOp(op);
+    if (currentState == SdCardManager::OpStatus::IDLE) {
+      return returnvalue::OK;
+    }
+    while (currentState == SdCardManager::OpStatus::ONGOING) {
       if (maxWaitTimeCd.hasTimedOut()) {
         return returnvalue::FAILED;
       }
+      TaskFactory::delayTask(50);
+      currentState = sdcMan->checkCurrentOp(op);
     }
     return returnvalue::OK;
   };
-  sif::debug << "hello 0" << std::endl;
   if (sdStatus.first != sd::SdState::OFF) {
     sdcMan->unmountSdCard(sd::SdCard::SLOT_0);
     result = waitingForFinish();
     if (result != returnvalue::OK) {
       return result;
     }
-    sdcMan->switchOffSdCard(sd::SdCard::SLOT_0, false);
+    sdcMan->switchOffSdCard(sd::SdCard::SLOT_0, sdStatus, false);
     result = waitingForFinish();
     if (result != returnvalue::OK) {
       return result;
     }
   }
-  sif::debug << "hello 1" << std::endl;
   if (sdStatus.second != sd::SdState::OFF) {
     sdcMan->unmountSdCard(sd::SdCard::SLOT_1);
     result = waitingForFinish();
     if (result != returnvalue::OK) {
       return result;
     }
-    sdcMan->switchOffSdCard(sd::SdCard::SLOT_1, false);
+    sdcMan->switchOffSdCard(sd::SdCard::SLOT_1, sdStatus, false);
     result = waitingForFinish();
     if (result != returnvalue::OK) {
       return result;
     }
   }
-  sif::debug << "hello 2" << std::endl;
   return result;
 }
 

bsp_q7s/fs/SdCardManager.cpp

@@ -125,13 +125,8 @@ ReturnValue_t SdCardManager::switchOnSdCard(sd::SdCard sdCard, bool doMountSdCar
   return mountSdCard(sdCard);
 }
 
-ReturnValue_t SdCardManager::switchOffSdCard(sd::SdCard sdCard, bool doUnmountSdCard,
-                                             SdStatePair* statusPair) {
-  std::pair<sd::SdState, sd::SdState> active;
-  ReturnValue_t result = getSdCardsStatus(active);
-  if (result != returnvalue::OK) {
-    return result;
-  }
+ReturnValue_t SdCardManager::switchOffSdCard(sd::SdCard sdCard, SdStatePair& sdStates,
+                                             bool doUnmountSdCard) {
   if (doUnmountSdCard) {
     if (not blocking) {
       sif::warning << "SdCardManager::switchOffSdCard: Two-step command but manager is"
@@ -147,17 +142,17 @@ ReturnValue_t SdCardManager::switchOffSdCard(sd::SdCard sdCard, bool doUnmountSd
     return returnvalue::FAILED;
   }
   if (sdCard == sd::SdCard::SLOT_0) {
-    if (active.first == sd::SdState::OFF) {
+    if (sdStates.first == sd::SdState::OFF) {
       return ALREADY_OFF;
     }
   } else if (sdCard == sd::SdCard::SLOT_1) {
-    if (active.second == sd::SdState::OFF) {
+    if (sdStates.second == sd::SdState::OFF) {
       return ALREADY_OFF;
     }
   }
 
   if (doUnmountSdCard) {
-    result = unmountSdCard(sdCard);
+    ReturnValue_t result = unmountSdCard(sdCard);
     if (result != returnvalue::OK) {
       return result;
     }
@@ -189,7 +184,7 @@ ReturnValue_t SdCardManager::setSdCardState(sd::SdCard sdCard, bool on) {
   command << "q7hw sd set " << sdstring << " " << statestring;
   cmdExecutor.load(command.str(), blocking, printCmdOutput);
   ReturnValue_t result = cmdExecutor.execute();
-  if (blocking and result != returnvalue::OK) {
+  if (result != returnvalue::OK) {
     utility::handleSystemError(cmdExecutor.getLastError(), "SdCardManager::setSdCardState");
   }
   return result;

bsp_q7s/fs/SdCardManager.h

@@ -114,8 +114,7 @@ class SdCardManager : public SystemObject, public SdCardMountedIF {
    * @return - returnvalue::OK on success, ALREADY_ON if it is already on,
    *           SYSTEM_CALL_ERROR on system error
    */
-  ReturnValue_t switchOffSdCard(sd::SdCard sdCard, bool doUnmountSdCard = true,
-                                SdStatePair* statusPair = nullptr);
+  ReturnValue_t switchOffSdCard(sd::SdCard sdCard, SdStatePair& sdStates, bool doUnmountSdCard);
 
   /**
    * Get the state of the SD cards. If the state file does not exist, this function will