Fix FileUplink packet sequence repeat and CRC #1378 (#3007)

* Added duplicated packet event/warning

* Added method to check for duplicated packet

* Added last packet write status member variable

* Duplicate file packets are now skipped

* Added duplicate file packet UT

* Fixed `m_lastPacketWriteStatus` initialization.

* Updated UT to test that the first packet isn't erroneously marked as a duplicate

Svc/FileUplink/Events.fppi

@@ -63,16 +63,25 @@ event PacketOutOfOrder(
   format "Received packet {} after packet {}" \
   throttle 20
 
+@ The File Uplink component encountered a duplicate packet during file receipt
+event PacketDuplicate(
+                        packetIndex: U32 @< The sequence index of the duplicate packet
+                      ) \
+  severity warning high \
+  id 7 \
+  format "Received a duplicate of packet {}" \
+  throttle 20
+
 @ The File Uplink component received a CANCEL packet
 event UplinkCanceled \
   severity activity high \
-  id 7 \
+  id 8 \
   format "Received CANCEL packet"
 
 @ Error decoding file packet
 event DecodeError(
                    status: I32 @< The sequence index of the out-of-order packet
                  ) \
   severity warning high \
-  id 8 \
+  id 9 \
   format "Unable to decode file packet. Status: {}"

Svc/FileUplink/FileUplink.cpp

@@ -25,6 +25,7 @@ namespace Svc {
       FileUplinkComponentBase(name),
       m_receiveMode(START),
       m_lastSequenceIndex(0),
+      m_lastPacketWriteStatus(Os::File::MAX_STATUS),
       m_filesReceived(this),
       m_packetsReceived(this),
       m_warnings(this)
@@ -120,7 +121,15 @@ namespace Svc {
       this->m_warnings.invalidReceiveMode(Fw::FilePacket::T_DATA);
       return;
     }
+
     const U32 sequenceIndex = dataPacket.asHeader().getSequenceIndex();
+
+    // skip this packet if it is a duplicate and it has already been written
+    if (this->m_lastPacketWriteStatus == Os::File::OP_OK &&
+        this->checkDuplicatedPacket(sequenceIndex)) {
+        return;
+    }
+
     this->checkSequenceIndex(sequenceIndex);
     const U32 byteOffset = dataPacket.getByteOffset();
     const U32 dataSize = dataPacket.getDataSize();
@@ -136,6 +145,8 @@ namespace Svc {
     if (status != Os::File::OP_OK) {
       this->m_warnings.fileWrite(this->m_file.name);
     }
+
+    this->m_lastPacketWriteStatus = status;
   }
 
   void FileUplink ::
@@ -174,6 +185,18 @@ namespace Svc {
     this->m_lastSequenceIndex = sequenceIndex;
   }
 
+  bool FileUplink ::
+    checkDuplicatedPacket(const U32 sequenceIndex)
+  {
+    // check for duplicate packet
+    if (sequenceIndex == this->m_lastSequenceIndex) {
+      this->m_warnings.packetDuplicate(sequenceIndex);
+      return true;
+    }
+
+    return false;
+  }
+
   void FileUplink ::
     compareChecksums(const Fw::FilePacket::EndPacket& endPacket)
   {
@@ -194,13 +217,15 @@ namespace Svc {
     this->m_file.osFile.close();
     this->m_receiveMode = START;
     this->m_lastSequenceIndex = 0;
+    this->m_lastPacketWriteStatus = Os::File::MAX_STATUS;
   }
 
   void FileUplink ::
     goToDataMode()
   {
     this->m_receiveMode = DATA;
     this->m_lastSequenceIndex = 0;
+    this->m_lastPacketWriteStatus = Os::File::MAX_STATUS;
   }
 
 }

Svc/FileUplink/FileUplink.hpp

@@ -161,6 +161,11 @@ namespace Svc {
               const U32 lastSequenceIndex
           );
 
+          //! Record a Duplicate Packet warning
+          void packetDuplicate(
+              const U32 sequenceIndex
+          );
+
           //! Record a File Write warning
           void fileWrite(Fw::LogStringArg& fileName);
 
@@ -246,6 +251,9 @@ namespace Svc {
       //! Check sequence index
       void checkSequenceIndex(const U32 sequenceIndex);
 
+      //! Check if a received packet is a duplicate
+      bool checkDuplicatedPacket(const U32 sequenceIndex);
+
       //! Compare checksums
       void compareChecksums(const Fw::FilePacket::EndPacket& endPacket);
 
@@ -267,6 +275,9 @@ namespace Svc {
       //! The sequence index of the last packet received
       U32 m_lastSequenceIndex;
 
+      //! The write status of the last packet received
+      Os::File::Status m_lastPacketWriteStatus;
+
       //! The file being assembled
       File m_file;
 

Svc/FileUplink/Warnings.cpp

@@ -57,6 +57,17 @@ namespace Svc {
     this->warning();
   }
 
+  void FileUplink::Warnings ::
+    packetDuplicate(
+        const U32 sequenceIndex
+    )
+  {
+    this->m_fileUplink->log_WARNING_HI_PacketDuplicate(
+        sequenceIndex
+    );
+    this->warning();
+  }
+
   void FileUplink::Warnings ::
     fileWrite(Fw::LogStringArg& fileName)
   {

Svc/FileUplink/test/ut/FileUplinkMain.cpp

@@ -49,6 +49,11 @@ TEST(FileUplink, PacketOutOfOrder) {
   tester.packetOutOfOrder();
 }
 
+TEST(FileUplink, PacketDuplicated) {
+  Svc::FileUplinkTester tester;
+  tester.packetDuplicated();
+}
+
 TEST(FileUplink, CancelPacketInStartMode) {
   Svc::FileUplinkTester tester;
   tester.cancelPacketInStartMode();

Svc/FileUplink/test/ut/FileUplinkTester.cpp

@@ -377,6 +377,64 @@ namespace Svc {
 
   }
 
+  void FileUplinkTester ::
+    packetDuplicated()
+  {
+    const char *const sourcePath = "source.bin";
+    const char *const destPath = "dest.bin";
+    U8 packetData[] = { 5, 6, 7, 8, 9 };
+    const size_t fileSize = 2 * PACKET_SIZE;
+
+    // Send the start packet (packet 0)
+    this->sendStartPacket(sourcePath, destPath, fileSize);
+    ASSERT_TLM_SIZE(1);
+    ASSERT_TLM_PacketsReceived(
+        0,
+        ++this->expectedPacketsReceived
+    );
+    ASSERT_EVENTS_SIZE(0);
+
+    ASSERT_EQ(0, component.m_lastSequenceIndex);
+    ASSERT_EQ(1, this->sequenceIndex);
+    ASSERT_EQ(Os::File::MAX_STATUS, component.m_lastPacketWriteStatus);
+
+    // Send data packet 1
+    const size_t byteOffset = 0;
+    this->sendDataPacket(byteOffset, packetData);
+    ASSERT_TLM_SIZE(1);
+    ASSERT_TLM_PacketsReceived(
+        0,
+        ++this->expectedPacketsReceived
+    );
+    ASSERT_TLM_Warnings_SIZE(0);
+
+    // capture the checksum after sending the first packet
+    const ::CFDP::Checksum expectedChecksum(component.m_file.m_checksum);
+
+    // Simulate duplication of packet 1
+    --this->sequenceIndex;
+
+    ASSERT_EQ(this->sequenceIndex, component.m_lastSequenceIndex);
+    ASSERT_EQ(Os::File::OP_OK, component.m_lastPacketWriteStatus);
+
+    // Send data packet 1 again
+    this->sendDataPacket(byteOffset, packetData);
+    ASSERT_TLM_SIZE(2);
+    ASSERT_TLM_PacketsReceived(
+        0,
+        ++this->expectedPacketsReceived
+    );
+    ASSERT_TLM_Warnings(0, 1);
+
+    ASSERT_EVENTS_SIZE(1);
+    ASSERT_EVENTS_PacketDuplicate(0, component.m_lastSequenceIndex);
+
+    // verify that the checksum hasn't changed
+    ASSERT_EQ(expectedChecksum, component.m_file.m_checksum);
+
+    this->removeFile("test.bin");
+  }
+
   void FileUplinkTester ::
     cancelPacketInStartMode()
   {

Svc/FileUplink/test/ut/FileUplinkTester.hpp

@@ -80,6 +80,10 @@ namespace Svc {
       //!
       void packetOutOfOrder();
 
+      //! Send a file with an duplicated packet
+      //!
+      void packetDuplicated();
+
       //! Send a CANCEL packet in START mode
       //!
       void cancelPacketInStartMode();