Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated CoDICE XTCE files from imap_xtce tool #812

Merged

Conversation

bourque
Copy link
Collaborator

@bourque bourque commented Sep 4, 2024

Change Summary

Overview

This PR updates the CoDICE XTCE files from running the new imap_xtce command line tool. As such, there is now a single telemetry definition file for all packet types. However, I need to retain the old P_COD_NHK.xml file until I can sort out a discrepancy with one of the fields that is causing an error (marked as a TODO in the code).

New Files

  • codice_packet_definition.xml
    • The new, combined CoDICE packet definition, output from the imap_xtce tool

Deleted Files

  • A whole bunch of individual packet definition files for each packet type

Updated Files

  • imap_processing/codice/codice_l0.py
    • Updated code to use new packet definition file
  • imap_processing/codice/codice_l1a.py
    • Removed redundant code that decoms packets, using codice_lo.decom_packets() instead
  • imap_processing/codice/constants.py
    • Removed the hard-coded XTCE file mapping, no longer needed

Closes #768

@bourque bourque added Ins: CoDICE Related to the CoDICE instrument Level: L0 Level 0 processing labels Sep 4, 2024
@bourque bourque requested a review from a team September 4, 2024 20:33
@bourque bourque self-assigned this Sep 4, 2024
@bourque bourque requested review from greglucas, subagonsouth, tech3371 and joeymukherjee and removed request for a team September 4, 2024 20:33
Copy link
Collaborator

@greglucas greglucas left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks good. I like the combination of these into a single file.

Comment on lines 39 to 43
# TODO: Currently need to use the 'old' packet definition for housekeeping
# because the test housekeeping packet has LAST_OPCODE=0, and the
# telemetry definition "STATES" tab doesn't allow for a 0 value for
# this. Need to figure out if the test packet is bad, or the telemetry
# definition is wrong.
Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This is an interesting case. Does an Enumeration need to exist for every possible value or should space_packet_parser just return the raw_value here and not decode anything rather than raising an error? Gavin might have some insight here as well.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

From what I understand from Gavin's comment in the xtce slack channel, it sounds like his preference would be to explicitly define what a value of 0 means in the packet definition file. I will ask Joey about this in our next tagup.

Comment on lines 51 to 55
packets: dict[int, xr.Dataset] = packet_file_to_datasets(
packet_file, xtce_packet_definition
)
packets = packet_file_to_datasets(packet_file, xtce_packet_definition)

return packets
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

can you return the dataset now without assigning since it's not packets anymore? or this file itself might go away like mine did for SWE and SWAPI once your HK issue is fixed.

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Good call, this should really be called datasets now instead of packets. But I think I will keep the module around, at least for now, as a way to distinguish L0 code from L1 code.

<xtce:EnumeratedParameterType name="COD_NHK.LAST_OPCODE" signed="false">
<xtce:IntegerDataEncoding sizeInBits="16" encoding="unsigned" />
<xtce:EnumerationList>
<xtce:Enumeration value="4096" label="COD_NOOP" />
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Can you add this to get around your above issue temporarily or will this cause issue later on?

Suggested change
<xtce:Enumeration value="4096" label="COD_NOOP" />
<xtce:Enumeration value="0" label="TESTING" />
<xtce:Enumeration value="4096" label="COD_NOOP" />

Copy link
Collaborator Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Based on Gavin's comments I think this will likely be the workaround I go with, but I want to ask Joey about this first.

@tech3371
Copy link
Contributor

tech3371 commented Sep 6, 2024

Nice consolidation!

@bourque
Copy link
Collaborator Author

bourque commented Sep 11, 2024

After discussing with Joey, a LAST_OPCODE value of 0 is unexpected to happen in-flight, and is likely in there now because Greg Dunn has been filling some values with zeros for simulated data. For now, I added <xtce:Enumeration value="0" label="ERROR" /> to the packet definition file as a workaround, but this should be revisited once I acquire updated simulated housekeeping data (mentioned in the TODO).

@bourque bourque merged commit ebea6cc into IMAP-Science-Operations-Center:dev Sep 11, 2024
17 checks passed
@bourque bourque deleted the codice-xtce-updates branch September 11, 2024 17:05
This pull request was closed.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Ins: CoDICE Related to the CoDICE instrument Level: L0 Level 0 processing
Projects
None yet
Development

Successfully merging this pull request may close these issues.

CoDICE: Add support for variable length data in XTCE definition files
3 participants