From 3e8de10866ca3976e14a2937679e27d0338d67ee Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 11 Jun 2021 10:37:48 +0200 Subject: [PATCH] refactored some components --- stm32h7/spi/SpiComIF.cpp | 146 +++++++++++++++++++++------------- stm32h7/spi/SpiComIF.h | 20 ++--- stm32h7/spi/SpiCookie.cpp | 8 ++ stm32h7/spi/SpiCookie.h | 4 + stm32h7/spi/spiDefinitions.h | 7 ++ stm32h7/spi/spiInterrupts.cpp | 3 +- 6 files changed, 120 insertions(+), 68 deletions(-) diff --git a/stm32h7/spi/SpiComIF.cpp b/stm32h7/spi/SpiComIF.cpp index 5f21115..732fb5e 100644 --- a/stm32h7/spi/SpiComIF.cpp +++ b/stm32h7/spi/SpiComIF.cpp @@ -11,10 +11,11 @@ #include "stm32h7xx_hal_gpio.h" SpiComIF::SpiComIF(object_id_t objectId): SystemObject(objectId) { - spi::assignTransferRxTxCompleteCallback(&spiTransferCompleteCallback, this); - spi::assignTransferRxCompleteCallback(&spiTransferRxCompleteCallback, this); - spi::assignTransferTxCompleteCallback(&spiTransferTxCompleteCallback, this); - spi::assignTransferErrorCallback(&spiTransferErrorCallback, this); + void* irqArgsVoided = reinterpret_cast(&irqArgs); + spi::assignTransferRxTxCompleteCallback(&spiTransferCompleteCallback, irqArgsVoided); + spi::assignTransferRxCompleteCallback(&spiTransferRxCompleteCallback, irqArgsVoided); + spi::assignTransferTxCompleteCallback(&spiTransferTxCompleteCallback, irqArgsVoided); + spi::assignTransferErrorCallback(&spiTransferErrorCallback, irqArgsVoided); } void SpiComIF::configureCacheMaintenanceOnTxBuffer(bool enable) { @@ -112,20 +113,20 @@ ReturnValue_t SpiComIF::initializeInterface(CookieIF *cookie) { spi::setSpiIrqMspFunctions(typedCfg); } else if(transferMode == spi::TransferModes::DMA) { - auto typedCfg = dynamic_cast(mspCfg); - if(typedCfg == nullptr) { - printCfgError("DMA MSP"); - return HasReturnvaluesIF::RETURN_FAILED; - } - // Check DMA handles - DMA_HandleTypeDef* txHandle = nullptr; - DMA_HandleTypeDef* rxHandle = nullptr; - spi::getDmaHandles(&txHandle, &rxHandle); - if(txHandle == nullptr or rxHandle == nullptr) { - printCfgError("DMA Handle"); - return HasReturnvaluesIF::RETURN_FAILED; - } - spi::setSpiDmaMspFunctions(typedCfg); + auto typedCfg = dynamic_cast(mspCfg); + if(typedCfg == nullptr) { + printCfgError("DMA MSP"); + return HasReturnvaluesIF::RETURN_FAILED; + } + // Check DMA handles + DMA_HandleTypeDef* txHandle = nullptr; + DMA_HandleTypeDef* rxHandle = nullptr; + spi::getDmaHandles(&txHandle, &rxHandle); + if(txHandle == nullptr or rxHandle == nullptr) { + printCfgError("DMA Handle"); + return HasReturnvaluesIF::RETURN_FAILED; + } + spi::setSpiDmaMspFunctions(typedCfg); } gpio::initializeGpioClock(gpioPort); @@ -160,6 +161,17 @@ ReturnValue_t SpiComIF::sendMessage(CookieIF *cookie, const uint8_t *sendData, s iter->second.currentTransferLen = sendLen; auto transferMode = spiCookie->getTransferMode(); + switch(spiCookie->getTransferState()) { + case(spi::TransferStates::IDLE): { + break; + } + case(spi::TransferStates::WAIT): + case(spi::TransferStates::FAILURE): + case(spi::TransferStates::SUCCESS): + default: { + return HasReturnvaluesIF::RETURN_FAILED; + } + } switch(transferMode) { case(spi::TransferModes::POLLING): { @@ -191,12 +203,37 @@ ReturnValue_t SpiComIF::readReceivedMessage(CookieIF *cookie, uint8_t **buffer, if(spiCookie == nullptr) { return NULLPOINTER; } - auto iter = spiDeviceMap.find(spiCookie->getDeviceAddress()); - if(iter == spiDeviceMap.end()) { + switch(spiCookie->getTransferState()) { + case(spi::TransferStates::SUCCESS): { + auto iter = spiDeviceMap.find(spiCookie->getDeviceAddress()); + if(iter == spiDeviceMap.end()) { + return HasReturnvaluesIF::RETURN_FAILED; + } + *buffer = iter->second.replyBuffer.data(); + *size = iter->second.currentTransferLen; + spiCookie->setTransferState(spi::TransferStates::IDLE); + break; + } + case(spi::TransferStates::FAILURE): { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "SpiComIF::readReceivedMessage: Transfer failure" << std::endl; +#else + sif::printWarning("SpiComIF::readReceivedMessage: Transfer failure\n"); +#endif +#endif + spiCookie->setTransferState(spi::TransferStates::IDLE); return HasReturnvaluesIF::RETURN_FAILED; } - *buffer = iter->second.replyBuffer.data(); - *size = iter->second.currentTransferLen; + case(spi::TransferStates::WAIT): + case(spi::TransferStates::IDLE): { + break; + } + default: { + return HasReturnvaluesIF::RETURN_FAILED; + } + } + return HasReturnvaluesIF::RETURN_OK; } @@ -212,6 +249,7 @@ ReturnValue_t SpiComIF::handlePollingSendOperation(uint8_t* recvPtr, SPI_HandleT if(returnval != HasReturnvaluesIF::RETURN_OK) { return returnval; } + spiCookie.setTransferState(spi::TransferStates::WAIT); HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_RESET); auto result = HAL_SPI_TransmitReceive(&spiHandle, const_cast(sendData), recvPtr, sendLen, defaultPollingTimeout); @@ -219,6 +257,7 @@ ReturnValue_t SpiComIF::handlePollingSendOperation(uint8_t* recvPtr, SPI_HandleT spiSemaphore->release(); switch(result) { case(HAL_OK): { + spiCookie.setTransferState(spi::TransferStates::SUCCESS); break; } case(HAL_TIMEOUT): { @@ -231,6 +270,7 @@ ReturnValue_t SpiComIF::handlePollingSendOperation(uint8_t* recvPtr, SPI_HandleT spiCookie.getDeviceAddress()); #endif #endif + spiCookie.setTransferState(spi::TransferStates::FAILURE); return spi::HAL_TIMEOUT_RETVAL; } case(HAL_ERROR): @@ -244,6 +284,7 @@ ReturnValue_t SpiComIF::handlePollingSendOperation(uint8_t* recvPtr, SPI_HandleT spiCookie.getDeviceAddress()); #endif #endif + spiCookie.setTransferState(spi::TransferStates::FAILURE); return spi::HAL_ERROR_RETVAL; } } @@ -323,9 +364,6 @@ ReturnValue_t SpiComIF::halErrorHandler(HAL_StatusTypeDef status, spi::TransferM ReturnValue_t SpiComIF::genericIrqSendSetup(uint8_t *recvPtr, SPI_HandleTypeDef& spiHandle, SpiCookie& spiCookie, const uint8_t *sendData, size_t sendLen) { - // These are required by the callback - currentGpioPort = spiCookie.getChipSelectGpioPort(); - currentGpioPin = spiCookie.getChipSelectGpioPin(); currentRecvPtr = recvPtr; currentRecvBuffSize = sendLen; @@ -339,53 +377,53 @@ ReturnValue_t SpiComIF::genericIrqSendSetup(uint8_t *recvPtr, SPI_HandleTypeDef& } // Cache the current SPI handle in any case spi::setSpiHandle(&spiHandle); - // The SPI handle is passed to the default SPI callback as a void argument + // Assign the IRQ arguments for the user callbacks + irqArgs.comIF = this; + irqArgs.spiCookie = &spiCookie; + // The SPI handle is passed to the default SPI callback as a void argument. This callback + // is different from the user callbacks specified above! spi::assignSpiUserArgs(spiCookie.getSpiIdx(), reinterpret_cast(&spiHandle)); - HAL_GPIO_WritePin(currentGpioPort, currentGpioPin, GPIO_PIN_RESET); + HAL_GPIO_WritePin(spiCookie.getChipSelectGpioPort(), spiCookie.getChipSelectGpioPin(), + GPIO_PIN_RESET); return HasReturnvaluesIF::RETURN_OK; } void SpiComIF::spiTransferTxCompleteCallback(SPI_HandleTypeDef *hspi, void *args) { - SpiComIF* spiComIF = reinterpret_cast(args); - if(spiComIF == nullptr) { - return; - } - genericIrqHandler(spiComIF, TransferStates::FAILURE); + genericIrqHandler(args, spi::TransferStates::SUCCESS); } void SpiComIF::spiTransferRxCompleteCallback(SPI_HandleTypeDef *hspi, void *args) { - SpiComIF* spiComIF = reinterpret_cast(args); - if(spiComIF == nullptr) { - return; - } - genericIrqHandler(spiComIF, TransferStates::FAILURE); + genericIrqHandler(args, spi::TransferStates::SUCCESS); } void SpiComIF::spiTransferCompleteCallback(SPI_HandleTypeDef *hspi, void *args) { - SpiComIF* spiComIF = reinterpret_cast(args); - if(spiComIF == nullptr) { - return; - } - genericIrqHandler(spiComIF, TransferStates::FAILURE); + genericIrqHandler(args, spi::TransferStates::SUCCESS); } void SpiComIF::spiTransferErrorCallback(SPI_HandleTypeDef *hspi, void *args) { - SpiComIF* spiComIF = reinterpret_cast(args); - if(spiComIF == nullptr) { - return; - } - genericIrqHandler(spiComIF, TransferStates::FAILURE); + genericIrqHandler(args, spi::TransferStates::FAILURE); } -void SpiComIF::genericIrqHandler(SpiComIF *spiComIF, TransferStates targetState) { - spiComIF->transferState = TransferStates::SUCCESS; +void SpiComIF::genericIrqHandler(void *irqArgsVoid, spi::TransferStates targetState) { + IrqArgs* irqArgs = reinterpret_cast(irqArgsVoid); + if(irqArgs == nullptr) { + return; + } + SpiCookie* spiCookie = irqArgs->spiCookie; + SpiComIF* comIF = irqArgs->comIF; + if(spiCookie == nullptr or comIF == nullptr) { + return; + } + + spiCookie->setTransferState(targetState); // Pull CS pin high again - HAL_GPIO_WritePin(spiComIF->currentGpioPort, spiComIF->currentGpioPin, GPIO_PIN_SET); + HAL_GPIO_WritePin(spiCookie->getChipSelectGpioPort(), spiCookie->getChipSelectGpioPin(), + GPIO_PIN_SET); // Release the task semaphore BaseType_t taskWoken = pdFALSE; - ReturnValue_t result = BinarySemaphore::releaseFromISR(spiComIF->spiSemaphore->getSemaphore(), + ReturnValue_t result = BinarySemaphore::releaseFromISR(comIF->spiSemaphore->getSemaphore(), &taskWoken); if(result != HasReturnvaluesIF::RETURN_OK) { // Configuration error @@ -393,10 +431,10 @@ void SpiComIF::genericIrqHandler(SpiComIF *spiComIF, TransferStates targetState) } // Perform cache maintenance operation for DMA transfers - if(spiComIF->currentTransferMode == spi::TransferModes::DMA) { + if(spiCookie->getTransferMode() == spi::TransferModes::DMA) { // Invalidate cache prior to access by CPU - SCB_InvalidateDCache_by_Addr ((uint32_t *) spiComIF->currentRecvPtr, - spiComIF->currentRecvBuffSize); + SCB_InvalidateDCache_by_Addr ((uint32_t *) comIF->currentRecvPtr, + comIF->currentRecvBuffSize); } /* Request a context switch if the SPI ComIF task was woken up and has a higher priority than the currently running task */ diff --git a/stm32h7/spi/SpiComIF.h b/stm32h7/spi/SpiComIF.h index 2a40a43..4b1ef80 100644 --- a/stm32h7/spi/SpiComIF.h +++ b/stm32h7/spi/SpiComIF.h @@ -15,13 +15,6 @@ class SpiCookie; -enum class TransferStates { - IDLE, - WAIT, - SUCCESS, - FAILURE -}; - /** * @brief This communication interface allows using generic device handlers with using * the STM32H7 SPI peripherals @@ -87,11 +80,17 @@ private: size_t currentTransferLen = 0; }; + struct IrqArgs { + SpiComIF* comIF = nullptr; + SpiCookie* spiCookie = nullptr; + }; + + IrqArgs irqArgs; + uint32_t defaultPollingTimeout = 50; SemaphoreIF::TimeoutType timeoutType = SemaphoreIF::TimeoutType::WAITING; dur_millis_t timeoutMs = 20; - spi::TransferModes currentTransferMode = spi::TransferModes::POLLING; BinarySemaphore* spiSemaphore = nullptr; bool cacheMaintenanceOnTxBuffer = true; @@ -99,11 +98,8 @@ private: using SpiDeviceMap = std::map; using SpiDeviceMapIter = SpiDeviceMap::iterator; - GPIO_TypeDef* currentGpioPort = nullptr; - uint16_t currentGpioPin = 0; uint8_t* currentRecvPtr = nullptr; size_t currentRecvBuffSize = 0; - volatile TransferStates transferState = TransferStates::IDLE; SpiDeviceMap spiDeviceMap; @@ -124,7 +120,7 @@ private: static void spiTransferCompleteCallback(SPI_HandleTypeDef *hspi, void* args); static void spiTransferErrorCallback(SPI_HandleTypeDef *hspi, void* args); - static void genericIrqHandler(SpiComIF* comIF, TransferStates targetState); + static void genericIrqHandler(void* irqArgs, spi::TransferStates targetState); void printCfgError(const char* const type); }; diff --git a/stm32h7/spi/SpiCookie.cpp b/stm32h7/spi/SpiCookie.cpp index f28df27..d583c41 100644 --- a/stm32h7/spi/SpiCookie.cpp +++ b/stm32h7/spi/SpiCookie.cpp @@ -68,3 +68,11 @@ void SpiCookie::deleteMspCfg() { spi::TransferModes SpiCookie::getTransferMode() const { return transferMode; } + +void SpiCookie::setTransferState(spi::TransferStates transferState) { + this->transferState = transferState; +} + +spi::TransferStates SpiCookie::getTransferState() const { + return this->transferState; +} diff --git a/stm32h7/spi/SpiCookie.h b/stm32h7/spi/SpiCookie.h index cb84ffd..45226b4 100644 --- a/stm32h7/spi/SpiCookie.h +++ b/stm32h7/spi/SpiCookie.h @@ -54,6 +54,7 @@ private: uint32_t spiSpeed; spi::SpiModes spiMode; spi::TransferModes transferMode; + volatile spi::TransferStates transferState = spi::TransferStates::IDLE; uint16_t chipSelectGpioPin; GPIO_TypeDef* chipSelectGpioPort; // The MSP configuration is cached here. Be careful when using this, it is automatically @@ -64,6 +65,9 @@ private: // Only the SpiComIF is allowed to use this to prevent dangling pointers issues spi::MspCfgBase* getMspCfg(); void deleteMspCfg(); + + void setTransferState(spi::TransferStates transferState); + spi::TransferStates getTransferState() const; }; diff --git a/stm32h7/spi/spiDefinitions.h b/stm32h7/spi/spiDefinitions.h index c22b853..772bf32 100644 --- a/stm32h7/spi/spiDefinitions.h +++ b/stm32h7/spi/spiDefinitions.h @@ -16,6 +16,13 @@ static constexpr ReturnValue_t HAL_TIMEOUT_RETVAL = HasReturnvaluesIF::makeRetur static constexpr ReturnValue_t HAL_BUSY_RETVAL = HasReturnvaluesIF::makeReturnCode(HAL_SPI_ID, 1); static constexpr ReturnValue_t HAL_ERROR_RETVAL = HasReturnvaluesIF::makeReturnCode(HAL_SPI_ID, 2); +enum class TransferStates { + IDLE, + WAIT, + SUCCESS, + FAILURE +}; + enum SpiBus { SPI_1, SPI_2 diff --git a/stm32h7/spi/spiInterrupts.cpp b/stm32h7/spi/spiInterrupts.cpp index 49182b8..83ba732 100644 --- a/stm32h7/spi/spiInterrupts.cpp +++ b/stm32h7/spi/spiInterrupts.cpp @@ -42,8 +42,7 @@ void spi::dmaTxIrqHandler(void* dmaHandle) { * @param None * @retval None */ -void spi::spiIrqHandler(void* spiHandle) -{ +void spi::spiIrqHandler(void* spiHandle) { if(spiHandle == nullptr) { return; }