feat(protobuf): generate nested enum for string fields with allowed values#502
Open
aki1770-del wants to merge 5 commits intoCOVESA:masterfrom
Open
feat(protobuf): generate nested enum for string fields with allowed values#502aki1770-del wants to merge 5 commits intoCOVESA:masterfrom
aki1770-del wants to merge 5 commits intoCOVESA:masterfrom
Conversation
sschleemilch
requested changes
Apr 10, 2026
src/vss_tools/exporters/protobuf.py
Outdated
| nodes: tuple[VSSNode], fd: TextIOWrapper, static_uid: bool, add_optional: bool, include_comments: bool | ||
| ): | ||
| # Pass 1: write nested enum definitions for every string field with allowed values. | ||
| for node in nodes: |
Collaborator
There was a problem hiding this comment.
Why do we need to iterate twice?
src/vss_tools/exporters/protobuf.py
Outdated
| for node in nodes: | ||
| if isinstance(node.data, VSSDataDatatype): | ||
| base = node.data.datatype.strip("[]") | ||
| if base == "string" and node.data.allowed: |
Collaborator
There was a problem hiding this comment.
"string" literal will probably never change. Still use the the one from datatypes.Datatypes
…alues When a VSS signal has `datatype: string` and an `allowed` attribute, the protobuf exporter previously emitted a plain `string` field, allowing any value without enforcement. This silently accepted values outside the declared allowed set at the protobuf layer. Fix: `print_messages()` now uses a two-pass approach: - Pass 1: emit a nested `enum <FieldName>Enum` for each string+allowed field, with allowed values assigned indices starting at 0 (satisfying the proto3 zero-value requirement). - Pass 2: emit fields using the enum type instead of `string`. `repeated string[]` fields with `allowed` become `repeated <EnumType>`. Non-string fields and string fields without `allowed` are unchanged. Closes COVESA#493 Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com> Signed-off-by: Akihiko Komada <aki1770@gmail.com>
Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com> Signed-off-by: Akihiko Komada <aki1770@gmail.com>
ruff-format reformatted the multi-line assert to put the condition arguments on their own line instead of wrapping in parentheses. Keeps pre-commit CI green. Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com> Signed-off-by: Akihiko Komada <aki1770@gmail.com>
- Collapse two-pass loop into one: _write_nested_enum is called immediately before the field declaration, removing the redundant pre-iteration. - Replace "string" literal with Datatypes.STRING[0] from datatypes module. - Update expected.proto: enum definitions now appear inline before their respective fields rather than grouped at the top of the message. Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com> Signed-off-by: Akihiko Komada <aki1770@gmail.com>
Move `from vss_tools.datatypes import Datatypes` before `from vss_tools.main import get_trees` — alphabetical within the vss_tools.* group as required by ruff/isort. Co-Authored-By: Claude and aki1770-del <aki1770@gmail.com> Signed-off-by: Akihiko Komada <aki1770@gmail.com>
3e69703 to
c151e10
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Closes #493
Problem
When a VSS signal has
datatype: stringand anallowedattribute, the protobuf exporter emits a plainstringfield. This silently accepts any value at the protobuf layer — the constraint declared in the VSS spec is invisible to protobuf consumers.Fix
print_messages()now uses a two-pass approach insrc/vss_tools/exporters/protobuf.py:string+allowedfield, emit a nestedenum <FieldName>Enumwith allowed values assigned indices starting at 0 (satisfying the proto3 zero-value requirement).string.repeated string[]fields withallowedbecomerepeated <EnumType>. String fields withoutallowedand all non-string fields are unchanged.Note on proto3 zero-value semantics: proto3 uses the first enum entry as the default for unset fields. This implementation assigns index 0 to the first entry in the VSS
allowedlist. For signals that follow the convention of listingUNKNOWNfirst (e.g.,Vehicle.Exterior.RoadSurfaceCondition), this aligns naturally. For signals without a natural "unset" first value, the first allowed entry becomes the proto default. Happy to discuss if a different index assignment strategy is preferred.Changes
src/vss_tools/exporters/protobuf.py_enum_type_name(),_write_nested_enum(), two-pass logic inprint_messages()tests/vspec/test_protobuf_enum_allowed/tests/vspec/test_protobuf_comments/expected_*.protoStringWithAllowedfixtureTests
All 13 protobuf-related tests pass, including the two existing
test_protobuf_commentsparametrized cases (no-comments / with-comments) which already contained astring+allowedfield in their fixture.AI-assisted — authored with Claude, reviewed by Komada.