From 44615c150b7690a3beccf459e3203561f17edf0a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 14 Sep 2022 14:00:20 +0200 Subject: [PATCH] add printout capabilities --- src/fsfw/cfdp/VarLenFields.h | 15 ++++++++ src/fsfw/cfdp/handler/DestHandler.cpp | 38 ++++++++++++------- src/fsfw/cfdp/handler/mib.h | 1 + .../ServiceInterfaceBuffer.cpp | 1 - src/fsfw_hal/common/CMakeLists.txt | 2 +- src/fsfw_hal/common/printChar.c | 2 +- 6 files changed, 43 insertions(+), 16 deletions(-) diff --git a/src/fsfw/cfdp/VarLenFields.h b/src/fsfw/cfdp/VarLenFields.h index 37a9cf5c..37602c75 100644 --- a/src/fsfw/cfdp/VarLenFields.h +++ b/src/fsfw/cfdp/VarLenFields.h @@ -7,6 +7,7 @@ #include "fsfw/cfdp/definitions.h" #include "fsfw/serialize/SerializeIF.h" +#include "fsfw/serviceinterface.h" #include "fsfw/util/UnsignedByteField.h" namespace cfdp { @@ -43,6 +44,14 @@ class VarLenField : public SerializeIF { [[nodiscard]] cfdp::WidthInBytes getWidth() const; [[nodiscard]] size_t getValue() const; +#if FSFW_CPP_OSTREAM_ENABLED == 1 + friend std::ostream &operator<<(std::ostream &os, const VarLenField &id) { + os << "dec: " << id.getValue() << ", hex: " << std::hex << std::setw(id.getWidth()) + << std::setfill('0') << id.getValue() << std::dec << std::setfill('0'); + return os; + } +#endif + private: ReturnValue_t deSerialize(const uint8_t **buffer, size_t *size, Endianness streamEndianness) override; @@ -83,6 +92,12 @@ struct TransactionId { return entityId == other.entityId and seqNum == other.seqNum; } +#if FSFW_CPP_OSTREAM_ENABLED == 1 + friend std::ostream &operator<<(std::ostream &os, const TransactionId &id) { + os << "Source ID { " << id.entityId << " }, Sequence Number " << id.seqNum.getValue(); + return os; + } +#endif EntityId entityId; TransactionSeqNum seqNum; }; diff --git a/src/fsfw/cfdp/handler/DestHandler.cpp b/src/fsfw/cfdp/handler/DestHandler.cpp index c4566814..d43d1482 100644 --- a/src/fsfw/cfdp/handler/DestHandler.cpp +++ b/src/fsfw/cfdp/handler/DestHandler.cpp @@ -252,7 +252,8 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, headerReader.getDestId(destId); RemoteEntityCfg* remoteCfg; if (not dp.remoteCfgTable.getRemoteCfg(destId, &remoteCfg)) { -// TODO: No remote config for dest ID. I consider this a configuration error. +// TODO: No remote config for dest ID. I consider this a configuration error, which is not +// covered by the standard. // Warning or error, yield or cache appropriate returnvalue #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "No remote config exists for destination ID" << std::endl; @@ -267,8 +268,9 @@ ReturnValue_t cfdp::DestHandler::handleMetadataParseError(ReturnValue_t result, ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, MetadataInfo& info) { if (fsmRes.state != CfdpStates::IDLE) { // According to standard, discard metadata PDU if we are busy - return returnvalue::OK; + return OK; } + ReturnValue_t result = OK; fsmRes.step = TransactionStep::TRANSACTION_START; if (reader.getTransmissionMode() == TransmissionModes::UNACKNOWLEDGED) { fsmRes.state = CfdpStates::BUSY_CLASS_1_NACKED; @@ -302,10 +304,26 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met #if FSFW_CPP_OSTREAM_ENABLED == 1 sif::warning << "cfdp::DestHandler" << __func__ << ": No remote configuration found for destination ID " - << tp.pduConf.destId.getValue() << std::endl; + << tp.pduConf.sourceId.getValue() << std::endl; #endif return FAILED; } + // If both dest name size and source name size are 0, we are dealing with a metadata only PDU, + // so there is no need to create a file or truncate an existing file + if (destNameSize > 0 and sourceNameSize > 0) { + FilesystemParams fparams(tp.destName.data()); + // TODO: Filesystem errors? + if (dp.user.vfs.fileExists(fparams)) { + dp.user.vfs.truncateFile(fparams); + } else { + result = dp.user.vfs.createFile(fparams); + if (result != OK) { + // TODO: Handle FS error. This is probably a case for the filestore rejection mechanism of + // CFDP. + // In any case, it does not really make sense to continue here + } + } + } fsmRes.step = TransactionStep::RECEIVING_FILE_DATA_PDUS; MetadataRecvdParams params(tp.transactionId, tp.pduConf.sourceId); params.fileSize = tp.fileSize.getSize(); @@ -313,15 +331,8 @@ ReturnValue_t cfdp::DestHandler::startTransaction(MetadataPduReader& reader, Met params.sourceFileName = tp.sourceName.data(); params.msgsToUserArray = dynamic_cast(userTlvVec.data()); params.msgsToUserLen = info.getOptionsLen(); - FilesystemParams fparams(tp.destName.data()); - // TODO: Filesystem errors? - if (dp.user.vfs.fileExists(fparams)) { - dp.user.vfs.truncateFile(fparams); - } else { - dp.user.vfs.createFile(fparams); - } dp.user.metadataRecvdIndication(params); - return OK; + return result; } cfdp::CfdpStates cfdp::DestHandler::getCfdpState() const { return fsmRes.state; } @@ -376,7 +387,8 @@ ReturnValue_t cfdp::DestHandler::checksumVerification() { params.size = readLen; auto result = dp.user.vfs.readFromFile(params, buf.data(), buf.size()); if (result != OK) { - // TODO: Better error handling + // TODO: I think this is a case for a filestore rejection, but it might sense to print + // a warning or trigger an event because this should generally not happen return FAILED; } crcCalc.add(buf.begin(), buf.begin() + readLen); @@ -391,7 +403,7 @@ ReturnValue_t cfdp::DestHandler::checksumVerification() { } else { // TODO: Proper error handling #if FSFW_CPP_OSTREAM_ENABLED == 1 - sif::error << "CRC check for file " << tp.sourceName.data() << " failed" << std::endl; + sif::warning << "CRC check for file " << tp.destName.data() << " failed" << std::endl; #endif tp.conditionCode = ConditionCode::FILE_CHECKSUM_FAILURE; } diff --git a/src/fsfw/cfdp/handler/mib.h b/src/fsfw/cfdp/handler/mib.h index 3b4c95e4..76284f2d 100644 --- a/src/fsfw/cfdp/handler/mib.h +++ b/src/fsfw/cfdp/handler/mib.h @@ -27,6 +27,7 @@ struct LocalEntityCfg { }; struct RemoteEntityCfg { + explicit RemoteEntityCfg(EntityId id) : remoteId(std::move(id)) {} EntityId remoteId; size_t maxFileSegmentLen = 2048; bool closureRequested = false; diff --git a/src/fsfw/serviceinterface/ServiceInterfaceBuffer.cpp b/src/fsfw/serviceinterface/ServiceInterfaceBuffer.cpp index 23892dcc..0e73be83 100644 --- a/src/fsfw/serviceinterface/ServiceInterfaceBuffer.cpp +++ b/src/fsfw/serviceinterface/ServiceInterfaceBuffer.cpp @@ -3,7 +3,6 @@ #if FSFW_CPP_OSTREAM_ENABLED == 1 #include - #include #include "fsfw/serviceinterface/serviceInterfaceDefintions.h" diff --git a/src/fsfw_hal/common/CMakeLists.txt b/src/fsfw_hal/common/CMakeLists.txt index 1cd9c678..2f4608f8 100644 --- a/src/fsfw_hal/common/CMakeLists.txt +++ b/src/fsfw_hal/common/CMakeLists.txt @@ -1,3 +1,3 @@ add_subdirectory(gpio) -target_sources(${LIB_FSFW_NAME} PRIVATE printChar.c) \ No newline at end of file +target_sources(${LIB_FSFW_NAME} PRIVATE printChar.c) diff --git a/src/fsfw_hal/common/printChar.c b/src/fsfw_hal/common/printChar.c index 6e02c1df..24fba5c8 100644 --- a/src/fsfw_hal/common/printChar.c +++ b/src/fsfw_hal/common/printChar.c @@ -1,5 +1,5 @@ -#include #include +#include void __attribute__((weak)) printChar(const char* character, bool errStream) { if (errStream) {