Skip to content

fix(ifex_to_protobuf): remove duplicate keys from type_translation dict - #176

Open
SoundMatt wants to merge 1 commit into
COVESA:masterfrom
SoundMatt:fix/ifex-to-protobuf-duplicate-dict-keys
Open

fix(ifex_to_protobuf): remove duplicate keys from type_translation dict#176
SoundMatt wants to merge 1 commit into
COVESA:masterfrom
SoundMatt:fix/ifex-to-protobuf-duplicate-dict-keys

Conversation

@SoundMatt

Copy link
Copy Markdown
Contributor

The type_translation dict in ifex_to_protobuf.py defined "int32", "int64", "uint32", and "uint64" twice each. Python silently uses the last value for any duplicate key, so those four IFEX types were being translated to sfixed32, sfixed64, fixed32, and fixed64 respectively — not the plain protobuf int32/int64/uint32/uint64 types that were set in the first (correct) definitions.

Remove the six duplicate entries.

The type_translation dict contained six entries that duplicated keys
already defined earlier in the same literal:

    "int32" : "sint32"   # duplicate of "int32" : "int32"
    "int64" : "sint64"   # duplicate of "int64" : "int64"
    "uint32": "fixed32"  # duplicate of "uint32": "uint32"
    "uint64": "fixed64"  # duplicate of "uint64": "uint64"
    "int32" : "sfixed32" # duplicate of "int32" (again)
    "int64" : "sfixed64" # duplicate of "int64" (again)

Python silently uses the last definition, so "int32", "int64",
"uint32", "uint64" were all mapping to the sfixed*/fixed* variants,
not the plain protobuf types.  Remove the duplicate entries.

Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>

Signed-off-by: Matt Jones <47545907+SoundMatt@users.noreply.github.com>
@gunnar-mb

Copy link
Copy Markdown
Collaborator

The forward-declaration and the way it is used is clearly wrong to have duplicates.
The reverse translation is still relevant (the types on the right side, such as fixed32 and sint32 are valid protobuf types) but that's already covered in the protobuf_to_ifex implementation.

So this change is correct, but it will change the translation. Just verify that this doesn't break any unit-tests.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants