From f735c2e9d4f9d0c2f9803332f4b63b7e435b01eb Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 11:51:51 +0100 Subject: [PATCH 01/16] assert size larger than 0 --- src/fsfw/container/FixedArrayList.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw/container/FixedArrayList.h b/src/fsfw/container/FixedArrayList.h index 97ade7e8..dde8eb16 100644 --- a/src/fsfw/container/FixedArrayList.h +++ b/src/fsfw/container/FixedArrayList.h @@ -12,6 +12,7 @@ template class FixedArrayList : public ArrayList { static_assert(MAX_SIZE <= std::numeric_limits::max(), "count_t is not large enough to hold MAX_SIZE"); + static_assert(MAX_SIZE > 0, "MAX_SIZE is 0"); private: T data[MAX_SIZE]; From 066dd0d397a4ff0d94f3d193e1ecb8e64f2e7541 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 11:52:20 +0100 Subject: [PATCH 02/16] changelog --- CHANGELOG.md | 4 ++++ 1 file changed, 4 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc07f78..3a1ce06e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Changed + +- Assert than `FixedArrayList` is larger than 0 at compile time. + # [v6.0.0] 2023-02-10 ## Fixes From b057250bfb3bb87943797c4b32eb17f22558907a Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 11:53:12 +0100 Subject: [PATCH 03/16] changelog --- CHANGELOG.md | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 3a1ce06e..c56885d6 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,7 +10,7 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Changed -- Assert than `FixedArrayList` is larger than 0 at compile time. +- Assert that `FixedArrayList` is larger than 0 at compile time. # [v6.0.0] 2023-02-10 From d98ed40e3d429c60f837e8d0763edb32b44c6f07 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:09:30 +0100 Subject: [PATCH 04/16] add CFDP subsystem ID --- src/fsfw/events/fwSubsystemIdRanges.h | 1 + 1 file changed, 1 insertion(+) diff --git a/src/fsfw/events/fwSubsystemIdRanges.h b/src/fsfw/events/fwSubsystemIdRanges.h index d8e4ade6..574ea070 100644 --- a/src/fsfw/events/fwSubsystemIdRanges.h +++ b/src/fsfw/events/fwSubsystemIdRanges.h @@ -33,6 +33,7 @@ enum : uint8_t { PUS_SERVICE_23 = 103, MGM_LIS3MDL = 106, MGM_RM3100 = 107, + CFDP = 108, FW_SUBSYSTEM_ID_RANGE }; From 6fc8f756a733e7887f5a101cfce0fe30c35834ec Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:13:55 +0100 Subject: [PATCH 05/16] update power switch IF --- src/fsfw/events/fwSubsystemIdRanges.h | 2 +- src/fsfw/power/PowerSwitchIF.h | 11 +++++++---- 2 files changed, 8 insertions(+), 5 deletions(-) diff --git a/src/fsfw/events/fwSubsystemIdRanges.h b/src/fsfw/events/fwSubsystemIdRanges.h index d8e4ade6..88958758 100644 --- a/src/fsfw/events/fwSubsystemIdRanges.h +++ b/src/fsfw/events/fwSubsystemIdRanges.h @@ -10,7 +10,7 @@ enum : uint8_t { CDH = 28, TCS_1 = 59, PCDU_1 = 42, - PCDU_2 = 43, + POWER_SWITCH_IF = 43, HEATER = 50, T_SENSORS = 52, FDIR = 70, diff --git a/src/fsfw/power/PowerSwitchIF.h b/src/fsfw/power/PowerSwitchIF.h index e8c9aff4..0b331e11 100644 --- a/src/fsfw/power/PowerSwitchIF.h +++ b/src/fsfw/power/PowerSwitchIF.h @@ -28,10 +28,12 @@ class PowerSwitchIF { static const ReturnValue_t SWITCH_TIMEOUT = MAKE_RETURN_CODE(2); static const ReturnValue_t FUSE_ON = MAKE_RETURN_CODE(3); static const ReturnValue_t FUSE_OFF = MAKE_RETURN_CODE(4); - static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::PCDU_2; - static const Event SWITCH_WENT_OFF = MAKE_EVENT( - 0, severity::LOW); //!< Someone detected that a switch went off which shouldn't. Severity: - //!< Low, Parameter1: switchId1, Parameter2: switchId2 + static const ReturnValue_t SWITCH_UNKNOWN = MAKE_RETURN_CODE(5); + + static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::POWER_SWITCH_IF; + //!< Someone detected that a switch went off which shouldn't. Severity: + //!< Low, Parameter1: switchId1, Parameter2: switchId2 + static const Event SWITCH_WENT_OFF = MAKE_EVENT(0, severity::LOW); /** * send a direct command to the Power Unit to enable/disable the specified switch. * @@ -50,6 +52,7 @@ class PowerSwitchIF { * @return * - @c SWITCH_ON if the specified switch is on. * - @c SWITCH_OFF if the specified switch is off. + * - @c SWITCH_UNKNOWN if the state of the specified switch is unknown. * - @c returnvalue::FAILED if an error occured */ virtual ReturnValue_t getSwitchState(power::Switch_t switchNr) const = 0; From 8f63a0e747e3ab44d5a43c1e4d690d516f0e75a7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:15:21 +0100 Subject: [PATCH 06/16] changelog --- CHANGELOG.md | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc07f78..05a8743e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Changed + +- Renamed `PCDU_2` subsystem ID to `POWER_SWITCH_IF`. +- Add new `PowerSwitchIF::SWITCH_UNKNOWN` returnvalue. + # [v6.0.0] 2023-02-10 ## Fixes From aa84e93603f32ce5aaa7c308e80ae084c3d2d004 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:17:36 +0100 Subject: [PATCH 07/16] small tweak for version getter --- src/fsfw/version.cpp | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/src/fsfw/version.cpp b/src/fsfw/version.cpp index 050187a9..27d44c03 100644 --- a/src/fsfw/version.cpp +++ b/src/fsfw/version.cpp @@ -1,6 +1,7 @@ #include "version.h" #include +#include #include "fsfw/FSFWVersion.h" @@ -20,7 +21,7 @@ fsfw::Version::Version(int major, int minor, int revision, const char* addInfo) void fsfw::Version::getVersion(char* str, size_t maxLen) const { size_t len = snprintf(str, maxLen, "%d.%d.%d", major, minor, revision); - if (addInfo != nullptr) { + if (addInfo != nullptr and std::strcmp(addInfo, "") != 0) { snprintf(str + len, maxLen - len, "-%s", addInfo); } } @@ -30,7 +31,7 @@ namespace fsfw { #if FSFW_CPP_OSTREAM_ENABLED == 1 std::ostream& operator<<(std::ostream& os, const Version& v) { os << v.major << "." << v.minor << "." << v.revision; - if (v.addInfo != nullptr) { + if (v.addInfo != nullptr and std::strcmp(v.addInfo, "") != 0) { os << "-" << v.addInfo; } return os; From 1f36c082ef83a6c8a30992d319221c5b3bb5c70e Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:21:50 +0100 Subject: [PATCH 08/16] bugfix and changelog for Linux getUptime --- CHANGELOG.md | 4 ++++ src/fsfw/osal/linux/Clock.cpp | 11 +++++++---- 2 files changed, 11 insertions(+), 4 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc07f78..4a6f185b 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,10 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Fixes + +- Linux OSAL `getUptime` fix: Check validity of `/proc/uptime` file before reading uptime. + # [v6.0.0] 2023-02-10 ## Fixes diff --git a/src/fsfw/osal/linux/Clock.cpp b/src/fsfw/osal/linux/Clock.cpp index bfdcf4e2..7dd960e5 100644 --- a/src/fsfw/osal/linux/Clock.cpp +++ b/src/fsfw/osal/linux/Clock.cpp @@ -76,14 +76,17 @@ timeval Clock::getUptime() { } ReturnValue_t Clock::getUptime(timeval* uptime) { - // TODO This is not posix compatible and delivers only seconds precision - // Linux specific file read but more precise. double uptimeSeconds; - if (std::ifstream("/proc/uptime", std::ios::in) >> uptimeSeconds) { + std::ifstream ifile("/proc/uptime"); + if (ifile.bad()) { + return returnvalue::FAILED; + } + if (ifile >> uptimeSeconds) { uptime->tv_sec = uptimeSeconds; uptime->tv_usec = uptimeSeconds * (double)1e6 - (uptime->tv_sec * 1e6); + return returnvalue::OK; } - return returnvalue::OK; + return returnvalue::FAILED; } // Wait for new FSFW Clock function delivering seconds uptime. From 5e3f5c4121317ab62a89a028301251ca59edbe33 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:25:39 +0100 Subject: [PATCH 09/16] fuse update --- src/fsfw/power/Fuse.h | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fsfw/power/Fuse.h b/src/fsfw/power/Fuse.h index e8b86cfd..c1b35899 100644 --- a/src/fsfw/power/Fuse.h +++ b/src/fsfw/power/Fuse.h @@ -32,7 +32,7 @@ class Fuse : public SystemObject, gp_id_t poolIdPower; }; - static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::PCDU_1; + static const uint8_t SUBSYSTEM_ID = SUBSYSTEM_ID::POWER_SWITCH_IF; //! PSS detected that current on a fuse is totally out of bounds. static const Event FUSE_CURRENT_HIGH = MAKE_EVENT(1, severity::LOW); //! PSS detected a fuse that went off. From 47503824d7ca4cf656b9f603c8e10daac85b3393 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Wed, 15 Mar 2023 12:27:39 +0100 Subject: [PATCH 10/16] health service fixes and changelog --- CHANGELOG.md | 5 +++++ src/fsfw/pus/CServiceHealthCommanding.cpp | 5 ++++- 2 files changed, 9 insertions(+), 1 deletion(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 0cc07f78..f12aa90e 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,11 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Fixes + +- PUS Health Service: Size check for set health command. +- PUS Health Service: Perform operation completion for announce health command. + # [v6.0.0] 2023-02-10 ## Fixes diff --git a/src/fsfw/pus/CServiceHealthCommanding.cpp b/src/fsfw/pus/CServiceHealthCommanding.cpp index 7faf8174..3ced4ffb 100644 --- a/src/fsfw/pus/CServiceHealthCommanding.cpp +++ b/src/fsfw/pus/CServiceHealthCommanding.cpp @@ -82,6 +82,9 @@ ReturnValue_t CServiceHealthCommanding::prepareCommand(CommandMessage *message, ReturnValue_t result = returnvalue::OK; switch (subservice) { case (Subservice::COMMAND_SET_HEALTH): { + if (tcDataLen != sizeof(object_id_t) + sizeof(HasHealthIF::HealthState)) { + return CommandingServiceBase::INVALID_TC; + } HealthSetCommand healthCommand; result = healthCommand.deSerialize(&tcData, &tcDataLen, SerializeIF::Endianness::BIG); if (result != returnvalue::OK) { @@ -93,7 +96,7 @@ ReturnValue_t CServiceHealthCommanding::prepareCommand(CommandMessage *message, } case (Subservice::COMMAND_ANNOUNCE_HEALTH): { HealthMessage::setHealthMessage(message, HealthMessage::HEALTH_ANNOUNCE); - break; + return CommandingServiceBase::EXECUTION_COMPLETE; } case (Subservice::COMMAND_ANNOUNCE_HEALTH_ALL): { ReturnValue_t result = iterateHealthTable(true); From 025b379e8bc053c7daa9267e3fa77c33749ef3c9 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Thu, 4 May 2023 14:04:55 +0200 Subject: [PATCH 11/16] bump ETL version --- CMakeLists.txt | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/CMakeLists.txt b/CMakeLists.txt index 5eddde7f..5d35e6ff 100644 --- a/CMakeLists.txt +++ b/CMakeLists.txt @@ -72,7 +72,7 @@ set(FSFW_ETL_LIB_MAJOR_VERSION 20 CACHE STRING "ETL library major version requirement") set(FSFW_ETL_LIB_VERSION - ${FSFW_ETL_LIB_MAJOR_VERSION}.28.0 + ${FSFW_ETL_LIB_MAJOR_VERSION}.35.14 CACHE STRING "ETL library exact version requirement") set(FSFW_ETL_LINK_TARGET etl::etl) From 4518fec65ce92c0cc0f569036da8cbcf1b1a3d1e Mon Sep 17 00:00:00 2001 From: Ulrich Mohr Date: Mon, 8 May 2023 15:25:47 +0200 Subject: [PATCH 12/16] CHANGELOG --- CHANGELOG.md | 19 ++++++++++++++++--- 1 file changed, 16 insertions(+), 3 deletions(-) diff --git a/CHANGELOG.md b/CHANGELOG.md index 01420436..693e59e7 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -10,15 +10,28 @@ and this project adheres to [Semantic Versioning](http://semver.org/). ## Fixes -- Linux OSAL `getUptime` fix: Check validity of `/proc/uptime` file before reading uptime. - PUS Health Service: Size check for set health command. -- PUS Health Service: Perform operation completion for announce health command. + Perform operation completion for announce health command. + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/746 +- Linux OSAL `getUptime` fix: Check validity of `/proc/uptime` file before reading uptime. + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/745 +- Small tweak for version getter + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/744 + +## Added + +- add CFDP subsystem ID + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/742 ## Changed - +- Bump ETL version to 20.35.14 + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/748 - Renamed `PCDU_2` subsystem ID to `POWER_SWITCH_IF`. + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/743 - Add new `PowerSwitchIF::SWITCH_UNKNOWN` returnvalue. + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/743 - Assert that `FixedArrayList` is larger than 0 at compile time. + https://egit.irs.uni-stuttgart.de/fsfw/fsfw/pulls/740 # [v6.0.0] 2023-02-10 From cae131edcf95e6ab8c6415abb31e529aea553ba7 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 15 May 2023 16:02:55 +0200 Subject: [PATCH 13/16] CFDP and unittest bugfixes --- src/fsfw/cfdp/pdu/HeaderCreator.cpp | 4 ++-- src/fsfw/cfdp/pdu/HeaderReader.cpp | 4 ++-- unittests/CatchFactory.cpp | 2 +- unittests/action/TestActionHelper.cpp | 22 +++++----------------- unittests/cfdp/pdu/testCfdpHeader.cpp | 10 +++++----- unittests/cfdp/pdu/testFileData.cpp | 2 +- unittests/cfdp/pdu/testFileDirective.cpp | 4 ++-- unittests/cfdp/testCfdp.cpp | 4 ++-- 8 files changed, 20 insertions(+), 32 deletions(-) diff --git a/src/fsfw/cfdp/pdu/HeaderCreator.cpp b/src/fsfw/cfdp/pdu/HeaderCreator.cpp index 29688575..2db3953c 100644 --- a/src/fsfw/cfdp/pdu/HeaderCreator.cpp +++ b/src/fsfw/cfdp/pdu/HeaderCreator.cpp @@ -24,8 +24,8 @@ ReturnValue_t HeaderCreator::serialize(uint8_t **buffer, size_t *size, size_t ma *buffer += 1; **buffer = pduDataFieldLen & 0x00ff; *buffer += 1; - **buffer = segmentationCtrl << 7 | pduConf.sourceId.getWidth() << 4 | segmentMetadataFlag << 3 | - pduConf.seqNum.getWidth(); + **buffer = segmentationCtrl << 7 | ((pduConf.sourceId.getWidth() - 1) << 4) | + segmentMetadataFlag << 3 | (pduConf.seqNum.getWidth() - 1); *buffer += 1; *size += 4; ReturnValue_t result = pduConf.sourceId.serialize(buffer, size, maxSize, streamEndianness); diff --git a/src/fsfw/cfdp/pdu/HeaderReader.cpp b/src/fsfw/cfdp/pdu/HeaderReader.cpp index 9edf2394..de3d2906 100644 --- a/src/fsfw/cfdp/pdu/HeaderReader.cpp +++ b/src/fsfw/cfdp/pdu/HeaderReader.cpp @@ -78,11 +78,11 @@ cfdp::SegmentationControl PduHeaderReader::getSegmentationControl() const { } cfdp::WidthInBytes PduHeaderReader::getLenEntityIds() const { - return static_cast((pointers.fixedHeader->fourthByte >> 4) & 0x07); + return static_cast(((pointers.fixedHeader->fourthByte >> 4) & 0b111) + 1); } cfdp::WidthInBytes PduHeaderReader::getLenSeqNum() const { - return static_cast(pointers.fixedHeader->fourthByte & 0x07); + return static_cast((pointers.fixedHeader->fourthByte & 0b111) + 1); } cfdp::SegmentMetadataFlag PduHeaderReader::getSegmentMetadataFlag() const { diff --git a/unittests/CatchFactory.cpp b/unittests/CatchFactory.cpp index 0d855cb3..37c62f94 100644 --- a/unittests/CatchFactory.cpp +++ b/unittests/CatchFactory.cpp @@ -30,7 +30,7 @@ */ void Factory::produceFrameworkObjects(void* args) { setStaticFrameworkObjectIds(); - new EventManager(objects::EVENT_MANAGER); + new EventManager(objects::EVENT_MANAGER, 120); new HealthTable(objects::HEALTH_TABLE); new InternalErrorReporter(objects::INTERNAL_ERROR_REPORTER); diff --git a/unittests/action/TestActionHelper.cpp b/unittests/action/TestActionHelper.cpp index d99700d7..57166d5e 100644 --- a/unittests/action/TestActionHelper.cpp +++ b/unittests/action/TestActionHelper.cpp @@ -27,12 +27,8 @@ TEST_CASE("Action Helper", "[action]") { CHECK(not testDhMock.executeActionCalled); REQUIRE(actionHelper.handleActionMessage(&actionMessage) == returnvalue::OK); CHECK(testDhMock.executeActionCalled); - // No message is sent if everything is alright. CHECK(not testMqMock.wasMessageSent()); - store_address_t invalidAddress; - ActionMessage::setCommand(&actionMessage, testActionId, invalidAddress); - actionHelper.handleActionMessage(&actionMessage); - CHECK(testMqMock.wasMessageSent()); + CHECK(not testMqMock.wasMessageSent()); const uint8_t* ptr = nullptr; size_t size = 0; REQUIRE(ipcStore->getData(paramAddress, &ptr, &size) == @@ -44,6 +40,10 @@ TEST_CASE("Action Helper", "[action]") { for (uint8_t i = 0; i < 3; i++) { REQUIRE(ptr[i] == (i + 1)); } + // Action message without application data is also valid + store_address_t invalidAddress; + ActionMessage::setCommand(&actionMessage, testActionId, invalidAddress); + actionHelper.handleActionMessage(&actionMessage); testDhMock.clearBuffer(); } @@ -95,17 +95,5 @@ TEST_CASE("Action Helper", "[action]") { REQUIRE(ActionMessage::getActionId(&testMessage) == testActionId); } - SECTION("Missing IPC Data") { - ActionMessage::setCommand(&actionMessage, testActionId, store_address_t::invalid()); - CHECK(not testDhMock.executeActionCalled); - REQUIRE(actionHelper.handleActionMessage(&actionMessage) == returnvalue::OK); - CommandMessage testMessage; - REQUIRE(testMqMock.getNextSentMessage(testMessage) == returnvalue::OK); - REQUIRE(testMessage.getCommand() == static_cast(ActionMessage::STEP_FAILED)); - REQUIRE(ActionMessage::getReturnCode(&testMessage) == - static_cast(StorageManagerIF::ILLEGAL_STORAGE_ID)); - REQUIRE(ActionMessage::getStep(&testMessage) == 0); - } - SECTION("Data Reply") {} } diff --git a/unittests/cfdp/pdu/testCfdpHeader.cpp b/unittests/cfdp/pdu/testCfdpHeader.cpp index 5f81bec9..1fc7dfd4 100644 --- a/unittests/cfdp/pdu/testCfdpHeader.cpp +++ b/unittests/cfdp/pdu/testCfdpHeader.cpp @@ -97,7 +97,7 @@ TEST_CASE("CFDP Header", "[cfdp]") { REQUIRE(creator.serialize(&serTarget, &serSize, serBuf.size(), SerializeIF::Endianness::BIG) == returnvalue::OK); CHECK(serBuf[0] == 0x3f); - CHECK(serBuf[3] == 0x99); + CHECK(serBuf[3] == 0x88); REQUIRE(creator.getCrcFlag() == true); REQUIRE(creator.getDirection() == cfdp::Direction::TOWARDS_SENDER); REQUIRE(creator.getLargeFileFlag() == true); @@ -127,7 +127,7 @@ TEST_CASE("CFDP Header", "[cfdp]") { REQUIRE(creator.getTransmissionMode() == cfdp::TransmissionMode::UNACKNOWLEDGED); REQUIRE(creator.getSegmentationControl() == true); // Last three bits are 2 now (length of seq number) and bit 1 to bit 3 is 4 (len entity IDs) - REQUIRE(serBuf[3] == 0b11001010); + REQUIRE(serBuf[3] == 0b10111001); uint32_t entityId = 0; size_t deSerSize = 0; SerializeAdapter::deSerialize(&entityId, serBuf.data() + 4, &deSerSize, @@ -175,7 +175,7 @@ TEST_CASE("CFDP Header", "[cfdp]") { REQUIRE(serBuf[1] == 0); REQUIRE(serBuf[2] == 0); // Entity and Transaction Sequence number are 1 byte large - REQUIRE(serBuf[3] == 0b00010001); + REQUIRE(serBuf[3] == 0b00000000); // Source ID REQUIRE(serBuf[4] == 0); // Transaction Seq Number @@ -220,7 +220,7 @@ TEST_CASE("CFDP Header", "[cfdp]") { REQUIRE(serBuf[1] == 0); REQUIRE(serBuf[2] == 0); // Entity and Transaction Sequence number are 1 byte large - REQUIRE(serBuf[3] == 0b00010001); + REQUIRE(serBuf[3] == 0b00000000); REQUIRE(serSize == 7); // Deser call not strictly necessary auto reader = PduHeaderReader(serBuf.data(), serBuf.size()); @@ -270,7 +270,7 @@ TEST_CASE("CFDP Header", "[cfdp]") { REQUIRE(reader.parseData() == returnvalue::OK); // Everything except version bit flipped to one now REQUIRE(serBuf[0] == 0x3f); - REQUIRE(serBuf[3] == 0b11001010); + REQUIRE(serBuf[3] == 0b10111001); REQUIRE(reader.getWholePduSize() == 14); REQUIRE(reader.getCrcFlag() == true); diff --git a/unittests/cfdp/pdu/testFileData.cpp b/unittests/cfdp/pdu/testFileData.cpp index 258ef9c1..39139378 100644 --- a/unittests/cfdp/pdu/testFileData.cpp +++ b/unittests/cfdp/pdu/testFileData.cpp @@ -68,7 +68,7 @@ TEST_CASE("File Data PDU", "[cfdp][pdu]") { // Bits 1 to 3 length of enitity IDs is 2 // Bit 4: Segment metadata flag is set // Bit 5 to seven: length of transaction seq num is 2 - REQUIRE(fileDataBuffer[3] == 0b10101010); + REQUIRE(fileDataBuffer[3] == 0b10011001); REQUIRE((fileDataBuffer[10] >> 6) & 0b11 == cfdp::RecordContinuationState::CONTAINS_START_AND_END); // Segment metadata length diff --git a/unittests/cfdp/pdu/testFileDirective.cpp b/unittests/cfdp/pdu/testFileDirective.cpp index e1158a1a..17e0d699 100644 --- a/unittests/cfdp/pdu/testFileDirective.cpp +++ b/unittests/cfdp/pdu/testFileDirective.cpp @@ -30,7 +30,7 @@ TEST_CASE("CFDP File Directive", "[cfdp][pdu]") { REQUIRE(serBuf[1] == 0); REQUIRE(serBuf[2] == 5); // Entity and Transaction Sequence number are 1 byte large - REQUIRE(serBuf[3] == 0b00010001); + REQUIRE(serBuf[3] == 0b00000000); // Source ID REQUIRE(serBuf[4] == 0); // Transaction Seq Number @@ -82,4 +82,4 @@ TEST_CASE("CFDP File Directive", "[cfdp][pdu]") { // Invalid file directive REQUIRE(fdDeser.parseData() == cfdp::INVALID_DIRECTIVE_FIELD); } -} \ No newline at end of file +} diff --git a/unittests/cfdp/testCfdp.cpp b/unittests/cfdp/testCfdp.cpp index eacc83de..467b5b2d 100644 --- a/unittests/cfdp/testCfdp.cpp +++ b/unittests/cfdp/testCfdp.cpp @@ -33,8 +33,8 @@ TEST_CASE("CFDP Base", "[cfdp]") { // PDU data field length is 5 (4 + Directive code octet) REQUIRE(serBuf[1] == 0); REQUIRE(serBuf[2] == 5); - // Entity and Transaction Sequence number are 1 byte large - REQUIRE(serBuf[3] == 0b00010001); + // Entity and Transaction Sequence number are 1 byte large, value minus one is stored + REQUIRE(serBuf[3] == 0b00000000); // Source ID REQUIRE(serBuf[4] == 0); // Transaction Seq Number From 1a77e6bb095b275f8223d7f71f74eb6de188ec54 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 15 May 2023 16:13:05 +0200 Subject: [PATCH 14/16] proper floating point comparison --- unittests/datapoollocal/testLocalPoolManager.cpp | 13 ++----------- 1 file changed, 2 insertions(+), 11 deletions(-) diff --git a/unittests/datapoollocal/testLocalPoolManager.cpp b/unittests/datapoollocal/testLocalPoolManager.cpp index 91cd011d..e463a123 100644 --- a/unittests/datapoollocal/testLocalPoolManager.cpp +++ b/unittests/datapoollocal/testLocalPoolManager.cpp @@ -9,6 +9,7 @@ #include #include +#include #include #include "CatchDefinitions.h" @@ -309,9 +310,7 @@ TEST_CASE("Local Pool Manager Tests", "[LocManTest]") { HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, false); CHECK(poolOwner.poolManager.handleHousekeepingMessage(&hkCmd) == returnvalue::OK); - /* For non-diagnostics and a specified minimum frequency of 0.2 seconds, the - resulting collection interval should be 1.0 second */ - CHECK(poolOwner.dataset.getCollectionInterval() == 1.0); + CHECK_THAT(poolOwner.dataset.getCollectionInterval(), Catch::Matchers::WithinAbs(0.4, 0.01)); REQUIRE(poolOwnerMock.wasMessageSent()); REQUIRE(poolOwnerMock.numberOfSentMessages() == 1); CHECK(poolOwnerMock.clearLastSentMessage() == returnvalue::OK); @@ -348,14 +347,6 @@ TEST_CASE("Local Pool Manager Tests", "[LocManTest]") { REQUIRE(poolOwnerMock.numberOfSentMessages() == 1); CHECK(poolOwnerMock.clearLastSentMessage() == returnvalue::OK); - HousekeepingMessage::setCollectionIntervalModificationCommand(&hkCmd, lpool::testSid, 0.4, - false); - CHECK(poolOwner.poolManager.handleHousekeepingMessage(&hkCmd) == - static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); - REQUIRE(poolOwnerMock.wasMessageSent()); - REQUIRE(poolOwnerMock.numberOfSentMessages() == 1); - CHECK(poolOwnerMock.clearLastSentMessage() == returnvalue::OK); - HousekeepingMessage::setStructureReportingCommand(&hkCmd, lpool::testSid, false); CHECK(poolOwner.poolManager.handleHousekeepingMessage(&hkCmd) == static_cast(LocalDataPoolManager::WRONG_HK_PACKET_TYPE)); From 4391823f01d792bcc078d47b60f7df7624f8cbe4 Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 15 May 2023 16:14:36 +0200 Subject: [PATCH 15/16] changelog --- CHANGELOG.md | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index a841e38d..2250a301 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -8,6 +8,12 @@ and this project adheres to [Semantic Versioning](http://semver.org/). # [unreleased] +## Fixed + +- Important bugfix in CFDP PDU header format: The entity length field and the transaction sequence + number fields stored the actual length of the field instead of the length minus 1 like specified + in the CFDP standard. + ## Changed - Health functions are virtual now. From e0adb3325f4a26f6c112ef4b83114319b67cdbff Mon Sep 17 00:00:00 2001 From: Robin Mueller Date: Mon, 22 May 2023 10:37:26 +0200 Subject: [PATCH 16/16] werks --- unittests/action/TestActionHelper.cpp | 1 - 1 file changed, 1 deletion(-) diff --git a/unittests/action/TestActionHelper.cpp b/unittests/action/TestActionHelper.cpp index 57166d5e..de021bb8 100644 --- a/unittests/action/TestActionHelper.cpp +++ b/unittests/action/TestActionHelper.cpp @@ -28,7 +28,6 @@ TEST_CASE("Action Helper", "[action]") { REQUIRE(actionHelper.handleActionMessage(&actionMessage) == returnvalue::OK); CHECK(testDhMock.executeActionCalled); CHECK(not testMqMock.wasMessageSent()); - CHECK(not testMqMock.wasMessageSent()); const uint8_t* ptr = nullptr; size_t size = 0; REQUIRE(ipcStore->getData(paramAddress, &ptr, &size) ==