From b7ff78712cbb9b24c3c46bdfc5d0213f3b127cab Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Nov 2023 13:18:05 +0100 Subject: [PATCH 1/7] revert some changes in com IF --- linux/acs/StrComHandler.cpp | 6 +++--- 1 file changed, 3 insertions(+), 3 deletions(-) diff --git a/linux/acs/StrComHandler.cpp b/linux/acs/StrComHandler.cpp index 2db73f35..0bb4d749 100644 --- a/linux/acs/StrComHandler.cpp +++ b/linux/acs/StrComHandler.cpp @@ -54,7 +54,7 @@ ReturnValue_t StrComHandler::performOperation(uint8_t operationCode) { switch (state) { case InternalState::POLL_ONE_REPLY: { // Stopwatch watch; - replyTimeout.setTimeout(200); + replyTimeout.setTimeout(400); readOneReply(static_cast(state)); { MutexGuard mg(lock); @@ -720,7 +720,7 @@ ReturnValue_t StrComHandler::readReceivedMessage(CookieIF* cookie, uint8_t** buf { MutexGuard mg(lock); if (state != InternalState::SLEEPING) { - return returnvalue::OK; + return BUSY; } replyWasReceived = this->replyWasReceived; } @@ -733,7 +733,7 @@ ReturnValue_t StrComHandler::readReceivedMessage(CookieIF* cookie, uint8_t** buf *size = replyLen; } replyLen = 0; - return returnvalue::OK; + return replyResult; } ReturnValue_t StrComHandler::unlockAndEraseRegions(uint32_t from, uint32_t to) { From 314df7a02105b29852b9e1d5f6393a17770526c0 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Nov 2023 16:48:40 +0100 Subject: [PATCH 2/7] bugfix for virt channel: clear invalid state --- mission/com/LiveTmTask.cpp | 9 +++++---- mission/com/TmStoreTaskBase.cpp | 7 ++++--- mission/com/VirtualChannel.cpp | 4 +++- mission/com/VirtualChannelWithQueue.cpp | 2 +- mission/tmtc/DirectTmSinkIF.h | 1 + 5 files changed, 14 insertions(+), 9 deletions(-) diff --git a/mission/com/LiveTmTask.cpp b/mission/com/LiveTmTask.cpp index 56fe5a51..c671090c 100644 --- a/mission/com/LiveTmTask.cpp +++ b/mission/com/LiveTmTask.cpp @@ -173,15 +173,16 @@ ReturnValue_t LiveTmTask::handleGenericTmQueue(MessageQueueIF& queue, bool isCfd size_t writtenSize = 0; result = channel.write(data, size, writtenSize); if (result == DirectTmSinkIF::PARTIALLY_WRITTEN) { - result = channel.handleWriteCompletionSynchronously(writtenSize, 200); + result = channel.handleWriteCompletionSynchronously(writtenSize, 400); if (result != returnvalue::OK) { // TODO: Event? Might lead to dangerous spam though.. sif::warning << "LiveTmTask: Synchronous write of last segment failed with code 0x" - << std::setw(4) << std::hex << result << std::dec << std::endl; + << std::setfill('0') << std::setw(4) << std::hex << result << std::dec + << std::endl; } } else if (result != returnvalue::OK) { - sif::error << "LiveTmTask: Channel write failed with code 0x" << std::hex << std::setw(4) - << result << std::dec << std::endl; + sif::error << "LiveTmTask: Channel write failed with code 0x" << std::setfill('0') << std::hex + << std::setw(4) << result << std::dec << std::endl; } } // Try delete in any case, ignore failures (which should not happen), it is more important to diff --git a/mission/com/TmStoreTaskBase.cpp b/mission/com/TmStoreTaskBase.cpp index 215cef08..9e86ba08 100644 --- a/mission/com/TmStoreTaskBase.cpp +++ b/mission/com/TmStoreTaskBase.cpp @@ -141,11 +141,12 @@ ReturnValue_t TmStoreTaskBase::performDump(PersistentTmStoreWithTmQueue& store, size_t writtenSize = 0; result = channel.write(tmReader.getFullData(), dumpedLen, writtenSize); if (result == VirtualChannelIF::PARTIALLY_WRITTEN) { - result = channel.handleWriteCompletionSynchronously(writtenSize, 200); + result = channel.handleWriteCompletionSynchronously(writtenSize, 400); if (result != returnvalue::OK) { // TODO: Event? Might lead to dangerous spam though.. - sif::warning << "PersistentTmStore: Synchronous write of last segment failed with code 0x" - << std::setw(4) << std::hex << result << std::dec << std::endl; + sif::warning << "LiveTmTask: Synchronous write of last segment failed with code 0x" + << std::setfill('0') << std::setw(4) << std::hex << result << std::dec + << std::endl; } } else if (result == DirectTmSinkIF::IS_BUSY) { sif::warning << "PersistentTmStore: Unexpected VC channel busy" << std::endl; diff --git a/mission/com/VirtualChannel.cpp b/mission/com/VirtualChannel.cpp index e531727e..a75959e6 100644 --- a/mission/com/VirtualChannel.cpp +++ b/mission/com/VirtualChannel.cpp @@ -74,5 +74,7 @@ ReturnValue_t VirtualChannel::handleWriteCompletionSynchronously(size_t& written return result; } } - return returnvalue::FAILED; + // Timeout. Cancel the transfer + cancelTransfer(); + return TIMEOUT; } diff --git a/mission/com/VirtualChannelWithQueue.cpp b/mission/com/VirtualChannelWithQueue.cpp index 0d9e4d11..fb3375dc 100644 --- a/mission/com/VirtualChannelWithQueue.cpp +++ b/mission/com/VirtualChannelWithQueue.cpp @@ -41,7 +41,7 @@ ReturnValue_t VirtualChannelWithQueue::handleNextTm(bool performWriteOp) { if (performWriteOp) { result = write(data, size, writtenSize); if (result == PARTIALLY_WRITTEN) { - result = handleWriteCompletionSynchronously(writtenSize, 200); + result = handleWriteCompletionSynchronously(writtenSize, 400); 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 ff8ab6fe..546b293e 100644 --- a/mission/tmtc/DirectTmSinkIF.h +++ b/mission/tmtc/DirectTmSinkIF.h @@ -15,6 +15,7 @@ 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 NO_WRITE_ACTIVE = returnvalue::makeCode(CLASS_ID, 2); + static constexpr ReturnValue_t TIMEOUT = returnvalue::makeCode(CLASS_ID, 3); /** * @brief Implements the functionality to write to a TM sink directly. From 352043cb51de23b5c598632e48e2fad23b0ea370 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Nov 2023 17:20:55 +0100 Subject: [PATCH 3/7] printout improvements --- linux/ipcore/PapbVcInterface.cpp | 1 + mission/com/LiveTmTask.cpp | 9 +++++++-- 2 files changed, 8 insertions(+), 2 deletions(-) diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index c02d0935..a176c3af 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -30,6 +30,7 @@ ReturnValue_t PapbVcInterface::initialize() { ReturnValue_t PapbVcInterface::write(const uint8_t* data, size_t size, size_t& writtenSize) { // There are no packets smaller than 4, this is considered a configuration error. if (size < 4) { + sif::warning << "PapbVcInterface::write: Passed packet smaller than 4 bytes" << std::endl; return returnvalue::FAILED; } // The user must call advance until completion before starting a new packet transfer. diff --git a/mission/com/LiveTmTask.cpp b/mission/com/LiveTmTask.cpp index c671090c..33b9322a 100644 --- a/mission/com/LiveTmTask.cpp +++ b/mission/com/LiveTmTask.cpp @@ -54,8 +54,13 @@ ReturnValue_t LiveTmTask::performOperation(uint8_t opCode) { } } } else if (result != MessageQueueIF::EMPTY) { - sif::warning << "LiveTmTask: TM queue failure, returncode 0x" << std::hex << std::setw(4) - << result << std::dec << std::endl; + const char* contextStr = "Regular TM queue"; + if (isCfdp) { + contextStr = "CFDP TM queue"; + } + sif::warning << "LiveTmTask: " << contextStr << " handling failure, returncode 0x" + << std::setfill('0') << std::hex << std::setw(4) << result << std::dec + << std::endl; } } From ef948af5f33a4aea98ecb6ec973b1f019e1dd2ca Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Nov 2023 17:22:36 +0100 Subject: [PATCH 4/7] small possible fix --- linux/ipcore/PapbVcInterface.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/linux/ipcore/PapbVcInterface.cpp b/linux/ipcore/PapbVcInterface.cpp index a176c3af..e4414363 100644 --- a/linux/ipcore/PapbVcInterface.cpp +++ b/linux/ipcore/PapbVcInterface.cpp @@ -84,6 +84,9 @@ ReturnValue_t PapbVcInterface::advanceWrite(size_t& writtenSize) { writtenSize++; } if (not pollReadyForOctet(MAX_BUSY_POLLS)) { + if (not pollReadyForPacket()) { + return PARTIALLY_WRITTEN; + } abortPacketTransfer(); return returnvalue::FAILED; } From 9e31c065632807e6c3b0eaf3c6add65c467af110 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 23 Nov 2023 17:47:44 +0100 Subject: [PATCH 5/7] changelog update --- CHANGELOG.md | 7 +++++++ 1 file changed, 7 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74bd77e3..7369c89f 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -16,6 +16,13 @@ will consitute of a breaking change warranting a new major release: # [unreleased] +## Fixed + +- Increase allowed time for PTME writers to finish partial transfers. A duration of 200 ms was + not sufficient for cases where 3 writers write concurrently. +- Fixed state issue for PTME writer object where the writer was not reset properly after a timeout + of a partial transfer. This bloked the whole VC. + # [v7.3.0] 2023-11-07 ## Changed From 14e545618c8a602e5cf8d23a633cd0809a6ae6a7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Tue, 28 Nov 2023 15:42:36 +0100 Subject: [PATCH 6/7] typo --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 7369c89f..19159f5c 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -21,7 +21,7 @@ will consitute of a breaking change warranting a new major release: - Increase allowed time for PTME writers to finish partial transfers. A duration of 200 ms was not sufficient for cases where 3 writers write concurrently. - Fixed state issue for PTME writer object where the writer was not reset properly after a timeout - of a partial transfer. This bloked the whole VC. + of a partial transfer. This was a major bug blocking the whole VC if it occured. # [v7.3.0] 2023-11-07 From 178da2fec15de2365c5d727a9e8e5cc7cb096a93 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 29 Nov 2023 14:17:17 +0100 Subject: [PATCH 7/7] changelog update --- CHANGELOG.md | 1 + 1 file changed, 1 insertion(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 74bd77e3..019a9ea3 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -23,6 +23,7 @@ will consitute of a breaking change warranting a new major release: - Changed PDEC addresses depending on which firmware version is used. It is suspected that the previous addresses were invalid and not properly covered by the Linux memory protection. The OBSW will use the old addresses for older FW versions. +- Reverted some STR ComIF behaviour back to an older software version. ## Added