From c2118197f1e81764180156a168d24d748a7aa235 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 03:17:32 +0000 Subject: [PATCH 1/8] Initial plan From 4dc0b221158e5c66f546e5ebf5f145615d61a863 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 03:25:37 +0000 Subject: [PATCH 2/8] Replace structured-classes with built-in struct module Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- pyproject.toml | 1 - src/struct_frame/py_gen.py | 521 ++++++++++++++++++++++----------- src/struct_frame/py_gen.py.bak | 320 ++++++++++++++++++++ 3 files changed, 665 insertions(+), 177 deletions(-) create mode 100644 src/struct_frame/py_gen.py.bak diff --git a/pyproject.toml b/pyproject.toml index 40e5495a..2cf40bd2 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -9,7 +9,6 @@ authors = [ { name="Rijesh Augustine", email="rijesh@mylonics.com" }, ] dependencies = [ - "structured-classes>=3.1.0", "proto-schema-parser>=1.4.5", ] description = "A framework for serializing data with headers" diff --git a/src/struct_frame/py_gen.py b/src/struct_frame/py_gen.py index e14a9108..19ebb8b5 100644 --- a/src/struct_frame/py_gen.py +++ b/src/struct_frame/py_gen.py @@ -6,19 +6,36 @@ StyleC = NamingStyleC() -py_types = {"uint8": "uint8", - "int8": "int8", - "uint16": "uint16", - "int16": "int16", - "uint32": "uint32", - "int32": "int32", - "bool": "bool8", - "float": "float32", - "double": "float64", - "uint64": 'uint64', - "int64": 'int64', - "string": "str", # Add string type support - } +# Mapping from proto types to Python struct format characters +py_struct_format = { + "uint8": "B", + "int8": "b", + "uint16": "H", + "int16": "h", + "uint32": "I", + "int32": "i", + "bool": "?", + "float": "f", + "double": "d", + "uint64": "Q", + "int64": "q", +} + +# Python type hints for fields +py_type_hints = { + "uint8": "int", + "int8": "int", + "uint16": "int", + "int16": "int", + "uint32": "int", + "int32": "int", + "bool": "bool", + "float": "float", + "double": "float", + "uint64": "int", + "int64": "int", + "string": "bytes", +} class EnumPyGen(): @@ -54,141 +71,305 @@ def generate(field): class FieldPyGen(): @staticmethod - def generate(field): - result = '' - - var_name = field.name + def get_type_hint(field): + """Get Python type hint for a field""" type_name = field.fieldType - - # Handle basic type resolution - if type_name in py_types: - base_type = py_types[type_name] + + if type_name in py_type_hints: + base_hint = py_type_hints[type_name] + elif field.isEnum: + base_hint = "int" # Enums are stored as uint8 else: - if field.isEnum: - # For enums, use uint8 as the serialization type (enums are stored as uint8) - # but keep the enum class name for documentation - base_type = 'uint8' - enum_class_name = '%s%s' % (pascalCase(field.package), type_name) - else: - base_type = '%s%s' % (pascalCase(field.package), type_name) - + # Nested message + base_hint = '%s%s' % (pascalCase(field.package), type_name) + # Handle arrays if field.is_array: if field.fieldType == "string": - # String arrays use char arrays - if field.size_option is not None: - # Fixed string array: char[array_size][string_size] - element_size = field.element_size if field.element_size else 16 - # Use Annotated with StaticStructArraySerializer for array of char arrays - type_annotation = f"Annotated[list[bytes], StaticStructArraySerializer({field.size_option}, typing.get_args(char[{element_size}])[1])] # Fixed string array: {field.size_option} strings, each {element_size} chars" - elif field.max_size is not None: - # Bounded string array - use nested struct - type_annotation = f"_BoundedStringArray_{var_name}" - else: - type_annotation = f"list[{base_type}] # String array (unbounded)" + return "List[bytes]" else: - # Non-string arrays - if field.size_option is not None: - # Fixed array: Use StaticStructArraySerializer - if field.isEnum or field.isDefaultType: - # For enums and primitives, use Annotated with StaticStructArraySerializer - # For enums, they're stored as uint8 - serializer_type = 'uint8' if field.isEnum else base_type - if field.isEnum: - type_annotation = f"Annotated[list, StaticStructArraySerializer({field.size_option}, typing.get_args(uint8)[1])] # Fixed array of {enum_class_name}: {field.size_option} elements" - else: - type_annotation = f"Annotated[list, StaticStructArraySerializer({field.size_option}, typing.get_args({base_type})[1])] # Fixed array: {field.size_option} elements" - else: - # For nested structs, we need to inline them (arrays of structs not directly supported) - # Mark this for special handling in MessagePyGen - type_annotation = f"_INLINE_STRUCT_ARRAY_{base_type}_{field.size_option}" - elif field.max_size is not None: - # Bounded array - use nested struct - type_annotation = f"_BoundedArray_{var_name}" - else: - type_annotation = f"list[{base_type}] # Array (unbounded)" - # Handle strings with size info + return f"List[{base_hint}]" + elif field.fieldType == "string": + return "bytes" + else: + return base_hint + + @staticmethod + def generate(field): + """Generate field definition with type hint""" + result = '' + + var_name = field.name + type_hint = FieldPyGen.get_type_hint(field) + + result += f' {var_name}: {type_hint}' + + # Add comments about special handling + if field.is_array: + if field.size_option is not None: + result += f' # Fixed array: {field.size_option} elements' + elif field.max_size is not None: + result += f' # Bounded array: max {field.max_size} elements' elif field.fieldType == "string": if field.size_option is not None: - # Fixed string - use char array (this works natively in structured library) - type_annotation = f"char[{field.size_option}] # Fixed string: {field.size_option} chars" + result += f' # Fixed string: {field.size_option} bytes' elif field.max_size is not None: - # Variable string - needs a nested struct with length prefix (like C) - type_annotation = f"_VariableString_{var_name}" - else: - # Fallback (shouldn't happen with validation) - type_annotation = "str # String (unbounded)" - else: - # Regular field - if field.isEnum: - # For enum fields, add comment about the enum type - type_annotation = f"{base_type} # Enum: {enum_class_name}" - else: - type_annotation = base_type - - result += ' %s: %s' % (var_name, type_annotation) - + result += f' # Variable string: max {field.max_size} bytes' + + if field.isEnum: + enum_class_name = '%s%s' % (pascalCase(field.package), field.fieldType) + result += f' # Enum: {enum_class_name}' + leading_comment = field.comments if leading_comment: for c in leading_comment: result = "#" + c + "\n" + result - + return result class MessagePyGen(): @staticmethod - def generate_variable_string_struct(field, msg_name): - """Generate a nested struct for variable-size strings""" - var_name = field.name - struct_name = f"_VariableString_{var_name}" - max_size = field.max_size + def get_struct_format(field): + """Get struct format string for a field""" + type_name = field.fieldType + + # Get base format character + if type_name in py_struct_format: + base_fmt = py_struct_format[type_name] + elif field.isEnum: + base_fmt = "B" # Enums are uint8 + elif type_name == "string": + # Strings need special handling + if field.size_option is not None: + return f"{field.size_option}s" + else: + return None # Variable strings handled separately + else: + # Nested message - no direct format + return None - result = f'class {struct_name}(Structured, byte_order=ByteOrder.LE, byte_order_mode=ByteOrderMode.OVERRIDE):\n' - result += f' length: uint8 # Actual length of the string\n' - result += f' data: char[{max_size}] # String data (null-padded)\n' + # Handle arrays + if field.is_array: + if field.size_option is not None: + # Fixed array + return f"{field.size_option}{base_fmt}" + else: + # Bounded/variable array - handled separately + return None - return result + '\n' + return base_fmt @staticmethod - def generate_bounded_array_struct(field, msg_name): - """Generate a nested struct for bounded/variable arrays""" - var_name = field.name - type_name = field.fieldType + def generate_pack_method(msg): + """Generate the pack() method""" + result = '\n def pack(self) -> bytes:\n' + result += ' """Pack the message into binary format"""\n' + result += ' data = b""\n' - # Determine the base type - if type_name in py_types: - base_type = py_types[type_name] - else: - if field.isEnum: - base_type = '%s%s' % (pascalCase(field.package), type_name) + for key, f in msg.fields.items(): + if f.fieldType == "string" and not f.is_array: + # String field + if f.size_option is not None: + # Fixed string + result += f' # Fixed string: {f.name}\n' + result += f' data += struct.pack("<{f.size_option}s", self.{f.name}[:{f.size_option}])\n' + elif f.max_size is not None: + # Variable string with length prefix + result += f' # Variable string: {f.name}\n' + result += f' str_data = self.{f.name}[:{f.max_size}]\n' + result += f' data += struct.pack(" Date: Thu, 6 Nov 2025 03:26:14 +0000 Subject: [PATCH 3/8] Remove backup file and add .bak to gitignore Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- .gitignore | 1 + src/struct_frame/py_gen.py.bak | 320 --------------------------------- 2 files changed, 1 insertion(+), 320 deletions(-) delete mode 100644 src/struct_frame/py_gen.py.bak diff --git a/.gitignore b/.gitignore index 5a3798f9..2e902fe8 100644 --- a/.gitignore +++ b/.gitignore @@ -35,3 +35,4 @@ Thumbs.db *.swp *.swo *~ +*.bak diff --git a/src/struct_frame/py_gen.py.bak b/src/struct_frame/py_gen.py.bak deleted file mode 100644 index e14a9108..00000000 --- a/src/struct_frame/py_gen.py.bak +++ /dev/null @@ -1,320 +0,0 @@ -#!/usr/bin/env python3 -# kate: replace-tabs on; indent-width 4; - -from struct_frame import version, NamingStyleC, CamelToSnakeCase, pascalCase -import time - -StyleC = NamingStyleC() - -py_types = {"uint8": "uint8", - "int8": "int8", - "uint16": "uint16", - "int16": "int16", - "uint32": "uint32", - "int32": "int32", - "bool": "bool8", - "float": "float32", - "double": "float64", - "uint64": 'uint64', - "int64": 'int64', - "string": "str", # Add string type support - } - - -class EnumPyGen(): - @staticmethod - def generate(field): - leading_comment = field.comments - - result = '' - if leading_comment: - for c in leading_comment: - result = '#%s\n' % c - - enumName = '%s%s' % (pascalCase(field.package), field.name) - result += 'class %s(Enum):\n' % (enumName) - - enum_length = len(field.data) - enum_values = [] - for index, (d) in enumerate(field.data): - leading_comment = field.data[d][1] - - if leading_comment: - for c in leading_comment: - enum_values.append("#" + c) - - enum_value = " %s_%s = %d" % (CamelToSnakeCase( - field.name).upper(), StyleC.enum_entry(d), field.data[d][0]) - - enum_values.append(enum_value) - - result += '\n'.join(enum_values) - return result - - -class FieldPyGen(): - @staticmethod - def generate(field): - result = '' - - var_name = field.name - type_name = field.fieldType - - # Handle basic type resolution - if type_name in py_types: - base_type = py_types[type_name] - else: - if field.isEnum: - # For enums, use uint8 as the serialization type (enums are stored as uint8) - # but keep the enum class name for documentation - base_type = 'uint8' - enum_class_name = '%s%s' % (pascalCase(field.package), type_name) - else: - base_type = '%s%s' % (pascalCase(field.package), type_name) - - # Handle arrays - if field.is_array: - if field.fieldType == "string": - # String arrays use char arrays - if field.size_option is not None: - # Fixed string array: char[array_size][string_size] - element_size = field.element_size if field.element_size else 16 - # Use Annotated with StaticStructArraySerializer for array of char arrays - type_annotation = f"Annotated[list[bytes], StaticStructArraySerializer({field.size_option}, typing.get_args(char[{element_size}])[1])] # Fixed string array: {field.size_option} strings, each {element_size} chars" - elif field.max_size is not None: - # Bounded string array - use nested struct - type_annotation = f"_BoundedStringArray_{var_name}" - else: - type_annotation = f"list[{base_type}] # String array (unbounded)" - else: - # Non-string arrays - if field.size_option is not None: - # Fixed array: Use StaticStructArraySerializer - if field.isEnum or field.isDefaultType: - # For enums and primitives, use Annotated with StaticStructArraySerializer - # For enums, they're stored as uint8 - serializer_type = 'uint8' if field.isEnum else base_type - if field.isEnum: - type_annotation = f"Annotated[list, StaticStructArraySerializer({field.size_option}, typing.get_args(uint8)[1])] # Fixed array of {enum_class_name}: {field.size_option} elements" - else: - type_annotation = f"Annotated[list, StaticStructArraySerializer({field.size_option}, typing.get_args({base_type})[1])] # Fixed array: {field.size_option} elements" - else: - # For nested structs, we need to inline them (arrays of structs not directly supported) - # Mark this for special handling in MessagePyGen - type_annotation = f"_INLINE_STRUCT_ARRAY_{base_type}_{field.size_option}" - elif field.max_size is not None: - # Bounded array - use nested struct - type_annotation = f"_BoundedArray_{var_name}" - else: - type_annotation = f"list[{base_type}] # Array (unbounded)" - # Handle strings with size info - elif field.fieldType == "string": - if field.size_option is not None: - # Fixed string - use char array (this works natively in structured library) - type_annotation = f"char[{field.size_option}] # Fixed string: {field.size_option} chars" - elif field.max_size is not None: - # Variable string - needs a nested struct with length prefix (like C) - type_annotation = f"_VariableString_{var_name}" - else: - # Fallback (shouldn't happen with validation) - type_annotation = "str # String (unbounded)" - else: - # Regular field - if field.isEnum: - # For enum fields, add comment about the enum type - type_annotation = f"{base_type} # Enum: {enum_class_name}" - else: - type_annotation = base_type - - result += ' %s: %s' % (var_name, type_annotation) - - leading_comment = field.comments - if leading_comment: - for c in leading_comment: - result = "#" + c + "\n" + result - - return result - - -class MessagePyGen(): - @staticmethod - def generate_variable_string_struct(field, msg_name): - """Generate a nested struct for variable-size strings""" - var_name = field.name - struct_name = f"_VariableString_{var_name}" - max_size = field.max_size - - result = f'class {struct_name}(Structured, byte_order=ByteOrder.LE, byte_order_mode=ByteOrderMode.OVERRIDE):\n' - result += f' length: uint8 # Actual length of the string\n' - result += f' data: char[{max_size}] # String data (null-padded)\n' - - return result + '\n' - - @staticmethod - def generate_bounded_array_struct(field, msg_name): - """Generate a nested struct for bounded/variable arrays""" - var_name = field.name - type_name = field.fieldType - - # Determine the base type - if type_name in py_types: - base_type = py_types[type_name] - else: - if field.isEnum: - base_type = '%s%s' % (pascalCase(field.package), type_name) - else: - base_type = '%s%s' % (pascalCase(field.package), type_name) - - if field.fieldType == "string": - # Bounded string array - struct_name = f"_BoundedStringArray_{var_name}" - element_size = field.element_size if field.element_size else 16 - result = f'class {struct_name}(Structured, byte_order=ByteOrder.LE, byte_order_mode=ByteOrderMode.OVERRIDE):\n' - result += f' count: uint8\n' - result += f' data: Annotated[list[bytes], StaticStructArraySerializer({field.max_size}, typing.get_args(char[{element_size}])[1])]\n' - else: - # Bounded numeric/enum/struct array - struct_name = f"_BoundedArray_{var_name}" - result = f'class {struct_name}(Structured, byte_order=ByteOrder.LE, byte_order_mode=ByteOrderMode.OVERRIDE):\n' - result += f' count: uint8\n' - - if field.isEnum or field.isDefaultType: - # For enums and primitives - serializer_type = 'uint8' if field.isEnum else base_type - result += f' data: Annotated[list, StaticStructArraySerializer({field.max_size}, typing.get_args({serializer_type})[1])]\n' - else: - # For nested structs, inline them - # Since we can't easily create arrays of Structured objects, we inline the fields - for i in range(field.max_size): - result += f' item{i}: {base_type}\n' - - return result + '\n' - - @staticmethod - def generate(msg): - leading_comment = msg.comments - - result = '' - if leading_comment: - for c in msg.comments: - result = '#%s\n' % c - - # First, generate nested structs for variable strings and bounded arrays - for key, f in msg.fields.items(): - if f.fieldType == "string" and f.max_size is not None and not f.is_array: - # Variable-size string field - result += MessagePyGen.generate_variable_string_struct(f, msg.name) - elif f.is_array and f.max_size is not None: - # Bounded array - result += MessagePyGen.generate_bounded_array_struct(f, msg.name) - - structName = '%s%s' % (pascalCase(msg.package), msg.name) - result += 'class %s(Structured, byte_order=ByteOrder.LE, byte_order_mode=ByteOrderMode.OVERRIDE):\n' % structName - result += ' msg_size = %s\n' % msg.size - if msg.id != None: - result += ' msg_id = %s\n' % msg.id - - # Generate fields, handling special cases - field_lines = [] - for key, f in msg.fields.items(): - field_code = FieldPyGen.generate(f) - - # Handle inline struct arrays (fixed-size arrays of nested structs) - if "_INLINE_STRUCT_ARRAY_" in field_code: - # Extract struct name and count - import re - match = re.search(r'_INLINE_STRUCT_ARRAY_(\w+)_(\d+)', field_code) - if match: - struct_name = match.group(1) - count = int(match.group(2)) - # Generate inlined fields - for i in range(count): - field_lines.append(f' {f.name}{i}: {struct_name} # Inlined struct array element {i}') - continue - - field_lines.append(field_code) - - result += '\n'.join(field_lines) - - result += '\n\n def __str__(self):\n' - result += f' out = "{msg.name} Msg, ID {msg.id}, Size {msg.size} \\n"\n' - for key, f in msg.fields.items(): - result += f' out += f"{key} = ' - result += '{self.' + key + '}\\n"\n' - result += f' out += "\\n"\n' - result += f' return out' - - result += '\n\n def to_dict(self, include_name = True, include_id = True):\n' - result += ' out = {}\n' - # Handle all field types including arrays - for key, f in msg.fields.items(): - if f.is_array: - if f.isDefaultType or f.isEnum or f.fieldType == "string": - # Array of primitives, enums, or strings - result += f' out["{key}"] = self.{key}\n' - else: - # Array of nested messages - convert each element - result += f' out["{key}"] = [item.to_dict(False, False) for item in self.{key}]\n' - elif f.isDefaultType or f.isEnum or f.fieldType == "string": - # Regular primitive, enum, or string field - result += f' out["{key}"] = self.{key}\n' - else: - # Nested message field - if getattr(f, 'flatten', False): - # Merge nested dict into parent - result += f' out.update(self.{key}.to_dict(False, False))\n' - else: - result += f' out["{key}"] = self.{key}.to_dict(False, False)\n' - result += ' if include_name:\n' - result += f' out["name"] = "{msg.name}"\n' - result += ' if include_id:\n' - result += f' out["msg_id"] = "{msg.id}"\n' - result += ' return out\n' - - return result - - @staticmethod - def get_initializer(msg, null_init): - if not msg.fields: - return '{0}' - - parts = [] - for field in msg.fields: - parts.append(field.get_initializer(null_init)) - return '{' + ', '.join(parts) + '}' - - -class FilePyGen(): - @staticmethod - def generate(package): - yield '# Automatically generated struct frame header \n' - yield '# Generated by %s at %s. \n\n' % (version, time.asctime()) - - yield 'from structured import *\n' - yield 'from enum import Enum\n' - yield 'import typing\n' - yield 'from typing import Annotated\n' - yield 'from structured.hint_types.arrays import StaticStructArraySerializer\n\n' - - if package.enums: - yield '# Enum definitions\n' - for key, enum in package.enums.items(): - yield EnumPyGen.generate(enum) + '\n\n' - - if package.messages: - yield '# Struct definitions \n' - # Need to sort messages to make sure dependecies are properly met - - for key, msg in package.sortedMessages().items(): - yield MessagePyGen.generate(msg) + '\n' - yield '\n' - - if package.messages: - - yield '%s_definitions = {\n' % package.name - for key, msg in package.sortedMessages().items(): - if msg.id != None: - structName = '%s%s' % (pascalCase(msg.package), msg.name) - yield ' %s: %s,\n' % (msg.id, structName) - - yield '}\n' From e2c971944031328ed92bb207978650f5232a7e31 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 03:28:55 +0000 Subject: [PATCH 4/8] Fix handling of fixed arrays of nested messages Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- src/struct_frame/py_gen.py | 66 +++++++++++++++++++++++++------------- 1 file changed, 44 insertions(+), 22 deletions(-) diff --git a/src/struct_frame/py_gen.py b/src/struct_frame/py_gen.py index 19ebb8b5..e64c6fe2 100644 --- a/src/struct_frame/py_gen.py +++ b/src/struct_frame/py_gen.py @@ -206,16 +206,27 @@ def generate_pack_method(msg): else: # Numeric/enum/struct array fmt = MessagePyGen.get_struct_format(f) - if fmt and f.size_option is not None: - # Fixed array of primitives/enums - result += f' # Fixed array: {f.name}\n' - result += f' for i in range({f.size_option}):\n' - result += f' val = self.{f.name}[i] if i < len(self.{f.name}) else 0\n' - if f.isEnum: - result += f' data += struct.pack(" Date: Thu, 6 Nov 2025 03:32:55 +0000 Subject: [PATCH 5/8] Refactor: Extract struct format size mapping to module constant Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- src/struct_frame/py_gen.py | 16 +++++++++++++--- 1 file changed, 13 insertions(+), 3 deletions(-) diff --git a/src/struct_frame/py_gen.py b/src/struct_frame/py_gen.py index e64c6fe2..6705e253 100644 --- a/src/struct_frame/py_gen.py +++ b/src/struct_frame/py_gen.py @@ -21,6 +21,16 @@ "int64": "q", } +# Mapping from struct format characters to their sizes in bytes +struct_format_sizes = { + 'b': 1, 'B': 1, + 'h': 2, 'H': 2, + 'i': 4, 'I': 4, + 'q': 8, 'Q': 8, + 'f': 4, 'd': 8, + '?': 1 +} + # Python type hints for fields py_type_hints = { "uint8": "int", @@ -329,7 +339,7 @@ def generate_unpack_method(msg): result += f' offset += 1\n' else: base_fmt = py_struct_format[f.fieldType] - size = {'b': 1, 'B': 1, 'h': 2, 'H': 2, 'i': 4, 'I': 4, 'q': 8, 'Q': 8, 'f': 4, 'd': 8, '?': 1}[base_fmt] + size = struct_format_sizes[base_fmt] result += f' val = struct.unpack_from("<{base_fmt}", data, offset)[0]\n' result += f' offset += {size}\n' result += f' fields["{f.name}"].append(val)\n' @@ -356,7 +366,7 @@ def generate_unpack_method(msg): result += f' offset += 1\n' else: base_fmt = py_struct_format[f.fieldType] - size = {'b': 1, 'B': 1, 'h': 2, 'H': 2, 'i': 4, 'I': 4, 'q': 8, 'Q': 8, 'f': 4, 'd': 8, '?': 1}[base_fmt] + size = struct_format_sizes[base_fmt] result += f' val = struct.unpack_from("<{base_fmt}", data, offset)[0]\n' result += f' offset += {size}\n' result += f' if i < count:\n' @@ -378,7 +388,7 @@ def generate_unpack_method(msg): fmt = MessagePyGen.get_struct_format(f) if fmt: # Simple type - size = {'b': 1, 'B': 1, 'h': 2, 'H': 2, 'i': 4, 'I': 4, 'q': 8, 'Q': 8, 'f': 4, 'd': 8, '?': 1}.get(fmt, 0) + size = struct_format_sizes.get(fmt, 0) if f.fieldType == "string": result += f' fields["{f.name}"] = struct.unpack_from("<{fmt}", data, offset)[0]\n' else: From 941a56b272e9cad86ae847642dfb0f07666de740 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 04:48:46 +0000 Subject: [PATCH 6/8] Fix code review issues and re-enable Python cross-platform tests - Fix string format size calculation bug for formats like '16s' - Remove duplicate code in unpack method - Fix checksum calculation in struct_frame_parser.py to include msg_id - Re-enable Python cross-platform tests - Update encoder/decoder test files to work with new struct-based generator - Update comments to remove references to structured-classes library Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- .../boilerplate/py/struct_frame_parser.py | 4 +++- src/struct_frame/py_gen.py | 9 ++++---- tests/cross_platform_test.py | 12 ++++------- tests/py/encoder_framed.py | 21 +++++++------------ tests/py/encoder_struct.py | 13 ++++++------ 5 files changed, 25 insertions(+), 34 deletions(-) diff --git a/src/struct_frame/boilerplate/py/struct_frame_parser.py b/src/struct_frame/boilerplate/py/struct_frame_parser.py index 70f36876..12186de9 100644 --- a/src/struct_frame/boilerplate/py/struct_frame_parser.py +++ b/src/struct_frame/boilerplate/py/struct_frame_parser.py @@ -64,7 +64,9 @@ def encode(self, data, msg_id): if (len(data)): for b in data: output.append(b) - checksum = fletcher_checksum_calculation(data) + # Calculate checksum on msg_id + data (consistent with validate_packet) + checksum_data = [msg_id] + list(data) + checksum = fletcher_checksum_calculation(checksum_data) output.append(checksum[0]) output.append(checksum[1]) diff --git a/src/struct_frame/py_gen.py b/src/struct_frame/py_gen.py index 6705e253..b04242c4 100644 --- a/src/struct_frame/py_gen.py +++ b/src/struct_frame/py_gen.py @@ -388,11 +388,12 @@ def generate_unpack_method(msg): fmt = MessagePyGen.get_struct_format(f) if fmt: # Simple type - size = struct_format_sizes.get(fmt, 0) - if f.fieldType == "string": - result += f' fields["{f.name}"] = struct.unpack_from("<{fmt}", data, offset)[0]\n' + # Handle multi-character struct formats like '16s' + if fmt.endswith('s') and len(fmt) > 1 and fmt[:-1].isdigit(): + size = int(fmt[:-1]) else: - result += f' fields["{f.name}"] = struct.unpack_from("<{fmt}", data, offset)[0]\n' + size = struct_format_sizes.get(fmt, 0) + result += f' fields["{f.name}"] = struct.unpack_from("<{fmt}", data, offset)[0]\n' result += f' offset += {size}\n' else: # Nested message diff --git a/tests/cross_platform_test.py b/tests/cross_platform_test.py index 19eed3fe..caa48dc8 100755 --- a/tests/cross_platform_test.py +++ b/tests/cross_platform_test.py @@ -266,17 +266,13 @@ def test_cross_platform(self, encoder_lang: str, decoder_lang: str, framed: bool def check_language_available(self, language: str, mode: str = "framed") -> bool: """Check if encoder/decoder for a language are available""" - # NOTE: Python and TypeScript are currently disabled due to known limitations: - # - Python: The structured-classes library's pack() method doesn't properly - # serialize variable-length strings and arrays. Only fixed-size primitive - # fields are included in the packed output, preventing proper cross-platform - # message encoding. + # NOTE: TypeScript is currently disabled due to known limitations: # - TypeScript: Generated code has a runtime error where the .Array() method # doesn't exist on the typed-struct builder object. This is a code generation # bug in struct-frame's TypeScript generator. # # Once these issues are resolved, remove the early return below. - if language in ["python", "typescript"]: + if language in ["typescript"]: return False if language == "c": @@ -327,8 +323,8 @@ def run_all_tests(self, test_struct=True, test_framed=True): print("="*60) self.log("Struct-based tests are currently NOT IMPLEMENTED", "WARNING") self.log( - "This is due to limitations in variable-length field serialization", "WARNING") - self.log("in the Python structured-classes library.", "WARNING") + "This is due to encoder/decoder implementations not being complete", "WARNING") + self.log("for all languages in struct mode.", "WARNING") self.log("TEST FAILED: Struct-based tests not implemented", "ERROR") # Test framed mode diff --git a/tests/py/encoder_framed.py b/tests/py/encoder_framed.py index 35829d72..47667916 100755 --- a/tests/py/encoder_framed.py +++ b/tests/py/encoder_framed.py @@ -11,24 +11,17 @@ sys.path.insert(0, os.path.join(os.path.dirname(__file__), '../generated/py')) try: - from serialization_test_sf import SerializationTestSerializationTestMessage, _VariableString_test_string, _BoundedArray_test_array + from serialization_test_sf import SerializationTestSerializationTestMessage from struct_frame_parser import BasicPacket - # Create variable string for test_string (matching the test_serialization.py format) - test_string = _VariableString_test_string( - 18, b"Hello from Python!") # length=18, data - - # Create bounded array for test_array (matching the test_serialization.py format) - test_array = _BoundedArray_test_array( - 3, [100, 200, 300, 0, 0]) # 3 elements in use, max 5 - # Create test message with known values + # The new struct-based generator uses direct values instead of wrapper classes msg = SerializationTestSerializationTestMessage( - 0xDEADBEEF, # magic_number - test_string, # test_string - 3.14159, # test_float - True, # test_bool - test_array # test_array + magic_number=0xDEADBEEF, + test_string=b"Hello from Python!", + test_float=3.14159, + test_bool=True, + test_array=[100, 200, 300] ) # Serialize with framing using BasicPacket diff --git a/tests/py/encoder_struct.py b/tests/py/encoder_struct.py index f11f5411..87d9e501 100755 --- a/tests/py/encoder_struct.py +++ b/tests/py/encoder_struct.py @@ -14,16 +14,15 @@ from serialization_test_sf import SerializationTestSerializationTestMessage # Create test message with known values + # The new struct-based generator uses bytes for strings msg = SerializationTestSerializationTestMessage( - 0xDEADBEEF, # magic_number - 3.14159, # test_float - True # test_bool + magic_number=0xDEADBEEF, + test_string=b"Hello from Python!", + test_float=3.14159, + test_bool=True, + test_array=[100, 200, 300] ) - # Set string and array fields - msg.test_string = "Hello from Python!" - msg.test_array = [100, 200, 300] - # Serialize to struct bytes (no framing) using pack() struct_bytes = msg.pack() From ec83663a01f4186a052b9f944999985ea25bb12f Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 04:56:06 +0000 Subject: [PATCH 7/8] Remove structured-classes from CI dependencies The CI workflow was still trying to install structured-classes which is no longer a dependency. This has been removed so the CI can properly run the Python compatibility tests. Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- .github/workflows/test.yml | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/.github/workflows/test.yml b/.github/workflows/test.yml index 3a63760d..b453b80f 100644 --- a/.github/workflows/test.yml +++ b/.github/workflows/test.yml @@ -36,7 +36,7 @@ jobs: - name: Install Python dependencies run: | python -m pip install --upgrade pip - pip install proto-schema-parser structured-classes + pip install proto-schema-parser - name: Install Node.js dependencies run: npm ci From 751f6afb82501c40e45eede0a21a8dede74abcde Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Thu, 6 Nov 2025 05:07:12 +0000 Subject: [PATCH 8/8] Fix Python cross-language compatibility tests MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Updated test_serialization.py to use the new struct-based generator API instead of the old structured-classes helper classes. This allows the Python test to generate the python_test_data.bin file, which enables the cross-language compatibility tests. Now all 6 cross-language tests are running and passing: - C → C++ - C → Python - C++ → C - C++ → Python - Python → C - Python → C++ Co-authored-by: rijesha <7819200+rijesha@users.noreply.github.com> --- tests/py/test_serialization.py | 17 ++++++++--------- 1 file changed, 8 insertions(+), 9 deletions(-) diff --git a/tests/py/test_serialization.py b/tests/py/test_serialization.py index e29fc254..c251af0b 100755 --- a/tests/py/test_serialization.py +++ b/tests/py/test_serialization.py @@ -34,18 +34,17 @@ def create_test_data(): """Create test data for cross-language compatibility testing""" try: sys.path.insert(0, '../generated/py') - from serialization_test_sf import ( - SerializationTestSerializationTestMessage, - _BoundedArray_test_array, - _VariableString_test_string - ) + from serialization_test_sf import SerializationTestSerializationTestMessage from struct_frame_parser import BasicPacket - test_string = _VariableString_test_string(18, b"Hello from Python!") - test_array = _BoundedArray_test_array(3, [100, 200, 300, 0, 0]) - + # Create test message with known values + # The new struct-based generator uses direct values instead of wrapper classes msg = SerializationTestSerializationTestMessage( - 0xDEADBEEF, test_string, 3.14159, True, test_array + magic_number=0xDEADBEEF, + test_string=b"Hello from Python!", + test_float=3.14159, + test_bool=True, + test_array=[100, 200, 300] ) packet = BasicPacket()