From f40f783cb43ca11388ac557848980fcde5a1ac15 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Sep 2021 17:58:44 +0200 Subject: [PATCH 01/24] GPIO code update Adds capability to define GPIO by label and by chip for Linux systems --- .../fsfw_hal/common/gpio/gpioDefinitions.h | 64 ++++++--- .../fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp | 127 ++++++++++++------ hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.h | 11 +- hal/src/fsfw_hal/linux/rpi/GpioRPi.cpp | 2 +- 4 files changed, 139 insertions(+), 65 deletions(-) diff --git a/hal/src/fsfw_hal/common/gpio/gpioDefinitions.h b/hal/src/fsfw_hal/common/gpio/gpioDefinitions.h index 710b2e2c..688d9c9b 100644 --- a/hal/src/fsfw_hal/common/gpio/gpioDefinitions.h +++ b/hal/src/fsfw_hal/common/gpio/gpioDefinitions.h @@ -26,7 +26,8 @@ enum GpioOperation { enum GpioTypes { NONE, - GPIO_REGULAR, + GPIO_REGULAR_BY_CHIP, + GPIO_REGULAR_BY_LABEL, CALLBACK }; @@ -68,28 +69,57 @@ public: int initValue = 0; }; -class GpiodRegular: public GpioBase { +class GpiodRegularBase: public GpioBase { public: - GpiodRegular() : - GpioBase(gpio::GpioTypes::GPIO_REGULAR, std::string(), gpio::Direction::IN, 0) { + GpiodRegularBase(gpio::GpioTypes gpioType, std::string consumer, gpio::Direction direction, + int initValue, int lineNum): GpioBase(gpioType, consumer, direction, initValue), + lineNum(lineNum) { } - ; - - GpiodRegular(std::string chipname_, int lineNum_, std::string consumer_, - gpio::Direction direction_, int initValue_) : - GpioBase(gpio::GpioTypes::GPIO_REGULAR, consumer_, direction_, initValue_), - chipname(chipname_), lineNum(lineNum_) { - } - - GpiodRegular(std::string chipname_, int lineNum_, std::string consumer_) : - GpioBase(gpio::GpioTypes::GPIO_REGULAR, consumer_, gpio::Direction::IN, 0), - chipname(chipname_), lineNum(lineNum_) { - } - std::string chipname; int lineNum = 0; struct gpiod_line* lineHandle = nullptr; }; +class GpiodRegularByChip: public GpiodRegularBase { +public: + GpiodRegularByChip() : + GpiodRegularBase(gpio::GpioTypes::GPIO_REGULAR_BY_CHIP, + std::string(), gpio::Direction::IN, gpio::LOW, 0) { + } + + GpiodRegularByChip(std::string chipname_, int lineNum_, std::string consumer_, + gpio::Direction direction_, int initValue_) : + GpiodRegularBase(gpio::GpioTypes::GPIO_REGULAR_BY_CHIP, + consumer_, direction_, initValue_, lineNum_), + chipname(chipname_){ + } + + GpiodRegularByChip(std::string chipname_, int lineNum_, std::string consumer_) : + GpiodRegularBase(gpio::GpioTypes::GPIO_REGULAR_BY_CHIP, consumer_, + gpio::Direction::IN, gpio::LOW, lineNum_), + chipname(chipname_) { + } + + std::string chipname; +}; + +class GpiodRegularByLabel: public GpiodRegularBase { +public: + GpiodRegularByLabel(std::string label_, int lineNum_, std::string consumer_, + gpio::Direction direction_, int initValue_) : + GpiodRegularBase(gpio::GpioTypes::GPIO_REGULAR_BY_LABEL, consumer_, + direction_, initValue_, lineNum_), + label(label_) { + } + + GpiodRegularByLabel(std::string label_, int lineNum_, std::string consumer_) : + GpiodRegularBase(gpio::GpioTypes::GPIO_REGULAR_BY_LABEL, consumer_, + gpio::Direction::IN, gpio::LOW, lineNum_), + label(label_) { + } + + std::string label; +}; + class GpioCallback: public GpioBase { public: GpioCallback(std::string consumer, gpio::Direction direction_, int initValue_, diff --git a/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp b/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp index eef61e58..15c3d118 100644 --- a/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp +++ b/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.cpp @@ -20,7 +20,7 @@ LinuxLibgpioIF::~LinuxLibgpioIF() { ReturnValue_t LinuxLibgpioIF::addGpios(GpioCookie* gpioCookie) { ReturnValue_t result; if(gpioCookie == nullptr) { - sif::error << "LinuxLibgpioIF::initialize: Invalid cookie" << std::endl; + sif::error << "LinuxLibgpioIF::addGpios: Invalid cookie" << std::endl; return RETURN_FAILED; } @@ -45,16 +45,25 @@ ReturnValue_t LinuxLibgpioIF::addGpios(GpioCookie* gpioCookie) { ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap& mapToAdd) { for(auto& gpioConfig: mapToAdd) { - switch(gpioConfig.second->gpioType) { + auto& gpioType = gpioConfig.second->gpioType; + switch(gpioType) { case(gpio::GpioTypes::NONE): { return GPIO_INVALID_INSTANCE; } - case(gpio::GpioTypes::GPIO_REGULAR): { - GpiodRegular* regularGpio = dynamic_cast(gpioConfig.second); + case(gpio::GpioTypes::GPIO_REGULAR_BY_CHIP): { + auto regularGpio = dynamic_cast(gpioConfig.second); if(regularGpio == nullptr) { return GPIO_INVALID_INSTANCE; } - configureRegularGpio(gpioConfig.first, regularGpio); + configureGpioByChip(gpioConfig.first, *regularGpio); + break; + } + case(gpio::GpioTypes::GPIO_REGULAR_BY_LABEL):{ + auto regularGpio = dynamic_cast(gpioConfig.second); + if(regularGpio == nullptr) { + return GPIO_INVALID_INSTANCE; + } + configureGpioByLabel(gpioConfig.first, *regularGpio); break; } case(gpio::GpioTypes::CALLBACK): { @@ -70,41 +79,59 @@ ReturnValue_t LinuxLibgpioIF::configureGpios(GpioMap& mapToAdd) { return RETURN_OK; } -ReturnValue_t LinuxLibgpioIF::configureRegularGpio(gpioId_t gpioId, GpiodRegular *regularGpio) { - std::string chipname; +ReturnValue_t LinuxLibgpioIF::configureGpioByLabel(gpioId_t gpioId, + GpiodRegularByLabel &gpioByLabel) { + std::string& label = gpioByLabel.label; + struct gpiod_chip* chip = gpiod_chip_open_by_label(label.c_str()); + if (chip == nullptr) { + sif::warning << "LinuxLibgpioIF::configureRegularGpio: Failed to open gpio from gpio " + << "group with label " << label << ". Gpio ID: " << gpioId << std::endl; + return RETURN_FAILED; + + } + std::string failOutput = "label: " + label; + return configureRegularGpio(gpioId, gpioByLabel.gpioType, chip, gpioByLabel, failOutput); +} + +ReturnValue_t LinuxLibgpioIF::configureGpioByChip(gpioId_t gpioId, + GpiodRegularByChip &gpioByChip) { + std::string& chipname = gpioByChip.chipname; + struct gpiod_chip* chip = gpiod_chip_open_by_name(chipname.c_str()); + if (chip == nullptr) { + sif::warning << "LinuxLibgpioIF::configureRegularGpio: Failed to open chip " + << chipname << ". Gpio ID: " << gpioId << std::endl; + return RETURN_FAILED; + } + std::string failOutput = "chipname: " + chipname; + return configureRegularGpio(gpioId, gpioByChip.gpioType, chip, gpioByChip, failOutput); +} + +ReturnValue_t LinuxLibgpioIF::configureRegularGpio(gpioId_t gpioId, gpio::GpioTypes gpioType, + struct gpiod_chip* chip, GpiodRegularBase& regularGpio, std::string failOutput) { unsigned int lineNum; - struct gpiod_chip *chip; gpio::Direction direction; std::string consumer; struct gpiod_line *lineHandle; int result = 0; - chipname = regularGpio->chipname; - chip = gpiod_chip_open_by_name(chipname.c_str()); - if (!chip) { - sif::warning << "LinuxLibgpioIF::configureRegularGpio: Failed to open chip " - << chipname << ". Gpio ID: " << gpioId << std::endl; - return RETURN_FAILED; - } - - lineNum = regularGpio->lineNum; + lineNum = regularGpio.lineNum; lineHandle = gpiod_chip_get_line(chip, lineNum); if (!lineHandle) { - sif::debug << "LinuxLibgpioIF::configureRegularGpio: Failed to open line " << std::endl; - sif::debug << "GPIO ID: " << gpioId << ", line number: " << lineNum << - ", chipname: " << chipname << std::endl; - sif::debug << "Check if linux GPIO configuration has changed. " << std::endl; + sif::warning << "LinuxLibgpioIF::configureRegularGpio: Failed to open line " << std::endl; + sif::warning << "GPIO ID: " << gpioId << ", line number: " << lineNum << + ", " << failOutput << std::endl; + sif::warning << "Check if Linux GPIO configuration has changed. " << std::endl; gpiod_chip_close(chip); return RETURN_FAILED; } - direction = regularGpio->direction; - consumer = regularGpio->consumer; + direction = regularGpio.direction; + consumer = regularGpio.consumer; /* Configure direction and add a description to the GPIO */ switch (direction) { case(gpio::OUT): { result = gpiod_line_request_output(lineHandle, consumer.c_str(), - regularGpio->initValue); + regularGpio.initValue); if (result < 0) { sif::error << "LinuxLibgpioIF::configureRegularGpio: Failed to request line " << lineNum << " from GPIO instance with ID: " << gpioId << std::endl; @@ -134,7 +161,7 @@ ReturnValue_t LinuxLibgpioIF::configureRegularGpio(gpioId_t gpioId, GpiodRegular * Write line handle to GPIO configuration instance so it can later be used to set or * read states of GPIOs. */ - regularGpio->lineHandle = lineHandle; + regularGpio.lineHandle = lineHandle; return RETURN_OK; } @@ -145,8 +172,14 @@ ReturnValue_t LinuxLibgpioIF::pullHigh(gpioId_t gpioId) { return UNKNOWN_GPIO_ID; } - if(gpioMapIter->second->gpioType == gpio::GpioTypes::GPIO_REGULAR) { - return driveGpio(gpioId, dynamic_cast(gpioMapIter->second), 1); + auto gpioType = gpioMapIter->second->gpioType; + if(gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_CHIP or + gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_LABEL) { + auto regularGpio = dynamic_cast(gpioMapIter->second); + if(regularGpio == nullptr) { + return GPIO_TYPE_FAILURE; + } + return driveGpio(gpioId, *regularGpio, gpio::HIGH); } else { auto gpioCallback = dynamic_cast(gpioMapIter->second); @@ -167,8 +200,14 @@ ReturnValue_t LinuxLibgpioIF::pullLow(gpioId_t gpioId) { return UNKNOWN_GPIO_ID; } - if(gpioMapIter->second->gpioType == gpio::GpioTypes::GPIO_REGULAR) { - return driveGpio(gpioId, dynamic_cast(gpioMapIter->second), 0); + auto& gpioType = gpioMapIter->second->gpioType; + if(gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_CHIP or + gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_LABEL) { + auto regularGpio = dynamic_cast(gpioMapIter->second); + if(regularGpio == nullptr) { + return GPIO_TYPE_FAILURE; + } + return driveGpio(gpioId, *regularGpio, gpio::LOW); } else { auto gpioCallback = dynamic_cast(gpioMapIter->second); @@ -183,12 +222,8 @@ ReturnValue_t LinuxLibgpioIF::pullLow(gpioId_t gpioId) { } ReturnValue_t LinuxLibgpioIF::driveGpio(gpioId_t gpioId, - GpiodRegular* regularGpio, unsigned int logicLevel) { - if(regularGpio == nullptr) { - return GPIO_TYPE_FAILURE; - } - - int result = gpiod_line_set_value(regularGpio->lineHandle, logicLevel); + GpiodRegularBase& regularGpio, gpio::Levels logicLevel) { + int result = gpiod_line_set_value(regularGpio.lineHandle, logicLevel); if (result < 0) { sif::warning << "LinuxLibgpioIF::driveGpio: Failed to pull GPIO with ID " << gpioId << " to logic level " << logicLevel << std::endl; @@ -204,9 +239,10 @@ ReturnValue_t LinuxLibgpioIF::readGpio(gpioId_t gpioId, int* gpioState) { sif::warning << "LinuxLibgpioIF::readGpio: Unknown GPIOD ID " << gpioId << std::endl; return UNKNOWN_GPIO_ID; } - - if(gpioMapIter->second->gpioType == gpio::GpioTypes::GPIO_REGULAR) { - GpiodRegular* regularGpio = dynamic_cast(gpioMapIter->second); + auto gpioType = gpioMapIter->second->gpioType; + if(gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_CHIP or + gpioType == gpio::GpioTypes::GPIO_REGULAR_BY_LABEL) { + auto regularGpio = dynamic_cast(gpioMapIter->second); if(regularGpio == nullptr) { return GPIO_TYPE_FAILURE; } @@ -225,13 +261,14 @@ ReturnValue_t LinuxLibgpioIF::checkForConflicts(GpioMap& mapToAdd){ ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; for(auto& gpioConfig: mapToAdd) { switch(gpioConfig.second->gpioType) { - case(gpio::GpioTypes::GPIO_REGULAR): { - auto regularGpio = dynamic_cast(gpioConfig.second); + case(gpio::GpioTypes::GPIO_REGULAR_BY_CHIP): + case(gpio::GpioTypes::GPIO_REGULAR_BY_LABEL): { + auto regularGpio = dynamic_cast(gpioConfig.second); if(regularGpio == nullptr) { return GPIO_TYPE_FAILURE; } /* Check for conflicts and remove duplicates if necessary */ - result = checkForConflictsRegularGpio(gpioConfig.first, regularGpio, mapToAdd); + result = checkForConflictsRegularGpio(gpioConfig.first, *regularGpio, mapToAdd); if(result != HasReturnvaluesIF::RETURN_OK) { status = result; } @@ -259,17 +296,19 @@ ReturnValue_t LinuxLibgpioIF::checkForConflicts(GpioMap& mapToAdd){ ReturnValue_t LinuxLibgpioIF::checkForConflictsRegularGpio(gpioId_t gpioIdToCheck, - GpiodRegular* gpioToCheck, GpioMap& mapToAdd) { + GpiodRegularBase& gpioToCheck, GpioMap& mapToAdd) { /* Cross check with private map */ gpioMapIter = gpioMap.find(gpioIdToCheck); if(gpioMapIter != gpioMap.end()) { - if(gpioMapIter->second->gpioType != gpio::GpioTypes::GPIO_REGULAR) { + auto& gpioType = gpioMapIter->second->gpioType; + if(gpioType != gpio::GpioTypes::GPIO_REGULAR_BY_CHIP and + gpioType != gpio::GpioTypes::GPIO_REGULAR_BY_LABEL) { sif::warning << "LinuxLibgpioIF::checkForConflicts: ID already exists for different " "GPIO type" << gpioIdToCheck << ". Removing duplicate." << std::endl; mapToAdd.erase(gpioIdToCheck); return HasReturnvaluesIF::RETURN_OK; } - auto ownRegularGpio = dynamic_cast(gpioMapIter->second); + auto ownRegularGpio = dynamic_cast(gpioMapIter->second); if(ownRegularGpio == nullptr) { return GPIO_TYPE_FAILURE; } diff --git a/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.h b/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.h index 00e1bdfe..31e4a7e8 100644 --- a/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.h +++ b/hal/src/fsfw_hal/linux/gpio/LinuxLibgpioIF.h @@ -6,6 +6,7 @@ #include class GpioCookie; +class GpiodRegularIF; /** * @brief This class implements the GpioIF for a linux based system. The @@ -47,9 +48,13 @@ private: * @param gpioId The GPIO ID of the GPIO to drive. * @param logiclevel The logic level to set. O or 1. */ - ReturnValue_t driveGpio(gpioId_t gpioId, GpiodRegular* regularGpio, unsigned int logiclevel); + ReturnValue_t driveGpio(gpioId_t gpioId, GpiodRegularBase& regularGpio, + gpio::Levels logicLevel); - ReturnValue_t configureRegularGpio(gpioId_t gpioId, GpiodRegular* regularGpio); + ReturnValue_t configureGpioByLabel(gpioId_t gpioId, GpiodRegularByLabel& gpioByLabel); + ReturnValue_t configureGpioByChip(gpioId_t gpioId, GpiodRegularByChip& gpioByChip); + ReturnValue_t configureRegularGpio(gpioId_t gpioId, gpio::GpioTypes gpioType, + struct gpiod_chip* chip, GpiodRegularBase& regularGpio, std::string failOutput); /** * @brief This function checks if GPIOs are already registered and whether @@ -62,7 +67,7 @@ private: */ ReturnValue_t checkForConflicts(GpioMap& mapToAdd); - ReturnValue_t checkForConflictsRegularGpio(gpioId_t gpiodId, GpiodRegular* regularGpio, + ReturnValue_t checkForConflictsRegularGpio(gpioId_t gpiodId, GpiodRegularBase& regularGpio, GpioMap& mapToAdd); ReturnValue_t checkForConflictsCallbackGpio(gpioId_t gpiodId, GpioCallback* regularGpio, GpioMap& mapToAdd); diff --git a/hal/src/fsfw_hal/linux/rpi/GpioRPi.cpp b/hal/src/fsfw_hal/linux/rpi/GpioRPi.cpp index 6a71f291..e1c274c0 100644 --- a/hal/src/fsfw_hal/linux/rpi/GpioRPi.cpp +++ b/hal/src/fsfw_hal/linux/rpi/GpioRPi.cpp @@ -12,7 +12,7 @@ ReturnValue_t gpio::createRpiGpioConfig(GpioCookie* cookie, gpioId_t gpioId, int return HasReturnvaluesIF::RETURN_FAILED; } - GpiodRegular* config = new GpiodRegular(); + auto config = new GpiodRegularByChip(); /* Default chipname for Raspberry Pi. There is still gpiochip1 for expansion, but most users will not need this */ config->chipname = "gpiochip0"; From bfae25ff2dc68362c639ea8080e9b42e0c1d4add Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Sep 2021 18:06:04 +0200 Subject: [PATCH 02/24] Updates for SPI 1. Better names for functions 2. Reply size is set to 0 --- hal/src/fsfw_hal/linux/spi/SpiComIF.cpp | 34 +++++++++++++++--------- hal/src/fsfw_hal/linux/spi/SpiComIF.h | 1 + hal/src/fsfw_hal/linux/spi/SpiCookie.cpp | 2 +- hal/src/fsfw_hal/linux/spi/SpiCookie.h | 6 ++--- 4 files changed, 26 insertions(+), 17 deletions(-) diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp index 48bf7449..9c4e66ae 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.cpp @@ -15,11 +15,6 @@ #include #include -/* Can be used for low-level debugging of the SPI bus */ -#ifndef FSFW_HAL_LINUX_SPI_WIRETAPPING -#define FSFW_HAL_LINUX_SPI_WIRETAPPING 0 -#endif - SpiComIF::SpiComIF(object_id_t objectId, GpioIF* gpioComIF): SystemObject(objectId), gpioComIF(gpioComIF) { if(gpioComIF == nullptr) { @@ -146,8 +141,8 @@ ReturnValue_t SpiComIF::sendMessage(CookieIF *cookie, const uint8_t *sendData, s if(sendLen > spiCookie->getMaxBufferSize()) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "SpiComIF::sendMessage: Too much data sent, send length" << sendLen << - "larger than maximum buffer length" << spiCookie->getMaxBufferSize() << std::endl; + sif::warning << "SpiComIF::sendMessage: Too much data sent, send length " << sendLen << + "larger than maximum buffer length " << spiCookie->getMaxBufferSize() << std::endl; #else sif::printWarning("SpiComIF::sendMessage: Too much data sent, send length %lu larger " "than maximum buffer length %lu!\n", static_cast(sendLen), @@ -193,7 +188,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie *spiCookie, const spiCookie->getSpiParameters(spiMode, spiSpeed, nullptr); setSpiSpeedAndMode(fileDescriptor, spiMode, spiSpeed); spiCookie->assignWriteBuffer(sendData); - spiCookie->assignTransferSize(sendLen); + spiCookie->setTransferSize(sendLen); bool fullDuplex = spiCookie->isFullDuplex(); gpioId_t gpioId = spiCookie->getChipSelectPin(); @@ -202,12 +197,26 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie *spiCookie, const if(gpioId != gpio::NO_GPIO) { result = spiMutex->lockMutex(timeoutType, timeoutMs); if (result != RETURN_OK) { +#if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::error << "SpiComIF::sendMessage: Failed to lock mutex" << std::endl; +#else + sif::printError("SpiComIF::sendMessage: Failed to lock mutex\n"); +#endif +#endif + return result; + } + ReturnValue_t result = gpioComIF->pullLow(gpioId); + if(result != HasReturnvaluesIF::RETURN_OK) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "SpiComIF::sendMessage: Pulling low CS pin failed" << std::endl; +#else + sif::printWarning("SpiComIF::sendMessage: Pulling low CS pin failed"); +#endif #endif return result; } - gpioComIF->pullLow(gpioId); } /* Execute transfer */ @@ -218,7 +227,7 @@ ReturnValue_t SpiComIF::performRegularSendOperation(SpiCookie *spiCookie, const utility::handleIoctlError("SpiComIF::sendMessage: ioctl error."); result = FULL_DUPLEX_TRANSFER_FAILED; } -#if FSFW_HAL_LINUX_SPI_WIRETAPPING == 1 +#if FSFW_HAL_SPI_WIRETAPPING == 1 performSpiWiretapping(spiCookie); #endif /* FSFW_LINUX_SPI_WIRETAPPING == 1 */ } @@ -335,6 +344,7 @@ ReturnValue_t SpiComIF::readReceivedMessage(CookieIF *cookie, uint8_t **buffer, *buffer = rxBuf; *size = spiCookie->getCurrentTransferSize(); + spiCookie->setTransferSize(0); return HasReturnvaluesIF::RETURN_OK; } @@ -388,11 +398,11 @@ GpioIF* SpiComIF::getGpioInterface() { void SpiComIF::setSpiSpeedAndMode(int spiFd, spi::SpiModes mode, uint32_t speed) { int retval = ioctl(spiFd, SPI_IOC_WR_MODE, reinterpret_cast(&mode)); if(retval != 0) { - utility::handleIoctlError("SpiTestClass::performRm3100Test: Setting SPI mode failed!"); + utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Setting SPI mode failed"); } retval = ioctl(spiFd, SPI_IOC_WR_MAX_SPEED_HZ, &speed); if(retval != 0) { - utility::handleIoctlError("SpiTestClass::performRm3100Test: Setting SPI speed failed!"); + utility::handleIoctlError("SpiComIF::setSpiSpeedAndMode: Setting SPI speed failed"); } } diff --git a/hal/src/fsfw_hal/linux/spi/SpiComIF.h b/hal/src/fsfw_hal/linux/spi/SpiComIF.h index d43e2505..bcca7462 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiComIF.h +++ b/hal/src/fsfw_hal/linux/spi/SpiComIF.h @@ -1,6 +1,7 @@ #ifndef LINUX_SPI_SPICOMIF_H_ #define LINUX_SPI_SPICOMIF_H_ +#include "fsfw/FSFW.h" #include "spiDefinitions.h" #include "returnvalues/classIds.h" #include "fsfw_hal/common/gpio/GpioIF.h" diff --git a/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp b/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp index 54d8aa16..f07954e9 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp +++ b/hal/src/fsfw_hal/linux/spi/SpiCookie.cpp @@ -121,7 +121,7 @@ bool SpiCookie::isFullDuplex() const { return not this->halfDuplex; } -void SpiCookie::assignTransferSize(size_t transferSize) { +void SpiCookie::setTransferSize(size_t transferSize) { spiTransferStruct.len = transferSize; } diff --git a/hal/src/fsfw_hal/linux/spi/SpiCookie.h b/hal/src/fsfw_hal/linux/spi/SpiCookie.h index acf7c77c..844fd421 100644 --- a/hal/src/fsfw_hal/linux/spi/SpiCookie.h +++ b/hal/src/fsfw_hal/linux/spi/SpiCookie.h @@ -103,10 +103,10 @@ public: void assignReadBuffer(uint8_t* rx); void assignWriteBuffer(const uint8_t* tx); /** - * Assign size for the next transfer. + * Set size for the next transfer. Set to 0 for no transfer * @param transferSize */ - void assignTransferSize(size_t transferSize); + void setTransferSize(size_t transferSize); size_t getCurrentTransferSize() const; struct UncommonParameters { @@ -158,8 +158,6 @@ private: std::string spiDev, const size_t maxSize, spi::SpiModes spiMode, uint32_t spiSpeed, spi::send_callback_function_t callback, void* args); - size_t currentTransferSize = 0; - address_t spiAddress; gpioId_t chipSelectPin; std::string spiDevice; From fc9b85d5db29079ec96031a160a98f5caab65a81 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Sep 2021 18:06:54 +0200 Subject: [PATCH 03/24] update FSFW.h.in --- src/fsfw/FSFW.h.in | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/src/fsfw/FSFW.h.in b/src/fsfw/FSFW.h.in index f0eb9365..96460ae4 100644 --- a/src/fsfw/FSFW.h.in +++ b/src/fsfw/FSFW.h.in @@ -16,6 +16,11 @@ #cmakedefine FSFW_ADD_MONITORING #cmakedefine FSFW_ADD_SGP4_PROPAGATOR +// Can be used for low-level debugging of the SPI bus +#ifndef FSFW_HAL_SPI_WIRETAPPING +#define FSFW_HAL_SPI_WIRETAPPING 0 +#endif + #ifndef FSFW_HAL_L3GD20_GYRO_DEBUG #define FSFW_HAL_L3GD20_GYRO_DEBUG 0 #endif /* FSFW_HAL_L3GD20_GYRO_DEBUG */ From 85c04dee2340da305684a347c01c391ef41dfcc7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 27 Sep 2021 11:12:38 +0200 Subject: [PATCH 04/24] increase limit of packets stored --- src/fsfw/tmtcservices/TmTcBridge.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/tmtcservices/TmTcBridge.h b/src/fsfw/tmtcservices/TmTcBridge.h index d3689d19..4980caff 100644 --- a/src/fsfw/tmtcservices/TmTcBridge.h +++ b/src/fsfw/tmtcservices/TmTcBridge.h @@ -19,7 +19,7 @@ class TmTcBridge : public AcceptsTelemetryIF, public: static constexpr uint8_t TMTC_RECEPTION_QUEUE_DEPTH = 20; static constexpr uint8_t LIMIT_STORED_DATA_SENT_PER_CYCLE = 15; - static constexpr uint8_t LIMIT_DOWNLINK_PACKETS_STORED = 20; + static constexpr uint8_t LIMIT_DOWNLINK_PACKETS_STORED = 200; static constexpr uint8_t DEFAULT_STORED_DATA_SENT_PER_CYCLE = 5; static constexpr uint8_t DEFAULT_DOWNLINK_PACKETS_STORED = 10; From 09299802f0cc4456c510f6d6dac3260b96c08777 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:01:01 +0200 Subject: [PATCH 05/24] TCP refactoring This refactoring keeps the TCP connection opened until the client closes it. It also increased the robustness of the TCP reception. Because TCP is stream based and usually applied to newline separated data, a special way to handle binary space packets is required. The new SpacePacketParser class takes care of this by taking TC packet IDs as as optional start markers to parse for space packets in a given buffer. The refactored TCP server uses a ring buffer, a reception buffer and the new parser to extract space packets from a stream in a safer way. --- src/fsfw/osal/common/TcpTmTcServer.cpp | 229 ++++++++++++++++---- src/fsfw/osal/common/TcpTmTcServer.h | 68 ++++-- src/fsfw/tmtcpacket/SpacePacket.h | 109 +++++----- src/fsfw/tmtcservices/SpacePacketParser.cpp | 77 +++++++ src/fsfw/tmtcservices/SpacePacketParser.h | 96 ++++++++ 5 files changed, 473 insertions(+), 106 deletions(-) create mode 100644 src/fsfw/tmtcservices/SpacePacketParser.cpp create mode 100644 src/fsfw/tmtcservices/SpacePacketParser.h diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 11ab71af..e320b46b 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -1,8 +1,11 @@ -#include "fsfw/osal/common/TcpTmTcServer.h" -#include "fsfw/osal/common/TcpTmTcBridge.h" -#include "fsfw/osal/common/tcpipHelpers.h" - #include "fsfw/platform.h" +#include "fsfw/FSFW.h" + +#include "TcpTmTcServer.h" +#include "TcpTmTcBridge.h" +#include "tcpipHelpers.h" + +#include "fsfw/tasks/TaskFactory.h" #include "fsfw/container/SharedRingBuffer.h" #include "fsfw/ipc/MessageQueueSenderIF.h" #include "fsfw/ipc/MutexGuard.h" @@ -17,6 +20,7 @@ #elif defined(PLATFORM_UNIX) #include #endif +#include #ifndef FSFW_TCP_RECV_WIRETAPPING_ENABLED #define FSFW_TCP_RECV_WIRETAPPING_ENABLED 0 @@ -25,12 +29,11 @@ const std::string TcpTmTcServer::DEFAULT_SERVER_PORT = tcpip::DEFAULT_SERVER_PORT; TcpTmTcServer::TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, - size_t receptionBufferSize, std::string customTcpServerPort): - SystemObject(objectId), tmtcBridgeId(tmtcTcpBridge), - tcpPort(customTcpServerPort), receptionBuffer(receptionBufferSize) { - if(tcpPort == "") { - tcpPort = DEFAULT_SERVER_PORT; - } + size_t receptionBufferSize, size_t ringBufferSize, std::string customTcpServerPort, + ReceptionModes receptionMode): + SystemObject(objectId), tmtcBridgeId(tmtcTcpBridge), receptionMode(receptionMode), + tcpConfig(customTcpServerPort), receptionBuffer(receptionBufferSize), + ringBuffer(ringBufferSize, true), validPacketIds() { } ReturnValue_t TcpTmTcServer::initialize() { @@ -41,6 +44,16 @@ ReturnValue_t TcpTmTcServer::initialize() { return result; } + switch(receptionMode) { + case(ReceptionModes::SPACE_PACKETS): { + spacePacketParser = new SpacePacketParser(validPacketIds); + if(spacePacketParser == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } + tcpConfig.tcpFlags |= MSG_DONTWAIT; + } + } + tcStore = ObjectManager::instance()->get(objects::TC_STORE); if (tcStore == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -63,7 +76,7 @@ ReturnValue_t TcpTmTcServer::initialize() { hints.ai_flags = AI_PASSIVE; // Listen to all addresses (0.0.0.0) by using AI_PASSIVE in the hint flags - retval = getaddrinfo(nullptr, tcpPort.c_str(), &hints, &addrResult); + retval = getaddrinfo(nullptr, tcpConfig.tcpPort.c_str(), &hints, &addrResult); if (retval != 0) { handleError(Protocol::TCP, ErrorSources::GETADDRINFO_CALL); return HasReturnvaluesIF::RETURN_FAILED; @@ -105,7 +118,7 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { // Listen for connection requests permanently for lifetime of program while(true) { - retval = listen(listenerTcpSocket, tcpBacklog); + retval = listen(listenerTcpSocket, tcpConfig.tcpBacklog); if(retval == SOCKET_ERROR) { handleError(Protocol::TCP, ErrorSources::LISTEN_CALL, 500); continue; @@ -123,11 +136,12 @@ ReturnValue_t TcpTmTcServer::performOperation(uint8_t opCode) { handleServerOperation(connSocket); // Done, shut down connection and go back to listening for client requests - retval = shutdown(connSocket, SHUT_SEND); + retval = shutdown(connSocket, SHUT_BOTH); if(retval != 0) { handleError(Protocol::TCP, ErrorSources::SHUTDOWN_CALL); } closeSocket(connSocket); + connSocket = 0; } return HasReturnvaluesIF::RETURN_OK; } @@ -144,51 +158,82 @@ ReturnValue_t TcpTmTcServer::initializeAfterTaskCreation() { return HasReturnvaluesIF::RETURN_OK; } -void TcpTmTcServer::handleServerOperation(socket_t connSocket) { - int retval = 0; - do { - // Read all telecommands sent by the client - retval = recv(connSocket, +void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { + while (true) { + int retval = recv( + connSocket, reinterpret_cast(receptionBuffer.data()), receptionBuffer.capacity(), - tcpFlags); - if (retval > 0) { - handleTcReception(retval); + tcpConfig.tcpFlags + ); + if(retval == 0) { + // Client closed connection + return; } - else if(retval == 0) { - // Client has finished sending telecommands, send telemetry now - handleTmSending(connSocket); + else if(retval > 0) { + // The ring buffer was configured for overwrite, so the returnvalue does not need to + // be checked for now + ringBuffer.writeData(receptionBuffer.data(), retval); } - else { - // Should not happen - tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL); + else if(retval < 0) { + if(errno == EAGAIN) { + // No data available. Check whether any packets have been read, then send back + // telemetry if available + bool tcAvailable = false; + bool tmSent = false; + size_t availableReadData = ringBuffer.getAvailableReadData(); + if(availableReadData > lastRingBufferSize) { + tcAvailable = true; + handleTcRingBufferData(availableReadData); + } + ReturnValue_t result = handleTmSending(connSocket, tmSent); + if(result == CONN_BROKEN) { + return; + } + if(not tcAvailable and not tmSent) { + TaskFactory::delayTask(DEFAULT_LOOP_DELAY_MS); + } + } + else { + tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL); + } } - } while(retval > 0); + } } -ReturnValue_t TcpTmTcServer::handleTcReception(size_t bytesRecvd) { +ReturnValue_t TcpTmTcServer::handleTcReception(uint8_t* spacePacket, size_t packetSize) { #if FSFW_TCP_RECV_WIRETAPPING_ENABLED == 1 arrayprinter::print(receptionBuffer.data(), bytesRead); #endif + if(spacePacket == nullptr or packetSize == 0) { + return HasReturnvaluesIF::RETURN_FAILED; + } store_address_t storeId; - ReturnValue_t result = tcStore->addData(&storeId, receptionBuffer.data(), bytesRecvd); + ReturnValue_t result = tcStore->addData(&storeId, spacePacket, packetSize); if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning<< "TcpTmTcServer::handleServerOperation: Data storage failed." << std::endl; - sif::warning << "Packet size: " << bytesRecvd << std::endl; + sif::warning << "TcpTmTcServer::handleServerOperation: Data storage with packet size" << + packetSize << " failed" << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Data storage with packet size %d " + "failed\n", packetSize); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif /* FSFW_VERBOSE_LEVEL >= 1 */ + return result; } TmTcMessage message(storeId); - result = MessageQueueSenderIF::sendMessage(targetTcDestination, &message); + result = MessageQueueSenderIF::sendMessage(targetTcDestination, &message); if (result != HasReturnvaluesIF::RETURN_OK) { #if FSFW_VERBOSE_LEVEL >= 1 #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::warning << "UdpTcPollingTask::handleSuccessfullTcRead: " + sif::warning << "TcpTmTcServer::handleServerOperation: " " Sending message to queue failed" << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: " + " Sending message to queue failed\n"); #endif /* FSFW_CPP_OSTREAM_ENABLED == 1 */ #endif /* FSFW_VERBOSE_LEVEL >= 1 */ tcStore->deleteData(storeId); @@ -196,21 +241,26 @@ ReturnValue_t TcpTmTcServer::handleTcReception(size_t bytesRecvd) { return result; } -void TcpTmTcServer::setTcpBacklog(uint8_t tcpBacklog) { - this->tcpBacklog = tcpBacklog; -} - std::string TcpTmTcServer::getTcpPort() const { - return tcpPort; + return tcpConfig.tcpPort; } -ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket) { +void TcpTmTcServer::setSpacePacketParsingOptions(std::vector validPacketIds) { + this->validPacketIds = validPacketIds; +} + +TcpTmTcServer::TcpConfig& TcpTmTcServer::getTcpConfigStruct() { + return tcpConfig; +} + +ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) { // Access to the FIFO is mutex protected because it is filled by the bridge MutexGuard(tmtcBridge->mutex, tmtcBridge->timeoutType, tmtcBridge->mutexTimeoutMs); store_address_t storeId; while((not tmtcBridge->tmFifo->empty()) and (tmtcBridge->packetSentCounter < tmtcBridge->sentPacketsPerCycle)) { - tmtcBridge->tmFifo->retrieve(&storeId); + // Send can fail, so only peek from the FIFO + tmtcBridge->tmFifo->peek(&storeId); // Using the store accessor will take care of deleting TM from the store automatically ConstStorageAccessor storeAccessor(storeId); @@ -221,10 +271,101 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket) { int retval = send(connSocket, reinterpret_cast(storeAccessor.data()), storeAccessor.size(), - tcpTmFlags); - if(retval != static_cast(storeAccessor.size())) { - tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::SEND_CALL); + tcpConfig.tcpTmFlags); + if(retval == static_cast(storeAccessor.size())) { + // Packet sent, clear FIFO entry + tmtcBridge->tmFifo->pop(); + tmSent = true; + + } + else if(retval <= 0) { + // Assume that the client has closed the connection here for now + handleSocketError(storeAccessor); + return CONN_BROKEN; } } return HasReturnvaluesIF::RETURN_OK; } + +ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { + ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + size_t readAmount = availableReadData; + lastRingBufferSize = availableReadData; + if(readAmount >= ringBuffer.getMaxSize()) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + // Possible configuration error, too much data or/and data coming in too fast, + // requiring larger buffers + sif::warning << "TcpTmTcServer::handleServerOperation: Ring buffer reached " << + "fill count" << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Ring buffer reached " + "fill count"); +#endif +#endif + } + if(readAmount >= receptionBuffer.size()) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + // Possible configuration error, too much data or/and data coming in too fast, + // requiring larger buffers + sif::warning << "TcpTmTcServer::handleServerOperation: " + "Reception buffer too small " << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Reception buffer too small\n"); +#endif +#endif + readAmount = receptionBuffer.size(); + } + ringBuffer.readData(receptionBuffer.data(), readAmount, true); + const uint8_t* bufPtr = receptionBuffer.data(); + const uint8_t** bufPtrPtr = &bufPtr; + size_t startIdx = 0; + size_t foundSize = 0; + size_t readLen = 0; + while(readLen < readAmount) { + result = spacePacketParser->parseSpacePackets(bufPtrPtr, readAmount, + startIdx, foundSize, readLen); + switch(result) { + case(SpacePacketParser::NO_PACKET_FOUND): + case(SpacePacketParser::SPLIT_PACKET): { + break; + } + case(HasReturnvaluesIF::RETURN_OK): { + result = handleTcReception(receptionBuffer.data() + startIdx, foundSize); + if(result != HasReturnvaluesIF::RETURN_OK) { + status = result; + } + } + } + ringBuffer.deleteData(foundSize); + lastRingBufferSize = ringBuffer.getAvailableReadData(); + std::memset(receptionBuffer.data() + startIdx, 0, foundSize); + } + return status; +} + +void TcpTmTcServer::handleSocketError(ConstStorageAccessor &accessor) { + // Don't delete data + accessor.release(); + auto socketError = getLastSocketError(); + switch(socketError) { +#if defined PLATFORM_WIN + case(WSAECONNRESET): { + // See https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-send + // Remote client might have shut down connection + return; + } +#else + case(EPIPE): { + // See https://man7.org/linux/man-pages/man2/send.2.html + // Remote client might have shut down connection + return; + } +#endif + default: { + tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::SEND_CALL); + } + } +} diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index c6916080..a0a31655 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -1,11 +1,13 @@ #ifndef FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ #define FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ +#include #include "TcpIpBase.h" #include "fsfw/platform.h" #include "fsfw/osal/common/tcpipHelpers.h" #include "fsfw/ipc/messageQueueDefinitions.h" +#include "fsfw/container/SimpleRingBuffer.h" #include "fsfw/ipc/MessageQueueIF.h" #include "fsfw/objectmanager/frameworkObjects.h" #include "fsfw/objectmanager/SystemObject.h" @@ -42,9 +44,37 @@ class TcpTmTcServer: public TcpIpBase, public ExecutableObjectIF { public: + enum class ReceptionModes { + SPACE_PACKETS + }; + + struct TcpConfig { + public: + TcpConfig(std::string tcpPort): tcpPort(tcpPort) {} + + /** + * Passed to the recv call + */ + int tcpFlags = 0; + int tcpBacklog = 3; + + /** + * Passed to the select call which is used to ensure non-blocking TC reception + */ + //uint32_t selectTimeoutMs = DEFAULT_SELECT_TIMEOUT_MS; + /** + * Passed to the send call + */ + int tcpTmFlags = 0; + + const std::string tcpPort; + }; + static const std::string DEFAULT_SERVER_PORT; static constexpr size_t ETHERNET_MTU_SIZE = 1500; + static constexpr size_t RING_BUFFER_SIZE = ETHERNET_MTU_SIZE * 3; + static constexpr uint32_t DEFAULT_LOOP_DELAY_MS = 200; /** * TCP Server Constructor @@ -55,11 +85,19 @@ public: * @param customTcpServerPort The user can specify another port than the default (7301) here. */ TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, - size_t receptionBufferSize = ETHERNET_MTU_SIZE + 1, - std::string customTcpServerPort = ""); + size_t receptionBufferSize = RING_BUFFER_SIZE, + size_t ringBufferSize = RING_BUFFER_SIZE, + std::string customTcpServerPort = DEFAULT_SERVER_PORT, + ReceptionModes receptionMode = ReceptionModes::SPACE_PACKETS); virtual~ TcpTmTcServer(); - void setTcpBacklog(uint8_t tcpBacklog); + /** + * Get a handle to the TCP configuration struct, which can be used to configure TCP + * properties + * @return + */ + TcpConfig& getTcpConfigStruct(); + void setSpacePacketParsingOptions(std::vector validPacketIds); ReturnValue_t initialize() override; ReturnValue_t performOperation(uint8_t opCode) override; @@ -71,25 +109,29 @@ protected: StorageManagerIF* tcStore = nullptr; StorageManagerIF* tmStore = nullptr; private: + static constexpr ReturnValue_t CONN_BROKEN = HasReturnvaluesIF::makeReturnCode(1, 0); //! TMTC bridge is cached. object_id_t tmtcBridgeId = objects::NO_OBJECT; TcpTmTcBridge* tmtcBridge = nullptr; - std::string tcpPort; - int tcpFlags = 0; - socket_t listenerTcpSocket = 0; + ReceptionModes receptionMode; + TcpConfig tcpConfig; struct sockaddr tcpAddress; + socket_t listenerTcpSocket = 0; + MessageQueueId_t targetTcDestination = MessageQueueIF::NO_QUEUE; - int tcpAddrLen = sizeof(tcpAddress); - int tcpBacklog = 3; std::vector receptionBuffer; - int tcpSockOpt = 0; - int tcpTmFlags = 0; + SimpleRingBuffer ringBuffer; + std::vector validPacketIds; + SpacePacketParser* spacePacketParser = nullptr; + uint8_t lastRingBufferSize = 0; - void handleServerOperation(socket_t connSocket); - ReturnValue_t handleTcReception(size_t bytesRecvd); - ReturnValue_t handleTmSending(socket_t connSocket); + virtual void handleServerOperation(socket_t& connSocket); + ReturnValue_t handleTcReception(uint8_t* spacePacket, size_t packetSize); + ReturnValue_t handleTmSending(socket_t connSocket, bool& tmSent); + ReturnValue_t handleTcRingBufferData(size_t availableReadData); + void handleSocketError(ConstStorageAccessor& accessor); }; #endif /* FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ */ diff --git a/src/fsfw/tmtcpacket/SpacePacket.h b/src/fsfw/tmtcpacket/SpacePacket.h index 49dd5ae5..9eec87a8 100644 --- a/src/fsfw/tmtcpacket/SpacePacket.h +++ b/src/fsfw/tmtcpacket/SpacePacket.h @@ -15,56 +15,67 @@ */ class SpacePacket: public SpacePacketBase { public: - static const uint16_t PACKET_MAX_SIZE = 1024; - /** - * The constructor initializes the packet and sets all header information - * according to the passed parameters. - * @param packetDataLength Sets the packet data length field and therefore specifies the size of the packet. - * @param isTelecommand Sets the packet type field to either TC (true) or TM (false). - * @param apid Sets the packet's APID field. The default value describes an idle packet. - * @param sequenceCount ets the packet's Source Sequence Count field. - */ - SpacePacket(uint16_t packetDataLength, bool isTelecommand = false, - uint16_t apid = APID_IDLE_PACKET, uint16_t sequenceCount = 0); - /** - * The class's default destructor. - */ - virtual ~SpacePacket(); - /** - * With this call, the complete data content (including the CCSDS Primary - * Header) is overwritten with the byte stream given. - * @param p_data Pointer to data to overwrite the content with - * @param packet_size Size of the data - * @return @li \c true if packet_size is smaller than \c MAX_PACKET_SIZE. - * @li \c false else. - */ - bool addWholeData(const uint8_t* p_data, uint32_t packet_size); + static const uint16_t PACKET_MAX_SIZE = 1024; + /** + * The constructor initializes the packet and sets all header information + * according to the passed parameters. + * @param packetDataLength Sets the packet data length field and therefore specifies + * the size of the packet. + * @param isTelecommand Sets the packet type field to either TC (true) or TM (false). + * @param apid Sets the packet's APID field. The default value describes an idle packet. + * @param sequenceCount ets the packet's Source Sequence Count field. + */ + SpacePacket(uint16_t packetDataLength, bool isTelecommand = false, + uint16_t apid = APID_IDLE_PACKET, uint16_t sequenceCount = 0); + /** + * The class's default destructor. + */ + virtual ~SpacePacket(); + + static constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { + uint16_t tcPacketId = (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); + return tcPacketId; + } + static constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { + uint16_t tmPacketId = (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); + return tmPacketId; + } + + /** + * With this call, the complete data content (including the CCSDS Primary + * Header) is overwritten with the byte stream given. + * @param p_data Pointer to data to overwrite the content with + * @param packet_size Size of the data + * @return @li \c true if packet_size is smaller than \c MAX_PACKET_SIZE. + * @li \c false else. + */ + bool addWholeData(const uint8_t* p_data, uint32_t packet_size); protected: - /** - * This structure defines the data structure of a Space Packet as local data. - * There's a buffer which corresponds to the Space Packet Data Field with a - * maximum size of \c PACKET_MAX_SIZE. - */ - struct PacketStructured { - CCSDSPrimaryHeader header; - uint8_t buffer[PACKET_MAX_SIZE]; - }; - /** - * This union simplifies accessing the full data content of the Space Packet. - * This is achieved by putting the \c PacketStructured struct in a union with - * a plain buffer. - */ - union SpacePacketData { - PacketStructured fields; - uint8_t byteStream[PACKET_MAX_SIZE + sizeof(CCSDSPrimaryHeader)]; - }; - /** - * This is the data representation of the class. - * It is a struct of CCSDS Primary Header and a data field, which again is - * packed in an union, so the data can be accessed as a byte stream without - * a cast. - */ - SpacePacketData localData; + /** + * This structure defines the data structure of a Space Packet as local data. + * There's a buffer which corresponds to the Space Packet Data Field with a + * maximum size of \c PACKET_MAX_SIZE. + */ + struct PacketStructured { + CCSDSPrimaryHeader header; + uint8_t buffer[PACKET_MAX_SIZE]; + }; + /** + * This union simplifies accessing the full data content of the Space Packet. + * This is achieved by putting the \c PacketStructured struct in a union with + * a plain buffer. + */ + union SpacePacketData { + PacketStructured fields; + uint8_t byteStream[PACKET_MAX_SIZE + sizeof(CCSDSPrimaryHeader)]; + }; + /** + * This is the data representation of the class. + * It is a struct of CCSDS Primary Header and a data field, which again is + * packed in an union, so the data can be accessed as a byte stream without + * a cast. + */ + SpacePacketData localData; }; #endif /* SPACEPACKET_H_ */ diff --git a/src/fsfw/tmtcservices/SpacePacketParser.cpp b/src/fsfw/tmtcservices/SpacePacketParser.cpp new file mode 100644 index 00000000..84f861cf --- /dev/null +++ b/src/fsfw/tmtcservices/SpacePacketParser.cpp @@ -0,0 +1,77 @@ +#include +#include +#include + +SpacePacketParser::SpacePacketParser(std::vector validPacketIds): + validPacketIds(validPacketIds) { +} + +ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t *buffer, + const size_t maxSize, size_t& startIndex, size_t& foundSize) { + const uint8_t** tempPtr = &buffer; + size_t readLen = 0; + return parseSpacePackets(tempPtr, maxSize, startIndex, foundSize, readLen); +} + +ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t **buffer, const size_t maxSize, + size_t &startIndex, size_t &foundSize, size_t& readLen) { + if(buffer == nullptr or maxSize < 5) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "SpacePacketParser::parseSpacePackets: Frame invalid" << std::endl; +#else + sif::printWarning("SpacePacketParser::parseSpacePackets: Frame invalid\n"); +#endif + return HasReturnvaluesIF::RETURN_FAILED; + } + const uint8_t* bufPtr = *buffer; + + auto verifyLengthField = [&](size_t idx) { + uint16_t lengthField = bufPtr[idx + 4] << 8 | bufPtr[idx + 5]; + size_t packetSize = lengthField + 7; + startIndex = idx; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + if(lengthField == 0) { + // Skip whole header for now + foundSize = 6; + result = NO_PACKET_FOUND; + } + else if(packetSize + idx > maxSize) { + // Don't increment buffer and read length here, user has to decide what to do + foundSize = packetSize; + return SPLIT_PACKET; + } + else { + foundSize = packetSize; + } + *buffer += foundSize; + readLen += foundSize; + return result; + }; + + size_t idx = 0; + // Space packet ID as start marker + if(validPacketIds.size() > 0) { + while(idx < maxSize - 5) { + uint16_t currentPacketId = bufPtr[idx] << 8 | bufPtr[idx + 1]; + if(std::find(validPacketIds.begin(), validPacketIds.end(), currentPacketId) != + validPacketIds.end()) { + if(idx + 5 >= maxSize) { + return SPLIT_PACKET; + } + return verifyLengthField(idx); + } + else { + idx++; + } + } + startIndex = 0; + foundSize = maxSize; + *buffer += foundSize; + readLen += foundSize; + return NO_PACKET_FOUND; + } + // Assume that the user verified a valid start of a space packet + else { + return verifyLengthField(idx); + } +} diff --git a/src/fsfw/tmtcservices/SpacePacketParser.h b/src/fsfw/tmtcservices/SpacePacketParser.h new file mode 100644 index 00000000..16b53ea4 --- /dev/null +++ b/src/fsfw/tmtcservices/SpacePacketParser.h @@ -0,0 +1,96 @@ +#ifndef FRAMEWORK_TMTCSERVICES_PUSPARSER_H_ +#define FRAMEWORK_TMTCSERVICES_PUSPARSER_H_ + +#include "fsfw/container/DynamicFIFO.h" +#include "fsfw/returnvalues/FwClassIds.h" + +#include +#include + +/** + * @brief This small helper class scans a given buffer for PUS packets. + * Can be used if PUS packets are serialized in a tightly packed frame. + * @details + * The parser uses the length field field of the space packets to find + * the respective space packet sizes. + * + * The parser parses a buffer by taking a pointer and the maximum size to scan. + * If space packets are found, they are stored in a FIFO which stores pairs + * consisting of the index in the buffer and the respective packet sizes. + * + * If the parser detects split packets (which means that the size of the + * next packet is larger than the remaining size to scan), it can either + * store that split packet or throw away the packet. + * @author R. Mueller + */ +class SpacePacketParser { +public: + //! The first entry is the index inside the buffer while the second index + //! is the size of the PUS packet starting at that index. + using IndexSizePair = std::pair; + + static constexpr uint8_t INTERFACE_ID = CLASS_ID::PUS_PARSER; + static constexpr ReturnValue_t NO_PACKET_FOUND = MAKE_RETURN_CODE(0x00); + static constexpr ReturnValue_t SPLIT_PACKET = MAKE_RETURN_CODE(0x01); + + /** + * @brief Parser constructor. + * @param maxExpectedPusPackets + * Maximum expected number of PUS packets. A good estimate is to divide + * the frame size by the minimum size of a PUS packet (12 bytes) + * @param storeSplitPackets + * Specifies whether split packets are also stored inside the FIFO, + * with the size being the remaining frame size. + */ + SpacePacketParser(std::vector validPacketIds); + + /** + * Parse a given frame for PUS packets + * @param frame + * @param frameSize + * @param foundPackets The number of found packets will be stored here + * @return + * -@c NO_PACKET_FOUND if no packet was found + * -@c SPLIT_PACKET if splitting is enabled and a split packet was found + * -@c RETURN_OK if a packet was found. The index and sizes are stored in the internal FIFO + */ + ReturnValue_t parseSpacePackets(const uint8_t* buffer, const size_t maxSize, + size_t& startIndex, size_t& foundSize); + + ReturnValue_t parseSpacePackets(const uint8_t **buffer, const size_t maxSize, + size_t& startIndex, size_t& foundSize, size_t& readLen); + /** + * Accessor function to get a reference to the internal FIFO which + * stores pairs of index and packet sizes. This FIFO is filled + * by the #parsePusPackets function. + * @return + */ + //DynamicFIFO& fifo(); + + /** + * Retrieve the next index and packet size pair from the FIFO. + * This also removes it from the FIFO. Please note that if the FIFO + * is empty, an empty pair will be returned. + * @return + */ + //IndexSizePair getNextFifoPair(); + +private: + /** + * A FIFO is used to store information about multiple PUS packets + * inside the receive buffer. The maximum number of entries is defined + * by the first constructor argument. + */ + //DynamicFIFO indexSizePairFIFO; + + std::vector validPacketIds; + + //bool storeSplitPackets = false; + +// ReturnValue_t readMultiplePackets(const uint8_t *frame, size_t frameSize, +// size_t startIndex, uint32_t& foundPackets); +// ReturnValue_t readNextPacket(const uint8_t *frame, +// size_t frameSize, size_t& startIndex, uint32_t& foundPackets); +}; + +#endif /* FRAMEWORK_TMTCSERVICES_PUSPARSER_H_ */ From e536918804255bf035ba78e5ddb9171b01f02761 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:09:56 +0200 Subject: [PATCH 06/24] wiretapping in runtime config now --- src/fsfw/osal/common/TcpTmTcServer.cpp | 24 +++++++++++++++--------- src/fsfw/osal/common/TcpTmTcServer.h | 5 ++++- 2 files changed, 19 insertions(+), 10 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index e320b46b..5be9373e 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -5,12 +5,13 @@ #include "TcpTmTcBridge.h" #include "tcpipHelpers.h" +#include "fsfw/tmtcservices/SpacePacketParser.h" #include "fsfw/tasks/TaskFactory.h" +#include "fsfw/globalfunctions/arrayprinter.h" #include "fsfw/container/SharedRingBuffer.h" #include "fsfw/ipc/MessageQueueSenderIF.h" #include "fsfw/ipc/MutexGuard.h" #include "fsfw/objectmanager/ObjectManager.h" - #include "fsfw/serviceinterface/ServiceInterface.h" #include "fsfw/tmtcservices/TmTcMessage.h" @@ -20,11 +21,6 @@ #elif defined(PLATFORM_UNIX) #include #endif -#include - -#ifndef FSFW_TCP_RECV_WIRETAPPING_ENABLED -#define FSFW_TCP_RECV_WIRETAPPING_ENABLED 0 -#endif const std::string TcpTmTcServer::DEFAULT_SERVER_PORT = tcpip::DEFAULT_SERVER_PORT; @@ -202,9 +198,11 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { } ReturnValue_t TcpTmTcServer::handleTcReception(uint8_t* spacePacket, size_t packetSize) { -#if FSFW_TCP_RECV_WIRETAPPING_ENABLED == 1 - arrayprinter::print(receptionBuffer.data(), bytesRead); -#endif + if(wiretappingEnabled) { + sif::info << "Received TC:" << std::endl; + arrayprinter::print(spacePacket, packetSize); + } + if(spacePacket == nullptr or packetSize == 0) { return HasReturnvaluesIF::RETURN_FAILED; } @@ -268,6 +266,10 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) if(result != HasReturnvaluesIF::RETURN_OK) { return result; } + if(wiretappingEnabled) { + sif::info << "Sending TM:" << std::endl; + arrayprinter::print(storeAccessor.data(), storeAccessor.size()); + } int retval = send(connSocket, reinterpret_cast(storeAccessor.data()), storeAccessor.size(), @@ -346,6 +348,10 @@ ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { return status; } +void TcpTmTcServer::enableWiretapping(bool enable) { + this->wiretappingEnabled = enable; +} + void TcpTmTcServer::handleSocketError(ConstStorageAccessor &accessor) { // Don't delete data accessor.release(); diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index a0a31655..d5214848 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -1,7 +1,6 @@ #ifndef FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ #define FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ -#include #include "TcpIpBase.h" #include "fsfw/platform.h" @@ -22,6 +21,7 @@ #include class TcpTmTcBridge; +class SpacePacketParser; /** * @brief TCP server implementation @@ -91,6 +91,8 @@ public: ReceptionModes receptionMode = ReceptionModes::SPACE_PACKETS); virtual~ TcpTmTcServer(); + void enableWiretapping(bool enable); + /** * Get a handle to the TCP configuration struct, which can be used to configure TCP * properties @@ -113,6 +115,7 @@ private: //! TMTC bridge is cached. object_id_t tmtcBridgeId = objects::NO_OBJECT; TcpTmTcBridge* tmtcBridge = nullptr; + bool wiretappingEnabled = false; ReceptionModes receptionMode; TcpConfig tcpConfig; From bbea5e33bc66d05589d6143c0e75931acd99de83 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:12:23 +0200 Subject: [PATCH 07/24] removed obsolete empty ctor --- src/fsfw/osal/common/TcpTmTcServer.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 5be9373e..fb421fc7 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -29,7 +29,7 @@ TcpTmTcServer::TcpTmTcServer(object_id_t objectId, object_id_t tmtcTcpBridge, ReceptionModes receptionMode): SystemObject(objectId), tmtcBridgeId(tmtcTcpBridge), receptionMode(receptionMode), tcpConfig(customTcpServerPort), receptionBuffer(receptionBufferSize), - ringBuffer(ringBufferSize, true), validPacketIds() { + ringBuffer(ringBufferSize, true) { } ReturnValue_t TcpTmTcServer::initialize() { From f02852d8d2bf40b2941ccd0816c1ed01d703fea7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:13:46 +0200 Subject: [PATCH 08/24] cmake lists file update --- src/fsfw/tmtcservices/CMakeLists.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw/tmtcservices/CMakeLists.txt b/src/fsfw/tmtcservices/CMakeLists.txt index c30af214..96cf99b5 100644 --- a/src/fsfw/tmtcservices/CMakeLists.txt +++ b/src/fsfw/tmtcservices/CMakeLists.txt @@ -6,4 +6,5 @@ target_sources(${LIB_FSFW_NAME} TmTcBridge.cpp TmTcMessage.cpp VerificationReporter.cpp + SpacePacketParser.cpp ) \ No newline at end of file From a4d6421510575b9eda7f27c1637963c91881f922 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:30:01 +0200 Subject: [PATCH 09/24] documentation and bugfixes --- src/fsfw/tmtcservices/SpacePacketParser.cpp | 2 +- src/fsfw/tmtcservices/SpacePacketParser.h | 100 ++++++++------------ 2 files changed, 42 insertions(+), 60 deletions(-) diff --git a/src/fsfw/tmtcservices/SpacePacketParser.cpp b/src/fsfw/tmtcservices/SpacePacketParser.cpp index 84f861cf..3d442458 100644 --- a/src/fsfw/tmtcservices/SpacePacketParser.cpp +++ b/src/fsfw/tmtcservices/SpacePacketParser.cpp @@ -44,7 +44,7 @@ ReturnValue_t SpacePacketParser::parseSpacePackets(const uint8_t **buffer, const foundSize = packetSize; } *buffer += foundSize; - readLen += foundSize; + readLen += idx + foundSize; return result; }; diff --git a/src/fsfw/tmtcservices/SpacePacketParser.h b/src/fsfw/tmtcservices/SpacePacketParser.h index 16b53ea4..82b15010 100644 --- a/src/fsfw/tmtcservices/SpacePacketParser.h +++ b/src/fsfw/tmtcservices/SpacePacketParser.h @@ -8,19 +8,11 @@ #include /** - * @brief This small helper class scans a given buffer for PUS packets. - * Can be used if PUS packets are serialized in a tightly packed frame. + * @brief This small helper class scans a given buffer for space packets. + * Can be used if space packets are serialized in a tightly packed frame. * @details - * The parser uses the length field field of the space packets to find - * the respective space packet sizes. - * - * The parser parses a buffer by taking a pointer and the maximum size to scan. - * If space packets are found, they are stored in a FIFO which stores pairs - * consisting of the index in the buffer and the respective packet sizes. - * - * If the parser detects split packets (which means that the size of the - * next packet is larger than the remaining size to scan), it can either - * store that split packet or throw away the packet. + * The parser uses the length field field and the 16-bit TC packet ID of the space packets to find + * find space packets in a given data stream * @author R. Mueller */ class SpacePacketParser { @@ -35,62 +27,52 @@ public: /** * @brief Parser constructor. - * @param maxExpectedPusPackets - * Maximum expected number of PUS packets. A good estimate is to divide - * the frame size by the minimum size of a PUS packet (12 bytes) - * @param storeSplitPackets - * Specifies whether split packets are also stored inside the FIFO, - * with the size being the remaining frame size. + * @param validPacketIds This vector contains the allowed 16-bit TC packet ID start markers + * The parser will search for these stark markers to detect the start of a space packet. + * It is also possible to pass an empty vector here, but this is not recommended. + * If an empty vector is passed, the parser will assume that the start of the given stream + * contains the start of a new space packet. */ SpacePacketParser(std::vector validPacketIds); + /** + * Parse a given frame for space packets but also increment the given buffer and assign the + * total number of bytes read so far + * @param buffer Parser will look for space packets in this buffer + * @param maxSize Maximum size of the buffer + * @param startIndex Start index of a found space packet + * @param foundSize Found size of the space packet + * @param readLen Length read so far. This value is incremented by the number of parsed + * bytes which also includes the size of a found packet + * -@c NO_PACKET_FOUND if no packet was found in the given buffer or the length field is + * invalid. foundSize will be set to the size of the space packet header. buffer and + * readLen will be incremented accordingly. + * -@c SPLIT_PACKET if a packet was found but the detected size exceeds maxSize. foundSize + * will be set to the detected packet size and startIndex will be set to the start of the + * detected packet. buffer and read length will not be incremented but the found length + * will be assigned. + * -@c RETURN_OK if a packet was found + */ + ReturnValue_t parseSpacePackets(const uint8_t **buffer, const size_t maxSize, + size_t& startIndex, size_t& foundSize, size_t& readLen); + /** - * Parse a given frame for PUS packets - * @param frame - * @param frameSize - * @param foundPackets The number of found packets will be stored here - * @return - * -@c NO_PACKET_FOUND if no packet was found - * -@c SPLIT_PACKET if splitting is enabled and a split packet was found - * -@c RETURN_OK if a packet was found. The index and sizes are stored in the internal FIFO + * Parse a given frame for space packets + * @param buffer Parser will look for space packets in this buffer + * @param maxSize Maximum size of the buffer + * @param startIndex Start index of a found space packet + * @param foundSize Found size of the space packet + * -@c NO_PACKET_FOUND if no packet was found in the given buffer or the length field is + * invalid. foundSize will be set to the size of the space packet header + * -@c SPLIT_PACKET if a packet was found but the detected size exceeds maxSize. foundSize + * will be set to the detected packet size and startIndex will be set to the start of the + * detected packet + * -@c RETURN_OK if a packet was found */ ReturnValue_t parseSpacePackets(const uint8_t* buffer, const size_t maxSize, size_t& startIndex, size_t& foundSize); - - ReturnValue_t parseSpacePackets(const uint8_t **buffer, const size_t maxSize, - size_t& startIndex, size_t& foundSize, size_t& readLen); - /** - * Accessor function to get a reference to the internal FIFO which - * stores pairs of index and packet sizes. This FIFO is filled - * by the #parsePusPackets function. - * @return - */ - //DynamicFIFO& fifo(); - - /** - * Retrieve the next index and packet size pair from the FIFO. - * This also removes it from the FIFO. Please note that if the FIFO - * is empty, an empty pair will be returned. - * @return - */ - //IndexSizePair getNextFifoPair(); - private: - /** - * A FIFO is used to store information about multiple PUS packets - * inside the receive buffer. The maximum number of entries is defined - * by the first constructor argument. - */ - //DynamicFIFO indexSizePairFIFO; - std::vector validPacketIds; - - //bool storeSplitPackets = false; - -// ReturnValue_t readMultiplePackets(const uint8_t *frame, size_t frameSize, -// size_t startIndex, uint32_t& foundPackets); -// ReturnValue_t readNextPacket(const uint8_t *frame, -// size_t frameSize, size_t& startIndex, uint32_t& foundPackets); }; #endif /* FRAMEWORK_TMTCSERVICES_PUSPARSER_H_ */ From 1622e23f1c15ddcf1d4b522c332c1ab156f91fd9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:32:58 +0200 Subject: [PATCH 10/24] delay configurable --- src/fsfw/osal/common/TcpTmTcServer.cpp | 2 +- src/fsfw/osal/common/TcpTmTcServer.h | 5 +++-- 2 files changed, 4 insertions(+), 3 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index fb421fc7..c3936146 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -187,7 +187,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { return; } if(not tcAvailable and not tmSent) { - TaskFactory::delayTask(DEFAULT_LOOP_DELAY_MS); + TaskFactory::delayTask(tcpConfig.tcpLoopDelay); } } else { diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index d5214848..da0e8bd5 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -59,9 +59,10 @@ public: int tcpBacklog = 3; /** - * Passed to the select call which is used to ensure non-blocking TC reception + * If no telecommands packets are being received and no telemetry is being sent, + * the TCP server will delay periodically by this amount to decrease the CPU load */ - //uint32_t selectTimeoutMs = DEFAULT_SELECT_TIMEOUT_MS; + uint32_t tcpLoopDelay = DEFAULT_LOOP_DELAY_MS ; /** * Passed to the send call */ From 93ad9be5d6074f9ff7452547456d0751e3dfe383 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:42:50 +0200 Subject: [PATCH 11/24] removed include --- hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.h | 1 - 1 file changed, 1 deletion(-) diff --git a/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.h b/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.h index 1ba680cb..6627cbb7 100644 --- a/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.h +++ b/hal/src/fsfw_hal/devicehandlers/MgmRM3100Handler.h @@ -2,7 +2,6 @@ #define MISSION_DEVICES_MGMRM3100HANDLER_H_ #include "fsfw/FSFW.h" -#include "devices/powerSwitcherList.h" #include "devicedefinitions/MgmRM3100HandlerDefs.h" #include "fsfw/devicehandlers/DeviceHandlerBase.h" From 5d9e5e222ad7e2dbfda692db5091f805ef488bc9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 15:45:45 +0200 Subject: [PATCH 12/24] removed C++14 featue --- .../devicehandlers/devicedefinitions/MgmRM3100HandlerDefs.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/hal/src/fsfw_hal/devicehandlers/devicedefinitions/MgmRM3100HandlerDefs.h b/hal/src/fsfw_hal/devicehandlers/devicedefinitions/MgmRM3100HandlerDefs.h index 0ee2c7f6..b6375692 100644 --- a/hal/src/fsfw_hal/devicehandlers/devicedefinitions/MgmRM3100HandlerDefs.h +++ b/hal/src/fsfw_hal/devicehandlers/devicedefinitions/MgmRM3100HandlerDefs.h @@ -49,7 +49,7 @@ static constexpr uint8_t TMRC_DEFAULT_VALUE = TMRC_DEFAULT_37HZ_VALUE; static constexpr uint8_t MEASUREMENT_REG_START = 0x24; static constexpr uint8_t BIST_REGISTER = 0x33; -static constexpr uint8_t DATA_READY_VAL = 0b1000'0000; +static constexpr uint8_t DATA_READY_VAL = 0b10000000; static constexpr uint8_t STATUS_REGISTER = 0x34; static constexpr uint8_t REVID_REGISTER = 0x36; From 5284eadca64199ed48fee02fd30882735f13b6c6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 16:30:51 +0200 Subject: [PATCH 13/24] changes for C++11 --- src/fsfw/returnvalues/FwClassIds.h | 1 + src/fsfw/tmtcpacket/SpacePacket.h | 17 ++++++++--------- src/fsfw/tmtcservices/SpacePacketParser.h | 2 +- 3 files changed, 10 insertions(+), 10 deletions(-) diff --git a/src/fsfw/returnvalues/FwClassIds.h b/src/fsfw/returnvalues/FwClassIds.h index 9ef6c3f2..6b09fd8a 100644 --- a/src/fsfw/returnvalues/FwClassIds.h +++ b/src/fsfw/returnvalues/FwClassIds.h @@ -79,6 +79,7 @@ enum: uint8_t { HAL_GPIO, //HGIO MGM_LIS3MDL, //MGMLIS3 MGM_RM3100, //MGMRM3100 + SPACE_PACKET_PARSER, //SPPA FW_CLASS_ID_COUNT // [EXPORT] : [END] }; diff --git a/src/fsfw/tmtcpacket/SpacePacket.h b/src/fsfw/tmtcpacket/SpacePacket.h index 9eec87a8..67746859 100644 --- a/src/fsfw/tmtcpacket/SpacePacket.h +++ b/src/fsfw/tmtcpacket/SpacePacket.h @@ -32,15 +32,6 @@ public: */ virtual ~SpacePacket(); - static constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { - uint16_t tcPacketId = (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); - return tcPacketId; - } - static constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { - uint16_t tmPacketId = (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); - return tmPacketId; - } - /** * With this call, the complete data content (including the CCSDS Primary * Header) is overwritten with the byte stream given. @@ -78,4 +69,12 @@ protected: SpacePacketData localData; }; +constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { + return (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); +} + +constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { + return (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); +} + #endif /* SPACEPACKET_H_ */ diff --git a/src/fsfw/tmtcservices/SpacePacketParser.h b/src/fsfw/tmtcservices/SpacePacketParser.h index 82b15010..bed3369b 100644 --- a/src/fsfw/tmtcservices/SpacePacketParser.h +++ b/src/fsfw/tmtcservices/SpacePacketParser.h @@ -21,7 +21,7 @@ public: //! is the size of the PUS packet starting at that index. using IndexSizePair = std::pair; - static constexpr uint8_t INTERFACE_ID = CLASS_ID::PUS_PARSER; + static constexpr uint8_t INTERFACE_ID = CLASS_ID::SPACE_PACKET_PARSER; static constexpr ReturnValue_t NO_PACKET_FOUND = MAKE_RETURN_CODE(0x00); static constexpr ReturnValue_t SPLIT_PACKET = MAKE_RETURN_CODE(0x01); From bf02f14772184394c51c500ccc6dc312839de746 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 16:31:53 +0200 Subject: [PATCH 14/24] C++11 adaptions --- src/fsfw/tmtcpacket/SpacePacket.h | 17 ++++++++--------- src/fsfw/tmtcservices/SpacePacketParser.h | 2 +- 2 files changed, 9 insertions(+), 10 deletions(-) diff --git a/src/fsfw/tmtcpacket/SpacePacket.h b/src/fsfw/tmtcpacket/SpacePacket.h index 9eec87a8..67746859 100644 --- a/src/fsfw/tmtcpacket/SpacePacket.h +++ b/src/fsfw/tmtcpacket/SpacePacket.h @@ -32,15 +32,6 @@ public: */ virtual ~SpacePacket(); - static constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { - uint16_t tcPacketId = (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); - return tcPacketId; - } - static constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { - uint16_t tmPacketId = (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); - return tmPacketId; - } - /** * With this call, the complete data content (including the CCSDS Primary * Header) is overwritten with the byte stream given. @@ -78,4 +69,12 @@ protected: SpacePacketData localData; }; +constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { + return (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); +} + +constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { + return (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); +} + #endif /* SPACEPACKET_H_ */ diff --git a/src/fsfw/tmtcservices/SpacePacketParser.h b/src/fsfw/tmtcservices/SpacePacketParser.h index 82b15010..bed3369b 100644 --- a/src/fsfw/tmtcservices/SpacePacketParser.h +++ b/src/fsfw/tmtcservices/SpacePacketParser.h @@ -21,7 +21,7 @@ public: //! is the size of the PUS packet starting at that index. using IndexSizePair = std::pair; - static constexpr uint8_t INTERFACE_ID = CLASS_ID::PUS_PARSER; + static constexpr uint8_t INTERFACE_ID = CLASS_ID::SPACE_PACKET_PARSER; static constexpr ReturnValue_t NO_PACKET_FOUND = MAKE_RETURN_CODE(0x00); static constexpr ReturnValue_t SPLIT_PACKET = MAKE_RETURN_CODE(0x01); From 936bac5abdd6e56e8f3be85b624620948fecf714 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 16:32:43 +0200 Subject: [PATCH 15/24] class id renamed --- src/fsfw/returnvalues/FwClassIds.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw/returnvalues/FwClassIds.h b/src/fsfw/returnvalues/FwClassIds.h index cdbf5657..337709ed 100644 --- a/src/fsfw/returnvalues/FwClassIds.h +++ b/src/fsfw/returnvalues/FwClassIds.h @@ -78,6 +78,7 @@ enum: uint8_t { HAL_GPIO, //HGIO MGM_LIS3MDL, //MGMLIS3 MGM_RM3100, //MGMRM3100 + SPACE_PACKET_PARSER, //SPPA FW_CLASS_ID_COUNT // [EXPORT] : [END] }; From d9744365d254560196a54661e9f1b37288b38462 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 17:26:02 +0200 Subject: [PATCH 16/24] all windows fixes --- src/fsfw/osal/common/TcpTmTcServer.cpp | 32 ++++++++++++++++++++++++-- src/fsfw/osal/common/TcpTmTcServer.h | 3 +++ 2 files changed, 33 insertions(+), 2 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index c3936146..16c834d4 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -46,7 +46,9 @@ ReturnValue_t TcpTmTcServer::initialize() { if(spacePacketParser == nullptr) { return HasReturnvaluesIF::RETURN_FAILED; } +#if defined PLATFORM_UNIX tcpConfig.tcpFlags |= MSG_DONTWAIT; +#endif } } @@ -155,6 +157,10 @@ ReturnValue_t TcpTmTcServer::initializeAfterTaskCreation() { } void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { +#if defined PLATFORM_WIN + setSocketNonBlocking(connSocket); +#endif + while (true) { int retval = recv( connSocket, @@ -172,7 +178,13 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { ringBuffer.writeData(receptionBuffer.data(), retval); } else if(retval < 0) { - if(errno == EAGAIN) { + int errorValue = GetLastError(); +#if defined PLATFORM_UNIX + int wouldBlockValue = EAGAIN; +#elif defined PLATFORM_WIN + int wouldBlockValue = WSAEWOULDBLOCK; +#endif + if(errorValue == wouldBlockValue) { // No data available. Check whether any packets have been read, then send back // telemetry if available bool tcAvailable = false; @@ -191,7 +203,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { } } else { - tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL); + tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL, 300); } } } @@ -375,3 +387,19 @@ void TcpTmTcServer::handleSocketError(ConstStorageAccessor &accessor) { } } } + +void TcpTmTcServer::setSocketNonBlocking(socket_t &connSocket) { + u_long iMode = 1; + int iResult = ioctlsocket(connSocket, FIONBIO, &iMode); + if(iResult != NO_ERROR) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TcpTmTcServer::handleServerOperation: Setting socket" + " non-blocking failed with error " << iResult; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Setting socket" + " non-blocking failed with error %d\n", iResult); +#endif +#endif + } +} diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index da0e8bd5..64726a30 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -136,6 +136,9 @@ private: ReturnValue_t handleTmSending(socket_t connSocket, bool& tmSent); ReturnValue_t handleTcRingBufferData(size_t availableReadData); void handleSocketError(ConstStorageAccessor& accessor); +#if defined PLATFORM_WIN + void setSocketNonBlocking(socket_t& connSocket); +#endif }; #endif /* FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ */ From 32b5060c626261bd2729210f318bcc127d0117c3 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 17:26:02 +0200 Subject: [PATCH 17/24] all windows fixes --- src/fsfw/osal/common/TcpTmTcServer.cpp | 158 +++++++++++++++++++++++-- src/fsfw/osal/common/TcpTmTcServer.h | 17 ++- 2 files changed, 162 insertions(+), 13 deletions(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 11ab71af..b22f4412 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -41,6 +41,17 @@ ReturnValue_t TcpTmTcServer::initialize() { return result; } + switch(receptionMode) { + case(ReceptionModes::SPACE_PACKETS): { + spacePacketParser = new SpacePacketParser(validPacketIds); + if(spacePacketParser == nullptr) { + return HasReturnvaluesIF::RETURN_FAILED; + } +#if defined PLATFORM_UNIX + tcpConfig.tcpFlags |= MSG_DONTWAIT; +#endif + } + } tcStore = ObjectManager::instance()->get(objects::TC_STORE); if (tcStore == nullptr) { #if FSFW_CPP_OSTREAM_ENABLED == 1 @@ -144,11 +155,14 @@ ReturnValue_t TcpTmTcServer::initializeAfterTaskCreation() { return HasReturnvaluesIF::RETURN_OK; } -void TcpTmTcServer::handleServerOperation(socket_t connSocket) { - int retval = 0; - do { - // Read all telecommands sent by the client - retval = recv(connSocket, +void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { +#if defined PLATFORM_WIN + setSocketNonBlocking(connSocket); +#endif + + while (true) { + int retval = recv( + connSocket, reinterpret_cast(receptionBuffer.data()), receptionBuffer.capacity(), tcpFlags); @@ -159,9 +173,34 @@ void TcpTmTcServer::handleServerOperation(socket_t connSocket) { // Client has finished sending telecommands, send telemetry now handleTmSending(connSocket); } - else { - // Should not happen - tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL); + else if(retval < 0) { + int errorValue = GetLastError(); +#if defined PLATFORM_UNIX + int wouldBlockValue = EAGAIN; +#elif defined PLATFORM_WIN + int wouldBlockValue = WSAEWOULDBLOCK; +#endif + if(errorValue == wouldBlockValue) { + // No data available. Check whether any packets have been read, then send back + // telemetry if available + bool tcAvailable = false; + bool tmSent = false; + size_t availableReadData = ringBuffer.getAvailableReadData(); + if(availableReadData > lastRingBufferSize) { + tcAvailable = true; + handleTcRingBufferData(availableReadData); + } + ReturnValue_t result = handleTmSending(connSocket, tmSent); + if(result == CONN_BROKEN) { + return; + } + if(not tcAvailable and not tmSent) { + TaskFactory::delayTask(tcpConfig.tcpLoopDelay); + } + } + else { + tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::RECV_CALL, 300); + } } } while(retval > 0); } @@ -228,3 +267,106 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket) { } return HasReturnvaluesIF::RETURN_OK; } + +ReturnValue_t TcpTmTcServer::handleTcRingBufferData(size_t availableReadData) { + ReturnValue_t status = HasReturnvaluesIF::RETURN_OK; + ReturnValue_t result = HasReturnvaluesIF::RETURN_OK; + size_t readAmount = availableReadData; + lastRingBufferSize = availableReadData; + if(readAmount >= ringBuffer.getMaxSize()) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + // Possible configuration error, too much data or/and data coming in too fast, + // requiring larger buffers + sif::warning << "TcpTmTcServer::handleServerOperation: Ring buffer reached " << + "fill count" << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Ring buffer reached " + "fill count"); +#endif +#endif + } + if(readAmount >= receptionBuffer.size()) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + // Possible configuration error, too much data or/and data coming in too fast, + // requiring larger buffers + sif::warning << "TcpTmTcServer::handleServerOperation: " + "Reception buffer too small " << std::endl; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Reception buffer too small\n"); +#endif +#endif + readAmount = receptionBuffer.size(); + } + ringBuffer.readData(receptionBuffer.data(), readAmount, true); + const uint8_t* bufPtr = receptionBuffer.data(); + const uint8_t** bufPtrPtr = &bufPtr; + size_t startIdx = 0; + size_t foundSize = 0; + size_t readLen = 0; + while(readLen < readAmount) { + result = spacePacketParser->parseSpacePackets(bufPtrPtr, readAmount, + startIdx, foundSize, readLen); + switch(result) { + case(SpacePacketParser::NO_PACKET_FOUND): + case(SpacePacketParser::SPLIT_PACKET): { + break; + } + case(HasReturnvaluesIF::RETURN_OK): { + result = handleTcReception(receptionBuffer.data() + startIdx, foundSize); + if(result != HasReturnvaluesIF::RETURN_OK) { + status = result; + } + } + } + ringBuffer.deleteData(foundSize); + lastRingBufferSize = ringBuffer.getAvailableReadData(); + std::memset(receptionBuffer.data() + startIdx, 0, foundSize); + } + return status; +} + +void TcpTmTcServer::enableWiretapping(bool enable) { + this->wiretappingEnabled = enable; +} + +void TcpTmTcServer::handleSocketError(ConstStorageAccessor &accessor) { + // Don't delete data + accessor.release(); + auto socketError = getLastSocketError(); + switch(socketError) { +#if defined PLATFORM_WIN + case(WSAECONNRESET): { + // See https://docs.microsoft.com/en-us/windows/win32/api/winsock2/nf-winsock2-send + // Remote client might have shut down connection + return; + } +#else + case(EPIPE): { + // See https://man7.org/linux/man-pages/man2/send.2.html + // Remote client might have shut down connection + return; + } +#endif + default: { + tcpip::handleError(tcpip::Protocol::TCP, tcpip::ErrorSources::SEND_CALL); + } + } +} + +void TcpTmTcServer::setSocketNonBlocking(socket_t &connSocket) { + u_long iMode = 1; + int iResult = ioctlsocket(connSocket, FIONBIO, &iMode); + if(iResult != NO_ERROR) { +#if FSFW_VERBOSE_LEVEL >= 1 +#if FSFW_CPP_OSTREAM_ENABLED == 1 + sif::warning << "TcpTmTcServer::handleServerOperation: Setting socket" + " non-blocking failed with error " << iResult; +#else + sif::printWarning("TcpTmTcServer::handleServerOperation: Setting socket" + " non-blocking failed with error %d\n", iResult); +#endif +#endif + } +} diff --git a/src/fsfw/osal/common/TcpTmTcServer.h b/src/fsfw/osal/common/TcpTmTcServer.h index c6916080..2104d859 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.h +++ b/src/fsfw/osal/common/TcpTmTcServer.h @@ -84,12 +84,19 @@ private: int tcpBacklog = 3; std::vector receptionBuffer; - int tcpSockOpt = 0; - int tcpTmFlags = 0; + SimpleRingBuffer ringBuffer; + std::vector validPacketIds; + SpacePacketParser* spacePacketParser = nullptr; + uint8_t lastRingBufferSize = 0; - void handleServerOperation(socket_t connSocket); - ReturnValue_t handleTcReception(size_t bytesRecvd); - ReturnValue_t handleTmSending(socket_t connSocket); + virtual void handleServerOperation(socket_t& connSocket); + ReturnValue_t handleTcReception(uint8_t* spacePacket, size_t packetSize); + ReturnValue_t handleTmSending(socket_t connSocket, bool& tmSent); + ReturnValue_t handleTcRingBufferData(size_t availableReadData); + void handleSocketError(ConstStorageAccessor& accessor); +#if defined PLATFORM_WIN + void setSocketNonBlocking(socket_t& connSocket); +#endif }; #endif /* FSFW_OSAL_COMMON_TCP_TMTC_SERVER_H_ */ From b1a9c90087bad07c81f9f96594edf7e2fa51d077 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 17:38:35 +0200 Subject: [PATCH 18/24] this should work for both OSes --- src/fsfw/osal/common/TcpTmTcServer.cpp | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 4348e21e..519df547 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -177,7 +177,7 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { ringBuffer.writeData(receptionBuffer.data(), retval); } else if(retval < 0) { - int errorValue = GetLastError(); + int errorValue = getLastSocketError(); #if defined PLATFORM_UNIX int wouldBlockValue = EAGAIN; #elif defined PLATFORM_WIN @@ -387,6 +387,7 @@ void TcpTmTcServer::handleSocketError(ConstStorageAccessor &accessor) { } } +#if defined PLATFORM_WIN void TcpTmTcServer::setSocketNonBlocking(socket_t &connSocket) { u_long iMode = 1; int iResult = ioctlsocket(connSocket, FIONBIO, &iMode); @@ -402,3 +403,4 @@ void TcpTmTcServer::setSocketNonBlocking(socket_t &connSocket) { #endif } } +#endif From cffd77ed325aa24bef3f22bc85dbd573c2db362e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Sep 2021 17:42:29 +0200 Subject: [PATCH 19/24] put functions in namespace --- src/fsfw/tmtcpacket/SpacePacket.h | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/src/fsfw/tmtcpacket/SpacePacket.h b/src/fsfw/tmtcpacket/SpacePacket.h index 67746859..fe8a1044 100644 --- a/src/fsfw/tmtcpacket/SpacePacket.h +++ b/src/fsfw/tmtcpacket/SpacePacket.h @@ -69,6 +69,8 @@ protected: SpacePacketData localData; }; +namespace spacepacket { + constexpr uint16_t getTcSpacePacketIdFromApid(uint16_t apid) { return (0x18 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); } @@ -77,4 +79,6 @@ constexpr uint16_t getTmSpacePacketIdFromApid(uint16_t apid) { return (0x08 << 8) | (((apid >> 8) & 0x07) << 8) | (apid & 0x00ff); } +} + #endif /* SPACEPACKET_H_ */ From 0a6a32a13003103ee6462d1c46f814e0c44ab07b Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Sep 2021 11:45:20 +0200 Subject: [PATCH 20/24] printout separation --- src/fsfw/osal/common/TcpTmTcServer.cpp | 8 ++++++++ 1 file changed, 8 insertions(+) diff --git a/src/fsfw/osal/common/TcpTmTcServer.cpp b/src/fsfw/osal/common/TcpTmTcServer.cpp index 519df547..8b34b1a3 100644 --- a/src/fsfw/osal/common/TcpTmTcServer.cpp +++ b/src/fsfw/osal/common/TcpTmTcServer.cpp @@ -210,7 +210,11 @@ void TcpTmTcServer::handleServerOperation(socket_t& connSocket) { ReturnValue_t TcpTmTcServer::handleTcReception(uint8_t* spacePacket, size_t packetSize) { if(wiretappingEnabled) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "Received TC:" << std::endl; +#else + sif::printInfo("Received TC:\n"); +#endif arrayprinter::print(spacePacket, packetSize); } @@ -278,7 +282,11 @@ ReturnValue_t TcpTmTcServer::handleTmSending(socket_t connSocket, bool& tmSent) return result; } if(wiretappingEnabled) { +#if FSFW_CPP_OSTREAM_ENABLED == 1 sif::info << "Sending TM:" << std::endl; +#else + sif::printInfo("Sending TM:\n"); +#endif arrayprinter::print(storeAccessor.data(), storeAccessor.size()); } int retval = send(connSocket, From 3d80271f0d01bce70afc51f9c2958cf5583272d7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Sep 2021 11:47:01 +0200 Subject: [PATCH 21/24] missing cast --- tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp b/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp index a82ac73a..8c2e55ed 100644 --- a/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp +++ b/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp @@ -103,7 +103,7 @@ TEST_CASE("DleEncoder" , "[DleEncoder]") { for(size_t faultyDestSize = 0; faultyDestSize < expectedVec.size(); faultyDestSize ++) { result = dleEncoder.encode(vecToEncode.data(), vecToEncode.size(), buffer.data(), faultyDestSize, &encodedLen); - REQUIRE(result == DleEncoder::STREAM_TOO_SHORT); + REQUIRE(result == static_cast(DleEncoder::STREAM_TOO_SHORT)); } }; From 76c3214a4f3db0344a4a6b9dd89deb11fedcf4f9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 30 Sep 2021 00:59:49 +0200 Subject: [PATCH 22/24] important bugfix --- src/fsfw/globalfunctions/DleEncoder.cpp | 2 +- tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp | 5 +++++ 2 files changed, 6 insertions(+), 1 deletion(-) diff --git a/src/fsfw/globalfunctions/DleEncoder.cpp b/src/fsfw/globalfunctions/DleEncoder.cpp index 47ea5c4e..f4691cc6 100644 --- a/src/fsfw/globalfunctions/DleEncoder.cpp +++ b/src/fsfw/globalfunctions/DleEncoder.cpp @@ -165,7 +165,7 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_ if (sourceStream[encodedIndex++] != STX_CHAR) { return DECODING_ERROR; } - while ((encodedIndex < sourceStreamLen) + while ((encodedIndex < sourceStreamLen - 1) and (decodedIndex < maxDestStreamlen) and (sourceStream[encodedIndex] != ETX_CHAR) and (sourceStream[encodedIndex] != STX_CHAR)) { diff --git a/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp b/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp index 8c2e55ed..cffd5308 100644 --- a/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp +++ b/tests/src/fsfw_tests/unit/globalfunctions/testDleEncoder.cpp @@ -218,5 +218,10 @@ TEST_CASE("DleEncoder" , "[DleEncoder]") { REQUIRE(result == static_cast(DleEncoder::DECODING_ERROR)); dleEncoder.setEscapeMode(true); + testArray1EncodedFaulty = TEST_ARRAY_1_ENCODED_ESCAPED; + testArray1EncodedFaulty[5] = 0; + result = dleEncoder.decode(testArray1EncodedFaulty.data(), testArray1EncodedFaulty.size(), + &readLen, buffer.data(), buffer.size(), &encodedLen); + REQUIRE(result == static_cast(DleEncoder::DECODING_ERROR)); } } From f67567d0b5fc4faf25fa99485e9bb86f3884e432 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 30 Sep 2021 16:49:30 +0200 Subject: [PATCH 23/24] refactoring, code more understandable --- src/fsfw/globalfunctions/DleEncoder.cpp | 48 +++++++++++++------------ 1 file changed, 25 insertions(+), 23 deletions(-) diff --git a/src/fsfw/globalfunctions/DleEncoder.cpp b/src/fsfw/globalfunctions/DleEncoder.cpp index f4691cc6..f77d5472 100644 --- a/src/fsfw/globalfunctions/DleEncoder.cpp +++ b/src/fsfw/globalfunctions/DleEncoder.cpp @@ -165,11 +165,9 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_ if (sourceStream[encodedIndex++] != STX_CHAR) { return DECODING_ERROR; } - while ((encodedIndex < sourceStreamLen - 1) - and (decodedIndex < maxDestStreamlen) - and (sourceStream[encodedIndex] != ETX_CHAR) - and (sourceStream[encodedIndex] != STX_CHAR)) { - if (sourceStream[encodedIndex] == DLE_CHAR) { + while ((encodedIndex < sourceStreamLen) and (decodedIndex < maxDestStreamlen)) { + switch(sourceStream[encodedIndex]) { + case(DLE_CHAR): { if(encodedIndex + 1 >= sourceStreamLen) { //reached the end of the sourceStream *readLen = sourceStreamLen; @@ -197,29 +195,33 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_ } } ++encodedIndex; + break; } - else { - destStream[decodedIndex] = sourceStream[encodedIndex]; - } - - ++encodedIndex; - ++decodedIndex; - } - if (sourceStream[encodedIndex] != ETX_CHAR) { - if(decodedIndex == maxDestStreamlen) { - //so far we did not find anything wrong here, so let user try again - *readLen = 0; - return STREAM_TOO_SHORT; - } - else { + case(STX_CHAR): { *readLen = ++encodedIndex; return DECODING_ERROR; } + case(ETX_CHAR): { + *readLen = ++encodedIndex; + *decodedLen = decodedIndex; + return RETURN_OK; + } + default: { + destStream[decodedIndex] = sourceStream[encodedIndex]; + break; + } + } + ++encodedIndex; + ++decodedIndex; } - else { - *readLen = ++encodedIndex; - *decodedLen = decodedIndex; - return RETURN_OK; + + if(decodedIndex == maxDestStreamlen) { + //so far we did not find anything wrong here, so let user try again + *readLen = 0; + return STREAM_TOO_SHORT; + } else { + *readLen = encodedIndex; + return DECODING_ERROR; } } From d81ed334d8d229543b15a30abcd03849af2837d1 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 4 Oct 2021 14:38:10 +0200 Subject: [PATCH 24/24] preserve STX char --- src/fsfw/globalfunctions/DleEncoder.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/globalfunctions/DleEncoder.cpp b/src/fsfw/globalfunctions/DleEncoder.cpp index f77d5472..91db5445 100644 --- a/src/fsfw/globalfunctions/DleEncoder.cpp +++ b/src/fsfw/globalfunctions/DleEncoder.cpp @@ -198,7 +198,7 @@ ReturnValue_t DleEncoder::decodeStreamEscaped(const uint8_t *sourceStream, size_ break; } case(STX_CHAR): { - *readLen = ++encodedIndex; + *readLen = encodedIndex; return DECODING_ERROR; } case(ETX_CHAR): {