Calculated CRC to be compared with existing CRC field in the packets

[tloubrieu-jpl]

Hello,

In our legacy CCSDS packet parser we had a function which added a calculated CRC to the parsed fields, whenever the calculated CRC was different from the one that we can read at the end of our packets, we knew we made something wrong when parsing the packet.

It does not sound like a CRC at the end of the packet is part of the CCSDS standard, how common is this practice ?

Now we use CCSDSpy to parse our packets, to implement this calculated CRC feature, I was thinking to write a converter but instead of having the list of field inputs as an argument to add the converter, I would need to have all the fields but the CRC field.

Any ideas on how I could implement that with CCSDSpy VariableLength ?

Thanks

Thomas

[tloubrieu-jpl]

Actually, I checked again, our legacy code was calculating the CRC from the un-parsed packet bits, from the beginning of the header to after the length of the packet length given in the header minus the length of the CRC field.

I don't believe there would be an easy way to re-code the parsed value and compute the CRC.

However, this calculated CRC was still useful to catch parsing errors since, if we are off some bits when we read the packet because of a mistake, we would not read the CRC at the right location and it would then differ from the calculated CRC.

[tloubrieu-jpl]

Hi again,

I think I will try to implement that feature as a super class of VariableLength which would have additional properties for the calculated CRC and the location of the existing CRC in the packet. I'll let know here how that works.

[ddasilva]

Good question. I haven't had to do this before, but I think this approach might be simpler for CRCs and Checksums in general: you could do a separate decoding pass that parses each packet into body (body below) and checksum fields (checksum_real below), and then use a converter to calculate the checksum from the body (checksum_calculated below).

If this works, it would be useful to include your CRC or checksum converter in the library for others to lean on. Might be easier than refactoring the functionality in a VariableLength superclass.

class CalculatedChecksum(converters.Converter):
    def __init__(self):
        pass
    def convert(self, body_array):
        # calculate checksum here

 pkt = ccsdspy.VariableLength([
      PacketArray(
           name="body",
           data_type="uint",
           bit_length=8,
           array_shape="expand",   # makes the body field expand
      ),
      PacketField(
           name="checksum_real",
           data_type="uint",
           bit_length=16
      ),
 ])

pkt.add_converted_field(
    "body",
    "checksum_calculated",
    CalculatedChecksum()
)
result = pkt.load('MyCCSDS.tlm')

assert np.all(result['checksum_real'] == result['checksum_calculated'])

[tloubrieu-jpl]

Thanks @ddasilva , that sounds simple enough, I will start from your code and write that converter in a PR.

[tloubrieu-jpl]

Hi @ddasilva ,

I believe I also need the CCSDS header fields to calculate the CRC. How can I integrate them to the converter ? Currently I am blocked in line

ccsdspy/ccsdspy/packet_types.py

Line 106 in 270209d

if not all_fields_present:

because the header fields are not in the _fields property.

Thanks

[ddasilva]

Understood! I'll take a look at this in the next couple days and try to reproduce the issue.

[tloubrieu-jpl]

Hi @ddasilva , when do you think you can work on that ? Would you like me to create a ticket to track that task ? Thanks

[ddasilva]

@tloubrieu-jpl Sorry, I left the github notification email in my inbox to remind me but accidentally cleaned it out. I'll take a look at this over the weekend.

[ddasilva]

Hi @tloubrieu-jpl,

This should be an easy fix to add_converted_field(). Replace this line:

fields_in_packet_set = {field._name for field in self._fields}

with these lines:

# Collect valid names of fields, which include primary header fields as well
# as fields defined in the packet.
fields_in_packet_set = set()

for field in _prepend_primary_header_fields(self._fields):
    fields_in_packet_set.add(field._name)

and you should be good to go. Basically the validation check was just too strict (a bug). If we encounter and further issues downstream, feel free to bring them up here.

[tloubrieu-jpl]

Thanks @ddasilva , I will come back to that later this week.