this is a lot better

bsp_q7s/boardtest/Q7STestTask.cpp

@@ -218,15 +218,25 @@ void Q7STestTask::testProtHandler() {
   bool opPerformed = false;
   ReturnValue_t result = returnvalue::OK;
   // If any chips are unlocked, lock them here
-  result = coreController->setBootCopyProtection(xsc::Chip::ALL_CHIP, xsc::Copy::ALL_COPY, true,
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_0, xsc::Copy::COPY_0, false);
-                                                 opPerformed, true);
+  if (result != returnvalue::OK) {
+    sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
+  }
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_0, xsc::Copy::COPY_1, false);
+  if (result != returnvalue::OK) {
+    sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
+  }
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_0, false);
+  if (result != returnvalue::OK) {
+    sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
+  }
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false);
   if (result != returnvalue::OK) {
     sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
   }
 
   // unlock own copy
-  result = coreController->setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, false,
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, false);
-                                                 opPerformed, true);
   if (result != returnvalue::OK) {
     sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
   }
@@ -239,8 +249,7 @@ void Q7STestTask::testProtHandler() {
   }
 
   // lock own copy
-  result = coreController->setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true,
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true);
-                                                 opPerformed, true);
   if (result != returnvalue::OK) {
     sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
   }
@@ -253,8 +262,7 @@ void Q7STestTask::testProtHandler() {
   }
 
   // unlock specific copy
-  result = coreController->setBootCopyProtection(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false,
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, false);
-                                                 opPerformed, true);
   if (result != returnvalue::OK) {
     sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
   }
@@ -267,8 +275,7 @@ void Q7STestTask::testProtHandler() {
   }
 
   // lock specific copy
-  result = coreController->setBootCopyProtection(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, true,
+  result = coreController->setBootCopyProtectionAndUpdateFile(xsc::Chip::CHIP_1, xsc::Copy::COPY_1, true);
-                                                 opPerformed, true);
   if (result != returnvalue::OK) {
     sif::warning << "Q7STestTask::testProtHandler: Op failed" << std::endl;
   }

bsp_q7s/core/CoreController.cpp

@@ -1232,16 +1232,15 @@ 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.
+
-  // Actually this function only ensures that reboots to the own image are protected..
+  // Ensure tha the target chip is writeprotected in any case.
-  ReturnValue_t result = setBootCopyProtection(xsc::Chip::SELF_CHIP, xsc::Copy::SELF_COPY, true,
+  bool wasProtected = handleBootCopyProt(chip, copy, true);
-                                               protOpPerformed, false);
+  if (wasProtected) {
-  if (result == returnvalue::OK and protOpPerformed) {
     // TODO: Would be nice to notify operator. But we can't use the filesystem anymore
     // and a reboot is imminent. Use scratch buffer?
     sif::info << "Running slot was writeprotected before reboot" << std::endl;
   }
-  return result;
+  return returnvalue::OK;
 }
 
 void CoreController::updateInternalSdInfo() {
@@ -1274,144 +1273,50 @@ ReturnValue_t CoreController::generateChipStateFile() {
   return returnvalue::OK;
 }
 
-ReturnValue_t CoreController::setBootCopyProtection(xsc::Chip targetChip, xsc::Copy targetCopy,
+ReturnValue_t CoreController::setBootCopyProtectionAndUpdateFile(xsc::Chip targetChip, xsc::Copy targetCopy,
-                                                    bool protect, bool &protOperationPerformed,
+                                                    bool protect) {
-                                                    bool updateProtFile) {
+  if(targetChip == xsc::Chip::ALL_CHIP or targetCopy == xsc::Copy::ALL_COPY) {
-  bool allChips = false;
+    return returnvalue::FAILED;
-  bool allCopies = false;
-  bool selfChip = false;
-  bool selfCopy = false;
-  protOperationPerformed = false;
-
-  switch (targetChip) {
-    case (xsc::Chip::ALL_CHIP): {
-      allChips = true;
-      break;
-    }
-    case (xsc::Chip::NO_CHIP): {
-      return returnvalue::OK;
-    }
-    case (xsc::Chip::SELF_CHIP): {
-      selfChip = true;
-      targetChip = CURRENT_CHIP;
-      break;
-    }
-    default: {
-      break;
-    }
-  }
-  switch (targetCopy) {
-    case (xsc::Copy::ALL_COPY): {
-      allCopies = true;
-      break;
-    }
-    case (xsc::Copy::NO_COPY): {
-      return returnvalue::OK;
-    }
-    case (xsc::Copy::SELF_COPY): {
-      selfCopy = true;
-      targetCopy = CURRENT_COPY;
-      break;
-    }
-    default: {
-      break;
-    }
   }
 
-  for (uint8_t arrIdx = 0; arrIdx < protArray.size(); arrIdx++) {
+  bool protOperationPerformed = handleBootCopyProt(targetChip, targetCopy, protect);
-    int result = handleBootCopyProtAtIndex(targetChip, targetCopy, protect, protOperationPerformed,
+  if (protOperationPerformed) {
-                                           selfChip, selfCopy, allChips, allCopies, arrIdx);
-    if (result != 0) {
-      break;
-    }
-  }
-  if (protOperationPerformed and updateProtFile) {
     updateProtInfo();
   }
   return returnvalue::OK;
 }
 
-int CoreController::handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy,
+bool CoreController::handleBootCopyProt(xsc::Chip targetChip, xsc::Copy targetCopy,
-                                              bool protect, bool &protOperationPerformed,
+                                              bool protect) {
-                                              bool selfChip, bool selfCopy, bool allChips,
-                                              bool allCopies, uint8_t arrIdx) {
-  bool currentProt = protArray[arrIdx];
   std::ostringstream oss;
-  bool performOp = false;
-  if (protect == currentProt) {
-    return 0;
-  }
-  if (protOperationPerformed) {
-    if ((selfChip and selfCopy) or (not allCopies and not allChips)) {
-      // No need to continue, only one operation was requested
-      return 1;
-    }
-  }
-  xsc::Chip currentChip;
-  xsc::Copy currentCopy;
   oss << "writeprotect ";
-  if (arrIdx == 0 or arrIdx == 1) {
+  if(targetChip == xsc::Chip::SELF_CHIP) {
-    oss << "0 ";
+    targetChip = CURRENT_CHIP;
-    currentChip = xsc::Chip::CHIP_0;
-  } else {
-    oss << "1 ";
-    currentChip = xsc::Chip::CHIP_1;
   }
-  if (arrIdx == 0 or arrIdx == 2) {
+  if(targetCopy == xsc::Copy::SELF_COPY) {
+    targetCopy = CURRENT_COPY;
+  }
+  if (targetChip == xsc::Chip::CHIP_0) {
     oss << "0 ";
-    currentCopy = xsc::Copy::COPY_0;
+  } else if(targetChip == xsc::Chip::SELF_CHIP) {
-  } else {
+    oss << "1 ";
+  }
+  if(targetCopy == xsc::Copy::COPY_0) {
+    oss << "0 ";
+  } else if(targetCopy == xsc::Copy::COPY_1) {
     oss << "1 ";
-    currentCopy = xsc::Copy::COPY_1;
   }
   if (protect) {
     oss << "1";
   } else {
     oss << "0";
   }
-
+  sif::info << "Executing command: " << oss.str() << std::endl;
-  int result = 0;
+  int result = std::system(oss.str().c_str());
-  if (allChips and allCopies) {
+  if(result == 0) {
-    performOp = true;
+    return true;
-  } else if (allChips) {
-    if ((selfCopy and CURRENT_COPY == targetCopy) or (currentCopy == targetCopy)) {
-      performOp = true;
-    }
-  } else if (allCopies) {
-    if ((selfChip and CURRENT_COPY == targetCopy) or (currentChip == targetChip)) {
-      performOp = true;
-    }
-  } else if (selfChip and (currentChip == targetChip)) {
-    if (selfCopy) {
-      if (currentCopy == targetCopy) {
-        performOp = true;
-      }
-    } else {
-      performOp = true;
-    }
-
-  } else if (selfCopy and (currentCopy == targetCopy)) {
-    if (selfChip) {
-      if (currentChip == targetChip) {
-        performOp = true;
-      }
-    } else {
-      performOp = true;
-    }
-  } else if ((targetChip == currentChip) and (targetCopy == currentCopy)) {
-    performOp = true;
   }
-  if (result != 0) {
+  return false;
-    utility::handleSystemError(result, "CoreController::checkAndSetBootCopyProtection");
-  }
-  if (performOp) {
-    // TODO: Lock operation take a long time. Use command executor? That would require a
-    // new state machine..
-    protOperationPerformed = true;
-    sif::info << "Executing command: " << oss.str() << std::endl;
-    result = std::system(oss.str().c_str());
-  }
-  return 0;
 }
 
 ReturnValue_t CoreController::updateProtInfo(bool regenerateChipStateFile) {
@@ -1456,7 +1361,6 @@ ReturnValue_t CoreController::handleProtInfoUpdateLine(std::string nextLine) {
   using namespace std;
   string word;
   uint8_t wordIdx = 0;
-  uint8_t arrayIdx = 0;
   istringstream iss(nextLine);
   xsc::Chip currentChip = xsc::Chip::CHIP_0;
   xsc::Copy currentCopy = xsc::Copy::COPY_0;
@@ -1468,28 +1372,11 @@ ReturnValue_t CoreController::handleProtInfoUpdateLine(std::string nextLine) {
       currentCopy = static_cast<xsc::Copy>(stoi(word));
     }
 
-    if (wordIdx == 3) {
-      if (currentChip == xsc::Chip::CHIP_0) {
-        if (currentCopy == xsc::Copy::COPY_0) {
-          arrayIdx = 0;
-        } else if (currentCopy == xsc::Copy::COPY_1) {
-          arrayIdx = 1;
-        }
-      }
-
-      else if (currentChip == xsc::Chip::CHIP_1) {
-        if (currentCopy == xsc::Copy::COPY_0) {
-          arrayIdx = 2;
-        } else if (currentCopy == xsc::Copy::COPY_1) {
-          arrayIdx = 3;
-        }
-      }
-    }
     if (wordIdx == 5) {
       if (word == "unlocked.") {
-        protArray[arrayIdx] = false;
+        protArray[currentChip][currentCopy] = false;
       } else {
-        protArray[arrayIdx] = true;
+        protArray[currentChip][currentCopy] = true;
       }
     }
     wordIdx++;

bsp_q7s/core/CoreController.h

@@ -199,8 +199,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe
    * @param updateProtFile Specify whether the protection info file is updated
    * @return
    */
-  ReturnValue_t setBootCopyProtection(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect,
+  ReturnValue_t setBootCopyProtectionAndUpdateFile(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect);
-                                      bool& protOperationPerformed, bool updateProtFile = true);
 
   bool sdInitFinished() const;
 
@@ -309,7 +308,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe
    * Index 2: Chip 1 Copy 0
    * Index 3: Chip 1 Copy 1
    */
-  std::array<bool, 4> protArray;
+  bool protArray[2][2] {};
   PeriodicOperationDivider opDivider5;
   PeriodicOperationDivider opDivider10;
 
@@ -374,9 +373,7 @@ class CoreController : public ExtendedControllerBase, public ReceivesParameterMe
   ReturnValue_t gracefulShutdownTasks(xsc::Chip chip, xsc::Copy copy, bool& protOpPerformed);
 
   ReturnValue_t handleProtInfoUpdateLine(std::string nextLine);
-  int handleBootCopyProtAtIndex(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect,
+  bool handleBootCopyProt(xsc::Chip targetChip, xsc::Copy targetCopy, bool protect);
-                                bool& protOperationPerformed, bool selfChip, bool selfCopy,
-                                bool allChips, bool allCopies, uint8_t arrIdx);
   void rebootWatchdogAlgorithm(RebootWatchdogFile& rf, bool& needsReboot, xsc::Chip& tgtChip,
                                xsc::Copy& tgtCopy);
   void resetRebootWatchdogCounters(xsc::Chip tgtChip, xsc::Copy tgtCopy);

tmtc

@@ -1 +1 @@
-Subproject commit a82cbff5a83eb37c68234bdeecd3b7d308d65eb1
+Subproject commit 95b69541751c9ea7146e0a342122da83bdb50f29