Adds a few simple checks of the primary header

[ehsteve]

This closes issue #21.

[ddasilva]

Overall looks very good, the only major request is that we start a proper warning design now, to keep things tidy for the future. Can you also add something to the release notes?

After the comments are addressed this can be merged.

[ehsteve]

I think i understand why the tests are failing. the tests were not previously using the pyproject.toml file configuration. I fixed that here and it is now finding files it should not be.

[ddasilva]

It looks like the tests are running ccsdspy/tests/data/hs/attic/expand_mnemnonics.py, which depends on pandas which isn't installed. This isn't really a test script, so maybe we should find a way to exclude it.

We can probably rename this file to _expandmnemonics.py and add `--ignore-glob='*.py` to the pytest call in the github actions

[ehsteve]

@ddasilva I made that change but it didn't fix it. I'm confused because I can't reproduce this issue on my machine. I installed into a clean environment and I actually get a different error. Are you able to reproduce?

ccsdspy/tests/test_hs.py ............F                                                                                  [ 24%]
ccsdspy/tests/test_packet_fields.py ....                                                                                [ 32%]
ccsdspy/tests/test_packet_types.py .......................                                                              [ 75%]
ccsdspy/tests/test_primary_header.py ......                                                                             [ 86%]
ccsdspy/tests/test_split.py ...                                                                                         [ 92%]
ccsdspy/tests/test_var_length.py ....                                                                                   [100%]

========================================================== FAILURES ===========================================================
_________________________________________________ test_hs_apid035_PacketArray _________________________________________________

    def test_hs_apid035_PacketArray():
        # Make FixedLength for normal packet
        dir_path = os.path.dirname(os.path.realpath(__file__))
        apid_dir = os.path.join(dir_path, "data", "hs", "apid035")
        defs_file_path = os.path.join(apid_dir, "defs.csv")
        dict_normal_defs = _load_apid_defs(defs_file_path)

        pkt_fields = []
        tmp = (
            dict_normal_defs["name"],
            dict_normal_defs["data_type"],
            dict_normal_defs["bit_length"],
        )

        for key, data_type, bit_length in zip(*tmp):
            pkt_fields.append(PacketField(name=key, data_type=data_type, bit_length=bit_length))

        normal_pkt = FixedLength(pkt_fields)

        # Make FixedLength for array packet
        fill_length = sum(dict_normal_defs["bit_length"])
        fill_length = 8 * 32  # 8 float32s at end

        array_pkt = FixedLength(
            [
                PacketField(name="unused", data_type="fill", bit_length=fill_length),
                PacketArray(name="PKT35_FLT_SIN_2H", data_type="float", bit_length=32, array_shape=8),
            ]
        )

        # Compare data
        ccsds_file_path = glob.glob(os.path.join(apid_dir, "*.tlm")).pop()
>       normal_result = normal_pkt.load(ccsds_file_path)

ccsdspy/tests/test_hs.py:149: 
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _
ccsdspy/packet_types.py:110: in load
    packet_arrays = _load(
ccsdspy/packet_types.py:534: in _load
    field_arrays = _decode_fixed_length(file_bytes, fields)
_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _

file_bytes = array([  8,  35, 207, ...,  53,  45,  96], dtype=uint8)
fields = [PacketField(name='CCSDS_VERSION_NUMBER', data_type='uint', bit_length=3, bit_offset=0, byte_order='big'), PacketField...big'), PacketField(name='CCSDS_SEQUENCE_COUNT', data_type='uint', bit_length=14, bit_offset=18, byte_order='big'), ...]

    def _decode_fixed_length(file_bytes, fields):
        """Decode a fixed length packet stream of a single APID.

        Parameters
        ----------
        file_bytes : array
           A NumPy array of uint8 type, holding the bytes of the file to decode.
        fields : list of ccsdspy.PacketField
           A list of fields, including the secondary header but excluding the
           primary header.

        Returns
        -------
        dictionary mapping field names to NumPy arrays, stored in the same order as
        the fields array passed.
        """
        # Setup a dictionary mapping a bit offset to each field. It is assumed
        # that the `fields` array contains entries for the secondary header.
        packet_nbytes = file_bytes[4] * 256 + file_bytes[5] + 7
        body_nbytes = sum(field._bit_length for field in fields) // 8
        counter = (packet_nbytes - body_nbytes) * 8

        bit_offset = {}

        for i, field in enumerate(fields):
            if i == 0 and field._bit_offset is not None:
                # case: using bit_offset to fix the start position
                bit_offset[field._name] = field._bit_offset
                counter = field._bit_offset + field._bit_length
            elif field._bit_offset is None:
                # case: floating start position such that packet def fills to
                # to end of packet. What's missing is assumed to be header at the beginning.
                bit_offset[field._name] = counter
                counter += field._bit_length
            elif field._bit_offset < counter:
                # case: bit_offset specifying to backtrack. This condition
                # seems odd and unlikely. Eg. one or more bits of a packet overlap?
                bit_offset[field._name] = field._bit_offset
                # don't update counter unless the the overlap goes past counter
                counter = max(field._bit_offset + field._bit_length, counter)
            elif field._bit_offset >= counter:
                # case: otherwise, bit_offset is ahead of counter and we're skipping
                # definition of 0 or more bits.
                bit_offset[field._name] = field._bit_offset
                counter = field._bit_offset + field._bit_length
            else:
                raise RuntimeError(
                    f"Unexpected case: could not compare"
                    f" bit_offset {field._bit_offset} with "
                    f"counter {counter} for field {field._name}"
                )

        if all(field._bit_offset is None for field in fields):
            assert counter == packet_nbytes * 8, "Field definition != packet length"
        elif counter > packet_nbytes * 8:
>           raise RuntimeError(
                ("Packet definition larger than packet length" f" by {counter-(packet_nbytes*8)} bits")
            )
E           RuntimeError: Packet definition larger than packet length by 16 bits

ccsdspy/decode.py:65: RuntimeError

---------- coverage: platform darwin, python 3.8.16-final-0 ----------
Name                       Stmts   Miss  Cover
----------------------------------------------
ccsdspy/__main__.py           28      4    86%
ccsdspy/decode.py            152      6    96%
ccsdspy/packet_fields.py      56      8    86%
ccsdspy/packet_types.py      136      6    96%
ccsdspy/utils.py              29      5    83%
----------------------------------------------
TOTAL                        401     29    93%

=================================================== short test summary info ===================================================
FAILED ccsdspy/tests/test_hs.py::test_hs_apid035_PacketArray - RuntimeError: Packet definition larger than packet length by 16 bits
================================================ 1 failed, 52 passed in 13.76s ================================================

[ddasilva]

There have been a number of problems with this hs test data-- where I discovered later that the data was wrong. Don't take this too seriously. I'll look into it!

[ddasilva]

@ehsteve I fixed this; it was a bug in the hs dataset. The packet definition didn't match the length of the actual packet as defined in the primary header. It really think the person who gave me this data was actively toying with it and gave me a dump of the fields he made X days/weeks before the tlm files he made (these type of problems have come up before).

It looks like this particular problem didn't come up before because the test wasn't parsing the primary header, so the "extra" body fields defined were just eating into the primary header space. I added a couple lines in this particular test that verify that the array fields being parsed have values (they are sin(t) test data, so I check they should be between -1 and 1).

You can pull my changes here: https://github.com/ddasilva/ccsdspy/tree/ccsds_header_check

Let me know if you need any more help!

[ddasilva]

Don't worry about refactoring a warning hierarchy for this pull request, if this happens it came come later

[ehsteve]

@ddasilva sorry it took so long but finally merged your changes into this PR. Looks like it is ready to go!

[codecov[bot]]

Codecov Report

Patch coverage: 95.45% and project coverage change: +1.86 :tada:

Comparison is base (7db6289) 92.42% compared to head (aed86d3) 94.29%.

Additional details and impacted files

```diff @@ Coverage Diff @@ ## main #55 +/- ## ========================================== + Coverage 92.42% 94.29% +1.86% ========================================== Files 5 5 Lines 383 403 +20 ========================================== + Hits 354 380 +26 + Misses 29 23 -6 ``` | [Impacted Files](https://codecov.io/gh/ddasilva/ccsdspy/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva) | Coverage Δ | | |---|---|---| | [ccsdspy/decode.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9kZWNvZGUucHk=) | `94.80% <66.66%> (-1.25%)` | :arrow_down: | | [ccsdspy/packet\_types.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9wYWNrZXRfdHlwZXMucHk=) | `96.32% <100.00%> (+1.40%)` | :arrow_up: | | [ccsdspy/packet\_fields.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/55?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9wYWNrZXRfZmllbGRzLnB5) | `98.21% <0.00%> (+12.50%)` | :arrow_up: | Help us with your feedback. Take ten seconds to tell us [how you rate us](https://about.codecov.io/nps?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva). Have a feature suggestion? [Share it here.](https://app.codecov.io/gh/feedback/?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva)

:umbrella: View full report at Codecov.
:loudspeaker: Do you have feedback about the report comment? Let us know in this issue.

[ddasilva]

Merged. Thanks @ehsteve !