From 72734f37190100a9e788ec38c0c8708d03d03d8c Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 29 Jan 2025 18:08:54 -0800 Subject: [PATCH 1/4] Add struct member index parsing to JSON loader --- src/fprime_gds/common/loaders/json_loader.py | 15 ++++++++++++--- 1 file changed, 12 insertions(+), 3 deletions(-) diff --git a/src/fprime_gds/common/loaders/json_loader.py b/src/fprime_gds/common/loaders/json_loader.py index c55b10a5..5494e981 100644 --- a/src/fprime_gds/common/loaders/json_loader.py +++ b/src/fprime_gds/common/loaders/json_loader.py @@ -178,7 +178,10 @@ def construct_serializable_type( SerializableType: The constructed serializable type. """ - struct_members = [] + # Note on struct_members: the order of the members list matter when calling construct_type() below. + # It should be ordered by incrementing index which corresponds to the order in the FPP declaration + # The JSON dictionary ordering is not guaranteed, so we use a dict() to sort by index below. + struct_members = {} for name, member_dict in qualified_type.get("members").items(): member_type_dict = member_dict["type"] member_type_obj = self.parse_type(member_type_dict) @@ -197,11 +200,17 @@ def construct_serializable_type( member_type_obj.FORMAT if hasattr(member_type_obj, "FORMAT") else "{}" ) description = member_type_dict.get("annotation", "") - struct_members.append((name, member_type_obj, fmt_str, description)) + member_index = member_dict["index"] + if member_index in struct_members: + raise KeyError( + f"Invalid dictionary: Duplicate index {member_index} in serializable type {type_name}" + ) + struct_members[member_index] = (name, member_type_obj, fmt_str, description) + # Construct the serializable type with list of members sorted by index ser_type = SerializableType.construct_type( type_name, - struct_members, + [struct_members[i] for i in sorted(struct_members)], ) self.parsed_types[type_name] = ser_type return ser_type From e0e2d756fb6d88927392db2cf4956c42f6f73c50 Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 29 Jan 2025 18:18:26 -0800 Subject: [PATCH 2/4] Add test case --- .../resources/RefTopologyDictionary.json | 32 +++++++++++++++++++ .../common/loaders/test_json_loader.py | 10 ++++++ 2 files changed, 42 insertions(+) diff --git a/test/fprime_gds/common/loaders/resources/RefTopologyDictionary.json b/test/fprime_gds/common/loaders/resources/RefTopologyDictionary.json index 70d8510b..175195bd 100644 --- a/test/fprime_gds/common/loaders/resources/RefTopologyDictionary.json +++ b/test/fprime_gds/common/loaders/resources/RefTopologyDictionary.json @@ -8,6 +8,38 @@ "dictionarySpecVersion" : "1.0.0" }, "typeDefinitions" : [ + { + "kind" : "struct", + "qualifiedName" : "Ref.TestMisorderedStructIndexes", + "members" : { + "ThisIsOne" : { + "type" : { + "name" : "U32", + "kind" : "integer", + "size" : 32, + "signed" : false + }, + "index" : 1 + }, + "ThisIsZero" : { + "type" : { + "name" : "U32", + "kind" : "integer", + "size" : 32, + "signed" : false + }, + "index" : 0 + }, + "ThisIsTwo" : { + "type" : { + "name" : "Ref.PacketRecvStatus", + "kind" : "qualifiedIdentifier" + }, + "index" : 2 + } + }, + "annotation" : "This type does not exist in Ref and is only used for GDS internal testing" + }, { "kind" : "struct", "qualifiedName" : "Ref.PacketStat", diff --git a/test/fprime_gds/common/loaders/test_json_loader.py b/test/fprime_gds/common/loaders/test_json_loader.py index 88f76401..2f5ee69e 100644 --- a/test/fprime_gds/common/loaders/test_json_loader.py +++ b/test/fprime_gds/common/loaders/test_json_loader.py @@ -100,6 +100,16 @@ def test_construct_serializable_type(loader): assert ref_choice_pair.MEMBER_LIST[1][2] == "{}" +def test_struct_with_unordered_members(loader): + misordered_member = loader.parse_type( + {"name": "Ref.TestMisorderedStructIndexes", "kind": "qualifiedIdentifier"} + ) + assert issubclass(misordered_member, SerializableType) + assert misordered_member.MEMBER_LIST[0][0] == "ThisIsZero" + assert misordered_member.MEMBER_LIST[1][0] == "ThisIsOne" + assert misordered_member.MEMBER_LIST[2][0] == "ThisIsTwo" + + def test_construct_primitive_types(loader): i32_type = loader.parse_type( {"name": "I32", "kind": "integer", "size": 32, "signed": True} From 28db2814ac07d59955df16c0b7186237b208c0de Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Wed, 29 Jan 2025 18:24:22 -0800 Subject: [PATCH 3/4] really looks like a word to me --- .github/actions/spelling/expect.txt | 1 + 1 file changed, 1 insertion(+) diff --git a/.github/actions/spelling/expect.txt b/.github/actions/spelling/expect.txt index b48310b4..01996f9f 100644 --- a/.github/actions/spelling/expect.txt +++ b/.github/actions/spelling/expect.txt @@ -415,6 +415,7 @@ metavar microsoft middleware millis +misordered mixin mkdir MML From b9c202177937fd0e58dbfa5b1dd78ddfcd5817fb Mon Sep 17 00:00:00 2001 From: thomas-bc Date: Thu, 30 Jan 2025 12:22:07 -0800 Subject: [PATCH 4/4] Review fix: improve readability --- src/fprime_gds/common/loaders/json_loader.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/src/fprime_gds/common/loaders/json_loader.py b/src/fprime_gds/common/loaders/json_loader.py index 5494e981..2f3f6f53 100644 --- a/src/fprime_gds/common/loaders/json_loader.py +++ b/src/fprime_gds/common/loaders/json_loader.py @@ -210,7 +210,7 @@ def construct_serializable_type( # Construct the serializable type with list of members sorted by index ser_type = SerializableType.construct_type( type_name, - [struct_members[i] for i in sorted(struct_members)], + [struct_members[i] for i in sorted(struct_members.keys())], ) self.parsed_types[type_name] = ser_type return ser_type