From 05aba7c0487c20de6387b059a88e6d9d9a896ec5 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Thu, 26 Jun 2025 10:43:32 -0700 Subject: [PATCH 1/2] Format Svc/Ccsds package --- Svc/Ccsds/ApidManager/ApidManager.hpp | 2 +- .../test/ut/ApidManagerTestMain.cpp | 25 ++------ .../ApidManager/test/ut/ApidManagerTester.cpp | 47 +++++++------- .../SpacePacketDeframer.cpp | 2 +- .../SpacePacketDeframer.hpp | 5 +- .../test/ut/SpacePacketDeframerTester.cpp | 25 +++++--- .../test/ut/SpacePacketDeframerTester.hpp | 4 +- .../SpacePacketFramer/SpacePacketFramer.cpp | 34 ++++++---- .../SpacePacketFramer/SpacePacketFramer.hpp | 1 - .../test/ut/SpacePacketFramerTester.cpp | 5 +- Svc/Ccsds/TcDeframer/TcDeframer.hpp | 3 +- .../TcDeframer/test/ut/TcDeframerTester.cpp | 64 ++++++++++--------- .../TcDeframer/test/ut/TcDeframerTester.hpp | 3 +- Svc/Ccsds/TmFramer/TmFramer.cpp | 5 +- Svc/Ccsds/TmFramer/test/ut/TmFramerTester.cpp | 20 +++--- 15 files changed, 121 insertions(+), 124 deletions(-) diff --git a/Svc/Ccsds/ApidManager/ApidManager.hpp b/Svc/Ccsds/ApidManager/ApidManager.hpp index db7f2ea9fc1..458a333e012 100644 --- a/Svc/Ccsds/ApidManager/ApidManager.hpp +++ b/Svc/Ccsds/ApidManager/ApidManager.hpp @@ -7,8 +7,8 @@ #ifndef Svc_Ccsds_ApidManager_HPP #define Svc_Ccsds_ApidManager_HPP -#include "Svc/Ccsds/ApidManager/ApidManagerComponentAc.hpp" #include "Fw/Com/ComPacket.hpp" +#include "Svc/Ccsds/ApidManager/ApidManagerComponentAc.hpp" namespace Svc { diff --git a/Svc/Ccsds/ApidManager/test/ut/ApidManagerTestMain.cpp b/Svc/Ccsds/ApidManager/test/ut/ApidManagerTestMain.cpp index d4698a1c0b3..06ff00f6217 100644 --- a/Svc/Ccsds/ApidManager/test/ut/ApidManagerTestMain.cpp +++ b/Svc/Ccsds/ApidManager/test/ut/ApidManagerTestMain.cpp @@ -26,9 +26,8 @@ TEST(ApidManager, ValidateSequenceCounts) { // Randomized testing TEST(ApidManager, RandomizedTesting) { - Svc::Ccsds::ApidManagerTester tester; - + Svc::Ccsds::ApidManagerTester::GetExistingSeqCount getExistingSeqCountRule; Svc::Ccsds::ApidManagerTester::GetNewSeqCountOk getNewSeqCountOkRule; Svc::Ccsds::ApidManagerTester::GetNewSeqCountTableFull getNewSeqCountTableFullRule; @@ -36,27 +35,15 @@ TEST(ApidManager, RandomizedTesting) { Svc::Ccsds::ApidManagerTester::ValidateSeqCountFailure validateSeqCountFailureRule; // Place these rules into a list of rules - STest::Rule* rules[] = { - &getExistingSeqCountRule, - &getNewSeqCountOkRule, - &getNewSeqCountTableFullRule, - &validateSeqCountOkRule, - &validateSeqCountFailureRule - }; + STest::Rule* rules[] = {&getExistingSeqCountRule, &getNewSeqCountOkRule, + &getNewSeqCountTableFullRule, &validateSeqCountOkRule, + &validateSeqCountFailureRule}; // Take the rules and place them into a random scenario - STest::RandomScenario random( - "Random Rules", - rules, - FW_NUM_ARRAY_ELEMENTS(rules) - ); + STest::RandomScenario random("Random Rules", rules, FW_NUM_ARRAY_ELEMENTS(rules)); // Create a bounded scenario wrapping the random scenario - STest::BoundedScenario bounded( - "Bounded Random Rules Scenario", - random, - 1000 - ); + STest::BoundedScenario bounded("Bounded Random Rules Scenario", random, 1000); // Run! const U32 numSteps = bounded.run(tester); printf("Ran %u steps.\n", numSteps); diff --git a/Svc/Ccsds/ApidManager/test/ut/ApidManagerTester.cpp b/Svc/Ccsds/ApidManager/test/ut/ApidManagerTester.cpp index d6aa81e2ab0..50efe479978 100644 --- a/Svc/Ccsds/ApidManager/test/ut/ApidManagerTester.cpp +++ b/Svc/Ccsds/ApidManager/test/ut/ApidManagerTester.cpp @@ -12,12 +12,10 @@ namespace Svc { namespace Ccsds { -static constexpr ComCfg::APID::T TEST_REGISTERED_APIDS[] = { - ComCfg::APID::FW_PACKET_COMMAND, - ComCfg::APID::FW_PACKET_TELEM, - ComCfg::APID::FW_PACKET_LOG, - ComCfg::APID::FW_PACKET_FILE -}; +static constexpr ComCfg::APID::T TEST_REGISTERED_APIDS[] = {ComCfg::APID::FW_PACKET_COMMAND, + ComCfg::APID::FW_PACKET_TELEM, ComCfg::APID::FW_PACKET_LOG, + ComCfg::APID::FW_PACKET_FILE}; + // ---------------------------------------------------------------------- // Construction and destruction // ---------------------------------------------------------------------- @@ -30,7 +28,6 @@ ApidManagerTester ::ApidManagerTester() for (FwIndexType i = 0; i < static_cast(FW_NUM_ARRAY_ELEMENTS(TEST_REGISTERED_APIDS)); i++) { this->component.m_apidSequences[i].apid = TEST_REGISTERED_APIDS[i]; this->shadow_seqCounts[TEST_REGISTERED_APIDS[i]] = 0; // Initialize shadow sequence counts to 0 - } } @@ -41,7 +38,7 @@ ApidManagerTester ::~ApidManagerTester() {} // ---------------------------------------------------------------------- bool ApidManagerTester::GetExistingSeqCount::precondition(const ApidManagerTester& testerState) { - return true; // Can always get existing sequence count + return true; // Can always get existing sequence count } void ApidManagerTester::GetExistingSeqCount::action(ApidManagerTester& testerState) { @@ -49,12 +46,11 @@ void ApidManagerTester::GetExistingSeqCount::action(ApidManagerTester& testerSta ComCfg::APID::T apid = testerState.shadow_getRandomTrackedApid(); U16 seqCount = testerState.invoke_to_getApidSeqCountIn(0, apid, 0); U16 shadowSeqCount = testerState.shadow_getAndIncrementSeqCount(apid); - ASSERT_EQ(seqCount, shadowSeqCount) - << "Sequence count for APID " << static_cast(apid) << " does not match shadow value." - << " Shadow: " << shadowSeqCount << ", Actual: " << seqCount; + ASSERT_EQ(seqCount, shadowSeqCount) << "Sequence count for APID " << static_cast(apid) + << " does not match shadow value." + << " Shadow: " << shadowSeqCount << ", Actual: " << seqCount; } - bool ApidManagerTester::GetNewSeqCountOk::precondition(const ApidManagerTester& testerState) { return testerState.shadow_isTableFull == false; } @@ -66,15 +62,15 @@ void ApidManagerTester::GetNewSeqCountOk::action(ApidManagerTester& testerState) bool isTableFull = !(testerState.shadow_seqCounts.size() < maxTrackedApidsVal); if (isTableFull) { testerState.shadow_isTableFull = true; - return; // Cannot get new sequence count if table is full - skip action + return; // Cannot get new sequence count if table is full - skip action } ComCfg::APID::T apid = testerState.shadow_getRandomUntrackedApid(); U16 seqCount = testerState.invoke_to_getApidSeqCountIn(0, apid, 0); U16 shadowSeqCount = testerState.shadow_getAndIncrementSeqCount(apid); - ASSERT_EQ(seqCount, shadowSeqCount) - << "Sequence count for APID " << static_cast(apid) << " does not match shadow value." - << " Shadow: " << shadowSeqCount << ", Actual: " << seqCount; + ASSERT_EQ(seqCount, shadowSeqCount) << "Sequence count for APID " << static_cast(apid) + << " does not match shadow value." + << " Shadow: " << shadowSeqCount << ", Actual: " << seqCount; } bool ApidManagerTester::GetNewSeqCountTableFull::precondition(const ApidManagerTester& testerState) { @@ -88,8 +84,7 @@ void ApidManagerTester::GetNewSeqCountTableFull::action(ApidManagerTester& teste // Use local constexpr to potentially avoid ODR-use of ApidManager::SEQUENCE_COUNT_ERROR constexpr U16 sequenceCountErrorVal = ApidManager::SEQUENCE_COUNT_ERROR; ASSERT_EQ(seqCount, sequenceCountErrorVal) - << "Expected SEQUENCE_COUNT_ERROR for untracked APID " << static_cast(apid) - << ", but got " << seqCount; + << "Expected SEQUENCE_COUNT_ERROR for untracked APID " << static_cast(apid) << ", but got " << seqCount; testerState.assertEvents_ApidTableFull_size(__FILE__, __LINE__, 1); testerState.assertEvents_ApidTableFull(__FILE__, __LINE__, 0, static_cast(apid)); } @@ -103,7 +98,7 @@ void ApidManagerTester::ValidateSeqCountOk::action(ApidManagerTester& testerStat ComCfg::APID::T apid = testerState.shadow_getRandomTrackedApid(); U16 shadow_expectedSeqCount = testerState.shadow_seqCounts[apid]; testerState.invoke_to_validateApidSeqCountIn(0, apid, shadow_expectedSeqCount); - testerState.shadow_validateApidSeqCount(apid, shadow_expectedSeqCount); // keep shadow state in sync + testerState.shadow_validateApidSeqCount(apid, shadow_expectedSeqCount); // keep shadow state in sync testerState.assertEvents_UnexpectedSequenceCount_size(__FILE__, __LINE__, 0); } @@ -116,16 +111,17 @@ void ApidManagerTester::ValidateSeqCountFailure::action(ApidManagerTester& teste testerState.clearHistory(); ComCfg::APID::T apid = testerState.shadow_getRandomTrackedApid(); U16 shadow_expectedSeqCount = testerState.shadow_seqCounts.at(apid); - U16 invalidSeqCount = static_cast((shadow_expectedSeqCount + 1) % (1 << SpacePacketSubfields::SeqCountWidth)); // Or any other value that's different, ensure wrap around + U16 invalidSeqCount = static_cast( + (shadow_expectedSeqCount + 1) % + (1 << SpacePacketSubfields::SeqCountWidth)); // Or any other value that's different, ensure wrap around // Invoke the port with the deliberately incorrect sequence count testerState.invoke_to_validateApidSeqCountIn(0, apid, invalidSeqCount); - testerState.shadow_validateApidSeqCount(apid, invalidSeqCount); // keep shadow state in sync + testerState.shadow_validateApidSeqCount(apid, invalidSeqCount); // keep shadow state in sync // Now, the event should be logged testerState.assertEvents_UnexpectedSequenceCount_size(__FILE__, __LINE__, 1); testerState.assertEvents_UnexpectedSequenceCount(__FILE__, __LINE__, 0, invalidSeqCount, shadow_expectedSeqCount); - } // ---------------------------------------------------------------------- @@ -138,14 +134,15 @@ U16 ApidManagerTester::shadow_getAndIncrementSeqCount(ComCfg::APID::T apid) { auto found = this->shadow_seqCounts.find(apid); if (found != this->shadow_seqCounts.end()) { U16 seqCount = found->second; - found->second = static_cast((seqCount + 1) % (1 << SpacePacketSubfields::SeqCountWidth)); // Increment for next call - return seqCount; // Return the current sequence count + found->second = + static_cast((seqCount + 1) % (1 << SpacePacketSubfields::SeqCountWidth)); // Increment for next call + return seqCount; // Return the current sequence count } // If APID not found, initialize a new entry if (this->shadow_seqCounts.size() < this->component.MAX_TRACKED_APIDS) { U16 seqCount = 0; this->shadow_seqCounts[apid] = static_cast(seqCount + 1); // increment for next call - return seqCount; // Return the initialized sequence count + return seqCount; // Return the initialized sequence count } return this->component.SEQUENCE_COUNT_ERROR; // Return error if APID not found } diff --git a/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.cpp b/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.cpp index 99e176ff050..8cd26cbf55e 100644 --- a/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.cpp +++ b/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.cpp @@ -68,7 +68,7 @@ void SpacePacketDeframer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, // Set data buffer to be of the encapsulated data: HEADER (6 bytes) | PACKET DATA data.setData(data.getData() + SpacePacketHeader::SERIALIZED_SIZE); - data.setSize(pkt_length); + data.setSize(pkt_length); this->dataOut_out(0, data, contextCopy); } diff --git a/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.hpp b/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.hpp index 784c22ea608..c84479323a7 100644 --- a/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.hpp +++ b/Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.hpp @@ -14,9 +14,9 @@ namespace Svc { namespace Ccsds { class SpacePacketDeframer final : public SpacePacketDeframerComponentBase { - friend class SpacePacketDeframerTester; + friend class SpacePacketDeframerTester; - public: + public: // ---------------------------------------------------------------------- // Component construction and destruction // ---------------------------------------------------------------------- @@ -46,7 +46,6 @@ class SpacePacketDeframer final : public SpacePacketDeframerComponentBase { void dataReturnIn_handler(FwIndexType portNum, //!< The port number Fw::Buffer& data, const ComCfg::FrameContext& context) override; - }; } // namespace Ccsds diff --git a/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.cpp b/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.cpp index 395d965b536..4ed50def52f 100644 --- a/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.cpp +++ b/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.cpp @@ -43,7 +43,8 @@ void SpacePacketDeframerTester ::testDataReturnPassthrough() { void SpacePacketDeframerTester ::testNominalDeframing() { ComCfg::APID::T apid = static_cast(STest::Random::lowerUpper(0, 0x7FF)); // random 11 bit APID U16 seqCount = static_cast(STest::Random::lowerUpper(0, 0x3FFF)); // random 14 bit sequence count - U16 dataLength = static_cast(STest::Random::lowerUpper(1, MAX_TEST_PACKET_DATA_SIZE)); // bytes of data, random length + U16 dataLength = + static_cast(STest::Random::lowerUpper(1, MAX_TEST_PACKET_DATA_SIZE)); // bytes of data, random length U8 data[dataLength]; U16 lengthToken = static_cast(dataLength - 1); // Length token is length - 1 for (FwIndexType i = 0; i < static_cast(dataLength); ++i) { @@ -73,8 +74,10 @@ void SpacePacketDeframerTester ::testNominalDeframing() { void SpacePacketDeframerTester ::testDeframingIncorrectLength() { ComCfg::APID::T apid = static_cast(STest::Random::lowerUpper(0, 0x7FF)); // random 11 bit APID U16 seqCount = static_cast(STest::Random::lowerUpper(0, 0x3FFF)); // random 14 bit sequence count - U16 realDataLength = static_cast(STest::Random::lowerUpper(1, MAX_TEST_PACKET_DATA_SIZE)); // bytes of data, random length - U16 invalidLengthToken = static_cast(realDataLength + 1); // Length token is greater than actual data available + U16 realDataLength = + static_cast(STest::Random::lowerUpper(1, MAX_TEST_PACKET_DATA_SIZE)); // bytes of data, random length + U16 invalidLengthToken = + static_cast(realDataLength + 1); // Length token is greater than actual data available U8 data[realDataLength]; Fw::Buffer buffer = this->assemblePacket(apid, seqCount, invalidLengthToken, data, realDataLength); @@ -91,25 +94,31 @@ void SpacePacketDeframerTester ::testDeframingIncorrectLength() { ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).context, nullContext); // Data should be the same as input // Event logging failure - ASSERT_EVENTS_SIZE(1); // No events should be generated in the nominal case + ASSERT_EVENTS_SIZE(1); // No events should be generated in the nominal case ASSERT_EVENTS_InvalidLength_SIZE(1); // No events should be generated in the nominal case - ASSERT_EVENTS_InvalidLength(0, static_cast(invalidLengthToken + 1), realDataLength); // Event logs the size in bytes, so add 1 to length token + ASSERT_EVENTS_InvalidLength(0, static_cast(invalidLengthToken + 1), + realDataLength); // Event logs the size in bytes, so add 1 to length token } // ---------------------------------------------------------------------- // Helper functions // ---------------------------------------------------------------------- -Fw::Buffer SpacePacketDeframerTester ::assemblePacket(U16 apid, U16 seqCount, U16 lengthToken, U8* packetData, U16 packetDataLen) { +Fw::Buffer SpacePacketDeframerTester ::assemblePacket(U16 apid, + U16 seqCount, + U16 lengthToken, + U8* packetData, + U16 packetDataLen) { SpacePacketHeader header; header.setpacketIdentification(apid); - header.setpacketSequenceControl(seqCount); // Sequence Flags = 0b11 (unsegmented) & unused Seq count + header.setpacketSequenceControl(seqCount); // Sequence Flags = 0b11 (unsegmented) & unused Seq count header.setpacketDataLength(lengthToken); Fw::ExternalSerializeBuffer serializer(static_cast(this->m_packetBuffer), sizeof(this->m_packetBuffer)); serializer.serialize(header); serializer.serialize(packetData, packetDataLen, Fw::Serialization::OMIT_LENGTH); - return Fw::Buffer(this->m_packetBuffer, static_cast(packetDataLen + SpacePacketHeader::SERIALIZED_SIZE)); + return Fw::Buffer(this->m_packetBuffer, + static_cast(packetDataLen + SpacePacketHeader::SERIALIZED_SIZE)); } } // namespace Ccsds diff --git a/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.hpp b/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.hpp index 69206e0c607..05dc9d7d456 100644 --- a/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.hpp +++ b/Svc/Ccsds/SpacePacketDeframer/test/ut/SpacePacketDeframerTester.hpp @@ -8,8 +8,8 @@ #define Svc_Ccsds_SpacePacketDeframerTester_HPP #include "Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframer.hpp" -#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" #include "Svc/Ccsds/SpacePacketDeframer/SpacePacketDeframerGTestBase.hpp" +#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" namespace Svc { @@ -71,7 +71,7 @@ class SpacePacketDeframerTester final : public SpacePacketDeframerGTestBase { SpacePacketDeframer component; //! Test buffer - static const FwSizeType MAX_TEST_PACKET_DATA_SIZE = 200; // this value needs to fit in a U8 for testing + static const FwSizeType MAX_TEST_PACKET_DATA_SIZE = 200; // this value needs to fit in a U8 for testing U8 m_packetBuffer[SpacePacketHeader::SERIALIZED_SIZE + MAX_TEST_PACKET_DATA_SIZE]; }; diff --git a/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp b/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp index 23c81aad643..05539ae6f3b 100644 --- a/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp +++ b/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.cpp @@ -5,8 +5,8 @@ // ====================================================================== #include "Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.hpp" -#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" #include "Svc/Ccsds/Types/FppConstantsAc.hpp" +#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" namespace Svc { @@ -28,8 +28,11 @@ void SpacePacketFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, c SpacePacketHeader header; Fw::SerializeStatus status; FwSizeType frameSize = SpacePacketHeader::SERIALIZED_SIZE + data.getSize(); - FW_ASSERT(data.getSize() <= std::numeric_limits::max() - SpacePacketHeader::SERIALIZED_SIZE, static_cast(data.getSize())); - FW_ASSERT(data.getSize() > 0, static_cast(data.getSize())); // Protocol specifies at least 1 byte of data for a valid packet + FW_ASSERT(data.getSize() <= std::numeric_limits::max() - SpacePacketHeader::SERIALIZED_SIZE, + static_cast(data.getSize())); + FW_ASSERT( + data.getSize() > 0, + static_cast(data.getSize())); // Protocol specifies at least 1 byte of data for a valid packet // Allocate frame buffer Fw::Buffer frameBuffer = this->bufferAllocate_out(0, static_cast(frameSize)); @@ -38,19 +41,23 @@ void SpacePacketFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, c // ----------------------------------------------- // Header // ----------------------------------------------- - // PVN is always 0 per Standard - Packet Type is 0 for Telemetry (downlink) - SecHdr flag is 0 for no secondary header + // PVN is always 0 per Standard - Packet Type is 0 for Telemetry (downlink) - SecHdr flag is 0 for no secondary + // header U16 packetIdentification = 0; ComCfg::APID::T apid = context.getapid(); - FW_ASSERT((apid >> SpacePacketSubfields::ApidWidth) == 0, static_cast(apid)); // apid must fit in 11 bits - packetIdentification |= static_cast(apid) & SpacePacketSubfields::ApidMask; // 11 bit APID + FW_ASSERT((apid >> SpacePacketSubfields::ApidWidth) == 0, + static_cast(apid)); // apid must fit in 11 bits + packetIdentification |= static_cast(apid) & SpacePacketSubfields::ApidMask; // 11 bit APID - U16 sequenceCount = this->getApidSeqCount_out(0, apid, 0); // retrieve the sequence count for this APID + U16 sequenceCount = this->getApidSeqCount_out(0, apid, 0); // retrieve the sequence count for this APID U16 packetSequenceControl = 0; - packetSequenceControl |= 0x3 << SpacePacketSubfields::SeqFlagsOffset; // Sequence Flags 0b11 = unsegmented User Data - packetSequenceControl |= sequenceCount & SpacePacketSubfields::SeqCountMask; // 14 bit sequence count + packetSequenceControl |= + 0x3 << SpacePacketSubfields::SeqFlagsOffset; // Sequence Flags 0b11 = unsegmented User Data + packetSequenceControl |= sequenceCount & SpacePacketSubfields::SeqCountMask; // 14 bit sequence count FW_ASSERT(data.getSize() <= std::numeric_limits::max(), static_cast(data.getSize())); - U16 packetDataLength = static_cast(data.getSize() - 1); // Standard specifies length is number of bytes minus 1 + U16 packetDataLength = + static_cast(data.getSize() - 1); // Standard specifies length is number of bytes minus 1 header.setpacketIdentification(packetIdentification); header.setpacketSequenceControl(packetSequenceControl); @@ -65,7 +72,7 @@ void SpacePacketFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, c FW_ASSERT(status == Fw::FW_SERIALIZE_OK, status); this->dataOut_out(0, frameBuffer, context); - this->dataReturnOut_out(0, data, context); // return ownership of the original data buffer + this->dataReturnOut_out(0, data, context); // return ownership of the original data buffer } void SpacePacketFramer ::comStatusIn_handler(FwIndexType portNum, Fw::Success& condition) { @@ -74,11 +81,12 @@ void SpacePacketFramer ::comStatusIn_handler(FwIndexType portNum, Fw::Success& c } } -void SpacePacketFramer ::dataReturnIn_handler(FwIndexType portNum, Fw::Buffer& frameBuffer, const ComCfg::FrameContext& context) { +void SpacePacketFramer ::dataReturnIn_handler(FwIndexType portNum, + Fw::Buffer& frameBuffer, + const ComCfg::FrameContext& context) { // dataReturnIn is the allocated buffer coming back from the dataOut port this->bufferDeallocate_out(0, frameBuffer); } - } // namespace Ccsds } // namespace Svc diff --git a/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.hpp b/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.hpp index 37543797622..3b5292fb2c5 100644 --- a/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.hpp +++ b/Svc/Ccsds/SpacePacketFramer/SpacePacketFramer.hpp @@ -56,7 +56,6 @@ class SpacePacketFramer final : public SpacePacketFramerComponentBase { void dataReturnIn_handler(FwIndexType portNum, //!< The port number Fw::Buffer& data, const ComCfg::FrameContext& context) override; - }; } // namespace Ccsds diff --git a/Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp b/Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp index 39634a8bd5b..b2629dda5eb 100644 --- a/Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp +++ b/Svc/Ccsds/SpacePacketFramer/test/ut/SpacePacketFramerTester.cpp @@ -63,7 +63,7 @@ void SpacePacketFramerTester::testNominalFraming() { U16 seqCount = static_cast(STest::Random::lowerUpper(0, 0x3FFF)); // random 14 bit sequence count ComCfg::FrameContext context; context.setapid(apid); - this->m_nextSeqCount = seqCount; // seqCount to be returned by getApidSeqCount output port + this->m_nextSeqCount = seqCount; // seqCount to be returned by getApidSeqCount output port this->invoke_to_dataIn(0, data, context); @@ -91,8 +91,7 @@ U16 SpacePacketFramerTester ::from_getApidSeqCount_handler(FwIndexType portNum, return this->m_nextSeqCount; } -Fw::Buffer SpacePacketFramerTester ::from_bufferAllocate_handler(FwIndexType portNum, - FwSizeType size) { +Fw::Buffer SpacePacketFramerTester ::from_bufferAllocate_handler(FwIndexType portNum, FwSizeType size) { return Fw::Buffer(this->m_internalDataBuffer, size); } diff --git a/Svc/Ccsds/TcDeframer/TcDeframer.hpp b/Svc/Ccsds/TcDeframer/TcDeframer.hpp index 3129563ed68..20508f85c36 100644 --- a/Svc/Ccsds/TcDeframer/TcDeframer.hpp +++ b/Svc/Ccsds/TcDeframer/TcDeframer.hpp @@ -12,7 +12,7 @@ namespace Svc { namespace Ccsds { class TcDeframer : public TcDeframerComponentBase { - friend class TcDeframerTester; + friend class TcDeframerTester; public: // ---------------------------------------------------------------------- @@ -61,7 +61,6 @@ class TcDeframer : public TcDeframerComponentBase { U16 m_vcId; //!< The virtual channel ID this deframer is configured to handle U16 m_spacecraftId; //!< The spacecraft ID this deframer is configured to handle bool m_acceptAllVcid = true; //!< Flag to accept all VCIDs - }; } // namespace Ccsds } // namespace Svc diff --git a/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.cpp b/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.cpp index 050800739dd..29d26439327 100644 --- a/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.cpp +++ b/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.cpp @@ -6,9 +6,9 @@ #include "TcDeframerTester.hpp" #include "STest/Random/Random.hpp" -#include "Svc/Ccsds/Utils/CRC16.hpp" #include "Svc/Ccsds/Types/TCHeaderSerializableAc.hpp" #include "Svc/Ccsds/Types/TCTrailerSerializableAc.hpp" +#include "Svc/Ccsds/Utils/CRC16.hpp" namespace Svc { @@ -35,7 +35,7 @@ void TcDeframerTester::testDataReturn() { Fw::Buffer buffer(data, sizeof(data)); ComCfg::FrameContext nullContext; this->invoke_to_dataReturnIn(0, buffer, nullContext); - ASSERT_from_dataReturnOut_SIZE(1); // incoming buffer should be deallocated + ASSERT_from_dataReturnOut_SIZE(1); // incoming buffer should be deallocated ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), data); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), sizeof(data)); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).context, nullContext); @@ -43,10 +43,10 @@ void TcDeframerTester::testDataReturn() { void TcDeframerTester::testNominalDeframing() { // Frame: 5 bytes (header) + bytes (data) + 2 bytes (trailer) - U16 scId = static_cast(STest::Random::lowerUpper(0, 0x3FF)); // random 10 bit Spacecraft ID - U8 vcId = static_cast(STest::Random::lowerUpper(0, 0x3F)); // random 6 bit virtual channel ID - U8 seqCount = static_cast(STest::Random::lowerUpper(0, 0xFF)); // random 8 bit sequence count - U8 payloadLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length + U16 scId = static_cast(STest::Random::lowerUpper(0, 0x3FF)); // random 10 bit Spacecraft ID + U8 vcId = static_cast(STest::Random::lowerUpper(0, 0x3F)); // random 6 bit virtual channel ID + U8 seqCount = static_cast(STest::Random::lowerUpper(0, 0xFF)); // random 8 bit sequence count + U8 payloadLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length U8 payload[payloadLength]; // Initialize payload with some data for (FwIndexType i = 0; i < payloadLength; i++) { @@ -69,8 +69,8 @@ void TcDeframerTester::testNominalDeframing() { void TcDeframerTester::testInvalidScId() { // Frame: 5 bytes (header) + 1 byte (data) + 2 bytes (trailer) - U16 scId = static_cast(STest::Random::lowerUpper(1, 0x3FF)); // random 10 bit Spacecraft ID - U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length + U16 scId = static_cast(STest::Random::lowerUpper(1, 0x3FF)); // random 10 bit Spacecraft ID + U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length U8 data[dataLength]; // Assemble frame with incorrect scId value @@ -81,62 +81,64 @@ void TcDeframerTester::testInvalidScId() { this->invoke_to_dataIn(0, buffer, nullContext); ASSERT_from_dataOut_SIZE(0); - ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), buffer.getData()); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), buffer.getSize()); - ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted - ASSERT_EVENTS_InvalidSpacecraftId_SIZE(1); // event was emitted for invalid spacecraft ID - ASSERT_EVENTS_InvalidSpacecraftId(0, static_cast(scId - 1), scId); // event was emitted for invalid spacecraft ID + ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted + ASSERT_EVENTS_InvalidSpacecraftId_SIZE(1); // event was emitted for invalid spacecraft ID + ASSERT_EVENTS_InvalidSpacecraftId(0, static_cast(scId - 1), + scId); // event was emitted for invalid spacecraft ID } - void TcDeframerTester::testInvalidVcId() { - U8 vcId = static_cast(STest::Random::lowerUpper(1, 0x3F)); // random 6 bit VCID - U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length + U8 vcId = static_cast(STest::Random::lowerUpper(1, 0x3F)); // random 6 bit VCID + U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length U8 data[dataLength]; // Assemble frame with incorrect vcId value Fw::Buffer buffer = this->assembleFrameBuffer(data, dataLength, 0, static_cast(vcId - 1)); ComCfg::FrameContext nullContext; - this->setComponentState(0, vcId, 0, false); // set the component in mode where only one VCID is accepted + this->setComponentState(0, vcId, 0, false); // set the component in mode where only one VCID is accepted this->invoke_to_dataIn(0, buffer, nullContext); ASSERT_from_dataOut_SIZE(0); - ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), buffer.getData()); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), buffer.getSize()); - ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted - ASSERT_EVENTS_InvalidVcId_SIZE(1); // event was emitted for invalid VCID - ASSERT_EVENTS_InvalidVcId(0, static_cast(vcId - 1), vcId); // event was emitted for invalid VCID + ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted + ASSERT_EVENTS_InvalidVcId_SIZE(1); // event was emitted for invalid VCID + ASSERT_EVENTS_InvalidVcId(0, static_cast(vcId - 1), vcId); // event was emitted for invalid VCID } void TcDeframerTester::testInvalidLengthToken() { - U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length + U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length U8 data[dataLength]; U8 incorrectLengthToken = static_cast(dataLength + TCHeader::SERIALIZED_SIZE + TCTrailer::SERIALIZED_SIZE + 1); Fw::Buffer buffer = this->assembleFrameBuffer(data, dataLength); - buffer.getData()[3] = incorrectLengthToken; // Override length token to invalid value + buffer.getData()[3] = incorrectLengthToken; // Override length token to invalid value ComCfg::FrameContext nullContext; this->setComponentState(); this->invoke_to_dataIn(0, buffer, nullContext); ASSERT_from_dataOut_SIZE(0); - ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated + ASSERT_from_dataReturnOut_SIZE(1); // invalid buffer was deallocated ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), buffer.getData()); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), buffer.getSize()); - ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted - ASSERT_EVENTS_InvalidFrameLength_SIZE(1); // event was emitted for invalid frame length + ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted + ASSERT_EVENTS_InvalidFrameLength_SIZE(1); // event was emitted for invalid frame length // event logs size in bytes which is length token + 1 - ASSERT_EVENTS_InvalidFrameLength(0, static_cast(incorrectLengthToken + 1), static_cast(dataLength + TCHeader::SERIALIZED_SIZE + TCTrailer::SERIALIZED_SIZE)); + ASSERT_EVENTS_InvalidFrameLength( + 0, static_cast(incorrectLengthToken + 1), + static_cast(dataLength + TCHeader::SERIALIZED_SIZE + TCTrailer::SERIALIZED_SIZE)); } void TcDeframerTester::testInvalidCrc() { - U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length + U8 dataLength = static_cast(STest::Random::lowerUpper(1, 200)); // bytes of data, random length U8 data[dataLength]; - + Fw::Buffer buffer = this->assembleFrameBuffer(data, dataLength); // Override CRC to invalid value buffer.getData()[TCHeader::SERIALIZED_SIZE + dataLength + 1] = 0x00; @@ -150,7 +152,7 @@ void TcDeframerTester::testInvalidCrc() { ASSERT_from_dataReturnOut_SIZE(1); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getData(), buffer.getData()); ASSERT_EQ(this->fromPortHistory_dataReturnOut->at(0).data.getSize(), buffer.getSize()); - ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted + ASSERT_EVENTS_SIZE(1); // exactly 1 event emitted ASSERT_EVENTS_InvalidCrc_SIZE(1); } @@ -162,10 +164,10 @@ void TcDeframerTester::setComponentState(U16 scid, U8 vcid, U8 sequenceNumber, b // this->component.m_acceptAllVcid = acceptAllVcid; } -Fw::Buffer TcDeframerTester::assembleFrameBuffer(U8* data, U8 dataLength, U16 scid, U8 vcid, U8 seqNumber){ +Fw::Buffer TcDeframerTester::assembleFrameBuffer(U8* data, U8 dataLength, U16 scid, U8 vcid, U8 seqNumber) { ::memset(this->m_frameData, 0, sizeof(this->m_frameData)); U16 frameLength = static_cast(TCHeader::SERIALIZED_SIZE + dataLength + TCTrailer::SERIALIZED_SIZE); - U16 frameLengthToken = static_cast(frameLength - 1); // length token is length - 1 + U16 frameLengthToken = static_cast(frameLength - 1); // length token is length - 1 // Header this->m_frameData[0] = static_cast(scid >> 8); this->m_frameData[1] = static_cast(scid & 0xFF); diff --git a/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.hpp b/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.hpp index e21bf966c7f..db94abb2b95 100644 --- a/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.hpp +++ b/Svc/Ccsds/TcDeframer/test/ut/TcDeframerTester.hpp @@ -49,7 +49,6 @@ class TcDeframerTester final : public TcDeframerGTestBase { void testInvalidLengthToken(); void testInvalidCrc(); - private: // ---------------------------------------------------------------------- // Helper functions @@ -74,7 +73,7 @@ class TcDeframerTester final : public TcDeframerGTestBase { //! The component under test TcDeframer component; - U8 m_frameData[300]; // data buffer used to produce test frames + U8 m_frameData[300]; // data buffer used to produce test frames }; } // namespace Ccsds diff --git a/Svc/Ccsds/TmFramer/TmFramer.cpp b/Svc/Ccsds/TmFramer/TmFramer.cpp index 86da6efe44b..33d6e01c174 100644 --- a/Svc/Ccsds/TmFramer/TmFramer.cpp +++ b/Svc/Ccsds/TmFramer/TmFramer.cpp @@ -28,8 +28,7 @@ TmFramer ::~TmFramer() {} void TmFramer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, const ComCfg::FrameContext& context) { FW_ASSERT(data.getSize() <= ComCfg::TmFrameFixedSize - TMHeader::SERIALIZED_SIZE - TMTrailer::SERIALIZED_SIZE, static_cast(data.getSize())); - FW_ASSERT(this->m_bufferState == BufferOwnershipState::OWNED, - static_cast(this->m_bufferState)); + FW_ASSERT(this->m_bufferState == BufferOwnershipState::OWNED, static_cast(this->m_bufferState)); // ----------------------------------------------- // Header @@ -121,7 +120,7 @@ void TmFramer ::fill_with_idle_packet(Fw::SerializeBufferBase& serializer) { header.setpacketIdentification(idleApid); header.setpacketSequenceControl( 0x3 << SpacePacketSubfields::SeqFlagsOffset); // Sequence Flags = 0b11 (unsegmented) & unused Seq count - header.setpacketDataLength(lengthToken); // this should be payload length - 1 ??? + header.setpacketDataLength(lengthToken); // Serialize header and idle data into the frame serializer.serialize(header); for (U16 i = static_cast(startIndex + SpacePacketHeader::SERIALIZED_SIZE); i < endIndex; i++) { diff --git a/Svc/Ccsds/TmFramer/test/ut/TmFramerTester.cpp b/Svc/Ccsds/TmFramer/test/ut/TmFramerTester.cpp index 82d2c22dfb0..1b4fd615ba1 100644 --- a/Svc/Ccsds/TmFramer/test/ut/TmFramerTester.cpp +++ b/Svc/Ccsds/TmFramer/test/ut/TmFramerTester.cpp @@ -5,9 +5,9 @@ // ====================================================================== #include "TmFramerTester.hpp" +#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" #include "Svc/Ccsds/Types/TMHeaderSerializableAc.hpp" #include "Svc/Ccsds/Types/TMTrailerSerializableAc.hpp" -#include "Svc/Ccsds/Types/SpacePacketHeaderSerializableAc.hpp" namespace Svc { @@ -76,7 +76,8 @@ void TmFramerTester ::testNominalFraming() { ASSERT_EQ(this->component.m_virtualFrameCount, outVcCount + 1); // Idle data should be filled at the offset of header + payload + the Space Packet Idle Packet header - FwSizeType expectedIdleDataOffset = TMHeader::SERIALIZED_SIZE + sizeof(bufferData) + SpacePacketHeader::SERIALIZED_SIZE; + FwSizeType expectedIdleDataOffset = + TMHeader::SERIALIZED_SIZE + sizeof(bufferData) + SpacePacketHeader::SERIALIZED_SIZE; // The frame is composed of the payload + a SpacePacket Idle Packet (Header + idle_pattern) const U8 idlePattern = this->component.IDLE_DATA_PATTERN; @@ -101,9 +102,9 @@ void TmFramerTester ::testSeqCountWrapAround() { // to test the wrap around of the sequence counts this->component.m_masterFrameCount = 250; this->component.m_virtualFrameCount = 250; - U8 countWrapAround = 250; // will wrap around to 0 after 255 + U8 countWrapAround = 250; // will wrap around to 0 after 255 for (U32 iter = 0; iter < 10; iter++) { - this->component.m_bufferState = TmFramer::BufferOwnershipState::OWNED; // reset state to OWNED + this->component.m_bufferState = TmFramer::BufferOwnershipState::OWNED; // reset state to OWNED this->invoke_to_dataIn(0, buffer, defaultContext); ASSERT_from_dataOut_SIZE(iter + 1); Fw::Buffer outBuffer = this->fromPortHistory_dataOut->at(iter).data; @@ -116,11 +117,12 @@ void TmFramerTester ::testSeqCountWrapAround() { } void TmFramerTester ::testInputBufferTooLarge() { - const FwSizeType tooLargeSize = ComCfg::TmFrameFixedSize; // This is too large since we need room for header+trailer as well + const FwSizeType tooLargeSize = + ComCfg::TmFrameFixedSize; // This is too large since we need room for header+trailer as well U8 bufferData[tooLargeSize]; Fw::Buffer buffer(bufferData, tooLargeSize); ComCfg::FrameContext defaultContext; - // Send a buffer larger than the + // Send a buffer larger than the ASSERT_DEATH_IF_SUPPORTED(this->invoke_to_dataIn(0, buffer, defaultContext), "TmFramer.cpp"); } @@ -130,13 +132,12 @@ void TmFramerTester ::testDataReturn() { ComCfg::FrameContext defaultContext; // Send a buffer that is not the internal buffer of the component, and expect an assertion ASSERT_DEATH_IF_SUPPORTED(this->invoke_to_dataReturnIn(0, buffer, defaultContext), "TmFramer.cpp"); - + // Now send the expected buffer and expect state to go back to OWNED this->component.m_bufferState = TmFramer::BufferOwnershipState::NOT_OWNED; Fw::Buffer internalBuffer(this->component.m_frameBuffer, sizeof(this->component.m_frameBuffer)); this->invoke_to_dataReturnIn(0, internalBuffer, defaultContext); ASSERT_EQ(this->component.m_bufferState, TmFramer::BufferOwnershipState::OWNED); - } void TmFramerTester ::testBufferOwnershipState() { @@ -147,9 +148,8 @@ void TmFramerTester ::testBufferOwnershipState() { this->component.m_bufferState = TmFramer::BufferOwnershipState::NOT_OWNED; ASSERT_DEATH_IF_SUPPORTED(this->invoke_to_dataIn(0, buffer, context), "TmFramer.cpp"); this->component.m_bufferState = TmFramer::BufferOwnershipState::OWNED; - this->invoke_to_dataIn(0, buffer, context); // this should work now + this->invoke_to_dataIn(0, buffer, context); // this should work now ASSERT_EQ(this->component.m_bufferState, TmFramer::BufferOwnershipState::NOT_OWNED); - } // ---------------------------------------------------------------------- From bc998a0796df49d11ffa25151ffc2b09c542ea0a Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Thu, 26 Jun 2025 10:43:55 -0700 Subject: [PATCH 2/2] Add check to CI --- .github/workflows/format-check.yml | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/workflows/format-check.yml b/.github/workflows/format-check.yml index c639f4443aa..4b03a1ff254 100644 --- a/.github/workflows/format-check.yml +++ b/.github/workflows/format-check.yml @@ -33,6 +33,7 @@ jobs: Fw/Buffer Fw/Cmd Fw/Com + Svc/Ccsds run: | fprime-util format --check --dirs $CHECKED_DIRS shell: bash