Fix Intermittent UT failure in FprimeDeframer (nasa#4177)

* Fix random UT failure in FprimeDeframer

* Update UTs and docs to reflect expectations

Author: thomas-bc

Svc/FprimeDeframer/FprimeDeframer.cpp

@@ -58,15 +58,20 @@ void FprimeDeframer ::dataIn_handler(FwIndexType portNum, Fw::Buffer& data, cons
         return;
     }
     // -------- Attempt to extract APID from Payload --------
-    // If PacketDescriptor translates to an invalid APID, let it default to FW_PACKET_UNKNOWN
-    // and let downstream components (e.g. custom router) handle it
-    FwPacketDescriptorType packetDescriptor;
-    status = deserializer.deserializeTo(packetDescriptor);
-    FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK, status);
     ComCfg::FrameContext contextCopy = context;
-    // If a valid descriptor is deserialized, set it in the context
-    if (packetDescriptor < ComCfg::Apid::INVALID_UNINITIALIZED) {
-        contextCopy.set_apid(static_cast<ComCfg::Apid::T>(packetDescriptor));
+    if (deserializer.getBuffLeft() < FprimeProtocol::FrameTrailer::SERIALIZED_SIZE + sizeof(FwPacketDescriptorType)) {
+        // Not enough data to read a valid FwPacketDescriptor, emit event and skip attempting to read an APID
+        this->log_WARNING_LO_PayloadTooShort();
+    } else {
+        // If PacketDescriptor translates to an invalid APID, let it default to FW_PACKET_UNKNOWN
+        // and let downstream components (e.g. custom router) handle it
+        FwPacketDescriptorType packetDescriptor;
+        status = deserializer.deserializeTo(packetDescriptor);
+        FW_ASSERT(status == Fw::SerializeStatus::FW_SERIALIZE_OK, status);
+        // If a valid descriptor is deserialized, set it in the context
+        if ((packetDescriptor < ComCfg::Apid::INVALID_UNINITIALIZED)) {
+            contextCopy.set_apid(static_cast<ComCfg::Apid::T>(packetDescriptor));
+        }
     }
 
     // ---------------- Validate Frame Trailer ----------------

Svc/FprimeDeframer/FprimeDeframer.fpp

@@ -30,6 +30,11 @@ module Svc {
       severity warning high \
       format "Frame dropped: The transmitted frame checksum does not match that computed by the receiver"
 
+    @ An invalid frame was received (not enough data to contain a valid FwPacketDescriptor type)
+    event PayloadTooShort \
+      severity warning low \
+      format "The received buffer is too short to contain a valid FwPacketDescriptor"
+
     ###############################################################################
     # Standard AC Ports for Events 
     ###############################################################################

Svc/FprimeDeframer/test/ut/FprimeDeframerTester.cpp

@@ -27,6 +27,10 @@ FprimeDeframerTester ::~FprimeDeframerTester() {}
 // ----------------------------------------------------------------------
 
 void FprimeDeframerTester ::testNominalFrame() {
+    // This tests a nominal frame with 1 byte of data - which per F Prime protocol
+    // does not contain a valid FwPacketDescriptor (2 bytes) and therefore emits a warning event
+    // See testNominalFrameApid() for a nominal frame with a valid FwPacketDescriptor
+
     // Get random byte of data
     U8 randomByte = static_cast<U8>(STest::Random::lowerUpper(1, 255));
     //           |  F´ start word        |     Length (= 1)      |   Data     |   Checksum (4 bytes)   |
@@ -41,7 +45,9 @@ void FprimeDeframerTester ::testNominalFrame() {
     ASSERT_EQ(this->fromPortHistory_dataOut->at(0).data.getData()[0], randomByte);
     // Not enough data to read a valid APID -> should default to FW_PACKET_UNKNOWN
     ASSERT_EQ(this->fromPortHistory_dataOut->at(0).context.get_apid(), ComCfg::Apid::FW_PACKET_UNKNOWN);
-    ASSERT_EVENTS_SIZE(0);  // no events emitted
+
+    ASSERT_EVENTS_SIZE(1);                  // one event emitted
+    ASSERT_EVENTS_PayloadTooShort_SIZE(1);  // event was emitted for payload too short
 }
 
 void FprimeDeframerTester ::testNominalFrameApid() {
@@ -88,8 +94,8 @@ void FprimeDeframerTester ::testIncorrectStartWord() {
 }
 
 void FprimeDeframerTester ::testIncorrectCrc() {
-    // Frame:     |   F´ start word      |      Length = 1       | Data |  INCORRECT Checksum  |
-    U8 data[13] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00};
+    // Frame:     |   F´ start word      |      Length = 1       |Data (2bytes)| INCORRECT Checksum  |
+    U8 data[14] = {0xDE, 0xAD, 0xBE, 0xEF, 0x00, 0x00, 0x00, 0x01, 0x00, 0x00, 0x00, 0x00, 0x00, 0x00};
     this->mockReceiveData(data, sizeof(data));
     ASSERT_from_dataOut_SIZE(0);        // nothing emitted on dataOut
     ASSERT_from_dataReturnOut_SIZE(1);  // invalid buffer was deallocated

Svc/FprimeProtocol/docs/sdd.md

@@ -5,7 +5,7 @@ The F Prime protocol is a minimal communications protocol that is used by defaul
 A frame for F Prime protocol consists of 4 fields:
 - Start word: A 32-bit start word that is used to identify the start of a frame. The start word is always `0xDEADBEEF`.
 - Payload length: A 32-bit field that specifies the length of the payload data in bytes.
-- Payload data: A variable-length field that contains the payload data (usually an [F Prime packet](../../../Fw/Com/ComPacket.hpp)), of length specified by the payload length field.
+- Payload data: A variable-length field that contains the payload data in the form of an [F Prime packet](../../../Fw/Com/ComPacket.hpp), of length specified by the payload length field. Note: an F Prime packet contains always at least a (configurable width) `FwPacketDescriptorType` field, which is used to identify the type of packet.
 - CRC: A 32-bit CRC field that is used to verify the integrity of the frame.
 
 ## F Prime frame format