Merge pull request 'lets hope this fixes the occasional error' (#444) from optimize_lock_handling into develop

Reviewed-on: #444

bsp_q7s/fs/SdCardManager.cpp

@@ -21,15 +21,15 @@ SdCardManager* SdCardManager::INSTANCE = nullptr;
 
 SdCardManager::SdCardManager() : SystemObject(objects::SDC_MANAGER), cmdExecutor(256) {
   sdLock = MutexFactory::instance()->createMutex();
-  ReturnValue_t result = sdLock->lockMutex();
+  prefLock = MutexFactory::instance()->createMutex();
-  if (result != returnvalue::OK) {
+  defaultLock = MutexFactory::instance()->createMutex();
+
+  MutexGuard mg(prefLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
+  if (mg.getLockResult() != returnvalue::OK) {
     sif::error << "SdCardManager::SdCardManager: Mutex lock failed" << std::endl;
   }
   uint8_t prefSdRaw = 0;
-  result = scratch::readNumber(scratch::PREFERED_SDC_KEY, prefSdRaw);
+  ReturnValue_t result = scratch::readNumber(scratch::PREFERED_SDC_KEY, prefSdRaw);
-  if (sdLock->unlockMutex() != returnvalue::OK) {
-    sif::error << "SdCardManager::SdCardManager: Mutex unlock failed" << std::endl;
-  }
 
   if (result != returnvalue::OK) {
     if (result == scratch::KEY_NOT_FOUND) {
@@ -37,14 +37,16 @@ SdCardManager::SdCardManager() : SystemObject(objects::SDC_MANAGER), cmdExecutor
                       "Preferred SD card not set. Setting to 0"
                    << std::endl;
       setPreferredSdCard(sd::SdCard::SLOT_0);
-      sdInfo.pref = sd::SdCard::SLOT_0;
+      scratch::writeNumber(scratch::PREFERED_SDC_KEY, static_cast<uint8_t>(sd::SdCard::SLOT_0));
+      prefSdRaw = sd::SdCard::SLOT_0;
+
     } else {
       // Should not happen.
       // TODO: Maybe trigger event?
       sif::error << "SdCardManager::SdCardManager: Reading preferred SD card from scratch"
                     "buffer failed"
                  << std::endl;
-      sdInfo.pref = sd::SdCard::SLOT_0;
+      prefSdRaw = sd::SdCard::SLOT_0;
     }
   }
   sdInfo.pref = static_cast<sd::SdCard>(prefSdRaw);
@@ -195,7 +197,7 @@ ReturnValue_t SdCardManager::setSdCardState(sd::SdCard sdCard, bool on) {
 
 ReturnValue_t SdCardManager::getSdCardsStatus(SdStatePair& active) {
   using namespace std;
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(sdLock, LOCK_TYPE, SD_LOCK_TIMEOUT, LOCK_CTX);
   if (not filesystem::exists(SD_STATE_FILE)) {
     return STATUS_FILE_NEXISTS;
   }
@@ -378,7 +380,7 @@ void SdCardManager::processSdStatusLine(std::pair<sd::SdState, sd::SdState>& act
 }
 
 std::optional<sd::SdCard> SdCardManager::getPreferredSdCard() const {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(prefLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   auto res = mg.getLockResult();
   if (res != returnvalue::OK) {
     sif::error << "SdCardManager::getPreferredSdCard: Lock error" << std::endl;
@@ -387,7 +389,7 @@ std::optional<sd::SdCard> SdCardManager::getPreferredSdCard() const {
 }
 
 ReturnValue_t SdCardManager::setPreferredSdCard(sd::SdCard sdCard) {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(prefLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   if (sdCard == sd::SdCard::BOTH) {
     return returnvalue::FAILED;
   }
@@ -399,7 +401,7 @@ ReturnValue_t SdCardManager::updateSdCardStateFile() {
   if (cmdExecutor.getCurrentState() == CommandExecutor::States::PENDING) {
     return CommandExecutor::COMMAND_PENDING;
   }
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  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);
@@ -411,7 +413,7 @@ ReturnValue_t SdCardManager::updateSdCardStateFile() {
 }
 
 const char* SdCardManager::getCurrentMountPrefix() const {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(defaultLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   if (currentPrefix.has_value()) {
     return currentPrefix.value().c_str();
   }
@@ -464,7 +466,7 @@ void SdCardManager::setPrintCommandOutput(bool print) { this->printCmdOutput = p
 
 bool SdCardManager::isSdCardUsable(std::optional<sd::SdCard> sdCard) {
   {
-    MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+    MutexGuard mg(defaultLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
     if (markedUnusable) {
       return false;
     }
@@ -560,7 +562,7 @@ ReturnValue_t SdCardManager::performFsck(sd::SdCard sdcard, bool printOutput, in
 }
 
 void SdCardManager::setActiveSdCard(sd::SdCard sdCard) {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(defaultLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   sdInfo.active = sdCard;
   if (sdInfo.active == sd::SdCard::SLOT_0) {
     currentPrefix = config::SD_0_MOUNT_POINT;
@@ -570,7 +572,7 @@ void SdCardManager::setActiveSdCard(sd::SdCard sdCard) {
 }
 
 std::optional<sd::SdCard> SdCardManager::getActiveSdCard() const {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(defaultLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   if (markedUnusable) {
     return std::nullopt;
   }
@@ -578,6 +580,6 @@ std::optional<sd::SdCard> SdCardManager::getActiveSdCard() const {
 }
 
 void SdCardManager::markUnusable() {
-  MutexGuard mg(sdLock, LOCK_TYPE, LOCK_TIMEOUT, LOCK_CTX);
+  MutexGuard mg(defaultLock, LOCK_TYPE, OTHER_TIMEOUT, LOCK_CTX);
   markedUnusable = true;
 }

bsp_q7s/fs/SdCardManager.h

@@ -224,8 +224,11 @@ class SdCardManager : public SystemObject, public SdCardMountedIF {
   bool printCmdOutput = true;
   bool markedUnusable = false;
   MutexIF* sdLock = nullptr;
+  MutexIF* prefLock = nullptr;
+  MutexIF* defaultLock = nullptr;
   static constexpr MutexIF::TimeoutType LOCK_TYPE = MutexIF::TimeoutType::WAITING;
-  static constexpr uint32_t LOCK_TIMEOUT = 200;
+  static constexpr uint32_t SD_LOCK_TIMEOUT = 250;
+  static constexpr uint32_t OTHER_TIMEOUT = 20;
   static constexpr char LOCK_CTX[] = "SdCardManager";
 
   SdCardManager();