Pull CCSDS config from dictionary if available #259

thomas-bc · 2025-10-30T23:51:39Z


*Related Issue(s)*	nasa/fprime#3664
*Has Unit Tests (y/n)*
*Documentation Included (y/n)*

Change Description

Fix nasa/fprime#3664

Refactor the ConfigManager logic a little bit to make it easier to work with and purging of the ConfigParser (INI files) that was unused
Refactor DataDescType and ApidType (almost aliases) to be Enum-like but pulled from the ConfigManager at runtime
Add functionality to FramerDeframer to pull config values at instantiation

…cleanup and refactor type loading

…pattern, code clarity

src/fprime_gds/common/communication/ccsds/space_data_link.py

test/fprime_gds/common/encoders/test_ch_encoder.py

test/fprime_gds/common/encoders/test_event_encoder.py

test/fprime_gds/common/encoders/test_pkt_encoder.py

Copilot

Pull Request Overview

This PR refactors the Ground Data System (GDS) configuration and type management system to improve how types and constants are loaded from JSON dictionaries. The changes move away from string-based type representations to using actual type class objects, and introduce support for loading constants from the dictionary.

Key changes:

Renamed FwTypeJsonLoader to TypeJsonLoader and expanded it to load all type definitions
Introduced a new ConstantJsonLoader to load constant values from the dictionary
Refactored ConfigManager to store type classes directly instead of string representations, adding set_type()/get_type() and set_constant()/get_constant() methods
Changed default FwPacketDescriptorType from U32Type to U16Type to match CCSDS standards
Removed the apid.py module and integrated its functionality into ApidType class
Updated tests to use new API and override default values where necessary

Reviewed Changes

Copilot reviewed 17 out of 17 changed files in this pull request and generated 6 comments.

Show a summary per file

File	Description
test/fprime_gds/common/loaders/test_json_loader.py	Updated imports, test assertions to use type classes, and formatting improvements
test/fprime_gds/common/encoders/test_pkt_encoder.py	Updated to use new `set_type()` API and override default `FwPacketDescriptorType`
test/fprime_gds/common/encoders/test_event_encoder.py	Updated to use new `set_type()` API and override default `FwPacketDescriptorType`
test/fprime_gds/common/encoders/test_ch_encoder.py	Updated to use new `set_type()` API and override default `FwPacketDescriptorType`
test/fprime_gds/common/distributor/test_distributor.py	Updated to use new `set_type()` API and override default `FwPacketDescriptorType`
test/fprime_gds/common/communication/ccsds/test_space_packet.py	Updated test data to match new default `U16Type` packet descriptor size
src/fprime_gds/executables/cli.py	Updated to use new loader names and iterate over types/constants with new API
src/fprime_gds/common/utils/data_desc_type.py	Refactored to use metaclass for dynamic enum loading from ConfigManager
src/fprime_gds/common/utils/config_manager.py	Refactored to store type classes directly and support constants
src/fprime_gds/common/models/dictionaries.py	Updated to use new loaders and property names
src/fprime_gds/common/loaders/type_json_loader.py	Renamed from `fw_type_json_loader.py` and expanded to load all types
src/fprime_gds/common/loaders/json_loader.py	Added `parse_type_definition()` method for processing type definitions
src/fprime_gds/common/loaders/constant_json_loader.py	New loader for constants from dictionary
src/fprime_gds/common/distributor/distributor.py	Removed key framing support code
src/fprime_gds/common/communication/ccsds/space_packet.py	Updated to use `ApidType.from_data()` instead of removed `APID` class
src/fprime_gds/common/communication/ccsds/space_data_link.py	Added support for loading spacecraft ID and frame size from dictionary constants
src/fprime_gds/common/communication/ccsds/apid.py	Removed file - functionality moved to `ApidType` class

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

Copilot · 2025-10-31T01:02:22Z

src/fprime_gds/common/loaders/constant_json_loader.py

+        Constructs and returns python dictionaries keyed on id and name
+
+        Args:
+            _: Unused argument (inherited)
+        Returns:
+            A tuple with two Fw type dictionaries (python type dict):
+            (id_dict, name_dict). The keys should be the type id and
+            name fields respectively and the values should be type name
+            strings. Note: An empty id dictionary is returned since there
+            are no id fields in the Fw type alias JSON dictionary entries.


The docstring describes the return value incorrectly. This is a constant loader, not a type loader. The description should reference constant dictionaries with name-to-int mappings, not 'Fw type dictionaries' with 'type name strings'. The return structure is actually (empty_id_dict, name_to_value_dict, versions).

Suggested change

Constructs and returns python dictionaries keyed on id and name

Args:

_: Unused argument (inherited)

Returns:

A tuple with two Fw type dictionaries (python type dict):

(id_dict, name_dict). The keys should be the type id and

name fields respectively and the values should be type name

strings. Note: An empty id dictionary is returned since there

are no id fields in the Fw type alias JSON dictionary entries.

Constructs and returns dictionaries for constants defined in the JSON dictionary.

Args:

_: Unused argument (inherited)

Returns:

A tuple containing:

- An empty id dictionary (constants do not have ids)

- A dictionary mapping constant qualified names to their values

- The versions information from the dictionary

Copilot · 2025-10-31T01:02:23Z

src/fprime_gds/common/utils/config_manager.py

-        # These configs give the types of fields in the binary data
+    def get_constant(self, name: str) -> int:
+        """
+        Retrieve a constant from the config for parsing by returning an instance


The docstring incorrectly states 'returning an instance of the associated constant.' Constants are integer values, not instances. Should be 'returning the value of the associated constant.'

Suggested change

Retrieve a constant from the config for parsing by returning an instance

Retrieve a constant from the config for parsing by returning the value

Copilot · 2025-10-31T01:02:23Z

src/fprime_gds/common/utils/config_manager.py

+        constant_value = self.__prop["constants"].get(name, None)
+        if constant_value is None:
+            raise ConfigBadTypeException(name, "Unknown constant name")
+        # Return an instance of the constant


The comment 'Return an instance of the constant' is incorrect. The method returns an integer value, not an instance. Should be '# Return the value of the constant'.

Suggested change

# Return an instance of the constant

# Return the value of the constant

Copilot · 2025-10-31T01:02:23Z

src/fprime_gds/common/utils/config_manager.py

+    def set_constant(self, name: str, value: int):
+        """
+        Set a constant in the config for parsing by associating a name with
+        a constant class.


The docstring incorrectly states 'associating a name with a constant class.' The method associates a name with an integer value, not a class. Should be 'associating a name with a constant value.'

Suggested change

a constant class.

a constant value.

Copilot · 2025-10-31T01:02:24Z

src/fprime_gds/common/communication/ccsds/space_data_link.py

+            pass # Config value not found, move on
+        try:
+            dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")
+        except ConfigBadTypeException:
+            pass # Config value not found, move on


Corrected spacing in comment. Should be '# Config value not found, move on' with a space after '#'.

Suggested change

pass # Config value not found, move on

try:

dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")

except ConfigBadTypeException:

pass # Config value not found, move on

pass # Config value not found, move on

try:

dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")

except ConfigBadTypeException:

pass # Config value not found, move on

Copilot · 2025-10-31T01:02:24Z

src/fprime_gds/common/communication/ccsds/space_data_link.py

+            pass # Config value not found, move on
+        try:
+            dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")
+        except ConfigBadTypeException:
+            pass # Config value not found, move on


Corrected spacing in comment. Should be '# Config value not found, move on' with a space after '#'.

Suggested change

pass # Config value not found, move on

try:

dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")

except ConfigBadTypeException:

pass # Config value not found, move on

pass # Config value not found, move on

try:

dict_frame_size = ConfigManager.get_instance().get_constant("ComCfg.TmFrameFixedSize")

except ConfigBadTypeException:

pass # Config value not found, move on

thomas-bc added 12 commits October 29, 2025 10:54

Merge remote-tracking branch 'nasa/devel' into 3664-ccsds-dictionary

9dc39cd

Clean up legacy code

4ccbc21

Update ConfigManager to support all types, scrub of INI config, code …

90dd328

…cleanup and refactor type loading

Fix unrelated tests and code cleanup

0f04517

Faster, Better, Cheaper backwards compatibility

1dd0427

Python is funnnn

b20a606

docstrings and logs

32659be

Code clarity

4e195e2

handle no ComCfg.Apid case, refactor old APID type into new matclass …

719b88c

…pattern, code clarity

Add constant dict loader into config, use in FramerDeframer

41173bd

fix docstring

73621de

remove unused imports

55f2b88

thomas-bc changed the title ~~Pull CCSDS config from dictionary~~ Pull CCSDS config from dictionary if available Oct 30, 2025

github-advanced-security bot found potential problems Oct 30, 2025

View reviewed changes

src/fprime_gds/common/communication/ccsds/space_data_link.py Fixed Show fixed Hide fixed

Fix tests and formatting

449e86f

github-advanced-security bot found potential problems Oct 31, 2025

View reviewed changes

test/fprime_gds/common/encoders/test_ch_encoder.py Fixed Show fixed Hide fixed

test/fprime_gds/common/encoders/test_event_encoder.py Fixed Show fixed Hide fixed

test/fprime_gds/common/encoders/test_pkt_encoder.py Fixed Show fixed Hide fixed

thomas-bc added 2 commits October 30, 2025 17:48

Fix test_distributor.py

1b83ccd

fix static analysis notes

80a8513

thomas-bc requested review from LeStarch and Copilot October 31, 2025 01:00

Copilot AI reviewed Oct 31, 2025

View reviewed changes

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Uh oh!

Pull CCSDS config from dictionary if available #259

Pull CCSDS config from dictionary if available #259

Uh oh!

thomas-bc commented Oct 30, 2025

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Copilot AI Oct 31, 2025

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant

	Retrieve a constant from the config for parsing by returning an instance
	Retrieve a constant from the config for parsing by returning the value

	# Return an instance of the constant
	# Return the value of the constant

Pull CCSDS config from dictionary if available #259

Are you sure you want to change the base?

Pull CCSDS config from dictionary if available #259

Uh oh!

Conversation

thomas-bc commented Oct 30, 2025

Change Description

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Uh oh!

Copilot AI left a comment

Choose a reason for hiding this comment

Pull Request Overview

Reviewed Changes

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Copilot AI Oct 31, 2025

Choose a reason for hiding this comment

Uh oh!

Reviewers

Assignees

Labels

Projects

Milestone

Development

Uh oh!

1 participant