From 770fee009770abf35f006038a5377ffc3cf24053 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Mar 2023 17:38:38 +0200 Subject: [PATCH 1/6] some minor tweaks --- bsp_q7s/core/ObjectFactory.cpp | 2 +- linux/ipcore/PapbVcInterface.cpp | 6 ++---- linux/ipcore/PapbVcInterface.h | 4 ++-- mission/com/CcsdsIpCoreHandler.cpp | 18 +++++++++++++----- mission/com/CcsdsIpCoreHandler.h | 2 +- mission/tmtc/PersistentSingleTmStoreTask.cpp | 3 ++- mission/tmtc/TmStoreTaskBase.cpp | 4 ++-- 7 files changed, 23 insertions(+), 16 deletions(-) diff --git a/bsp_q7s/core/ObjectFactory.cpp b/bsp_q7s/core/ObjectFactory.cpp index 00e87330..8ed42ff9 100644 --- a/bsp_q7s/core/ObjectFactory.cpp +++ b/bsp_q7s/core/ObjectFactory.cpp @@ -21,9 +21,9 @@ #include #include #include +#include #include #include -#include #include #include #include diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index eacfce33..5849aecd 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -65,7 +65,7 @@ ReturnValue_t PapbVcInterface::pollPapbBusySignal(uint32_t maxPollRetries) const gpio::Levels papbBusyState = gpio::Levels::LOW; ReturnValue_t result; uint32_t busyIdx = 0; - nextDelay.tv_nsec = 0; + nextDelay.tv_nsec = FIRST_DELAY_PAPB_POLLING_NS; while (true) { /** Check if PAPB interface is ready to receive data */ @@ -87,9 +87,7 @@ ReturnValue_t PapbVcInterface::pollPapbBusySignal(uint32_t maxPollRetries) const // Ignore signal handling here for now. nanosleep(&nextDelay, &remDelay); // Adaptive delay. - if (nextDelay.tv_nsec == 0) { - nextDelay.tv_nsec = FIRST_NON_NULL_DELAY_NS; - } else if (nextDelay.tv_nsec * 2 <= MAX_DELAY_PAPB_POLLING_NS) { + if (nextDelay.tv_nsec * 2 <= MAX_DELAY_PAPB_POLLING_NS) { nextDelay.tv_nsec *= 2; } } diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index b4454dae..cfc885fc 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -76,7 +76,7 @@ class PapbVcInterface : public VirtualChannelIF { */ static const int DATA_REG_OFFSET = 256; - static constexpr long int FIRST_NON_NULL_DELAY_NS = 10; + static constexpr long int FIRST_DELAY_PAPB_POLLING_NS = 10; static constexpr long int MAX_DELAY_PAPB_POLLING_NS = 40; LinuxLibgpioIF* gpioComIF = nullptr; @@ -89,7 +89,7 @@ class PapbVcInterface : public VirtualChannelIF { std::string uioFile; int mapNum = 0; mutable struct timespec nextDelay = {.tv_sec = 0, .tv_nsec = 0}; - const struct timespec BETWEEN_POLL_DELAY = {.tv_sec = 0, .tv_nsec = 5}; + const struct timespec BETWEEN_POLL_DELAY = {.tv_sec = 0, .tv_nsec = 2}; mutable struct timespec remDelay; volatile uint32_t* vcBaseReg = nullptr; diff --git a/mission/com/CcsdsIpCoreHandler.cpp b/mission/com/CcsdsIpCoreHandler.cpp index 806096b8..212acd8d 100644 --- a/mission/com/CcsdsIpCoreHandler.cpp +++ b/mission/com/CcsdsIpCoreHandler.cpp @@ -165,9 +165,11 @@ ReturnValue_t CcsdsIpCoreHandler::executeAction(ActionId_t actionId, MessageQueu break; } case EN_TRANSMITTER: { - enableTransmit(); if (mode == HasModesIF::MODE_OFF) { + enableTransmit(true); mode = HasModesIF::MODE_ON; + } else { + enableTransmit(false); } return EXECUTION_FINISHED; } @@ -205,9 +207,11 @@ ReturnValue_t CcsdsIpCoreHandler::executeAction(ActionId_t actionId, MessageQueu void CcsdsIpCoreHandler::updateLinkState() { linkState = LINK_UP; } -void CcsdsIpCoreHandler::enableTransmit() { - // Reset PTME on each transmit enable. - updateBatPriorityFromParam(); +void CcsdsIpCoreHandler::enableTransmit(bool resetPtmeUpdateParams) { + if (resetPtmeUpdateParams) { + // Reset PTME on each transmit enable. + updateBatPriorityFromParam(); + } #ifndef TE0720_1CFA gpioIF->pullHigh(ptmeGpios.enableTxClock); gpioIF->pullHigh(ptmeGpios.enableTxData); @@ -243,7 +247,11 @@ void CcsdsIpCoreHandler::startTransition(Mode_t mode, Submode_t submode) { } }; if (mode == HasModesIF::MODE_ON) { - enableTransmit(); + if (this->mode != HasModesIF::MODE_ON) { + enableTransmit(true); + } else { + enableTransmit(false); + } if (submode == static_cast(com::CcsdsSubmode::DATARATE_DEFAULT)) { com::Datarate currentDatarate = com::getCurrentDatarate(); if (currentDatarate == com::Datarate::LOW_RATE_MODULATION_BPSK) { diff --git a/mission/com/CcsdsIpCoreHandler.h b/mission/com/CcsdsIpCoreHandler.h index 0fad0aae..f01d7ab3 100644 --- a/mission/com/CcsdsIpCoreHandler.h +++ b/mission/com/CcsdsIpCoreHandler.h @@ -172,7 +172,7 @@ class CcsdsIpCoreHandler : public SystemObject, /** * @brief Starts transmit timer and enables transmitter. */ - void enableTransmit(); + void enableTransmit(bool resetPtmeUpdateParams); /** * @brief Disables the transmitter by pulling the enable tx clock and tx data pin of the diff --git a/mission/tmtc/PersistentSingleTmStoreTask.cpp b/mission/tmtc/PersistentSingleTmStoreTask.cpp index a2d57208..1c93f14e 100644 --- a/mission/tmtc/PersistentSingleTmStoreTask.cpp +++ b/mission/tmtc/PersistentSingleTmStoreTask.cpp @@ -20,8 +20,9 @@ ReturnValue_t PersistentSingleTmStoreTask::performOperation(uint8_t opCode) { if (not busy) { TaskFactory::delayTask(100); } else if (dumpContext.vcBusyDuringDump) { + sif::debug << "VC busy, delaying" << std::endl; // TODO: Might not be necessary - TaskFactory::delayTask(10); + TaskFactory::delayTask(2); } } } diff --git a/mission/tmtc/TmStoreTaskBase.cpp b/mission/tmtc/TmStoreTaskBase.cpp index 7ca03488..993c127d 100644 --- a/mission/tmtc/TmStoreTaskBase.cpp +++ b/mission/tmtc/TmStoreTaskBase.cpp @@ -115,10 +115,10 @@ ReturnValue_t TmStoreTaskBase::handleOneDump(PersistentTmStoreWithTmQueue& store } else { // The PTME might be at full load, so it might sense to delay for a bit to let it do // its work until some more bandwidth is available. Set a flag here so the upper layer can - // do ths. + // do this. dumpContext.vcBusyDuringDump = true; dumpContext.ptmeBusyCounter++; - if (dumpContext.ptmeBusyCounter == 100) { + if (dumpContext.ptmeBusyCounter == 200) { // If this happens, something is probably wrong. sif::warning << "PTME busy for longer period. Cancelling dump" << std::endl; cancelDump(dumpContext, store, channel.isTxOn()); From 27370fcd44a94015f66ef0a9afea25386b2b2a47 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 30 Mar 2023 13:36:44 +0200 Subject: [PATCH 2/6] multi byte write support but does not work.. --- linux/ipcore/PapbVcInterface.cpp | 45 +++++++++++++++++++++++++++++--- linux/ipcore/PapbVcInterface.h | 8 +++--- 2 files changed, 47 insertions(+), 6 deletions(-) diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index 5849aecd..4aeba804 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -2,6 +2,7 @@ #include #include +#include #include #include "fsfw/serviceinterface/ServiceInterface.h" @@ -28,11 +29,46 @@ ReturnValue_t PapbVcInterface::initialize() { } ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) { + // There are no packets smaller than 4, this is considered a configuration error. + if (size < 4) { + return returnvalue::FAILED; + } if (pollPapbBusySignal(0) == returnvalue::OK) { - startPacketTransfer(); + startPacketTransfer(ByteWidthCfg::ONE); } else { return DirectTmSinkIF::IS_BUSY; } + // TODO: This should work but does not.. + // size_t idx = 0; + // while (idx < size) { + // + // nanosleep(&BETWEEN_POLL_DELAY, &remDelay); + // if ((size - idx) < 4) { + // *vcBaseReg = CONFIG_DATA_INPUT | (size - idx - 1); + // usleep(1); + // } + // if (pollPapbBusySignal(2) == returnvalue::OK) { + // // vcBaseReg + DATA_REG_OFFSET + 3 = static_cast(data + idx); + // // vcBaseReg + DATA_REG_OFFSET + 2 = static_cast(data + idx + 1); + // // vcBaseReg + DATA_REG_OFFSET + 1 = static_cast(data + idx + 2); + // // vcBaseReg + DATA_REG_OFFSET = static_cast(data + idx + 3); + // + // // std::memcpy((vcBaseReg + DATA_REG_OFFSET), data + idx , nextWriteSize); + // *(vcBaseReg + DATA_REG_OFFSET) = *reinterpret_cast(data + idx); + // //uint8_t* byteReg = reinterpret_cast(vcBaseReg + DATA_REG_OFFSET); + // + // //byteReg[0] = data[idx]; + // //byteReg[1] = data[idx]; + // } else { + // abortPacketTransfer(); + // return returnvalue::FAILED; + // } + // // TODO: Change this after the bugfix. Right now, the PAPB ignores the content of the byte + // // width configuration.5 + // // It's okay to increment by a larger amount for the last segment here, loop will be over + // // in any case. + // idx += 4; + // } for (size_t idx = 0; idx < size; idx++) { // This delay is super-important, DO NOT REMOVE! // Polling the GPIO too often can mess up the scheduler. @@ -40,6 +76,7 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) { // FPGA module which can accept packets and then takes care of dumping that packet into // the PTME. DMA would be an ideal solution for this. nanosleep(&BETWEEN_POLL_DELAY, &remDelay); + if (pollPapbBusySignal(2) == returnvalue::OK) { *(vcBaseReg + DATA_REG_OFFSET) = static_cast(data[idx]); } else { @@ -57,7 +94,9 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) { return returnvalue::OK; } -void PapbVcInterface::startPacketTransfer() { *vcBaseReg = CONFIG_START; } +void PapbVcInterface::startPacketTransfer(ByteWidthCfg initWidth) { + *vcBaseReg = CONFIG_DATA_INPUT | initWidth; +} void PapbVcInterface::completePacketTransfer() { *vcBaseReg = CONFIG_END; } @@ -65,7 +104,7 @@ ReturnValue_t PapbVcInterface::pollPapbBusySignal(uint32_t maxPollRetries) const gpio::Levels papbBusyState = gpio::Levels::LOW; ReturnValue_t result; uint32_t busyIdx = 0; - nextDelay.tv_nsec = FIRST_DELAY_PAPB_POLLING_NS; + nextDelay.tv_nsec = 0; while (true) { /** Check if PAPB interface is ready to receive data */ diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index cfc885fc..61999c7d 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -50,13 +50,14 @@ class PapbVcInterface : public VirtualChannelIF { static const ReturnValue_t PAPB_BUSY = MAKE_RETURN_CODE(0xA0); + enum ByteWidthCfg : uint32_t { ONE = 0b00, TWO = 0b01, THREE = 0b10, FOUR = 0b11 }; /** * Configuration bits: * bit[1:0]: Size of data (1,2,3 or 4 bytes). 1 Byte <=> b00 * bit[2]: Set this bit to 1 to abort a transferred packet * bit[3]: Signals to VcInterface the start of a new telemetry packet */ - static constexpr uint32_t CONFIG_START = 0b00001000; + static constexpr uint32_t CONFIG_DATA_INPUT = 0b00001000; /** * Abort a transferred packet. @@ -88,11 +89,12 @@ class PapbVcInterface : public VirtualChannelIF { std::string uioFile; int mapNum = 0; + volatile uint32_t dummy = 0; mutable struct timespec nextDelay = {.tv_sec = 0, .tv_nsec = 0}; const struct timespec BETWEEN_POLL_DELAY = {.tv_sec = 0, .tv_nsec = 2}; mutable struct timespec remDelay; - volatile uint32_t* vcBaseReg = nullptr; + uint32_t* vcBaseReg = nullptr; uint32_t vcOffset = 0; @@ -100,7 +102,7 @@ class PapbVcInterface : public VirtualChannelIF { * @brief This function sends the config byte to the virtual channel of the PTME IP Core * to initiate a packet transfer. */ - void startPacketTransfer(); + void startPacketTransfer(ByteWidthCfg initWidth); void abortPacketTransfer(); From bb7a6162835767f38a9465915814bd037971cb5e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 17:35:53 +0200 Subject: [PATCH 3/6] some minor tweaks --- linux/ipcore/PapbVcInterface.cpp | 2 +- linux/ipcore/PapbVcInterface.h | 1 - 2 files changed, 1 insertion(+), 2 deletions(-) diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index e7d6fd3d..0a7a15fc 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -37,7 +37,7 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size) { } else { return DirectTmSinkIF::IS_BUSY; } - // TODO: This should work but does not.. + // TODO: This should work but does not.. :( // size_t idx = 0; // while (idx < size) { // diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index 82bad56a..1fed52ef 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -91,7 +91,6 @@ class PapbVcInterface : public VirtualChannelIF { std::string uioFile; int mapNum = 0; - volatile uint32_t dummy = 0; mutable struct timespec nextDelay = {.tv_sec = 0, .tv_nsec = 0}; const struct timespec BETWEEN_POLL_DELAY = {.tv_sec = 0, .tv_nsec = 10}; mutable struct timespec remDelay; From f51656a8134cd81d017cd940f25c862a8d17bf21 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 17:36:49 +0200 Subject: [PATCH 4/6] add back volatile keyword --- linux/ipcore/PapbVcInterface.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index 1fed52ef..e54def5d 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -95,7 +95,7 @@ class PapbVcInterface : public VirtualChannelIF { const struct timespec BETWEEN_POLL_DELAY = {.tv_sec = 0, .tv_nsec = 10}; mutable struct timespec remDelay; - uint32_t* vcBaseReg = nullptr; + volatile uint32_t* vcBaseReg = nullptr; uint32_t vcOffset = 0; From fcf3437410881a0d7d16bb1f68b56b0f19495cd6 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 17:39:13 +0200 Subject: [PATCH 5/6] some more tweaks --- mission/com/CcsdsIpCoreHandler.h | 2 +- mission/com/PersistentLogTmStoreTask.cpp | 1 + 2 files changed, 2 insertions(+), 1 deletion(-) diff --git a/mission/com/CcsdsIpCoreHandler.h b/mission/com/CcsdsIpCoreHandler.h index 520403a5..785f84a5 100644 --- a/mission/com/CcsdsIpCoreHandler.h +++ b/mission/com/CcsdsIpCoreHandler.h @@ -180,7 +180,7 @@ class CcsdsIpCoreHandler : public SystemObject, /** * @brief Starts transmit timer and enables transmitter. */ - void enableTransmit(bool resetPtmeUpdateParams); + void enableTransmit(); /** * @brief Disables the transmitter by pulling the enable tx clock and tx data pin of the diff --git a/mission/com/PersistentLogTmStoreTask.cpp b/mission/com/PersistentLogTmStoreTask.cpp index 8ba7200f..0fc02461 100644 --- a/mission/com/PersistentLogTmStoreTask.cpp +++ b/mission/com/PersistentLogTmStoreTask.cpp @@ -43,6 +43,7 @@ ReturnValue_t PersistentLogTmStoreTask::performOperation(uint8_t opCode) { TaskFactory::delayTask(100); } else if (vcBusyDuringDump) { // TODO: Might not be necessary + sif::debug << "VC busy, delaying" << std::endl; TaskFactory::delayTask(10); } } From 2f9de8c36e9c87555ab63c0ba1dea8950cb3d56c Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Fri, 31 Mar 2023 17:41:01 +0200 Subject: [PATCH 6/6] some tweaks --- linux/ipcore/PapbVcInterface.cpp | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index 0a7a15fc..60968cc6 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -101,7 +101,7 @@ void PapbVcInterface::completePacketTransfer() { *vcBaseReg = CONFIG_END; } ReturnValue_t PapbVcInterface::pollInterfaceReadiness(uint32_t maxPollRetries, bool checkReadyState) const { uint32_t busyIdx = 0; - nextDelay.tv_nsec = FIRST_NON_NULL_DELAY_NS; + nextDelay.tv_nsec = FIRST_DELAY_PAPB_POLLING_NS; while (true) { // Check if PAPB interface is ready to receive data. Use the configuration register for this.