refactored SPI mutex handling

hal/src/fsfw_hal/linux/spi/ManualCsLockGuard.h

@@ -0,0 +1,42 @@
+#pragma once
+
+#include "fsfw/ipc/MutexIF.h"
+#include "fsfw/returnvalues/HasReturnvaluesIF.h"
+#include "fsfw_hal/common/gpio/GpioIF.h"
+
+class ManualCsLockWrapper : public HasReturnvaluesIF {
+ public:
+  ManualCsLockWrapper(MutexIF* lock, GpioIF* gpioIF, SpiCookie* cookie,
+                      MutexIF::TimeoutType type = MutexIF::TimeoutType::BLOCKING,
+                      uint32_t timeoutMs = 0)
+      : lock(lock), gpioIF(gpioIF), cookie(cookie), type(type), timeoutMs(timeoutMs) {
+    if (cookie == nullptr) {
+      // TODO: Error? Or maybe throw exception..
+      return;
+    }
+    cookie->setCsLockManual(true);
+    lockResult = lock->lockMutex(type, timeoutMs);
+    if (lockResult != RETURN_OK) {
+      return;
+    }
+    gpioResult = gpioIF->pullLow(cookie->getChipSelectPin());
+  }
+
+  ~ManualCsLockWrapper() {
+    if (lockResult == RETURN_OK) {
+      lock->unlockMutex();
+    }
+    if (gpioResult == RETURN_OK) {
+      gpioIF->pullHigh(cookie->getChipSelectPin());
+    }
+  }
+  ReturnValue_t lockResult;
+  ReturnValue_t gpioResult;
+
+ private:
+  MutexIF* lock;
+  GpioIF* gpioIF;
+  SpiCookie* cookie;
+  MutexIF::TimeoutType type;
+  uint32_t timeoutMs = 0;
+};

hal/src/fsfw_hal/linux/spi/SpiComIF.cpp

@@ -194,16 +194,22 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const
 
   bool fullDuplex = spiCookie->isFullDuplex();
   gpioId_t gpioId = spiCookie->getChipSelectPin();
+  bool csLockManual = spiCookie->getCsLockManual();
 
-  /* Pull SPI CS low. For now, no support for active high given  */
+  MutexIF::TimeoutType csType;
-  if (gpioId != gpio::NO_GPIO) {
+  dur_millis_t csTimeout = 0;
-    result = csMutex->lockMutex(timeoutType, timeoutMs);
+  // Pull SPI CS low. For now, no support for active high given
+  if (gpioId != gpio::NO_GPIO and not csLockManual) {
+    spiCookie->getMutexParams(csType, csTimeout);
+    result = csMutex->lockMutex(csType, csTimeout);
     if (result != RETURN_OK) {
 #if FSFW_VERBOSE_LEVEL >= 1
 #if FSFW_CPP_OSTREAM_ENABLED == 1
-      sif::error << "SpiComIF::sendMessage: Failed to lock mutex" << std::endl;
+      sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code "
+                 << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec
+                 << std::endl;
 #else
-      sif::printError("SpiComIF::sendMessage: Failed to lock mutex\n");
+      sif::printError("SpiComIF::sendMessage: Failed to lock mutex with code %d\n", result);
 #endif
 #endif
       return result;
@@ -249,7 +255,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie* spiCookie, const
     }
   }
 
-  if (gpioId != gpio::NO_GPIO) {
+  if (gpioId != gpio::NO_GPIO and not csLockManual) {
     gpioComIF->pullHigh(gpioId);
     result = csMutex->unlockMutex();
     if (result != RETURN_OK) {
@@ -292,12 +298,22 @@ ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) {
     return result;
   }
 
+  bool csLockManual = spiCookie->getCsLockManual();
   gpioId_t gpioId = spiCookie->getChipSelectPin();
-  if (gpioId != gpio::NO_GPIO) {
+  MutexIF::TimeoutType csType;
-    result = csMutex->lockMutex(timeoutType, timeoutMs);
+  dur_millis_t csTimeout = 0;
+  if (gpioId != gpio::NO_GPIO and not csLockManual) {
+    spiCookie->getMutexParams(csType, csTimeout);
+    result = csMutex->lockMutex(csType, csTimeout);
     if (result != RETURN_OK) {
+#if FSFW_VERBOSE_LEVEL >= 1
 #if FSFW_CPP_OSTREAM_ENABLED == 1
-      sif::error << "SpiComIF::getSendSuccess: Failed to lock mutex" << std::endl;
+      sif::error << "SpiComIF::sendMessage: Failed to lock mutex with code "
+                 << "0x" << std::hex << std::setfill('0') << std::setw(4) << result << std::dec
+                 << std::endl;
+#else
+      sif::printError("SpiComIF::sendMessage: Failed to lock mutex with code %d\n", result);
+#endif
 #endif
       return result;
     }
@@ -315,7 +331,7 @@ ReturnValue_t SpiComIF::performHalfDuplexReception(SpiCookie* spiCookie) {
     result = HALF_DUPLEX_TRANSFER_FAILED;
   }
 
-  if (gpioId != gpio::NO_GPIO) {
+  if (gpioId != gpio::NO_GPIO and not csLockManual) {
     gpioComIF->pullHigh(gpioId);
     result = csMutex->unlockMutex();
     if (result != RETURN_OK) {
@@ -346,15 +362,7 @@ ReturnValue_t SpiComIF::readReceivedMessage(CookieIF* cookie, uint8_t** buffer,
   return HasReturnvaluesIF::RETURN_OK;
 }
 
-MutexIF* SpiComIF::getMutex(MutexIF::TimeoutType* timeoutType, uint32_t* timeoutMs) {
+MutexIF* SpiComIF::getCsMutex() { return csMutex; }
-  if (timeoutType != nullptr) {
-    *timeoutType = this->timeoutType;
-  }
-  if (timeoutMs != nullptr) {
-    *timeoutMs = this->timeoutMs;
-  }
-  return csMutex;
-}
 
 void SpiComIF::performSpiWiretapping(SpiCookie* spiCookie) {
   if (spiCookie == nullptr) {
@@ -426,8 +434,3 @@ void SpiComIF::updateLinePolarity(int spiFd) {
     utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Updating SPI default clock failed");
   }
 }
-
-void SpiComIF::setMutexParams(MutexIF::TimeoutType timeoutType_, uint32_t timeoutMs_) {
-  timeoutType = timeoutType_;
-  timeoutMs = timeoutMs_;
-}

hal/src/fsfw_hal/linux/spi/SpiComIF.h

@@ -22,8 +22,6 @@ class SpiCookie;
  */
 class SpiComIF : public DeviceCommunicationIF, public SystemObject {
  public:
-  static constexpr dur_millis_t DEFAULT_MUTEX_TIMEOUT = 20;
-
   static constexpr uint8_t CLASS_ID = CLASS_ID::HAL_SPI;
   static constexpr ReturnValue_t OPENING_FILE_FAILED =
       HasReturnvaluesIF::makeReturnCode(CLASS_ID, 0);
@@ -46,7 +44,7 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject {
    * @brief   This function returns the mutex which can be used to protect the spi bus when
    *          the chip select must be driven from outside of the com if.
    */
-  MutexIF* getMutex(MutexIF::TimeoutType* timeoutType = nullptr, uint32_t* timeoutMs = nullptr);
+  MutexIF* getCsMutex();
   void setMutexParams(MutexIF::TimeoutType timeoutType, uint32_t timeoutMs);
 
   /**
@@ -93,8 +91,8 @@ class SpiComIF : public DeviceCommunicationIF, public SystemObject {
    * pulled high
    */
   MutexIF* csMutex = nullptr;
-  MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING;
+  // MutexIF::TimeoutType timeoutType = MutexIF::TimeoutType::WAITING;
-  uint32_t timeoutMs = DEFAULT_MUTEX_TIMEOUT;
+  // uint32_t timeoutMs = DEFAULT_MUTEX_TIMEOUT;
   spi_ioc_transfer clockUpdateTransfer = {};
 
   using SpiDeviceMap = std::unordered_map<address_t, SpiInstance>;

hal/src/fsfw_hal/linux/spi/SpiCookie.cpp

@@ -104,3 +104,17 @@ void SpiCookie::getCallback(spi::send_callback_function_t* callback, void** args
   *callback = this->sendCallback;
   *args = this->callbackArgs;
 }
+
+void SpiCookie::setCsLockManual(bool enable) { manualCsLock = enable; }
+
+bool SpiCookie::getCsLockManual() const { return manualCsLock; }
+
+void SpiCookie::getMutexParams(MutexIF::TimeoutType& csTimeoutType, dur_millis_t& csTimeout) const {
+  csTimeoutType = this->csTimeoutType;
+  csTimeout = this->csTimeout;
+}
+
+void SpiCookie::setMutexParams(MutexIF::TimeoutType csTimeoutType, dur_millis_t csTimeout) {
+  this->csTimeoutType = csTimeoutType;
+  this->csTimeout = csTimeout;
+}

hal/src/fsfw_hal/linux/spi/SpiCookie.h

@@ -2,6 +2,8 @@
 #define LINUX_SPI_SPICOOKIE_H_
 
 #include <fsfw/devicehandlers/CookieIF.h>
+#include <fsfw/ipc/MutexIF.h>
+#include <fsfw/timemanager/clockDefinitions.h>
 #include <linux/spi/spidev.h>
 
 #include "../../common/gpio/gpioDefinitions.h"
@@ -20,6 +22,8 @@
  */
 class SpiCookie : public CookieIF {
  public:
+  static constexpr dur_millis_t DEFAULT_MUTEX_TIMEOUT = 20;
+
   /**
    * Each SPI device will have a corresponding cookie. The cookie is used by the communication
    * interface and contains device specific information like the largest expected size to be
@@ -137,9 +141,42 @@ class SpiCookie : public CookieIF {
    */
   void activateCsDeselect(bool deselectCs, uint16_t delayUsecs);
 
+  void getMutexParams(MutexIF::TimeoutType& csTimeoutType, dur_millis_t& csTimeout) const;
+  void setMutexParams(MutexIF::TimeoutType csTimeoutType, dur_millis_t csTimeout);
+
+  void setCsLockManual(bool enable);
+  bool getCsLockManual() const;
+
   spi_ioc_transfer* getTransferStructHandle();
 
  private:
+  address_t spiAddress;
+  gpioId_t chipSelectPin;
+
+  spi::SpiComIfModes comIfMode;
+
+  // Required for regular mode
+  const size_t maxSize;
+  spi::SpiModes spiMode;
+  /**
+   * If this is set to true, the SPI ComIF will not perform any mutex locking for the
+   * CS mechanism. The user is responsible to locking and unlocking the mutex for the
+   * whole duration of the transfers.
+   */
+  bool manualCsLock = false;
+  uint32_t spiSpeed;
+  bool halfDuplex = false;
+
+  MutexIF::TimeoutType csTimeoutType = MutexIF::TimeoutType::WAITING;
+  dur_millis_t csTimeout = DEFAULT_MUTEX_TIMEOUT;
+
+  // Required for callback mode
+  spi::send_callback_function_t sendCallback = nullptr;
+  void* callbackArgs = nullptr;
+
+  struct spi_ioc_transfer spiTransferStruct = {};
+  UncommonParameters uncommonParameters;
+
   /**
    * Internal constructor which initializes every field
    * @param spiAddress
@@ -154,24 +191,6 @@ class SpiCookie : public CookieIF {
   SpiCookie(spi::SpiComIfModes comIfMode, address_t spiAddress, gpioId_t chipSelect,
             const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed,
             spi::send_callback_function_t callback, void* args);
-
-  address_t spiAddress;
-  gpioId_t chipSelectPin;
-
-  spi::SpiComIfModes comIfMode;
-
-  // Required for regular mode
-  const size_t maxSize;
-  spi::SpiModes spiMode;
-  uint32_t spiSpeed;
-  bool halfDuplex = false;
-
-  // Required for callback mode
-  spi::send_callback_function_t sendCallback = nullptr;
-  void* callbackArgs = nullptr;
-
-  struct spi_ioc_transfer spiTransferStruct = {};
-  UncommonParameters uncommonParameters;
 };
 
 #endif /* LINUX_SPI_SPICOOKIE_H_ */