Repository: medley56/space_packet_parser

Issue:
Add support for continuation packets
# Context

Continuation packets occur in CCSDS Packets when the data fields are too long for a single packet delivery, or if logically a team wants to break up the packets into smaller chunks. The packet data should be concatenated together when we encounter continuation packets in the XTCE parsing logic.

# Implementation Plan

Add checks for the sequence flags within the packet generation code and do the concatenation for users.

Hints from issue comments:

Solution patch:
diff --git a/docs/source/changelog.md b/docs/source/changelog.md
index 255f79c..9026bf6 100644
--- a/docs/source/changelog.md
+++ b/docs/source/changelog.md
@@ -16,6 +16,13 @@ Release notes for the `space_packet_parser` library
   streams as needed.
 - Add a ``ccsds_packet_generator()`` function that iterates through raw
   bytes and yields individual CCSDS packets.
+- Add continuation packet support to the XTCE parsing and packet generation.
+  This adds logic to concatenate packet data fields together across successive
+  packets (if there was too much data to fit in a single CCSDS packet or it
+  was logically better to split by other teams).
+  - Add warnings if packets are out of sequence within a given apid.
+  - Add ability to remove secondary header bytes from subsequent packets.
+    ``definition.packet_generator(data, combine_segmented_packets=True, secondary_header_bytes=4)``
 
 ### v5.0.1 (released)
 - BUGFIX: Allow raw_value representation for enums with falsy raw values. Previously these defaulted to the enum label.
diff --git a/space_packet_parser/definitions.py b/space_packet_parser/definitions.py
index b54c9ce..9f2961d 100644
--- a/space_packet_parser/definitions.py
+++ b/space_packet_parser/definitions.py
@@ -353,6 +353,8 @@ def packet_generator(  # pylint: disable=too-many-branches,too-many-statements
             parse_bad_pkts: bool = True,
             root_container_name="CCSDSPacket",
             ccsds_headers_only: bool = False,
+            combine_segmented_packets: bool = False,
+            secondary_header_bytes: int = 0,
             yield_unrecognized_packet_errors: bool = False,
             show_progress: bool = False,
             buffer_read_size_bytes: Optional[int] = None,
@@ -380,6 +382,15 @@ def packet_generator(  # pylint: disable=too-many-branches,too-many-statements
             Default False. If True, only parses the packet headers (does not use the provided packet definition).
             ``space_packet_parser.packets.ccsds_packet_generator`` can be used directly to parse only the CCSDS headers
             without needing a packet definition.
+        combine_segmented_packets : bool
+            Default False. If True, combines segmented packets into a single packet for parsing. This is useful for
+            parsing packets that are split into multiple packets due to size constraints. The packet data is combined
+            by concatenating the data from each packet together. The combined packet is then parsed as a single packet.
+        secondary_header_bytes : int
+            Default 0. The length of the secondary header in bytes.
+            This is used to skip the secondary header of segmented packets.
+            The byte layout within the returned packet has all data concatenated together as follows:
+            [packet0header, packet0secondaryheader, packet0data, packet1data, packet2data, ...].
         yield_unrecognized_packet_errors : bool
             Default False.
             If False, UnrecognizedPacketTypeErrors are caught silently and parsing continues to the next packet.
@@ -405,6 +416,11 @@ def packet_generator(  # pylint: disable=too-many-branches,too-many-statements
             If yield_unrecognized_packet_errors is True, it will yield an unraised exception object,
             which can be raised or used for debugging purposes.
         """
+        # Used to keep track of any continuation packets that we encounter
+        # gathering them all up before combining them into a single packet
+        # for the XTCE to parse, lookup is by APID.
+        # _segmented_packets[APID] = [RawPacketData, ...]
+        _segmented_packets = {}
 
         # Iterate over individual packets in the binary data
         for raw_packet_data in packets.ccsds_generator(binary_data,
@@ -415,7 +431,34 @@ def packet_generator(  # pylint: disable=too-many-branches,too-many-statements
                 yield raw_packet_data
                 continue
 
-            packet = packets.CCSDSPacket(raw_data=raw_packet_data)
+            if not combine_segmented_packets or raw_packet_data.sequence_flags == packets.SequenceFlags.UNSEGMENTED:
+                packet = packets.CCSDSPacket(raw_data=raw_packet_data)
+            elif raw_packet_data.sequence_flags == packets.SequenceFlags.FIRST:
+                _segmented_packets[raw_packet_data.apid] = [raw_packet_data]
+                continue
+            elif not _segmented_packets.get(raw_packet_data.apid, []):
+                warnings.warn("Continuation packet found without declaring the start, skipping this packet.")
+                continue
+            elif raw_packet_data.sequence_flags == packets.SequenceFlags.CONTINUATION:
+                _segmented_packets[raw_packet_data.apid].append(raw_packet_data)
+                continue
+            else:  # raw_packet_data.sequence_flags == packets.SequenceFlags.LAST:
+                _segmented_packets[raw_packet_data.apid].append(raw_packet_data)
+                # We have received the final packet, close it up and combine all of
+                # the segmented packets into a single "packet" for XTCE parsing
+                sequence_counts = [p.sequence_count for p in _segmented_packets[raw_packet_data.apid]]
+                if not all((sequence_counts[i + 1] - sequence_counts[i]) % 16384 == 1
+                           for i in range(len(sequence_counts) - 1)):
+                    warnings.warn(f"Continuation packets for apid {raw_packet_data.apid} "
+                                  f"are not in sequence {sequence_counts}, skipping these packets.")
+                    continue
+                # Add all content (including header) from the first packet
+                raw_data = _segmented_packets[raw_packet_data.apid][0]
+                # Add the continuation packets to the first packet, skipping the headers
+                for p in _segmented_packets[raw_packet_data.apid][1:]:
+                    raw_data += p[raw_packet_data.HEADER_LENGTH_BYTES + secondary_header_bytes:]
+                packet = packets.CCSDSPacket(raw_data=raw_data)
+
             # Now do the actual parsing of the packet data
             try:
                 packet = self.parse_ccsds_packet(packet, root_container_name=root_container_name)


Test patch:
diff --git a/tests/unit/test_packets.py b/tests/unit/test_packets.py
index 08028e2..8c7d584 100644
--- a/tests/unit/test_packets.py
+++ b/tests/unit/test_packets.py
@@ -2,7 +2,7 @@
 # Standard
 import pytest
 # Local
-from space_packet_parser import packets
+from space_packet_parser import definitions, packets
 
 
 @pytest.mark.parametrize(("input_var", "input_value"),
@@ -101,3 +101,68 @@ def test_ccsds_packet_data_lookups():
 
     with pytest.raises(KeyError):
         packet[10]
+
+
+def test_continuation_packets(test_data_dir):
+    # This definition has 65 bytes worth of data
+    d = definitions.XtcePacketDefinition(test_data_dir / "test_xtce.xml")
+    # We can put that all in one unsegmented packet, just to verify this is working as expected
+    raw_bytes = packets.create_ccsds_packet(data=b"0"*65, apid=11, sequence_flags=packets.SequenceFlags.UNSEGMENTED)
+    orig_packets = list(d.packet_generator(raw_bytes))
+    assert len(orig_packets) == 1
+    # Remove the sequence flags, counter, and packet length, as they are expected to vary across tests
+    def remove_keys(d):
+        d.pop("SEQ_FLGS")
+        d.pop("PKT_LEN")
+        d.pop("SRC_SEQ_CTR")
+    remove_keys(orig_packets[0])
+
+    # Now we will split the data across 2 CCSDS packets, but expect them to be combined into one for parsing purposes
+    p0 = packets.create_ccsds_packet(data=b"0"*64, apid=11, sequence_flags=packets.SequenceFlags.FIRST, sequence_count=0)
+    p1 = packets.create_ccsds_packet(data=b"0"*1, apid=11, sequence_flags=packets.SequenceFlags.LAST, sequence_count=1)
+    raw_bytes = p0 + p1
+    result_packets = list(d.packet_generator(raw_bytes, combine_segmented_packets=True))
+    remove_keys(result_packets[0])
+    assert result_packets == orig_packets
+
+    # Now we will split the data across 3 CCSDS packets and test the sequence_count wrap-around
+    p0 = packets.create_ccsds_packet(data=b"0"*63, apid=11, sequence_flags=packets.SequenceFlags.FIRST, sequence_count=16382)
+    p1 = packets.create_ccsds_packet(data=b"0"*1, apid=11, sequence_flags=packets.SequenceFlags.CONTINUATION, sequence_count=16383)
+    p2 = packets.create_ccsds_packet(data=b"0"*1, apid=11, sequence_flags=packets.SequenceFlags.LAST, sequence_count=0)
+    raw_bytes = p0 + p1 + p2
+    result_packets = list(d.packet_generator(raw_bytes, combine_segmented_packets=True))
+    remove_keys(result_packets[0])
+    assert result_packets == orig_packets
+
+    # Test stripping secondary headers (4 bytes per packet), should keep the first packet's header, but skip the following
+    # Add in 4 1s to the 2nd and 3rd packet that should be removed
+    p0 = packets.create_ccsds_packet(data=b"0"*63, apid=11, sequence_flags=packets.SequenceFlags.FIRST, sequence_count=16382)
+    p1 = packets.create_ccsds_packet(data=b"1"*4 + b"0"*1, apid=11, sequence_flags=packets.SequenceFlags.CONTINUATION, sequence_count=16383)
+    p2 = packets.create_ccsds_packet(data=b"1"*4 + b"0"*1, apid=11, sequence_flags=packets.SequenceFlags.LAST, sequence_count=0)
+    raw_bytes = p0 + p1 + p2
+    result_packets = list(d.packet_generator(raw_bytes, combine_segmented_packets=True, secondary_header_bytes=4))
+    remove_keys(result_packets[0])
+    assert result_packets == orig_packets
+
+
+def test_continuation_packet_warnings(test_data_dir):
+    # This definition has 65 bytes worth of data
+    d = definitions.XtcePacketDefinition(test_data_dir / "test_xtce.xml")
+
+    # CONTINUATION / LAST without FIRST
+    p0 = packets.create_ccsds_packet(data=b"0"*65, apid=11, sequence_flags=packets.SequenceFlags.CONTINUATION)
+    p1 = packets.create_ccsds_packet(data=b"0"*65, apid=11, sequence_flags=packets.SequenceFlags.LAST)
+    raw_bytes = p0 + p1
+    with pytest.warns(match="Continuation packet found without declaring the start"):
+        list(d.packet_generator(raw_bytes, combine_segmented_packets=True))
+        # Nothing expected to be returned
+        assert len(list(d.packet_generator(raw_bytes, combine_segmented_packets=True))) == 0
+
+    # Out of sequence packets
+    p0 = packets.create_ccsds_packet(data=b"0"*65, apid=11, sequence_flags=packets.SequenceFlags.FIRST, sequence_count=1)
+    p1 = packets.create_ccsds_packet(data=b"0"*65, apid=11, sequence_flags=packets.SequenceFlags.LAST, sequence_count=0)
+    raw_bytes = p0 + p1
+
+    with pytest.warns(match="not in sequence"):
+        # Nothing expected to be returned
+        assert len(list(d.packet_generator(raw_bytes, combine_segmented_packets=True))) == 0