diff --git a/bsp_q7s/objectFactory.cpp b/bsp_q7s/objectFactory.cpp index 211aa071..c8e6f555 100644 --- a/bsp_q7s/objectFactory.cpp +++ b/bsp_q7s/objectFactory.cpp @@ -750,14 +750,18 @@ ReturnValue_t ObjectFactory::createCcsdsComponents(CcsdsComponentArgs& args) { gpioChecker(args.gpioComIF.addGpios(gpioCookiePtmeIp), "PTME PAPB VCs"); // Creating virtual channel interfaces - VirtualChannelIF* vc0 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC0); - VirtualChannelIF* vc1 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC1); - VirtualChannelIF* vc2 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC2); - VirtualChannelIF* vc3 = new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_EMPTY, - q7s::UIO_PTME, q7s::uiomapids::PTME_VC3); + VirtualChannelIF* vc0 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC0_PAPB_EMPTY, q7s::UIO_PTME, + q7s::uiomapids::PTME_VC0, config::MAX_SPACEPACKET_TC_SIZE); + VirtualChannelIF* vc1 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC1_PAPB_EMPTY, q7s::UIO_PTME, + q7s::uiomapids::PTME_VC1, config::MAX_SPACEPACKET_TC_SIZE); + VirtualChannelIF* vc2 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC2_PAPB_EMPTY, q7s::UIO_PTME, + q7s::uiomapids::PTME_VC2, config::MAX_SPACEPACKET_TC_SIZE); + VirtualChannelIF* vc3 = + new PapbVcInterface(&args.gpioComIF, gpioIds::VC3_PAPB_EMPTY, q7s::UIO_PTME, + q7s::uiomapids::PTME_VC3, config::MAX_SPACEPACKET_TC_SIZE); // Creating ptme object and adding virtual channel interfaces Ptme* ptme = new Ptme(objects::PTME); ptme->addVcInterface(ccsds::VC0, vc0); diff --git a/common/config/eive/definitions.h b/common/config/eive/definitions.h index 777e8924..ee1fd5b7 100644 --- a/common/config/eive/definitions.h +++ b/common/config/eive/definitions.h @@ -35,6 +35,8 @@ static constexpr uint32_t STR_IMG_HELPER_QUEUE_SIZE = 50; static constexpr uint8_t LIVE_TM = 0; +static constexpr size_t MAX_SPACEPACKET_TC_SIZE = 2048; + /* Limits for filename and path checks */ static constexpr uint32_t MAX_PATH_SIZE = 200; static constexpr uint32_t MAX_FILENAME_SIZE = 100; diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index 88684772..d5207c0b 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -8,8 +8,12 @@ #include "fsfw/serviceinterface/ServiceInterface.h" PapbVcInterface::PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, - std::string uioFile, int mapNum) - : gpioComIF(gpioComIF), papbEmptyId(papbEmptyId), uioFile(std::move(uioFile)), mapNum(mapNum) {} + std::string uioFile, int mapNum, size_t maxPacketSize) + : gpioComIF(gpioComIF), + papbEmptyId(papbEmptyId), + packetBuf(maxPacketSize), + uioFile(std::move(uioFile)), + mapNum(mapNum) {} PapbVcInterface::~PapbVcInterface() {} @@ -29,9 +33,17 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size, size_t& w return returnvalue::FAILED; } // The user must call finishWrite before starting a new packet transfer. - if (partialWriteActive) { - return INCOMPLETE_PARTIAL_WRITE; + if (writeActive) { + return IS_BUSY; } + if (size > packetBuf.capacity()) { + sif::error << "PapbVcInterface: Packet with size " << size << " larger than maximum configured" + << " byte size " << packetBuf.capacity() << std::endl; + return returnvalue::FAILED; + } + std::memcpy(packetBuf.data(), data, size); + currentPacketSize = size; + currentPacketIndex = 0; if (pollReadyForPacket()) { startPacketTransfer(ByteWidthCfg::ONE); } else { @@ -41,14 +53,17 @@ ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size, size_t& w abortPacketTransfer(); return returnvalue::FAILED; } - return finishWriteInternal(data, 0, size, writtenSize, false); + return advanceWrite(writtenSize); } void PapbVcInterface::startPacketTransfer(ByteWidthCfg initWidth) { *vcBaseReg = CONFIG_DATA_INPUT | initWidth; } -void PapbVcInterface::completePacketTransfer() { *vcBaseReg = CONFIG_END; } +void PapbVcInterface::completePacketTransfer() { + *vcBaseReg = CONFIG_END; + writeActive = false; +} bool PapbVcInterface::pollReadyForPacket() const { // Check if PAPB interface is ready to receive data. Use the configuration register for this. @@ -57,34 +72,20 @@ bool PapbVcInterface::pollReadyForPacket() const { return (reg >> 6) & 0b1; } -ReturnValue_t PapbVcInterface::finishWrite(const uint8_t* data, size_t start, - size_t remainingSize) { +ReturnValue_t PapbVcInterface::advanceWrite(size_t& writtenSize) { if (not pollReadyForPacket()) { - return returnvalue::FAILED; + return IS_BUSY; } - size_t dummy = 0; - return finishWriteInternal(data, start, remainingSize, dummy, true); -} - -ReturnValue_t PapbVcInterface::finishWriteInternal(const uint8_t* data, size_t start, - size_t remainingSize, size_t& writtenSize, - bool abortOnPartialWrite) { - for (size_t idx = 0; idx < remainingSize; idx++) { + for (size_t idx = currentPacketIndex; idx < currentPacketSize; idx++) { if (not pollReadyForOctet(MAX_BUSY_POLLS)) { if (not pollReadyForPacket()) { - writtenSize = start + idx; - partialWriteActive = true; - if (abortOnPartialWrite) { - abortPacketTransfer(); - partialWriteActive = false; - return returnvalue::FAILED; - } return PARTIALLY_WRITTEN; } abortPacketTransfer(); return returnvalue::FAILED; } - *(vcBaseReg + DATA_REG_OFFSET) = static_cast(data[start + idx]); + *(vcBaseReg + DATA_REG_OFFSET) = static_cast(packetBuf[currentPacketIndex]); + writtenSize++; } if (not pollReadyForOctet(MAX_BUSY_POLLS)) { abortPacketTransfer(); diff --git a/linux/ipcore/PapbVcInterface.h b/linux/ipcore/PapbVcInterface.h index 35bd0439..eb707b62 100644 --- a/linux/ipcore/PapbVcInterface.h +++ b/linux/ipcore/PapbVcInterface.h @@ -6,6 +6,7 @@ #include #include +#include #include "OBSWConfig.h" #include "fsfw/returnvalues/returnvalue.h" @@ -30,7 +31,8 @@ class PapbVcInterface : public VirtualChannelIF { * @param uioFile UIO file providing access to the PAPB bus * @param mapNum Map number of UIO map associated with this virtual channel */ - PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, std::string uioFile, int mapNum); + PapbVcInterface(LinuxLibgpioIF* gpioComIF, gpioId_t papbEmptyId, std::string uioFile, int mapNum, + size_t maxPacketSize); virtual ~PapbVcInterface(); bool isBusy() const override; @@ -42,9 +44,9 @@ class PapbVcInterface : public VirtualChannelIF { */ ReturnValue_t write(const uint8_t* data, size_t size, size_t& writtenSize) override; - ReturnValue_t finishWrite(const uint8_t* data, size_t start, size_t remainingSize) override; - ReturnValue_t finishWriteInternal(const uint8_t* data, size_t start, size_t remainingSize, - size_t& writtenSize, bool abortOnPartialWrite); + ReturnValue_t advanceWrite(size_t& remainingSize) override; + // ReturnValue_t finishWriteInternal(const uint8_t* data, size_t start, size_t remainingSize, + // size_t& writtenSize, bool abortOnPartialWrite); void cancelTransfer() override; @@ -90,9 +92,12 @@ class PapbVcInterface : public VirtualChannelIF { /** High when external buffer memory of virtual channel is empty */ gpioId_t papbEmptyId = gpio::NO_GPIO; + std::vector packetBuf; std::string uioFile; int mapNum = 0; - bool partialWriteActive = false; + bool writeActive = false; + size_t currentPacketIndex = 0; + size_t currentPacketSize = 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; diff --git a/mission/com/LiveTmTask.cpp b/mission/com/LiveTmTask.cpp index 5b337030..15630be5 100644 --- a/mission/com/LiveTmTask.cpp +++ b/mission/com/LiveTmTask.cpp @@ -53,11 +53,11 @@ ReturnValue_t LiveTmTask::performOperation(uint8_t opCode) { consecutiveRegularCounter++; } } + } else if (result != MessageQueueIF::EMPTY) { + sif::warning << "LiveTmTask: TM queue failure, returncode 0x" << std::hex << std::setw(4) + << result << std::dec << std::endl; } } - if (channelIsBusy) { - sif::debug << "busy" << std::endl; - } if (channelIsBusy and !throttlePeriodOngoing) { // Throttle CFDP packet creator. It is by far the most relevant data creator, so throttling // it is the easiest way to handle back pressure for now in a sensible way. @@ -174,12 +174,12 @@ ReturnValue_t LiveTmTask::handleGenericTmQueue(MessageQueueIF& queue, bool isCfd } if (!ptmeLocked) { - size_t partiallyWrittenSize = 0; - result = channel.write(data, size, partiallyWrittenSize); + size_t writtenSize = 0; + result = channel.write(data, size, writtenSize); if (result == DirectTmSinkIF::PARTIALLY_WRITTEN) { // Already throttle CFDP. throttleCfdp(); - result = channel.handleLastWriteSynchronously(data, size, partiallyWrittenSize, 200); + result = channel.handleWriteCompletionSynchronously(writtenSize, 200); if (result != returnvalue::OK) { // TODO: Event? Might lead to dangerous spam though.. sif::warning << "LiveTmTask: Synchronous write of last segment failed with code 0x" diff --git a/mission/com/TmStoreTaskBase.cpp b/mission/com/TmStoreTaskBase.cpp index c51d6f9b..215cef08 100644 --- a/mission/com/TmStoreTaskBase.cpp +++ b/mission/com/TmStoreTaskBase.cpp @@ -138,11 +138,10 @@ ReturnValue_t TmStoreTaskBase::performDump(PersistentTmStoreWithTmQueue& store, return result; } dumpedLen = tmReader.getFullPacketLen(); - size_t partiallyWrittenSize = 0; - result = channel.write(tmReader.getFullData(), dumpedLen, partiallyWrittenSize); + size_t writtenSize = 0; + result = channel.write(tmReader.getFullData(), dumpedLen, writtenSize); if (result == VirtualChannelIF::PARTIALLY_WRITTEN) { - result = channel.handleLastWriteSynchronously(tmReader.getFullData(), partiallyWrittenSize, - dumpedLen - partiallyWrittenSize, 200); + result = channel.handleWriteCompletionSynchronously(writtenSize, 200); if (result != returnvalue::OK) { // TODO: Event? Might lead to dangerous spam though.. sif::warning << "PersistentTmStore: Synchronous write of last segment failed with code 0x" diff --git a/mission/com/VirtualChannel.cpp b/mission/com/VirtualChannel.cpp index 7ada00aa..d6d792f4 100644 --- a/mission/com/VirtualChannel.cpp +++ b/mission/com/VirtualChannel.cpp @@ -21,11 +21,11 @@ ReturnValue_t VirtualChannel::write(const uint8_t* data, size_t size, size_t& wr uint8_t VirtualChannel::getVcid() const { return vcId; } -ReturnValue_t VirtualChannel::finishWrite(const uint8_t* data, size_t start, size_t remainingSize) { +ReturnValue_t VirtualChannel::advanceWrite(size_t& writtenSize) { if (!ptme.containsVc(vcId)) { return CHANNEL_DOES_NOT_EXIST; } - return ptme.getVirtChannel(vcId)->finishWrite(data, start, remainingSize); + return ptme.getVirtChannel(vcId)->advanceWrite(writtenSize); } const char* VirtualChannel::getName() const { return vcName.c_str(); } @@ -46,20 +46,27 @@ void VirtualChannel::cancelTransfer() { bool VirtualChannel::isTxOn() const { return txOn; } -ReturnValue_t VirtualChannel::handleLastWriteSynchronously(const uint8_t* data, size_t start, - size_t remLen, unsigned maxDelayMs) { +ReturnValue_t VirtualChannel::handleWriteCompletionSynchronously(size_t& writtenSize, + unsigned maxCompletionTimeMs) { unsigned delayMs = 0; while (true) { if (isBusy()) { - if (delayMs >= maxDelayMs) { + if (delayMs >= maxCompletionTimeMs) { break; } TaskFactory::delayTask(10); delayMs += 10; continue; } - sif::debug << "last write after" << delayMs << std::endl; - return finishWrite(data, start, remLen); + ReturnValue_t result = advanceWrite(writtenSize); + if (result == returnvalue::OK) { + sif::debug << "last write after" << delayMs << std::endl; + // Transfer complete + return result; + } else if (result != PARTIALLY_WRITTEN) { + // Some error where we can not or should not continue the transfer. + return result; + } } return returnvalue::FAILED; } diff --git a/mission/com/VirtualChannel.h b/mission/com/VirtualChannel.h index 3d61bd13..d8ac0228 100644 --- a/mission/com/VirtualChannel.h +++ b/mission/com/VirtualChannel.h @@ -32,9 +32,9 @@ class VirtualChannel : public SystemObject, public VirtualChannelIF { ReturnValue_t sendNextTm(const uint8_t* data, size_t size, size_t& writtenSize); bool isBusy() const override; ReturnValue_t write(const uint8_t* data, size_t size, size_t& writtenSize) override; - ReturnValue_t finishWrite(const uint8_t* data, size_t start, size_t remainingSize) override; - ReturnValue_t handleLastWriteSynchronously(const uint8_t* data, size_t start, size_t remLen, - unsigned maxDelayMs); + ReturnValue_t advanceWrite(size_t& writtenSize) override; + ReturnValue_t handleWriteCompletionSynchronously(size_t& writtenSize, + unsigned maxCompletionTimeMs); void cancelTransfer() override; uint8_t getVcid() const; bool isTxOn() const; diff --git a/mission/com/VirtualChannelWithQueue.cpp b/mission/com/VirtualChannelWithQueue.cpp index 53f2ee52..0d9e4d11 100644 --- a/mission/com/VirtualChannelWithQueue.cpp +++ b/mission/com/VirtualChannelWithQueue.cpp @@ -37,11 +37,11 @@ ReturnValue_t VirtualChannelWithQueue::handleNextTm(bool performWriteOp) { } // TODO: Hnadle partial write handling - size_t partiallyWrittenSize = 0; + size_t writtenSize = 0; if (performWriteOp) { - result = write(data, size, partiallyWrittenSize); + result = write(data, size, writtenSize); if (result == PARTIALLY_WRITTEN) { - result = handleLastWriteSynchronously(data, size, partiallyWrittenSize, 200); + result = handleWriteCompletionSynchronously(writtenSize, 200); if (result != returnvalue::OK) { // TODO: Event? Might lead to dangerous spam though.. sif::warning diff --git a/mission/tmtc/DirectTmSinkIF.h b/mission/tmtc/DirectTmSinkIF.h index e1fcf54f..f5b43039 100644 --- a/mission/tmtc/DirectTmSinkIF.h +++ b/mission/tmtc/DirectTmSinkIF.h @@ -14,7 +14,6 @@ class DirectTmSinkIF { static constexpr ReturnValue_t IS_BUSY = returnvalue::makeCode(CLASS_ID, 0); static constexpr ReturnValue_t PARTIALLY_WRITTEN = returnvalue::makeCode(CLASS_ID, 1); - static constexpr ReturnValue_t INCOMPLETE_PARTIAL_WRITE = returnvalue::makeCode(CLASS_ID, 2); /** * @brief Implements the functionality to write to a TM sink directly @@ -27,7 +26,7 @@ class DirectTmSinkIF { */ virtual ReturnValue_t write(const uint8_t* data, size_t size, size_t& writtenSize) = 0; - virtual ReturnValue_t finishWrite(const uint8_t* data, size_t start, size_t remainingSize) = 0; + virtual ReturnValue_t advanceWrite(size_t& writtenSize) = 0; virtual bool isBusy() const = 0; };