From cc7250fcf5fb5e9ca890b3688d47d22a8956b14e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 17:08:59 +0200 Subject: [PATCH 01/13] second cmake fix --- src/CMakeLists.txt | 2 -- 1 file changed, 2 deletions(-) diff --git a/src/CMakeLists.txt b/src/CMakeLists.txt index 5a8f139b..e4670807 100644 --- a/src/CMakeLists.txt +++ b/src/CMakeLists.txt @@ -16,5 +16,3 @@ target_include_directories(${LIB_FSFW_NAME} PRIVATE target_include_directories(${LIB_FSFW_NAME} INTERFACE ${CMAKE_CURRENT_BINARY_DIR} ) - -configure_file(fsfw/FSFW.h.in fsfw/FSFW.h) From 42458725e86352e1fc38b806235ea5e8eb2b318c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 17:10:37 +0200 Subject: [PATCH 02/13] more important fix --- CMakeLists.txt | 16 ++++++++-------- 1 file changed, 8 insertions(+), 8 deletions(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 923d5cc5..e78e8929 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -93,14 +93,6 @@ target_include_directories(${LIB_FSFW_NAME} INTERFACE ${CMAKE_CURRENT_BINARY_DIR} ) -if(FSFW_BUILD_UNITTESTS) - configure_file(src/fsfw/FSFW.h.in fsfw/FSFW.h) - configure_file(src/fsfw/FSFWVersion.h.in fsfw/FSFWVersion.h) -else() - configure_file(src/fsfw/FSFW.h.in FSFW.h) - configure_file(src/fsfw/FSFWVersion.h.in FSFWVersion.h) -endif() - if(NOT CMAKE_CXX_STANDARD) set(CMAKE_CXX_STANDARD 11) set(CMAKE_CXX_STANDARD_REQUIRED True) @@ -155,6 +147,14 @@ else() set(OS_FSFW "host") endif() +if(FSFW_BUILD_UNITTESTS) + configure_file(src/fsfw/FSFW.h.in fsfw/FSFW.h) + configure_file(src/fsfw/FSFWVersion.h.in fsfw/FSFWVersion.h) +else() + configure_file(src/fsfw/FSFW.h.in FSFW.h) + configure_file(src/fsfw/FSFWVersion.h.in FSFWVersion.h) +endif() + message(STATUS "Compiling FSFW for the ${FSFW_OS_NAME} operating system.") add_subdirectory(src) From 3448a8c01b4e1cc3171e93f68b600dbfa9501962 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 17:23:14 +0200 Subject: [PATCH 03/13] SPI ComIF updates 1. Make setting a chip select pin optional 2. Make ComIF member functions public --- hal/src/fsfw_hal/stm32h7/spi/SpiComIF.cpp | 38 +++++++++++++++-------- hal/src/fsfw_hal/stm32h7/spi/SpiComIF.h | 3 +- 2 files changed, 26 insertions(+), 15 deletions(-) diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.cpp b/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.cpp index 1813aac0..4c4f7744 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.cpp @@ -138,12 +138,14 @@ ReturnValue_t SpiComIF::initializeInterface(CookieIF *cookie) { spi::setSpiDmaMspFunctions(typedCfg); } - gpio::initializeGpioClock(gpioPort); - GPIO_InitTypeDef chipSelect = {}; - chipSelect.Pin = gpioPin; - chipSelect.Mode = GPIO_MODE_OUTPUT_PP; - HAL_GPIO_Init(gpioPort, &chipSelect); - HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_SET); + if(gpioPort != nullptr) { + gpio::initializeGpioClock(gpioPort); + GPIO_InitTypeDef chipSelect = {}; + chipSelect.Pin = gpioPin; + chipSelect.Mode = GPIO_MODE_OUTPUT_PP; + HAL_GPIO_Init(gpioPort, &chipSelect); + HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_SET); + } if(HAL_SPI_Init(&spiHandle) != HAL_OK) { sif::printWarning("SpiComIF::initialize: Error initializing SPI\n"); @@ -259,10 +261,15 @@ ReturnValue_t SpiComIF::handlePollingSendOperation(uint8_t* recvPtr, SPI_HandleT return returnval; } spiCookie.setTransferState(spi::TransferStates::WAIT); - HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_RESET); + if(gpioPort != nullptr) { + HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_RESET); + } + auto result = HAL_SPI_TransmitReceive(&spiHandle, const_cast(sendData), recvPtr, sendLen, defaultPollingTimeout); - HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_SET); + if(gpioPort != nullptr) { + HAL_GPIO_WritePin(gpioPort, gpioPin, GPIO_PIN_SET); + } spiSemaphore->release(); switch(result) { case(HAL_OK): { @@ -392,8 +399,10 @@ ReturnValue_t SpiComIF::genericIrqSendSetup(uint8_t *recvPtr, SPI_HandleTypeDef& // 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(spiCookie.getChipSelectGpioPort(), spiCookie.getChipSelectGpioPin(), - GPIO_PIN_RESET); + if(spiCookie.getChipSelectGpioPort() != nullptr) { + HAL_GPIO_WritePin(spiCookie.getChipSelectGpioPort(), spiCookie.getChipSelectGpioPin(), + GPIO_PIN_RESET); + } return HasReturnvaluesIF::RETURN_OK; } @@ -426,9 +435,12 @@ void SpiComIF::genericIrqHandler(void *irqArgsVoid, spi::TransferStates targetSt spiCookie->setTransferState(targetState); - // Pull CS pin high again - HAL_GPIO_WritePin(spiCookie->getChipSelectGpioPort(), spiCookie->getChipSelectGpioPin(), - GPIO_PIN_SET); + if(spiCookie->getChipSelectGpioPort() != nullptr) { + // Pull CS pin high again + HAL_GPIO_WritePin(spiCookie->getChipSelectGpioPort(), spiCookie->getChipSelectGpioPin(), + GPIO_PIN_SET); + } + #if defined FSFW_OSAL_FREERTOS // Release the task semaphore diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.h b/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.h index 9548e102..cb6c4cf8 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiComIF.h @@ -60,7 +60,6 @@ public: void addDmaHandles(DMA_HandleTypeDef* txHandle, DMA_HandleTypeDef* rxHandle); ReturnValue_t initialize() override; -protected: // DeviceCommunicationIF overrides virtual ReturnValue_t initializeInterface(CookieIF * cookie) override; @@ -72,7 +71,7 @@ protected: virtual ReturnValue_t readReceivedMessage(CookieIF *cookie, uint8_t **buffer, size_t *size) override; -private: +protected: struct SpiInstance { SpiInstance(size_t maxRecvSize): replyBuffer(std::vector(maxRecvSize)) {} From d675621b73e86449ffe88298bcc09d5582b1c5c9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 17:31:04 +0200 Subject: [PATCH 04/13] grouping CS gpio definition --- hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp | 10 +++++----- hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h | 15 ++++++++++++--- 2 files changed, 17 insertions(+), 8 deletions(-) diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp index 88f1e1f1..e9cbac8e 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp @@ -3,10 +3,10 @@ SpiCookie::SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferModes transferMode, spi::MspCfgBase* mspCfg, uint32_t spiSpeed, spi::SpiModes spiMode, - uint16_t chipSelectGpioPin, GPIO_TypeDef* chipSelectGpioPort, size_t maxRecvSize): + size_t maxRecvSize, GpioPair csGpio): deviceAddress(deviceAddress), spiIdx(spiIdx), spiSpeed(spiSpeed), spiMode(spiMode), - transferMode(transferMode), chipSelectGpioPin(chipSelectGpioPin), - chipSelectGpioPort(chipSelectGpioPort), mspCfg(mspCfg), maxRecvSize(maxRecvSize) { + transferMode(transferMode), csGpio(csGpio), + mspCfg(mspCfg), maxRecvSize(maxRecvSize) { spiHandle.Init.DataSize = SPI_DATASIZE_8BIT; spiHandle.Init.FirstBit = SPI_FIRSTBIT_MSB; spiHandle.Init.TIMode = SPI_TIMODE_DISABLE; @@ -24,11 +24,11 @@ SpiCookie::SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferM } uint16_t SpiCookie::getChipSelectGpioPin() const { - return chipSelectGpioPin; + return csGpio.second; } GPIO_TypeDef* SpiCookie::getChipSelectGpioPort() { - return chipSelectGpioPort; + return csGpio.first; } address_t SpiCookie::getDeviceAddress() const { diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h index 45226b4a..f5698999 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h @@ -8,6 +8,8 @@ #include "stm32h743xx.h" +#include + /** * @brief SPI cookie implementation for the STM32H7 device family * @details @@ -18,6 +20,12 @@ class SpiCookie: public CookieIF { friend class SpiComIF; public: + /** + * Typedef for STM32 GPIO pair where the first entry is the port used (e.g. GPIOA) + * and the second entry is the pin number + */ + using GpioPair = std::pair; + /** * Allows construction of a SPI cookie for a connected SPI device * @param deviceAddress @@ -32,10 +40,11 @@ public: * definitions supplied in the MCU header file! (e.g. GPIO_PIN_X) * @param chipSelectGpioPort GPIO port (e.g. GPIOA) * @param maxRecvSize Maximum expected receive size. Chose as small as possible. + * @param csGpio Optional CS GPIO definition. */ SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferModes transferMode, spi::MspCfgBase* mspCfg, uint32_t spiSpeed, spi::SpiModes spiMode, - uint16_t chipSelectGpioPin, GPIO_TypeDef* chipSelectGpioPort, size_t maxRecvSize); + size_t maxRecvSize, GpioPair csGpio = GpioPair(nullptr, 0)); uint16_t getChipSelectGpioPin() const; GPIO_TypeDef* getChipSelectGpioPort(); @@ -55,8 +64,8 @@ private: spi::SpiModes spiMode; spi::TransferModes transferMode; volatile spi::TransferStates transferState = spi::TransferStates::IDLE; - uint16_t chipSelectGpioPin; - GPIO_TypeDef* chipSelectGpioPort; + GpioPair csGpio; + // The MSP configuration is cached here. Be careful when using this, it is automatically // deleted by the SPI communication interface if it is not required anymore! spi::MspCfgBase* mspCfg = nullptr; From cb7399b9998e9ec2d6439c5eb75c8312641770a4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 18:05:18 +0200 Subject: [PATCH 05/13] msp init improvements --- hal/src/fsfw_hal/stm32h7/definitions.h | 25 ++++++++++ hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp | 6 +-- hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h | 10 ++-- hal/src/fsfw_hal/stm32h7/spi/mspInit.cpp | 28 +++++------ hal/src/fsfw_hal/stm32h7/spi/mspInit.h | 50 +++++++++++++------ .../fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp | 22 ++++---- 6 files changed, 90 insertions(+), 51 deletions(-) create mode 100644 hal/src/fsfw_hal/stm32h7/definitions.h diff --git a/hal/src/fsfw_hal/stm32h7/definitions.h b/hal/src/fsfw_hal/stm32h7/definitions.h new file mode 100644 index 00000000..af63a541 --- /dev/null +++ b/hal/src/fsfw_hal/stm32h7/definitions.h @@ -0,0 +1,25 @@ +#ifndef FSFW_HAL_STM32H7_DEFINITIONS_H_ +#define FSFW_HAL_STM32H7_DEFINITIONS_H_ + +#include +#include "stm32h7xx.h" + +namespace stm32h7 { + +/** + * Typedef for STM32 GPIO pair where the first entry is the port used (e.g. GPIOA) + * and the second entry is the pin number + */ +struct GpioCfg { + GpioCfg(): port(nullptr), pin(0), altFnc(0) {}; + + GpioCfg(GPIO_TypeDef* port, uint16_t pin, uint8_t altFnc = 0): + port(port), pin(pin), altFnc(altFnc) {}; + GPIO_TypeDef* port; + uint16_t pin; + uint8_t altFnc; +}; + +} + +#endif /* #ifndef FSFW_HAL_STM32H7_DEFINITIONS_H_ */ diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp index e9cbac8e..200d4651 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.cpp @@ -3,7 +3,7 @@ SpiCookie::SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferModes transferMode, spi::MspCfgBase* mspCfg, uint32_t spiSpeed, spi::SpiModes spiMode, - size_t maxRecvSize, GpioPair csGpio): + size_t maxRecvSize, stm32h7::GpioCfg csGpio): deviceAddress(deviceAddress), spiIdx(spiIdx), spiSpeed(spiSpeed), spiMode(spiMode), transferMode(transferMode), csGpio(csGpio), mspCfg(mspCfg), maxRecvSize(maxRecvSize) { @@ -24,11 +24,11 @@ SpiCookie::SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferM } uint16_t SpiCookie::getChipSelectGpioPin() const { - return csGpio.second; + return csGpio.pin; } GPIO_TypeDef* SpiCookie::getChipSelectGpioPort() { - return csGpio.first; + return csGpio.port; } address_t SpiCookie::getDeviceAddress() const { diff --git a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h index f5698999..56c6e800 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h +++ b/hal/src/fsfw_hal/stm32h7/spi/SpiCookie.h @@ -3,6 +3,7 @@ #include "spiDefinitions.h" #include "mspInit.h" +#include "../definitions.h" #include "fsfw/devicehandlers/CookieIF.h" @@ -20,11 +21,6 @@ class SpiCookie: public CookieIF { friend class SpiComIF; public: - /** - * Typedef for STM32 GPIO pair where the first entry is the port used (e.g. GPIOA) - * and the second entry is the pin number - */ - using GpioPair = std::pair; /** * Allows construction of a SPI cookie for a connected SPI device @@ -44,7 +40,7 @@ public: */ SpiCookie(address_t deviceAddress, spi::SpiBus spiIdx, spi::TransferModes transferMode, spi::MspCfgBase* mspCfg, uint32_t spiSpeed, spi::SpiModes spiMode, - size_t maxRecvSize, GpioPair csGpio = GpioPair(nullptr, 0)); + size_t maxRecvSize, stm32h7::GpioCfg csGpio = stm32h7::GpioCfg(nullptr, 0, 0)); uint16_t getChipSelectGpioPin() const; GPIO_TypeDef* getChipSelectGpioPort(); @@ -64,7 +60,7 @@ private: spi::SpiModes spiMode; spi::TransferModes transferMode; volatile spi::TransferStates transferState = spi::TransferStates::IDLE; - GpioPair csGpio; + stm32h7::GpioCfg csGpio; // The MSP configuration is cached here. Be careful when using this, it is automatically // deleted by the SPI communication interface if it is not required anymore! diff --git a/hal/src/fsfw_hal/stm32h7/spi/mspInit.cpp b/hal/src/fsfw_hal/stm32h7/spi/mspInit.cpp index 4df61f9b..b7ff2f70 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/mspInit.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/mspInit.cpp @@ -118,40 +118,40 @@ void spi::halMspInitPolling(SPI_HandleTypeDef* hspi, MspCfgBase* cfgBase) { GPIO_InitTypeDef GPIO_InitStruct = {}; /*##-1- Enable peripherals and GPIO Clocks #################################*/ /* Enable GPIO TX/RX clock */ - cfg->setupMacroWrapper(); + cfg->setupCb(); /*##-2- Configure peripheral GPIO ##########################################*/ /* SPI SCK GPIO pin configuration */ - GPIO_InitStruct.Pin = cfg->sckPin; + GPIO_InitStruct.Pin = cfg->sck.pin; GPIO_InitStruct.Mode = GPIO_MODE_AF_PP; GPIO_InitStruct.Pull = GPIO_PULLDOWN; GPIO_InitStruct.Speed = GPIO_SPEED_FREQ_HIGH; - GPIO_InitStruct.Alternate = cfg->sckAlternateFunction; - HAL_GPIO_Init(cfg->sckPort, &GPIO_InitStruct); + GPIO_InitStruct.Alternate = cfg->sck.altFnc; + HAL_GPIO_Init(cfg->sck.port, &GPIO_InitStruct); /* SPI MISO GPIO pin configuration */ - GPIO_InitStruct.Pin = cfg->misoPin; - GPIO_InitStruct.Alternate = cfg->misoAlternateFunction; - HAL_GPIO_Init(cfg->misoPort, &GPIO_InitStruct); + GPIO_InitStruct.Pin = cfg->miso.pin; + GPIO_InitStruct.Alternate = cfg->miso.altFnc; + HAL_GPIO_Init(cfg->miso.port, &GPIO_InitStruct); /* SPI MOSI GPIO pin configuration */ - GPIO_InitStruct.Pin = cfg->mosiPin; - GPIO_InitStruct.Alternate = cfg->mosiAlternateFunction; - HAL_GPIO_Init(cfg->mosiPort, &GPIO_InitStruct); + GPIO_InitStruct.Pin = cfg->mosi.pin; + GPIO_InitStruct.Alternate = cfg->mosi.altFnc; + HAL_GPIO_Init(cfg->mosi.port, &GPIO_InitStruct); } void spi::halMspDeinitPolling(SPI_HandleTypeDef* hspi, MspCfgBase* cfgBase) { auto cfg = reinterpret_cast(cfgBase); // Reset peripherals - cfg->cleanUpMacroWrapper(); + cfg->cleanupCb(); // Disable peripherals and GPIO Clocks /* Configure SPI SCK as alternate function */ - HAL_GPIO_DeInit(cfg->sckPort, cfg->sckPin); + HAL_GPIO_DeInit(cfg->sck.port, cfg->sck.pin); /* Configure SPI MISO as alternate function */ - HAL_GPIO_DeInit(cfg->misoPort, cfg->misoPin); + HAL_GPIO_DeInit(cfg->miso.port, cfg->miso.pin); /* Configure SPI MOSI as alternate function */ - HAL_GPIO_DeInit(cfg->mosiPort, cfg->mosiPin); + HAL_GPIO_DeInit(cfg->mosi.port, cfg->mosi.pin); } void spi::halMspInitInterrupt(SPI_HandleTypeDef* hspi, MspCfgBase* cfgBase) { diff --git a/hal/src/fsfw_hal/stm32h7/spi/mspInit.h b/hal/src/fsfw_hal/stm32h7/spi/mspInit.h index e6de2f8e..0fb553f7 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/mspInit.h +++ b/hal/src/fsfw_hal/stm32h7/spi/mspInit.h @@ -2,6 +2,7 @@ #define FSFW_HAL_STM32H7_SPI_MSPINIT_H_ #include "spiDefinitions.h" +#include "../definitions.h" #include "../dma.h" #include "stm32h7xx_hal_spi.h" @@ -12,6 +13,8 @@ extern "C" { #endif +using mspCb = void (*) (void); + /** * @brief This file provides MSP implementation for DMA, IRQ and Polling mode for the * SPI peripheral. This configuration is required for the SPI communication to work. @@ -19,27 +22,37 @@ extern "C" { namespace spi { struct MspCfgBase { + MspCfgBase(); + MspCfgBase(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, + mspCb cleanupCb = nullptr, mspCb setupCb = nullptr): + sck(sck), mosi(mosi), miso(miso), cleanupCb(cleanupCb), + setupCb(setupCb) {} + virtual ~MspCfgBase() = default; - void (* cleanUpMacroWrapper) (void) = nullptr; - void (* setupMacroWrapper) (void) = nullptr; + stm32h7::GpioCfg sck; + stm32h7::GpioCfg mosi; + stm32h7::GpioCfg miso; - GPIO_TypeDef* sckPort = nullptr; - uint32_t sckPin = 0; - uint8_t sckAlternateFunction = 0; - GPIO_TypeDef* mosiPort = nullptr; - uint32_t mosiPin = 0; - uint8_t mosiAlternateFunction = 0; - GPIO_TypeDef* misoPort = nullptr; - uint32_t misoPin = 0; - uint8_t misoAlternateFunction = 0; + mspCb cleanupCb = nullptr; + mspCb setupCb = nullptr; }; -struct MspPollingConfigStruct: public MspCfgBase {}; +struct MspPollingConfigStruct: public MspCfgBase { + MspPollingConfigStruct(): MspCfgBase() {}; + MspPollingConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, + mspCb cleanupCb = nullptr, mspCb setupCb = nullptr): + MspCfgBase(sck, mosi, miso, cleanupCb, setupCb) {} +}; /* A valid instance of this struct must be passed to the MSP initialization function as a void* argument */ struct MspIrqConfigStruct: public MspPollingConfigStruct { + MspIrqConfigStruct(): MspPollingConfigStruct() {}; + MspIrqConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, + mspCb cleanupCb = nullptr, mspCb setupCb = nullptr): + MspPollingConfigStruct(sck, mosi, miso, cleanupCb, setupCb) {} + SpiBus spiBus = SpiBus::SPI_1; user_handler_t spiIrqHandler = nullptr; user_args_t spiUserArgs = nullptr; @@ -53,11 +66,16 @@ struct MspIrqConfigStruct: public MspPollingConfigStruct { /* A valid instance of this struct must be passed to the MSP initialization function as a void* argument */ struct MspDmaConfigStruct: public MspIrqConfigStruct { + MspDmaConfigStruct(): MspIrqConfigStruct() {}; + MspDmaConfigStruct(stm32h7::GpioCfg sck, stm32h7::GpioCfg mosi, stm32h7::GpioCfg miso, + mspCb cleanupCb = nullptr, mspCb setupCb = nullptr): + MspIrqConfigStruct(sck, mosi, miso, cleanupCb, setupCb) {} void (* dmaClkEnableWrapper) (void) = nullptr; - dma::DMAIndexes txDmaIndex; - dma::DMAIndexes rxDmaIndex; - dma::DMAStreams txDmaStream; - dma::DMAStreams rxDmaStream; + + dma::DMAIndexes txDmaIndex = dma::DMAIndexes::DMA_1; + dma::DMAIndexes rxDmaIndex = dma::DMAIndexes::DMA_1; + dma::DMAStreams txDmaStream = dma::DMAStreams::STREAM_0; + dma::DMAStreams rxDmaStream = dma::DMAStreams::STREAM_0; IRQn_Type txDmaIrqNumber = DMA1_Stream0_IRQn; IRQn_Type rxDmaIrqNumber = DMA1_Stream1_IRQn; // Priorities for NVIC diff --git a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp b/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp index 43194704..8247d002 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp @@ -23,17 +23,17 @@ void spiDmaClockEnableWrapper() { } void spi::h743zi::standardPollingCfg(MspPollingConfigStruct& cfg) { - cfg.setupMacroWrapper = &spiSetupWrapper; - cfg.cleanUpMacroWrapper = &spiCleanUpWrapper; - cfg.sckPort = GPIOA; - cfg.sckPin = GPIO_PIN_5; - cfg.misoPort = GPIOA; - cfg.misoPin = GPIO_PIN_6; - cfg.mosiPort = GPIOA; - cfg.mosiPin = GPIO_PIN_7; - cfg.sckAlternateFunction = GPIO_AF5_SPI1; - cfg.mosiAlternateFunction = GPIO_AF5_SPI1; - cfg.misoAlternateFunction = GPIO_AF5_SPI1; + cfg.setupCb = &spiSetupWrapper; + cfg.cleanupCb = &spiCleanUpWrapper; + cfg.sck.port = GPIOA; + cfg.sck.pin = GPIO_PIN_5; + cfg.miso.port = GPIOA; + cfg.miso.pin = GPIO_PIN_6; + cfg.mosi.port = GPIOA; + cfg.mosi.pin = GPIO_PIN_7; + cfg.sck.altFnc = GPIO_AF5_SPI1; + cfg.mosi.altFnc = GPIO_AF5_SPI1; + cfg.miso.altFnc = GPIO_AF5_SPI1; } void spi::h743zi::standardInterruptCfg(MspIrqConfigStruct& cfg, IrqPriorities spiIrqPrio, From 7c855592d05ecfc017e9806f87e40a11f8c75a8a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 27 Oct 2021 18:11:56 +0200 Subject: [PATCH 06/13] more cleaning up --- hal/src/fsfw_hal/stm32h7/devicetest/GyroL3GD20H.cpp | 8 ++++---- hal/src/fsfw_hal/stm32h7/spi/CMakeLists.txt | 2 +- .../spi/{stm32h743ziSpi.cpp => stm32h743zi.cpp} | 10 +++++----- .../stm32h7/spi/{stm32h743ziSpi.h => stm32h743zi.h} | 11 +++++------ 4 files changed, 15 insertions(+), 16 deletions(-) rename hal/src/fsfw_hal/stm32h7/spi/{stm32h743ziSpi.cpp => stm32h743zi.cpp} (88%) rename hal/src/fsfw_hal/stm32h7/spi/{stm32h743ziSpi.h => stm32h743zi.h} (64%) diff --git a/hal/src/fsfw_hal/stm32h7/devicetest/GyroL3GD20H.cpp b/hal/src/fsfw_hal/stm32h7/devicetest/GyroL3GD20H.cpp index 051be344..d1fdd1e5 100644 --- a/hal/src/fsfw_hal/stm32h7/devicetest/GyroL3GD20H.cpp +++ b/hal/src/fsfw_hal/stm32h7/devicetest/GyroL3GD20H.cpp @@ -4,7 +4,7 @@ #include "fsfw_hal/stm32h7/spi/spiDefinitions.h" #include "fsfw_hal/stm32h7/spi/spiCore.h" #include "fsfw_hal/stm32h7/spi/spiInterrupts.h" -#include "fsfw_hal/stm32h7/spi/stm32h743ziSpi.h" +#include "fsfw_hal/stm32h7/spi/stm32h743zi.h" #include "fsfw/tasks/TaskFactory.h" #include "fsfw/serviceinterface/ServiceInterface.h" @@ -33,20 +33,20 @@ GyroL3GD20H::GyroL3GD20H(SPI_HandleTypeDef *spiHandle, spi::TransferModes transf mspCfg = new spi::MspDmaConfigStruct(); auto typedCfg = dynamic_cast(mspCfg); spi::setDmaHandles(txDmaHandle, rxDmaHandle); - spi::h743zi::standardDmaCfg(*typedCfg, IrqPriorities::HIGHEST_FREERTOS, + stm32h7::h743zi::standardDmaCfg(*typedCfg, IrqPriorities::HIGHEST_FREERTOS, IrqPriorities::HIGHEST_FREERTOS, IrqPriorities::HIGHEST_FREERTOS); spi::setSpiDmaMspFunctions(typedCfg); } else if(transferMode == spi::TransferModes::INTERRUPT) { mspCfg = new spi::MspIrqConfigStruct(); auto typedCfg = dynamic_cast(mspCfg); - spi::h743zi::standardInterruptCfg(*typedCfg, IrqPriorities::HIGHEST_FREERTOS); + stm32h7::h743zi::standardInterruptCfg(*typedCfg, IrqPriorities::HIGHEST_FREERTOS); spi::setSpiIrqMspFunctions(typedCfg); } else if(transferMode == spi::TransferModes::POLLING) { mspCfg = new spi::MspPollingConfigStruct(); auto typedCfg = dynamic_cast(mspCfg); - spi::h743zi::standardPollingCfg(*typedCfg); + stm32h7::h743zi::standardPollingCfg(*typedCfg); spi::setSpiPollingMspFunctions(typedCfg); } diff --git a/hal/src/fsfw_hal/stm32h7/spi/CMakeLists.txt b/hal/src/fsfw_hal/stm32h7/spi/CMakeLists.txt index e28c35aa..aa5541bc 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/CMakeLists.txt +++ b/hal/src/fsfw_hal/stm32h7/spi/CMakeLists.txt @@ -5,5 +5,5 @@ target_sources(${LIB_FSFW_NAME} PRIVATE mspInit.cpp SpiCookie.cpp SpiComIF.cpp - stm32h743ziSpi.cpp + stm32h743zi.cpp ) diff --git a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp b/hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.cpp similarity index 88% rename from hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp rename to hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.cpp index 8247d002..1bafccd5 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.cpp +++ b/hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.cpp @@ -1,4 +1,4 @@ -#include "fsfw_hal/stm32h7/spi/stm32h743ziSpi.h" +#include "fsfw_hal/stm32h7/spi/stm32h743zi.h" #include "fsfw_hal/stm32h7/spi/spiCore.h" #include "fsfw_hal/stm32h7/spi/spiInterrupts.h" @@ -22,7 +22,7 @@ void spiDmaClockEnableWrapper() { __HAL_RCC_DMA2_CLK_ENABLE(); } -void spi::h743zi::standardPollingCfg(MspPollingConfigStruct& cfg) { +void stm32h7::h743zi::standardPollingCfg(spi::MspPollingConfigStruct& cfg) { cfg.setupCb = &spiSetupWrapper; cfg.cleanupCb = &spiCleanUpWrapper; cfg.sck.port = GPIOA; @@ -36,13 +36,13 @@ void spi::h743zi::standardPollingCfg(MspPollingConfigStruct& cfg) { cfg.miso.altFnc = GPIO_AF5_SPI1; } -void spi::h743zi::standardInterruptCfg(MspIrqConfigStruct& cfg, IrqPriorities spiIrqPrio, +void stm32h7::h743zi::standardInterruptCfg(spi::MspIrqConfigStruct& cfg, IrqPriorities spiIrqPrio, IrqPriorities spiSubprio) { // High, but works on FreeRTOS as well (priorities range from 0 to 15) cfg.preEmptPriority = spiIrqPrio; cfg.subpriority = spiSubprio; cfg.spiIrqNumber = SPI1_IRQn; - cfg.spiBus = SpiBus::SPI_1; + cfg.spiBus = spi::SpiBus::SPI_1; user_handler_t spiUserHandler = nullptr; user_args_t spiUserArgs = nullptr; getSpiUserHandler(spi::SpiBus::SPI_1, &spiUserHandler, &spiUserArgs); @@ -55,7 +55,7 @@ void spi::h743zi::standardInterruptCfg(MspIrqConfigStruct& cfg, IrqPriorities sp standardPollingCfg(cfg); } -void spi::h743zi::standardDmaCfg(MspDmaConfigStruct& cfg, IrqPriorities spiIrqPrio, +void stm32h7::h743zi::standardDmaCfg(spi::MspDmaConfigStruct& cfg, IrqPriorities spiIrqPrio, IrqPriorities txIrqPrio, IrqPriorities rxIrqPrio, IrqPriorities spiSubprio, IrqPriorities txSubprio, IrqPriorities rxSubprio) { cfg.dmaClkEnableWrapper = &spiDmaClockEnableWrapper; diff --git a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.h b/hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.h similarity index 64% rename from hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.h rename to hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.h index 87689add..daa95554 100644 --- a/hal/src/fsfw_hal/stm32h7/spi/stm32h743ziSpi.h +++ b/hal/src/fsfw_hal/stm32h7/spi/stm32h743zi.h @@ -3,21 +3,20 @@ #include "mspInit.h" -namespace spi { +namespace stm32h7 { namespace h743zi { -void standardPollingCfg(MspPollingConfigStruct& cfg); -void standardInterruptCfg(MspIrqConfigStruct& cfg, IrqPriorities spiIrqPrio, +void standardPollingCfg(spi::MspPollingConfigStruct& cfg); +void standardInterruptCfg(spi::MspIrqConfigStruct& cfg, IrqPriorities spiIrqPrio, IrqPriorities spiSubprio = HIGHEST); -void standardDmaCfg(MspDmaConfigStruct& cfg, IrqPriorities spiIrqPrio, +void standardDmaCfg(spi::MspDmaConfigStruct& cfg, IrqPriorities spiIrqPrio, IrqPriorities txIrqPrio, IrqPriorities rxIrqPrio, IrqPriorities spiSubprio = HIGHEST, IrqPriorities txSubPrio = HIGHEST, IrqPriorities rxSubprio = HIGHEST); + } } - - #endif /* FSFW_HAL_STM32H7_SPI_STM32H743ZISPI_H_ */ From 36aaf3d75800db7e2d9f7229fdd95068fdb17481 Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Wed, 27 Oct 2021 20:41:04 +0200 Subject: [PATCH 07/13] say hi to my new friend valgrind --- tests/src/fsfw_tests/unit/container/RingBufferTest.cpp | 4 ++-- .../fsfw_tests/unit/datapoollocal/LocalPoolManagerTest.cpp | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/tests/src/fsfw_tests/unit/container/RingBufferTest.cpp b/tests/src/fsfw_tests/unit/container/RingBufferTest.cpp index 819401ab..0be8b2a7 100644 --- a/tests/src/fsfw_tests/unit/container/RingBufferTest.cpp +++ b/tests/src/fsfw_tests/unit/container/RingBufferTest.cpp @@ -78,7 +78,7 @@ TEST_CASE("Ring Buffer Test" , "[RingBufferTest]") { TEST_CASE("Ring Buffer Test2" , "[RingBufferTest2]") { uint8_t testData[13]= {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; uint8_t readBuffer[10] = {13, 13, 13, 13, 13, 13, 13, 13, 13, 13}; - uint8_t* newBuffer = new uint8_t[10]; + uint8_t* newBuffer = new uint8_t[15]; SimpleRingBuffer ringBuffer(newBuffer, 10, true, 5); SECTION("Simple Test") { @@ -168,7 +168,7 @@ TEST_CASE("Ring Buffer Test2" , "[RingBufferTest2]") { TEST_CASE("Ring Buffer Test3" , "[RingBufferTest3]") { uint8_t testData[13]= {0, 1, 2, 3, 4, 5, 6, 7, 8, 9, 10, 11, 12}; uint8_t readBuffer[10] = {13, 13, 13, 13, 13, 13, 13, 13, 13, 13}; - uint8_t* newBuffer = new uint8_t[10]; + uint8_t* newBuffer = new uint8_t[25]; SimpleRingBuffer ringBuffer(newBuffer, 10, true, 15); SECTION("Simple Test") { diff --git a/tests/src/fsfw_tests/unit/datapoollocal/LocalPoolManagerTest.cpp b/tests/src/fsfw_tests/unit/datapoollocal/LocalPoolManagerTest.cpp index 7b2f9412..b1160254 100644 --- a/tests/src/fsfw_tests/unit/datapoollocal/LocalPoolManagerTest.cpp +++ b/tests/src/fsfw_tests/unit/datapoollocal/LocalPoolManagerTest.cpp @@ -143,7 +143,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(cdsShort.msDay_h == Catch::Approx(timeCdsNow.msDay_h).margin(1)); CHECK(cdsShort.msDay_hh == Catch::Approx(timeCdsNow.msDay_hh).margin(1)); CHECK(cdsShort.msDay_l == Catch::Approx(timeCdsNow.msDay_l).margin(1)); - CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(1)); + CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(5)); } SECTION("VariableSnapshotTest") { @@ -205,7 +205,7 @@ TEST_CASE("LocalPoolManagerTest" , "[LocManTest]") { CHECK(cdsShort.msDay_h == Catch::Approx(timeCdsNow.msDay_h).margin(1)); CHECK(cdsShort.msDay_hh == Catch::Approx(timeCdsNow.msDay_hh).margin(1)); CHECK(cdsShort.msDay_l == Catch::Approx(timeCdsNow.msDay_l).margin(1)); - CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(1)); + CHECK(cdsShort.msDay_ll == Catch::Approx(timeCdsNow.msDay_ll).margin(5)); } SECTION("VariableNotificationTest") { From a53992fdc9127c435f7bdf1a68cdf1329aab31ff Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Wed, 27 Oct 2021 21:32:40 +0200 Subject: [PATCH 08/13] introducing valgrind --- automation/Dockerfile | 2 +- automation/Jenkinsfile | 13 +++++++++++++ 2 files changed, 14 insertions(+), 1 deletion(-) diff --git a/automation/Dockerfile b/automation/Dockerfile index 0526d8f0..93a4fe7d 100644 --- a/automation/Dockerfile +++ b/automation/Dockerfile @@ -5,4 +5,4 @@ RUN apt-get --yes upgrade #tzdata is a dependency, won't install otherwise ARG DEBIAN_FRONTEND=noninteractive -RUN apt-get --yes install gcc g++ cmake lcov git nano +RUN apt-get --yes install gcc g++ cmake make lcov git valgrind nano diff --git a/automation/Jenkinsfile b/automation/Jenkinsfile index 0cb973bd..d4a8e2ab 100644 --- a/automation/Jenkinsfile +++ b/automation/Jenkinsfile @@ -55,5 +55,18 @@ pipeline { } } } + stage('Valgrind') { + agent { + dockerfile { + dir 'automation' + reuseNode true + } + } + steps { + dir(BUILDDIR) { + sh 'valgrind --leak-check=full --error-exitcode=1 ./fsfw-tests' + } + } + } } } From 6d5eb5b387b05c8c9615de0544bd0fc355137a52 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Nov 2021 18:48:02 +0100 Subject: [PATCH 09/13] Op Divider and bitutility updates - Added unittests for `PeriodicOperationDivider` and the `bitutil` helpers - Some API changes: Removed redundant bit part, because these functions are already in a namespace - Some bugfixes for `PeriodicOperationDivider` --- .../devicehandlers/MgmRM3100Handler.cpp | 2 +- .../datapoollocal/LocalPoolDataSetBase.cpp | 6 +- .../PeriodicOperationDivider.cpp | 39 +++++----- .../PeriodicOperationDivider.h | 77 +++++++++---------- src/fsfw/globalfunctions/bitutility.cpp | 11 +-- src/fsfw/globalfunctions/bitutility.h | 37 +++++++-- .../unit/datapoollocal/DataSetTest.cpp | 24 ++++-- .../unit/globalfunctions/CMakeLists.txt | 2 + .../unit/globalfunctions/testBitutil.cpp | 64 +++++++++++++++ .../unit/globalfunctions/testOpDivider.cpp | 64 +++++++++++++++ 10 files changed, 242 insertions(+), 84 deletions(-) create mode 100644 tests/src/fsfw_tests/unit/globalfunctions/testBitutil.cpp create mode 100644 tests/src/fsfw_tests/unit/globalfunctions/testOpDivider.cpp diff --git a/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp b/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp index 124eebbc..db4ea607 100644 --- a/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp +++ b/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.cpp @@ -186,7 +186,7 @@ ReturnValue_t MgmRM3100Handler::interpretDeviceReply(DeviceCommandId_t id, const uint8_t cmmValue = packet[1]; // We clear the seventh bit in any case // because this one is zero sometimes for some reason - bitutil::bitClear(&cmmValue, 6); + bitutil::clear(&cmmValue, 6); if(cmmValue == cmmRegValue and internalState == InternalState::READ_CMM) { commandExecuted = true; } diff --git a/src/fsfw/datapoollocal/LocalPoolDataSetBase.cpp b/src/fsfw/datapoollocal/LocalPoolDataSetBase.cpp index 5422c68a..d1ac0c7f 100644 --- a/src/fsfw/datapoollocal/LocalPoolDataSetBase.cpp +++ b/src/fsfw/datapoollocal/LocalPoolDataSetBase.cpp @@ -110,7 +110,7 @@ ReturnValue_t LocalPoolDataSetBase::serializeWithValidityBuffer(uint8_t **buffer for (uint16_t count = 0; count < fillCount; count++) { if(registeredVariables[count]->isValid()) { /* Set bit at correct position */ - bitutil::bitSet(validityPtr + validBufferIndex, validBufferIndexBit); + bitutil::set(validityPtr + validBufferIndex, validBufferIndexBit); } if(validBufferIndexBit == 7) { validBufferIndex ++; @@ -156,8 +156,8 @@ ReturnValue_t LocalPoolDataSetBase::deSerializeWithValidityBuffer( uint8_t validBufferIndexBit = 0; for (uint16_t count = 0; count < fillCount; count++) { // set validity buffer here. - bool nextVarValid = bitutil::bitGet(*buffer + - validBufferIndex, validBufferIndexBit); + bool nextVarValid = false; + bitutil::get(*buffer + validBufferIndex, validBufferIndexBit, nextVarValid); registeredVariables[count]->setValid(nextVarValid); if(validBufferIndexBit == 7) { diff --git a/src/fsfw/globalfunctions/PeriodicOperationDivider.cpp b/src/fsfw/globalfunctions/PeriodicOperationDivider.cpp index 62cc6f4c..ac6a78e4 100644 --- a/src/fsfw/globalfunctions/PeriodicOperationDivider.cpp +++ b/src/fsfw/globalfunctions/PeriodicOperationDivider.cpp @@ -2,43 +2,40 @@ PeriodicOperationDivider::PeriodicOperationDivider(uint32_t divider, - bool resetAutomatically): resetAutomatically(resetAutomatically), - counter(divider), divider(divider) { + bool resetAutomatically): resetAutomatically(resetAutomatically), + divider(divider) { } bool PeriodicOperationDivider::checkAndIncrement() { - bool opNecessary = check(); - if(opNecessary) { - if(resetAutomatically) { - counter = 0; - } - return opNecessary; - } - counter ++; - return opNecessary; + bool opNecessary = check(); + if(opNecessary and resetAutomatically) { + resetCounter(); + } + else { + counter++; + } + return opNecessary; } bool PeriodicOperationDivider::check() { - if(counter >= divider) { - return true; - } - return false; + if(counter >= divider) { + return true; + } + return false; } - - void PeriodicOperationDivider::resetCounter() { - counter = 0; + counter = 1; } void PeriodicOperationDivider::setDivider(uint32_t newDivider) { - divider = newDivider; + divider = newDivider; } uint32_t PeriodicOperationDivider::getCounter() const { - return counter; + return counter; } uint32_t PeriodicOperationDivider::getDivider() const { - return divider; + return divider; } diff --git a/src/fsfw/globalfunctions/PeriodicOperationDivider.h b/src/fsfw/globalfunctions/PeriodicOperationDivider.h index 7f7fb469..636849c0 100644 --- a/src/fsfw/globalfunctions/PeriodicOperationDivider.h +++ b/src/fsfw/globalfunctions/PeriodicOperationDivider.h @@ -13,51 +13,50 @@ */ class PeriodicOperationDivider { public: - /** - * Initialize with the desired divider and specify whether the internal - * counter will be reset automatically. - * @param divider - * @param resetAutomatically - */ - PeriodicOperationDivider(uint32_t divider, bool resetAutomatically = true); + /** + * Initialize with the desired divider and specify whether the internal + * counter will be reset automatically. + * @param divider Value of 0 or 1 will cause #check and #checkAndIncrement to always return + * true + * @param resetAutomatically + */ + PeriodicOperationDivider(uint32_t divider, bool resetAutomatically = true); + /** + * Check whether operation is necessary. If an operation is necessary and the class has been + * configured to be reset automatically, the counter will be reset to 1 automatically + * + * @return + * -@c true if the counter is larger or equal to the divider + * -@c false otherwise + */ + bool checkAndIncrement(); - /** - * Check whether operation is necessary. - * If an operation is necessary and the class has been - * configured to be reset automatically, the counter will be reset. - * - * @return - * -@c true if the counter is larger or equal to the divider - * -@c false otherwise - */ - bool checkAndIncrement(); + /** + * Checks whether an operation is necessary. This function will not increment the counter. + * @return + * -@c true if the counter is larger or equal to the divider + * -@c false otherwise + */ + bool check(); - /** - * Checks whether an operation is necessary. - * This function will not increment the counter! - * @return - * -@c true if the counter is larger or equal to the divider - * -@c false otherwise - */ - bool check(); + /** + * Can be used to reset the counter to 1 manually + */ + void resetCounter(); + uint32_t getCounter() const; - /** - * Can be used to reset the counter to 0 manually. - */ - void resetCounter(); - uint32_t getCounter() const; + /** + * Can be used to set a new divider value. + * @param newDivider + */ + void setDivider(uint32_t newDivider); + uint32_t getDivider() const; - /** - * Can be used to set a new divider value. - * @param newDivider - */ - void setDivider(uint32_t newDivider); - uint32_t getDivider() const; private: - bool resetAutomatically = true; - uint32_t counter = 0; - uint32_t divider = 0; + bool resetAutomatically = true; + uint32_t counter = 1; + uint32_t divider = 0; }; diff --git a/src/fsfw/globalfunctions/bitutility.cpp b/src/fsfw/globalfunctions/bitutility.cpp index 628e30c2..54e94a95 100644 --- a/src/fsfw/globalfunctions/bitutility.cpp +++ b/src/fsfw/globalfunctions/bitutility.cpp @@ -1,6 +1,6 @@ #include "fsfw/globalfunctions/bitutility.h" -void bitutil::bitSet(uint8_t *byte, uint8_t position) { +void bitutil::set(uint8_t *byte, uint8_t position) { if(position > 7) { return; } @@ -8,7 +8,7 @@ void bitutil::bitSet(uint8_t *byte, uint8_t position) { *byte |= 1 << shiftNumber; } -void bitutil::bitToggle(uint8_t *byte, uint8_t position) { +void bitutil::toggle(uint8_t *byte, uint8_t position) { if(position > 7) { return; } @@ -16,7 +16,7 @@ void bitutil::bitToggle(uint8_t *byte, uint8_t position) { *byte ^= 1 << shiftNumber; } -void bitutil::bitClear(uint8_t *byte, uint8_t position) { +void bitutil::clear(uint8_t *byte, uint8_t position) { if(position > 7) { return; } @@ -24,10 +24,11 @@ void bitutil::bitClear(uint8_t *byte, uint8_t position) { *byte &= ~(1 << shiftNumber); } -bool bitutil::bitGet(const uint8_t *byte, uint8_t position) { +bool bitutil::get(const uint8_t *byte, uint8_t position, bool& bit) { if(position > 7) { return false; } uint8_t shiftNumber = position + (7 - 2 * position); - return *byte & (1 << shiftNumber); + bit = *byte & (1 << shiftNumber); + return true; } diff --git a/src/fsfw/globalfunctions/bitutility.h b/src/fsfw/globalfunctions/bitutility.h index 1fc1290d..00f19310 100644 --- a/src/fsfw/globalfunctions/bitutility.h +++ b/src/fsfw/globalfunctions/bitutility.h @@ -5,13 +5,36 @@ namespace bitutil { -/* Helper functions for manipulating the individual bits of a byte. -Position refers to n-th bit of a byte, going from 0 (most significant bit) to -7 (least significant bit) */ -void bitSet(uint8_t* byte, uint8_t position); -void bitToggle(uint8_t* byte, uint8_t position); -void bitClear(uint8_t* byte, uint8_t position); -bool bitGet(const uint8_t* byte, uint8_t position); +// Helper functions for manipulating the individual bits of a byte. +// Position refers to n-th bit of a byte, going from 0 (most significant bit) to +// 7 (least significant bit) + +/** + * @brief Set the bit in a given byte + * @param byte + * @param position + */ +void set(uint8_t* byte, uint8_t position); +/** + * @brief Toggle the bit in a given byte + * @param byte + * @param position + */ +void toggle(uint8_t* byte, uint8_t position); +/** + * @brief Clear the bit in a given byte + * @param byte + * @param position + */ +void clear(uint8_t* byte, uint8_t position); +/** + * @brief Get the bit in a given byte + * @param byte + * @param position + * @param If the input is valid, this will be set to true if the bit is set and false otherwise. + * @return False if position is invalid, True otherwise + */ +bool get(const uint8_t* byte, uint8_t position, bool& bit); } diff --git a/tests/src/fsfw_tests/unit/datapoollocal/DataSetTest.cpp b/tests/src/fsfw_tests/unit/datapoollocal/DataSetTest.cpp index c967b241..e84f07b6 100644 --- a/tests/src/fsfw_tests/unit/datapoollocal/DataSetTest.cpp +++ b/tests/src/fsfw_tests/unit/datapoollocal/DataSetTest.cpp @@ -171,14 +171,19 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { /* We can do it like this because the buffer only has one byte for less than 8 variables */ uint8_t* validityByte = buffer + sizeof(buffer) - 1; - CHECK(bitutil::bitGet(validityByte, 0) == true); - CHECK(bitutil::bitGet(validityByte, 1) == false); - CHECK(bitutil::bitGet(validityByte, 2) == true); + bool bitSet = false; + bitutil::get(validityByte, 0, bitSet); + + CHECK(bitSet == true); + bitutil::get(validityByte, 1, bitSet); + CHECK(bitSet == false); + bitutil::get(validityByte, 2, bitSet); + CHECK(bitSet == true); /* Now we manipulate the validity buffer for the deserialization */ - bitutil::bitClear(validityByte, 0); - bitutil::bitSet(validityByte, 1); - bitutil::bitClear(validityByte, 2); + bitutil::clear(validityByte, 0); + bitutil::set(validityByte, 1); + bitutil::clear(validityByte, 2); /* Zero out everything except validity buffer */ std::memset(buffer, 0, sizeof(buffer) - 1); sizeToDeserialize = maxSize; @@ -239,8 +244,11 @@ TEST_CASE("DataSetTest" , "[DataSetTest]") { std::memcpy(validityBuffer.data(), buffer + 9 + sizeof(uint16_t) * 3, 2); /* The first 9 variables should be valid */ CHECK(validityBuffer[0] == 0xff); - CHECK(bitutil::bitGet(validityBuffer.data() + 1, 0) == true); - CHECK(bitutil::bitGet(validityBuffer.data() + 1, 1) == false); + bool bitSet = false; + bitutil::get(validityBuffer.data() + 1, 0, bitSet); + CHECK(bitSet == true); + bitutil::get(validityBuffer.data() + 1, 1, bitSet); + CHECK(bitSet == false); /* Now we invert the validity */ validityBuffer[0] = 0; diff --git a/tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt b/tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt index 209ce75f..3b29f23f 100644 --- a/tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt +++ b/tests/src/fsfw_tests/unit/globalfunctions/CMakeLists.txt @@ -1,3 +1,5 @@ target_sources(${FSFW_TEST_TGT} PRIVATE testDleEncoder.cpp + testOpDivider.cpp + testBitutil.cpp ) diff --git a/tests/src/fsfw_tests/unit/globalfunctions/testBitutil.cpp b/tests/src/fsfw_tests/unit/globalfunctions/testBitutil.cpp new file mode 100644 index 00000000..2627adcf --- /dev/null +++ b/tests/src/fsfw_tests/unit/globalfunctions/testBitutil.cpp @@ -0,0 +1,64 @@ +#include "fsfw/globalfunctions/bitutility.h" +#include + +TEST_CASE("Bitutility" , "[Bitutility]") { + uint8_t dummyByte = 0; + bool bitSet = false; + for(uint8_t pos = 0; pos < 8; pos++) { + bitutil::set(&dummyByte, pos); + REQUIRE(dummyByte == (1 << (7 - pos))); + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == 1); + dummyByte = 0; + } + + dummyByte = 0xff; + for(uint8_t pos = 0; pos < 8; pos++) { + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == 1); + bitutil::clear(&dummyByte, pos); + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == 0); + dummyByte = 0xff; + } + + dummyByte = 0xf0; + for(uint8_t pos = 0; pos < 8; pos++) { + if(pos < 4) { + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == 1); + bitutil::toggle(&dummyByte, pos); + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == 0); + } + else { + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == false); + bitutil::toggle(&dummyByte, pos); + bitutil::get(&dummyByte, pos, bitSet); + REQUIRE(bitSet == true); + } + } + REQUIRE(dummyByte == 0x0f); + + dummyByte = 0; + bitutil::set(&dummyByte, 8); + REQUIRE(dummyByte == 0); + bitutil::set(&dummyByte, -1); + REQUIRE(dummyByte == 0); + dummyByte = 0xff; + bitutil::clear(&dummyByte, 8); + REQUIRE(dummyByte == 0xff); + bitutil::clear(&dummyByte, -1); + REQUIRE(dummyByte == 0xff); + dummyByte = 0x00; + bitutil::toggle(&dummyByte, 8); + REQUIRE(dummyByte == 0x00); + bitutil::toggle(&dummyByte, -1); + REQUIRE(dummyByte == 0x00); + + REQUIRE(bitutil::get(&dummyByte, 8, bitSet) == false); +} + + + diff --git a/tests/src/fsfw_tests/unit/globalfunctions/testOpDivider.cpp b/tests/src/fsfw_tests/unit/globalfunctions/testOpDivider.cpp new file mode 100644 index 00000000..c02ada83 --- /dev/null +++ b/tests/src/fsfw_tests/unit/globalfunctions/testOpDivider.cpp @@ -0,0 +1,64 @@ +#include "fsfw/globalfunctions/PeriodicOperationDivider.h" +#include + +TEST_CASE("OpDivider" , "[OpDivider]") { + auto opDivider = PeriodicOperationDivider(1); + REQUIRE(opDivider.getDivider() == 1); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.check() == true); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.check() == true); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.checkAndIncrement() == true); + + opDivider.setDivider(0); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.checkAndIncrement() == true); + + opDivider.setDivider(2); + opDivider.resetCounter(); + REQUIRE(opDivider.getDivider() == 2); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.check() == false); + REQUIRE(opDivider.checkAndIncrement() == false); + REQUIRE(opDivider.getCounter() == 2); + REQUIRE(opDivider.check() == true); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.check() == false); + REQUIRE(opDivider.checkAndIncrement() == false); + REQUIRE(opDivider.getCounter() == 2); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.checkAndIncrement() == false); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.checkAndIncrement() == false); + + opDivider.setDivider(3); + opDivider.resetCounter(); + REQUIRE(opDivider.checkAndIncrement() == false); + REQUIRE(opDivider.checkAndIncrement() == false); + REQUIRE(opDivider.getCounter() == 3); + REQUIRE(opDivider.checkAndIncrement() == true); + REQUIRE(opDivider.getCounter() == 1); + REQUIRE(opDivider.checkAndIncrement() == false); + + auto opDividerNonResetting = PeriodicOperationDivider(2, false); + REQUIRE(opDividerNonResetting.getCounter() == 1); + REQUIRE(opDividerNonResetting.check() == false); + REQUIRE(opDividerNonResetting.checkAndIncrement() == false); + REQUIRE(opDividerNonResetting.getCounter() == 2); + REQUIRE(opDividerNonResetting.check() == true); + REQUIRE(opDividerNonResetting.checkAndIncrement() == true); + REQUIRE(opDividerNonResetting.getCounter() == 3); + REQUIRE(opDividerNonResetting.checkAndIncrement() == true); + REQUIRE(opDividerNonResetting.getCounter() == 4); + opDividerNonResetting.resetCounter(); + REQUIRE(opDividerNonResetting.getCounter() == 1); + REQUIRE(opDividerNonResetting.check() == false); + REQUIRE(opDividerNonResetting.checkAndIncrement() == false); + REQUIRE(opDividerNonResetting.getCounter() == 2); +} From 0176c07886822b697496bff1afea906b451ac75a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Nov 2021 18:49:29 +0100 Subject: [PATCH 10/13] use IF instead of void pointer --- src/fsfw/memory/FileSystemArgsIF.h | 13 +++++++++++++ src/fsfw/memory/HasFileSystemIF.h | 27 +++++++++++++++++---------- 2 files changed, 30 insertions(+), 10 deletions(-) create mode 100644 src/fsfw/memory/FileSystemArgsIF.h diff --git a/src/fsfw/memory/FileSystemArgsIF.h b/src/fsfw/memory/FileSystemArgsIF.h new file mode 100644 index 00000000..67d423ff --- /dev/null +++ b/src/fsfw/memory/FileSystemArgsIF.h @@ -0,0 +1,13 @@ +#ifndef FSFW_SRC_FSFW_MEMORY_FILESYSTEMARGS_H_ +#define FSFW_SRC_FSFW_MEMORY_FILESYSTEMARGS_H_ + +/** + * Empty base interface which can be implemented by to pass arguments via the HasFileSystemIF. + * Users can then dynamic_cast the base pointer to the require child pointer. + */ +class FileSystemArgsIF { +public: + virtual~ FileSystemArgsIF() {}; +}; + +#endif /* FSFW_SRC_FSFW_MEMORY_FILESYSTEMARGS_H_ */ diff --git a/src/fsfw/memory/HasFileSystemIF.h b/src/fsfw/memory/HasFileSystemIF.h index ec941f59..be2f8923 100644 --- a/src/fsfw/memory/HasFileSystemIF.h +++ b/src/fsfw/memory/HasFileSystemIF.h @@ -1,9 +1,10 @@ #ifndef FSFW_MEMORY_HASFILESYSTEMIF_H_ #define FSFW_MEMORY_HASFILESYSTEMIF_H_ -#include "../returnvalues/HasReturnvaluesIF.h" -#include "../returnvalues/FwClassIds.h" -#include "../ipc/messageQueueDefinitions.h" +#include "FileSystemArgsIF.h" +#include "fsfw/returnvalues/HasReturnvaluesIF.h" +#include "fsfw/returnvalues/FwClassIds.h" +#include "fsfw/ipc/messageQueueDefinitions.h" #include @@ -59,7 +60,7 @@ public: */ virtual ReturnValue_t appendToFile(const char* repositoryPath, const char* filename, const uint8_t* data, size_t size, - uint16_t packetNumber, void* args = nullptr) = 0; + uint16_t packetNumber, FileSystemArgsIF* args = nullptr) = 0; /** * @brief Generic function to create a new file. @@ -72,7 +73,7 @@ public: */ virtual ReturnValue_t createFile(const char* repositoryPath, const char* filename, const uint8_t* data = nullptr, - size_t size = 0, void* args = nullptr) = 0; + size_t size = 0, FileSystemArgsIF* args = nullptr) = 0; /** * @brief Generic function to delete a file. @@ -81,24 +82,30 @@ public: * @param args Any other arguments which an implementation might require * @return */ - virtual ReturnValue_t deleteFile(const char* repositoryPath, - const char* filename, void* args = nullptr) = 0; + virtual ReturnValue_t removeFile(const char* repositoryPath, + const char* filename, FileSystemArgsIF* args = nullptr) = 0; /** * @brief Generic function to create a directory * @param repositoryPath + * @param Equivalent to the -p flag in Unix systems. If some required parent directories + * do not exist, create them as well * @param args Any other arguments which an implementation might require * @return */ - virtual ReturnValue_t createDirectory(const char* repositoryPath, void* args = nullptr) = 0; + virtual ReturnValue_t createDirectory(const char* repositoryPath, const char* dirname, + bool createParentDirs, FileSystemArgsIF* args = nullptr) = 0; /** * @brief Generic function to remove a directory * @param repositoryPath * @param args Any other arguments which an implementation might require */ - virtual ReturnValue_t removeDirectory(const char* repositoryPath, - bool deleteRecurively = false, void* args = nullptr) = 0; + virtual ReturnValue_t removeDirectory(const char* repositoryPath, const char* dirname, + bool deleteRecurively = false, FileSystemArgsIF* args = nullptr) = 0; + + virtual ReturnValue_t renameFile(const char* repositoryPath, const char* oldFilename, + const char* newFilename, FileSystemArgsIF* args = nullptr) = 0; }; From 30217aa42b9ca23b1a0a8a8a972953277b124c34 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 10 Nov 2021 18:51:56 +0100 Subject: [PATCH 11/13] updated SerializeAdapter.h - Updates `SerializerAdapter` to also take simple pointer and simply assign the serialized and deSerialized size - Added related unittests --- src/fsfw/serialize/SerializeAdapter.h | 400 +++++++++++------- .../unit/serialize/TestSerialization.cpp | 297 ++++++++----- 2 files changed, 429 insertions(+), 268 deletions(-) diff --git a/src/fsfw/serialize/SerializeAdapter.h b/src/fsfw/serialize/SerializeAdapter.h index e6cd247e..2831472c 100644 --- a/src/fsfw/serialize/SerializeAdapter.h +++ b/src/fsfw/serialize/SerializeAdapter.h @@ -7,7 +7,7 @@ #include #include - /** +/** * @brief These adapters provides an interface to use the SerializeIF functions * with arbitrary template objects to facilitate and simplify the * serialization of classes with different multiple different data types @@ -20,174 +20,250 @@ */ class SerializeAdapter { public: - /*** - * This function can be used to serialize a trivial copy-able type or a - * child of SerializeIF. - * The right template to be called is determined in the function itself. - * For objects of non trivial copy-able type this function is almost never - * called by the user directly. Instead helpers for specific types like - * SerialArrayListAdapter or SerialLinkedListAdapter is the right choice here. - * - * @param[in] object Object to serialize, the used type is deduced from this pointer - * @param[in/out] buffer Buffer to serialize into. Will be moved by the function. - * @param[in/out] size Size of current written buffer. Will be incremented by the function. - * @param[in] maxSize Max size of Buffer - * @param[in] streamEndianness Endianness of serialized element as in according to SerializeIF::Endianness - * @return - * - @c BUFFER_TOO_SHORT The given buffer in is too short - * - @c RETURN_FAILED Generic Error - * - @c RETURN_OK Successful serialization - */ - template - static ReturnValue_t serialize(const T *object, uint8_t **buffer, - size_t *size, size_t maxSize, - SerializeIF::Endianness streamEndianness) { - InternalSerializeAdapter::value> adapter; - return adapter.serialize(object, buffer, size, maxSize, - streamEndianness); - } - /** - * Function to return the serialized size of the object in the pointer. - * May be a trivially copy-able object or a Child of SerializeIF - * - * @param object Pointer to Object - * @return Serialized size of object - */ - template - static size_t getSerializedSize(const T *object){ - InternalSerializeAdapter::value> adapter; - return adapter.getSerializedSize(object); - } - /** - * @brief - * Deserializes a object from a given buffer of given size. - * Object Must be trivially copy-able or a child of SerializeIF. - * - * @details - * Buffer will be moved to the current read location. Size will be decreased by the function. - * - * @param[in/out] buffer Buffer to deSerialize from. Will be moved by the function. - * @param[in/out] size Remaining size of the buffer to read from. Will be decreased by function. - * @param[in] streamEndianness Endianness as in according to SerializeIF::Endianness - * @return - * - @c STREAM_TOO_SHORT The input stream is too short to deSerialize the object - * - @c TOO_MANY_ELEMENTS The buffer has more inputs than expected - * - @c RETURN_FAILED Generic Error - * - @c RETURN_OK Successful deserialization - */ - template - static ReturnValue_t deSerialize(T *object, const uint8_t **buffer, - size_t *size, SerializeIF::Endianness streamEndianness) { - InternalSerializeAdapter::value> adapter; - return adapter.deSerialize(object, buffer, size, streamEndianness); - } + /*** + * @brief Serialize a trivial copy-able type or a child of SerializeIF. + * @details + * The right template to be called is determined in the function itself. + * For objects of non trivial copy-able type this function is almost never + * called by the user directly. Instead helpers for specific types like + * SerialArrayListAdapter or SerialLinkedListAdapter are the right choice here. + * + * @param[in] object: Object to serialize, the used type is deduced from this pointer + * @param[in/out] buffer: Pointer to the buffer to serialize into. Buffer position will be + * incremented by the function. + * @param[in/out] size: Pointer to size of current written buffer. + * SIze will be incremented by the function. + * @param[in] maxSize: Max size of Buffer + * @param[in] streamEndianness: Endianness of serialized element as in according to + * SerializeIF::Endianness + * @return + * - @c BUFFER_TOO_SHORT The given buffer in is too short + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful serialization + */ + template + static ReturnValue_t serialize(const T *object, uint8_t **buffer, + size_t *size, size_t maxSize, + SerializeIF::Endianness streamEndianness) { + InternalSerializeAdapter::value> adapter; + return adapter.serialize(object, buffer, size, maxSize, + streamEndianness); + } + + /*** + * This function can be used to serialize a trivial copy-able type or a child of SerializeIF. + * The right template to be called is determined in the function itself. + * For objects of non trivial copy-able type this function is almost never + * called by the user directly. Instead helpers for specific types like + * SerialArrayListAdapter or SerialLinkedListAdapter are the right choice here. + * + * @param[in] object: Object to serialize, the used type is deduced from this pointer + * @param[in/out] buffer: Buffer to serialize into. + * @param[out] serSize: Serialized size + * @param[in] maxSize: Max size of buffer + * @param[in] streamEndianness: Endianness of serialized element as in according to + * SerializeIF::Endianness + * @return + * - @c BUFFER_TOO_SHORT The given buffer in is too short + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful serialization + */ + template + static ReturnValue_t serialize(const T *object, uint8_t* const buffer, size_t* serSize, + size_t maxSize, SerializeIF::Endianness streamEndianness) { + if(object == nullptr or buffer == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + InternalSerializeAdapter::value> adapter; + uint8_t** tempPtr = const_cast(&buffer); + size_t tmpSize = 0; + ReturnValue_t result = adapter.serialize(object, tempPtr, &tmpSize, maxSize, + streamEndianness); + if(serSize != nullptr) { + *serSize = tmpSize; + } + return result; + } + + /** + * @brief Function to return the serialized size of the object in the pointer. + * @details + * May be a trivially copy-able object or a child of SerializeIF. + * + * @param object Pointer to Object + * @return Serialized size of object + */ + template + static size_t getSerializedSize(const T *object){ + InternalSerializeAdapter::value> adapter; + return adapter.getSerializedSize(object); + } + + /** + * @brief Deserializes a object from a given buffer of given size. + * + * @details + * Object Must be trivially copy-able or a child of SerializeIF. + * Buffer will be moved to the current read location. Size will be decreased by the function. + * + * @param[in] object: Pointer to object to deserialize + * @param[in/out] buffer: Pointer to the buffer to deSerialize from. Buffer position will be + * incremented by the function + * @param[in/out] size: Pointer to remaining size of the buffer to read from. + * Will be decreased by function. + * @param[in] streamEndianness: Endianness as in according to SerializeIF::Endianness + * @return + * - @c STREAM_TOO_SHORT The input stream is too short to deSerialize the object + * - @c TOO_MANY_ELEMENTS The buffer has more inputs than expected + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful deserialization + */ + template + static ReturnValue_t deSerialize(T *object, const uint8_t **buffer, + size_t *size, SerializeIF::Endianness streamEndianness) { + InternalSerializeAdapter::value> adapter; + return adapter.deSerialize(object, buffer, size, streamEndianness); + } + + /** + * @brief Deserializes a object from a given buffer of given size. + * + * @details + * Object Must be trivially copy-able or a child of SerializeIF. + * + * @param[in] object: Pointer to object to deserialize + * @param[in] buffer: Buffer to deSerialize from + * @param[out] deserSize: Deserialized length + * @param[in] streamEndianness: Endianness as in according to SerializeIF::Endianness + * @return + * - @c STREAM_TOO_SHORT The input stream is too short to deSerialize the object + * - @c TOO_MANY_ELEMENTS The buffer has more inputs than expected + * - @c RETURN_FAILED Generic Error + * - @c RETURN_OK Successful deserialization + */ + template + static ReturnValue_t deSerialize(T *object, const uint8_t* buffer, + size_t* deserSize, SerializeIF::Endianness streamEndianness) { + if(object == nullptr or buffer == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + InternalSerializeAdapter::value> adapter; + const uint8_t** tempPtr = &buffer; + size_t maxVal = -1; + ReturnValue_t result = adapter.deSerialize(object, tempPtr, &maxVal, streamEndianness); + if(deserSize != nullptr) { + *deserSize = -1 - maxVal; + } + return result; + } + private: - /** - * Internal template to deduce the right function calls at compile time - */ - template class InternalSerializeAdapter; + /** + * Internal template to deduce the right function calls at compile time + */ + template class InternalSerializeAdapter; - /** - * Template to be used if T is not a child of SerializeIF - * - * @tparam T T must be trivially_copyable - */ - template - class InternalSerializeAdapter { - static_assert (std::is_trivially_copyable::value, - "If a type needs to be serialized it must be a child of " - "SerializeIF or trivially copy-able"); - public: - static ReturnValue_t serialize(const T *object, uint8_t **buffer, - size_t *size, size_t max_size, - SerializeIF::Endianness streamEndianness) { - size_t ignoredSize = 0; - if (size == nullptr) { - size = &ignoredSize; - } - // Check remaining size is large enough and check integer - // overflow of *size - size_t newSize = sizeof(T) + *size; - if ((newSize <= max_size) and (newSize > *size)) { - T tmp; - switch (streamEndianness) { - case SerializeIF::Endianness::BIG: - tmp = EndianConverter::convertBigEndian(*object); - break; - case SerializeIF::Endianness::LITTLE: - tmp = EndianConverter::convertLittleEndian(*object); - break; - default: - case SerializeIF::Endianness::MACHINE: - tmp = *object; - break; - } - std::memcpy(*buffer, &tmp, sizeof(T)); - *size += sizeof(T); - (*buffer) += sizeof(T); - return HasReturnvaluesIF::RETURN_OK; - } else { - return SerializeIF::BUFFER_TOO_SHORT; - } - } + /** + * Template to be used if T is not a child of SerializeIF + * + * @tparam T T must be trivially_copyable + */ + template + class InternalSerializeAdapter { + static_assert (std::is_trivially_copyable::value, + "If a type needs to be serialized it must be a child of " + "SerializeIF or trivially copy-able"); + public: + static ReturnValue_t serialize(const T *object, uint8_t **buffer, + size_t *size, size_t max_size, + SerializeIF::Endianness streamEndianness) { + size_t ignoredSize = 0; + if (size == nullptr) { + size = &ignoredSize; + } + // Check remaining size is large enough and check integer + // overflow of *size + size_t newSize = sizeof(T) + *size; + if ((newSize <= max_size) and (newSize > *size)) { + T tmp; + switch (streamEndianness) { + case SerializeIF::Endianness::BIG: + tmp = EndianConverter::convertBigEndian(*object); + break; + case SerializeIF::Endianness::LITTLE: + tmp = EndianConverter::convertLittleEndian(*object); + break; + default: + case SerializeIF::Endianness::MACHINE: + tmp = *object; + break; + } + std::memcpy(*buffer, &tmp, sizeof(T)); + *size += sizeof(T); + (*buffer) += sizeof(T); + return HasReturnvaluesIF::RETURN_OK; + } else { + return SerializeIF::BUFFER_TOO_SHORT; + } + } - ReturnValue_t deSerialize(T *object, const uint8_t **buffer, - size_t *size, SerializeIF::Endianness streamEndianness) { - T tmp; - if (*size >= sizeof(T)) { - *size -= sizeof(T); - std::memcpy(&tmp, *buffer, sizeof(T)); - switch (streamEndianness) { - case SerializeIF::Endianness::BIG: - *object = EndianConverter::convertBigEndian(tmp); - break; - case SerializeIF::Endianness::LITTLE: - *object = EndianConverter::convertLittleEndian(tmp); - break; - default: - case SerializeIF::Endianness::MACHINE: - *object = tmp; - break; - } + ReturnValue_t deSerialize(T *object, const uint8_t **buffer, + size_t *size, SerializeIF::Endianness streamEndianness) { + T tmp; + if (*size >= sizeof(T)) { + *size -= sizeof(T); + std::memcpy(&tmp, *buffer, sizeof(T)); + switch (streamEndianness) { + case SerializeIF::Endianness::BIG: + *object = EndianConverter::convertBigEndian(tmp); + break; + case SerializeIF::Endianness::LITTLE: + *object = EndianConverter::convertLittleEndian(tmp); + break; + default: + case SerializeIF::Endianness::MACHINE: + *object = tmp; + break; + } - *buffer += sizeof(T); - return HasReturnvaluesIF::RETURN_OK; - } else { - return SerializeIF::STREAM_TOO_SHORT; - } - } + *buffer += sizeof(T); + return HasReturnvaluesIF::RETURN_OK; + } else { + return SerializeIF::STREAM_TOO_SHORT; + } + } - uint32_t getSerializedSize(const T *object) { - return sizeof(T); - } - }; + uint32_t getSerializedSize(const T *object) { + return sizeof(T); + } + }; - /** - * Template for objects that inherit from SerializeIF - * - * @tparam T A child of SerializeIF - */ - template - class InternalSerializeAdapter { - public: - ReturnValue_t serialize(const T *object, uint8_t **buffer, size_t *size, - size_t max_size, - SerializeIF::Endianness streamEndianness) const { - size_t ignoredSize = 0; - if (size == nullptr) { - size = &ignoredSize; - } - return object->serialize(buffer, size, max_size, streamEndianness); - } - size_t getSerializedSize(const T *object) const { - return object->getSerializedSize(); - } + /** + * Template for objects that inherit from SerializeIF + * + * @tparam T A child of SerializeIF + */ + template + class InternalSerializeAdapter { + public: + ReturnValue_t serialize(const T *object, uint8_t **buffer, size_t *size, + size_t max_size, + SerializeIF::Endianness streamEndianness) const { + size_t ignoredSize = 0; + if (size == nullptr) { + size = &ignoredSize; + } + return object->serialize(buffer, size, max_size, streamEndianness); + } + size_t getSerializedSize(const T *object) const { + return object->getSerializedSize(); + } - ReturnValue_t deSerialize(T *object, const uint8_t **buffer, - size_t *size, SerializeIF::Endianness streamEndianness) { - return object->deSerialize(buffer, size, streamEndianness); - } - }; + ReturnValue_t deSerialize(T *object, const uint8_t **buffer, + size_t *size, SerializeIF::Endianness streamEndianness) { + return object->deSerialize(buffer, size, streamEndianness); + } + }; }; #endif /* _FSFW_SERIALIZE_SERIALIZEADAPTER_H_ */ diff --git a/tests/src/fsfw_tests/unit/serialize/TestSerialization.cpp b/tests/src/fsfw_tests/unit/serialize/TestSerialization.cpp index 64deae3b..f883fe78 100644 --- a/tests/src/fsfw_tests/unit/serialize/TestSerialization.cpp +++ b/tests/src/fsfw_tests/unit/serialize/TestSerialization.cpp @@ -3,128 +3,213 @@ #include #include +#include #include -static bool test_value_bool = true; -static uint8_t tv_uint8 {5}; -static uint16_t tv_uint16 {283}; -static uint32_t tv_uint32 {929221}; -static uint64_t tv_uint64 {2929329429}; +static bool testBool = true; +static uint8_t tvUint8 {5}; +static uint16_t tvUint16 {283}; +static uint32_t tvUint32 {929221}; +static uint64_t tvUint64 {2929329429}; -static int8_t tv_int8 {-16}; -static int16_t tv_int16 {-829}; -static int32_t tv_int32 {-2312}; +static int8_t tvInt8 {-16}; +static int16_t tvInt16 {-829}; +static int32_t tvInt32 {-2312}; -static float tv_float {8.2149214}; -static float tv_sfloat = {-922.2321321}; -static double tv_double {9.2132142141e8}; -static double tv_sdouble {-2.2421e19}; +static float tvFloat {8.2149214}; +static float tvSfloat = {-922.2321321}; +static double tvDouble {9.2132142141e8}; +static double tvSdouble {-2.2421e19}; -static std::array test_array; +static std::array TEST_ARRAY; -TEST_CASE( "Serialization size tests", "[TestSerialization]") { +TEST_CASE( "Serialization size tests", "[SerSizeTest]") { //REQUIRE(unitTestClass.test_autoserialization() == 0); - REQUIRE(SerializeAdapter::getSerializedSize(&test_value_bool) == - sizeof(test_value_bool)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_uint8) == - sizeof(tv_uint8)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_uint16) == - sizeof(tv_uint16)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_uint32 ) == - sizeof(tv_uint32)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_uint64) == - sizeof(tv_uint64)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_int8) == - sizeof(tv_int8)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_int16) == - sizeof(tv_int16)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_int32) == - sizeof(tv_int32)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_float) == - sizeof(tv_float)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_sfloat) == - sizeof(tv_sfloat )); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_double) == - sizeof(tv_double)); - REQUIRE(SerializeAdapter::getSerializedSize(&tv_sdouble) == - sizeof(tv_sdouble)); + REQUIRE(SerializeAdapter::getSerializedSize(&testBool) == + sizeof(testBool)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvUint8) == + sizeof(tvUint8)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvUint16) == + sizeof(tvUint16)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvUint32 ) == + sizeof(tvUint32)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvUint64) == + sizeof(tvUint64)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvInt8) == + sizeof(tvInt8)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvInt16) == + sizeof(tvInt16)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvInt32) == + sizeof(tvInt32)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvFloat) == + sizeof(tvFloat)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvSfloat) == + sizeof(tvSfloat )); + REQUIRE(SerializeAdapter::getSerializedSize(&tvDouble) == + sizeof(tvDouble)); + REQUIRE(SerializeAdapter::getSerializedSize(&tvSdouble) == + sizeof(tvSdouble)); } +TEST_CASE("Auto Serialize Adapter", "[SerAdapter]") { + size_t serializedSize = 0; + uint8_t * pArray = TEST_ARRAY.data(); -TEST_CASE("Auto Serialize Adapter testing", "[single-file]") { - size_t serialized_size = 0; - uint8_t * p_array = test_array.data(); + SECTION("SerDe") { + size_t deserSize = 0; + SerializeAdapter::serialize(&testBool, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 1); + REQUIRE(TEST_ARRAY[0] == true); + bool readBack = false; + SerializeAdapter::deSerialize(&readBack, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 1); + REQUIRE(readBack == true); + SerializeAdapter::serialize(&tvUint8, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 1); + REQUIRE(TEST_ARRAY[0] == 5); + uint8_t readBackUint8 = 0; + uint8_t* const testPtr = TEST_ARRAY.data(); + uint8_t* const shouldStayConst = testPtr; + SerializeAdapter::deSerialize(&readBackUint8, testPtr, &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(testPtr == shouldStayConst); + REQUIRE(deserSize == 1); + REQUIRE(readBackUint8 == 5); + SerializeAdapter::serialize(&tvUint16, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 2); + deserSize = 0; + uint16_t readBackUint16 = 0; + SerializeAdapter::deSerialize(&readBackUint16, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 2); + REQUIRE(readBackUint16 == 283); - SECTION("Serializing...") { - SerializeAdapter::serialize(&test_value_bool, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_uint8, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_uint16, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_uint32, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_int8, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_int16, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_int32, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_uint64, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_float, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_double, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_sfloat, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - SerializeAdapter::serialize(&tv_sdouble, &p_array, - &serialized_size, test_array.size(), SerializeIF::Endianness::MACHINE); - REQUIRE (serialized_size == 47); + SerializeAdapter::serialize(&tvUint32, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 4); + uint32_t readBackUint32 = 0; + deserSize = 0; + SerializeAdapter::deSerialize(&readBackUint32, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 4); + REQUIRE(readBackUint32 == 929221); + + SerializeAdapter::serialize(&tvInt16, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 2); + int16_t readBackInt16 = 0; + SerializeAdapter::deSerialize(&readBackInt16, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(readBackInt16 == -829); + REQUIRE(deserSize == 2); + + SerializeAdapter::serialize(&tvFloat, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + float readBackFloat = 0.0; + SerializeAdapter::deSerialize(&readBackFloat, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(readBackFloat == Catch::Approx(8.214921)); + + SerializeAdapter::serialize(&tvSdouble, TEST_ARRAY.data(), &deserSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + double readBackSignedDouble = 0.0; + SerializeAdapter::deSerialize(&readBackSignedDouble, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(readBackSignedDouble == Catch::Approx(-2.2421e19)); + + uint8_t testBuf[4] = {1, 2, 3, 4}; + SerialBufferAdapter bufferAdapter(testBuf, sizeof(testBuf)); + SerializeAdapter::serialize(&bufferAdapter, TEST_ARRAY.data(), &deserSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 4); + for(uint8_t idx = 0; idx < 4; idx++) { + REQUIRE(TEST_ARRAY[idx] == idx + 1); + } + deserSize = 0; + testBuf[0] = 0; + testBuf[1] = 12; + SerializeAdapter::deSerialize(&bufferAdapter, TEST_ARRAY.data(), &deserSize, + SerializeIF::Endianness::MACHINE); + REQUIRE(deserSize == 4); + for(uint8_t idx = 0; idx < 4; idx++) { + REQUIRE(testBuf[idx] == idx + 1); + } + } + + SECTION("Serialize incrementing") { + SerializeAdapter::serialize(&testBool, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvUint8, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvUint16, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvUint32, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvInt8, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvInt16, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvInt32, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvUint64, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvFloat, &pArray, &serializedSize, + TEST_ARRAY.size(), SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvDouble, &pArray, &serializedSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvSfloat, &pArray, &serializedSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + SerializeAdapter::serialize(&tvSdouble, &pArray, &serializedSize, TEST_ARRAY.size(), + SerializeIF::Endianness::MACHINE); + REQUIRE (serializedSize == 47); } - SECTION("Deserializing") { - p_array = test_array.data(); - size_t remaining_size = serialized_size; - SerializeAdapter::deSerialize(&test_value_bool, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_uint8, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_uint16, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_uint32, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_int8, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_int16, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_int32, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_uint64, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_float, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_double, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_sfloat, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); - SerializeAdapter::deSerialize(&tv_sdouble, - const_cast(&p_array), &remaining_size, SerializeIF::Endianness::MACHINE); + SECTION("Deserialize decrementing") { + pArray = TEST_ARRAY.data(); + size_t remaining_size = serializedSize; + SerializeAdapter::deSerialize(&testBool, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvUint8, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvUint16, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvUint32, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvInt8, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvInt16, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvInt32, const_cast(&pArray), + &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvUint64, + const_cast(&pArray), &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvFloat, + const_cast(&pArray), &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvDouble, + const_cast(&pArray), &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvSfloat, + const_cast(&pArray), &remaining_size, SerializeIF::Endianness::MACHINE); + SerializeAdapter::deSerialize(&tvSdouble, + const_cast(&pArray), &remaining_size, SerializeIF::Endianness::MACHINE); - REQUIRE(test_value_bool == true); - REQUIRE(tv_uint8 == 5); - REQUIRE(tv_uint16 == 283); - REQUIRE(tv_uint32 == 929221); - REQUIRE(tv_uint64 == 2929329429); - REQUIRE(tv_int8 == -16); - REQUIRE(tv_int16 == -829); - REQUIRE(tv_int32 == -2312); + REQUIRE(testBool == true); + REQUIRE(tvUint8 == 5); + REQUIRE(tvUint16 == 283); + REQUIRE(tvUint32 == 929221); + REQUIRE(tvUint64 == 2929329429); + REQUIRE(tvInt8 == -16); + REQUIRE(tvInt16 == -829); + REQUIRE(tvInt32 == -2312); - REQUIRE(tv_float == Catch::Approx(8.214921)); - REQUIRE(tv_double == Catch::Approx(9.2132142141e8)); - REQUIRE(tv_sfloat == Catch::Approx(-922.2321321)); - REQUIRE(tv_sdouble == Catch::Approx(-2.2421e19)); + REQUIRE(tvFloat == Catch::Approx(8.214921)); + REQUIRE(tvDouble == Catch::Approx(9.2132142141e8)); + REQUIRE(tvSfloat == Catch::Approx(-922.2321321)); + REQUIRE(tvSdouble == Catch::Approx(-2.2421e19)); } } From 05c4f4fadca0ba73f1e51bd4ac50d278df11aabd Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 18 Nov 2021 19:56:24 +0100 Subject: [PATCH 12/13] Bugfix for Packet ID getters - Also added related unittests --- src/fsfw/tmtcpacket/SpacePacket.h | 2 +- tests/src/fsfw_tests/unit/tmtcpacket/CMakeLists.txt | 2 +- tests/src/fsfw_tests/unit/tmtcpacket/testCcsds.cpp | 11 +++++++++++ 3 files changed, 13 insertions(+), 2 deletions(-) create mode 100644 tests/src/fsfw_tests/unit/tmtcpacket/testCcsds.cpp diff --git a/src/fsfw/tmtcpacket/SpacePacket.h b/src/fsfw/tmtcpacket/SpacePacket.h index 16673319..a092712c 100644 --- a/src/fsfw/tmtcpacket/SpacePacket.h +++ b/src/fsfw/tmtcpacket/SpacePacket.h @@ -73,7 +73,7 @@ namespace spacepacket { constexpr uint16_t getSpacePacketIdFromApid(bool isTc, uint16_t apid, bool secondaryHeaderFlag = true) { - return (((isTc << 5) & 0x10) | ((secondaryHeaderFlag << 4) & 0x08) | + return ((isTc << 4) | (secondaryHeaderFlag << 3) | ((apid >> 8) & 0x07)) << 8 | (apid & 0x00ff); } diff --git a/tests/src/fsfw_tests/unit/tmtcpacket/CMakeLists.txt b/tests/src/fsfw_tests/unit/tmtcpacket/CMakeLists.txt index 36838b24..958bda40 100644 --- a/tests/src/fsfw_tests/unit/tmtcpacket/CMakeLists.txt +++ b/tests/src/fsfw_tests/unit/tmtcpacket/CMakeLists.txt @@ -1,3 +1,3 @@ target_sources(${FSFW_TEST_TGT} PRIVATE - PusTmTest.cpp + testCcsds.cpp ) diff --git a/tests/src/fsfw_tests/unit/tmtcpacket/testCcsds.cpp b/tests/src/fsfw_tests/unit/tmtcpacket/testCcsds.cpp new file mode 100644 index 00000000..8f531805 --- /dev/null +++ b/tests/src/fsfw_tests/unit/tmtcpacket/testCcsds.cpp @@ -0,0 +1,11 @@ +#include + +#include "fsfw/tmtcpacket/SpacePacket.h" + +TEST_CASE( "CCSDS Test" , "[ccsds]") { + REQUIRE(spacepacket::getTcSpacePacketIdFromApid(0x22) == 0x1822); + REQUIRE(spacepacket::getTmSpacePacketIdFromApid(0x22) == 0x0822); + + REQUIRE(spacepacket::getTcSpacePacketIdFromApid(0x7ff) == 0x1fff); + REQUIRE(spacepacket::getTmSpacePacketIdFromApid(0x7ff) == 0xfff); +} From 00dced31ee17f06a112b473b4b54fea8982c5f92 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 19 Nov 2021 13:50:46 +0100 Subject: [PATCH 13/13] update unittest helper scripts - Added functionality to open HTML report immediately - Added another helper script to automatically generate unittest build folder --- scripts/coverage.py | 7 +++++++ scripts/gen-unittest.sh | 3 +++ 2 files changed, 10 insertions(+) create mode 100755 scripts/gen-unittest.sh diff --git a/scripts/coverage.py b/scripts/coverage.py index 71b6fb03..b5bd7745 100755 --- a/scripts/coverage.py +++ b/scripts/coverage.py @@ -6,6 +6,7 @@ import platform import sys import time import argparse +import webbrowser from typing import List @@ -18,6 +19,10 @@ information how to set up the build folder. def main(): parser = argparse.ArgumentParser(description="Processing arguments for LCOV helper script.") + parser.add_argument( + '-o', '--open', action='store_true', help='Open coverage data in webbrowser' + ) + args = parser.parse_args() build_dir_list = [] if not os.path.isfile('README.md'): @@ -41,6 +46,8 @@ def main(): print("Multiple build directories found!") build_directory = determine_build_dir(build_dir_list) perform_lcov_operation(build_directory) + if os.path.isdir('fsfw-tests_coverage') and args.open: + webbrowser.open('fsfw-tests_coverage/index.html') def check_for_cmake_build_dir(build_dir_dict: list): diff --git a/scripts/gen-unittest.sh b/scripts/gen-unittest.sh new file mode 100755 index 00000000..9ca8c399 --- /dev/null +++ b/scripts/gen-unittest.sh @@ -0,0 +1,3 @@ +#!/bin/sh +mkdir build-Unittest && cd build-Unittest +cmake -DFSFW_BUILD_UNITTESTS=ON -DFSFW_OSAL=host -DCMAKE_BUILD_TYPE=Debug ..