Refactor spec parser and consolidate ASN.1 constants#7
Merged
Conversation
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Signed-off-by: Yogev Neumann <xqgex@users.noreply.github.com>
Signed-off-by: Yogev Neumann <xqgex@users.noreply.github.com>
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
This comment was marked as outdated.
Contributor
Python CI ReportCommit: Python TestsTest Output✅ All tests passed Coverage: 96%Per-file coverage (10 modules)
Details (907 statements, 39 missed)
Python Static Analysis
mypypylintflake8ruffcodespell |
Contributor
CI ReportCommit: Static Analysis
cppcheckClick to expandclang-tidyClick to expandSanitizers (ASan + UBSan)Test Output✅ No issues detected Valgrind Memory CheckTest Output✅ No memory issues detected |
There was a problem hiding this comment.
Pull request overview
Refactors the J2735 spec parsing/tooling code by centralizing duplicated ASN.1 constants, reducing duplication in SpecEntry block parsing, and moving fixed-width type collection onto the specification object to simplify downstream generators.
Changes:
- Centralizes ASN.1 tokens and type-definition regex patterns in
tools/j2735_asn1_constants.pyand updates parser/constraints to import them. - Refactors
SpecEntry.from_*_blockparsing by extracting shared section extraction + two ASN.1 parsing strategies (multiline vs DOTALL). - Moves fixed-width type collection into
J2735Specification.collect_fixed_width_types()and reuses it in bitwidth/size generators; adds workflow concurrency + updates stale header references in templates/docs.
Reviewed changes
Copilot reviewed 12 out of 12 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/j2735_asn1_constants.py | New centralized ASN.1 tokens + compiled regex patterns shared across tools. |
| tools/j2735_spec_parser.py | Uses centralized constants; refactors SpecEntry parsing helpers; adds collect_fixed_width_types(). |
| tools/j2735_spec_constraints.py | Uses centralized ASN.1 tokens instead of duplicated private constants. |
| tools/j2735_c_generator_bitwidth_constants.py | Reuses spec.collect_fixed_width_types() instead of duplicating collection logic. |
| tools/j2735_c_generator_size_constants.py | Reuses spec.collect_fixed_width_types() instead of duplicating collection logic. |
| tools/tests/spec/test_spec_entry.py | Adds focused unit coverage for the three SpecEntry.from_*_block constructors. |
| tools/templates/sequence/sequence_size.j2 | Updates dependency comment to reference J2735_internal_inline.h. |
| .github/workflows/python.yml | Adds workflow concurrency to cancel in-progress runs on new pushes. |
| .github/workflows/pr-compliance.yml | Adds workflow concurrency to cancel in-progress runs on new pushes. |
| .github/workflows/ci.yml | Adds workflow concurrency to cancel in-progress runs on new pushes. |
| .github/instructions/j2735_design.instructions.md | Updates stale header reference in design instructions. |
| .github/ISSUE_TEMPLATE/bug_report.yml | Updates stale include example to use J2735_api.h. |
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.
Pull Request
Description
Consolidate duplicated ASN.1 constants, move collection logic to its owning class, and refactor triplicated block-parsing in
SpecEntry.New
j2735_asn1_constants.py: Single source of truth for 5 ASN.1 syntax tokens (ASN1_ASSIGNMENT,ASN1_COMMENT_PREFIX, etc.) and 2 compiled regex patterns (ASN1_TYPE_DEF_PATTERN,ASN1_TYPE_DEF_DOTALL_PATTERN), previously duplicated acrossj2735_spec_constraints.pyandj2735_spec_parser.py.collect_fixed_width_types()→ class method: Moved from standalone function inj2735_c_generator_jinja.pyto method onJ2735Specification, co-locating the query with the data it operates on.SpecEntryblock-parsing refactor: The threefrom_*_blockclassmethods (Data Element, Data Frame, Message) shared ~40 lines of duplicated Use/ASN.1/Remarks extraction. Extracted three@staticmethodhelpers:_extract_block_sections— shared regex extraction (with doctest)_parse_asn1_multiline— line-walking strategy for Data Elements_parse_asn1_dotall— DOTALL regex strategy for Data Frames and MessagesTest-first methodology: 40 unit tests written and verified green against the old code before applying the refactor, then verified green again after.
Ghost reference cleanup: Fixed 3 stale references to removed modules (
j2735_c_generator.py) in bug report template, design instructions, and a Jinja2 template comment.Related Issue
N/A
Additional Notes
N/A
Type of Change
Checklist
make pre-pushand all checks passgit commit -s