CCSDSPy / ccsdspy

I/O interface and utilities for CCSDS binary spacecraft data in Python. Library used in flight missions at NASA, NOAA, and SWRI
https://ccsdspy.org
BSD 3-Clause "New" or "Revised" License
78 stars 19 forks source link

Extensions to VariableLength: support arrays whose length is determined by another field #57

Closed ddasilva closed 1 year ago

ddasilva commented 1 year ago

This change updates the VariableLength class to support variable length fields where the length is determined by another field in the packet. For example, you could have fields data1_len which sets the length of the data array, and then data2_len which follows it and sets the length of the data2 array. I have been told that some instruments generate these types of packets, which were currently unsupported by the array_shape="expand" method.

I added a new test for this and all the previous tests pass. I updated the user's guide for variable length packets and added to the changelog.



   import ccsdspy
   from ccsdspy import PacketField, PacketArray

    pkt = ccsdspy.VariableLength([
         PacketField(
              name='SHCOARSE',
              data_type='uint',
              bit_length=32
         ),
         PacketField(
              name='data_len',
              data_type='uint',
              bit_length=8,
         ),  
         PacketArray(
              name="data",
              data_type="uint",
              bit_length=16,
              array_shape="data_len",  # links data to data_len
         ),
         PacketField(
              name="checksum",
              data_type="uint",
              bit_length=16
         ),
    ])
codecov-commenter commented 1 year ago

Codecov Report

Patch coverage: 97.80% and project coverage change: +0.10 :tada:

Comparison is base (e15f2bf) 94.82% compared to head (4cb0354) 94.92%.

Additional details and impacted files ```diff @@ Coverage Diff @@ ## main #57 +/- ## ========================================== + Coverage 94.82% 94.92% +0.10% ========================================== Files 6 6 Lines 464 493 +29 ========================================== + Hits 440 468 +28 - Misses 24 25 +1 ``` | [Impacted Files](https://codecov.io/gh/ddasilva/ccsdspy/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva) | Coverage Δ | | |---|---|---| | [ccsdspy/packet\_fields.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9wYWNrZXRfZmllbGRzLnB5) | `96.61% <80.00%> (-1.61%)` | :arrow_down: | | [ccsdspy/packet\_types.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9wYWNrZXRfdHlwZXMucHk=) | `95.71% <83.33%> (-0.61%)` | :arrow_down: | | [ccsdspy/decode.py](https://codecov.io/gh/ddasilva/ccsdspy/pull/57?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=Daniel+da+Silva#diff-Y2NzZHNweS9kZWNvZGUucHk=) | `95.43% <100.00%> (+1.14%)` | :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 commented 1 year ago

@ehsteve Let me know what you think of this at a high level. There's a lot of dense code added, no pressure to review it

ddasilva commented 1 year ago

Closes #58

ddasilva commented 1 year ago

Build of Updated documentation (hidden from public): https://ccsdspy.readthedocs.io/en/var-length-prev-field/user-guide/variablelength.html

ehsteve commented 1 year ago

Hey @ddasilva maybe I'm not understanding something but given that the packet length is already in the ccsds header, isn't it always possible to work out the length of a variable field with this new functionality. Seems to me like any data length field is redundant unless you are relaxing the requirement on having only one expanding field.

ddasilva commented 1 year ago

That is correct, you have multiple variable length fields per packet.

ddasilva commented 1 year ago

And there is a unit test that does just this!

ddasilva commented 1 year ago

I updated the user guide to show the example having two reference-based variable fields, and added a statement to the user guide about how there is not a limit to the number of these reference-based variable fields, but there is for expanding fields.

https://ccsdspy.readthedocs.io/en/var-length-prev-field/user-guide/variablelength.html

ddasilva commented 1 year ago

@ehsteve I merged this because several weeks went by and I assumed you didn't have any comments on this. If you have comments, let me know and we can go back to discuss