Conversation
Signed-off-by: Yogev Neumann <xqgex@users.noreply.github.com>
Contributor
Python CI ReportCommit: Python TestsTest Output✅ All tests passed Coverage: 98%
Details (3237 statements, 60 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
This PR consolidates Data Frame SEQUENCE templates and enforces consistent J2735_INTERNAL_* naming for all internal macros. It follows PR #3's pattern of type-specific template architecture.
Changes:
- Renamed all internal macros to use
J2735_INTERNAL_prefix (OFF, OPT, PREFIX_BITS, ROOT_SIZE_BITS, WIDTH) - Redesigned wire format API from byte-level
ByteSegment/compute_wire_format()to high-levelWireVariant/get_sequence_variants() - Modularized templates: split monolithic
assemble_df.j2into focusedassemble_df_sequence.j2with new sub-templates (asn1_definition.j2,wire_format_table.j2,wire_format_compact.j2,wire_format_section.j2) - Renamed all Jinja filters with
filter_prefix and addedfilter_is_signedandfilter_format_range - Added comprehensive Python test coverage (~2,100 lines across 8 new test files)
- Enhanced C tests with boundary value and misalignment tests
- Added Doxyfile for API documentation generation
Reviewed changes
Copilot reviewed 46 out of 48 changed files in this pull request and generated no comments.
Show a summary per file
| File | Description |
|---|---|
| tools/j2735_c_generator_wire_format.py | Complete redesign from byte-level to variant-based wire format generation |
| tools/j2735_c_generator_jinja.py | Filter renaming and two new filters for signed detection and range formatting |
| tools/j2735_c_generator_data_frame.py | Updated to use new wire format API |
| tools/templates/assemble_df_sequence.j2 | New focused template for SEQUENCE types only |
| tools/templates/wire_format_*.j2 | Three new templates for wire format rendering (section, table, compact) |
| tools/templates/asn1_definition.j2 | New template for ASN.1 definition blocks |
| tools/templates/sequence/sequence_internal_*.j2 | Six templates renamed to match _internal_ convention |
| tools/tests/conftest.py | Extensive improvements with synthetic type builders and enhanced mock specs |
| tools/tests/c_generator/test_wire_format_*.py | Two new comprehensive test files replacing old byte-level tests |
| src/J2735_internal_DF_*.h | Regenerated headers with new naming convention and improved documentation |
| tests/J2735_internal_DF__test. | Enhanced with boundary and misalignment tests |
| Doxyfile | New configuration for API documentation |
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 Data Frame SEQUENCE templates and enforce
J2735_INTERNAL_*naming for all internal macros.Naming consistency: All internal macros that are not part of the public API now use the
J2735_INTERNAL_prefix consistently:J2735_OFF_→J2735_INTERNAL_OFF_J2735_OPT_→J2735_INTERNAL_OPT_J2735_PREFIX_BITS_→J2735_INTERNAL_PREFIX_BITS_J2735_ROOT_SIZE_BITS_→J2735_INTERNAL_ROOT_SIZE_BITS_Wire format redesign: Replaced the byte-level
ByteSegment/compute_wire_format()Python API with a higher-levelWireVariant/get_sequence_variants()API. Layout rendering (Unicode box-drawing tables) is now handled entirely by Jinja templates, making the Python layer responsible only for determining which variants exist (fixed, extensible with/without extensions, optional absent/present).Template modularization:
assemble_df.j2→assemble_df_sequence.j2(focused on SEQUENCE types only)wire_format.j2(Markdown table) →wire_format_section.j2+wire_format_table.j2(column-based) +wire_format_compact.j2(row-based for large types)asn1_definition.j2for rendering ASN.1 type definitions inside Doxygen comments_internal_conventionJinja filter housekeeping: All filters renamed with
filter_prefix for clarity. Two new filters added:filter_is_signedandfilter_format_range.Test coverage: Added 8 new Python test files (~2,100 lines), expanded
conftest.pywithmake_optional_mock_spec()builder and lightweight synthetic type helpers (make_integer_field,make_bitstring_field,make_sequence). New tests cover every sub-template, wire format variant generation, and structural invariants (box-drawing symmetry, bit position continuity).Other: Added Doxyfile for API documentation generation.
Related Issue
Follows #3 (Consolidate Data Element templates)
Additional Notes
The generated C headers (
BSMcoreData,IntersectionReferenceID,PathPrediction) have been regenerated and verified against the existing C test suite.Type of Change
Checklist
make pre-pushand all checks passgit commit -s