Skip to content

Commit 83d98d0

Browse files
authored
Fix Doxygen wire-format rendering, unify internal constants, and add misalignment tests (#9)
## Pull Request ### Description Systematic quality sweep across generated headers, Jinja2 templates, Python tooling, and C tests to resolve naming inconsistencies, improve Doxygen output, harden the code generator, and expand test coverage. 1. **`ROOT_SIZE` → `J2735_BW_*` constant unification:** Removed per-header `J2735_INTERNAL_ROOT_SIZE_<TYPE>` constants from all 15 non-extensible DE headers. `_RAW_READ` and `_SIZE` macros now reference the canonical `J2735_BW_*` constants from `J2735_internal_constants.h`, eliminating the dual-name problem. 2. **`_Static_assert` for non-extensible DE headers:** Added compile-time assertions to 7 non-extensible DE headers that were missing them, ensuring the root bitstring size matches the `J2735_BW_*` constant at compile time. 3. **Doxygen wire-format tag fixes:** Replaced `@code`/`@endcode` with `@verbatim`/`@endverbatim` for ASCII-art wire diagrams across all 15 DE headers and 4 DF headers (Doxygen was mangling box-drawing characters as C code). Added `@par` section prefixes for proper Doxygen section grouping. 4. **Hex range width normalization:** Updated `bitstring_internal_get_all.j2` template to derive hex format width from `return_type`, fixing 10 headers where `uint8_t` return types incorrectly showed `0x0000–0x00FF` instead of `0x00–0xFF`. 5. **Jinja2 filter ASN.1 hyphen support:** Extended `filter_screaming_snake()` and `filter_snake_case()` to handle hyphenated ASN.1 names (e.g., `Offset-B10` → `OFFSET_B_10`, `Node-LL-24B` → `NODE_LL_24_B`) with 4 new doctests. 6. **`_sum_field_bits()` validation:** Replaced silent `None → 0` coercion with an explicit `ValueError` for unresolved field types, preventing silent corruption of wire-format calculations. 7. **Python code quality:** Replaced `hasattr`/`getattr` chains with `isinstance` checks in `filter_format_range()` and `filter_is_signed()`; added `@lru_cache` to `create_jinja_env()`; added `StrictUndefined` to the Jinja2 environment; replaced magic numbers `1`/`7` with `_UPER_EXTENSION_MARKER_BITS`/`_UPER_NSNNWN_SMALL_BITS` named constants. 8. **Misalignment tests:** Added `test_vehicle_event_flags_misaligned_access()` and 3 UPER misalignment tests (`read_length_determinant`, `read_nsnnwn`, `skip_extensions`) per testing standards requirement. 9. **Template documentation unification:** Unified docstring format across all bitstring Jinja2 templates to use consistent `Context variables:`/`Filters used:`/`Output format:` sections. 10. **License header template for `.j2` files:** Added explicit Jinja2 template example to `license_header.instructions.md` and normalized indentation in 6 bitstring templates. 11. **Test cleanup:** Removed redundant per-line `cppcheck-suppress` comments in 4 DF test files (replaced with block-level `cppcheck-suppress-begin`/`end`); removed unused `<inttypes.h>` and `<stdbool.h>` includes from 5 DF/UPER test files; clarified test header `@brief` descriptions across 15 DE test headers. 12. **`asn1_definition.j2` unification:** Generalized the ASN.1 definition template to handle SEQUENCE, CHOICE, and BIT STRING via `typedef.type_class.name` branching; removed inline ASN.1 blocks from `assemble_df_choice.j2` and `assemble_de_bitstring.j2`. 13. **Minor fixes:** Renamed `_TEMPLATE_NAME` → `_SIZE_CONSTANTS_TEMPLATE_NAME` for consistency with other module-level constants; added `kw_only=True` to 2 wire-format dataclasses; split long `@note` box lines in `assemble_de_bitstring.j2`. ### Related Issue N/A ### Additional Notes N/A ### Type of Change - [x] Bug fix (non-breaking change that fixes an issue) - [ ] New feature (non-breaking change that adds functionality) - [ ] Breaking change (fix or feature that would cause existing functionality to change) - [x] Documentation update - [ ] Performance improvement ### Checklist - [x] My code follows the project's zero-copy coding style - [x] I have read the [CONTRIBUTING.md](../CONTRIBUTING.md) guidelines - [x] I have added/updated documentation as needed - [x] I have added tests that prove my fix/feature works - [x] I have run `make pre-push` and all checks pass - [x] I have signed my commits (DCO) using `git commit -s` --------- Signed-off-by: Yogev Neumann <xqgex@users.noreply.github.com>
1 parent e32835c commit 83d98d0

76 files changed

Lines changed: 2332 additions & 949 deletions

File tree

Some content is hidden

Large Commits have some content hidden by default. Use the searchbox below for content that may be hidden.

.github/instructions/license_header.instructions.md

Lines changed: 26 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -31,6 +31,7 @@ Use Doxygen comments.
3131
* SPDX-License-Identifier: Apache-2.0
3232
* SPDX-FileCopyrightText: 2026 Yogev Neumann
3333
*/
34+
<THE REST OF THE FILE>
3435
```
3536

3637
### Python / YAML / Makefile / Shell / Gitignore
@@ -54,12 +55,34 @@ Use hash comments.
5455
#
5556
# SPDX-License-Identifier: Apache-2.0
5657
# SPDX-FileCopyrightText: 2026 Yogev Neumann
57-
<THE REST OF THE FILE>>
58+
<THE REST OF THE FILE>
5859
```
5960

60-
### Jinja2 Templates
61+
### Jinja2 `.j2` Templates
6162

62-
Templates should also have the license as a comment at the top of each file wrapped with `{#-` and `-#}`.
63+
Use a comment at the top of each file wrapped with `{#-` and `-#}`.
64+
65+
```jinja2
66+
{#-
67+
Copyright 2026 Yogev Neumann
68+
69+
Licensed under the Apache License, Version 2.0 (the "License");
70+
you may not use this file except in compliance with the License.
71+
You may obtain a copy of the License at
72+
73+
http://www.apache.org/licenses/LICENSE-2.0
74+
75+
Unless required by applicable law or agreed to in writing, software
76+
distributed under the License is distributed on an "AS IS" BASIS,
77+
WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
78+
See the License for the specific language governing permissions and
79+
limitations under the License.
80+
81+
SPDX-License-Identifier: Apache-2.0
82+
SPDX-FileCopyrightText: 2026 Yogev Neumann
83+
-#}
84+
<THE REST OF THE FILE>
85+
```
6386

6487
## Exceptions
6588

src/J2735_internal_DE_AllowedManeuvers.h

Lines changed: 12 additions & 15 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
* @author Yogev Neumann
2222
* @brief J2735 AllowedManeuvers Definition and Access Macros.
2323
*
24+
* @verbatim
2425
* AllowedManeuvers ::= BIT STRING {
2526
* maneuverStraightAllowed (0),
2627
* maneuverLeftAllowed (1),
@@ -35,46 +36,42 @@
3536
* caution (10),
3637
* reserved1 (11)
3738
* } (SIZE (12))
39+
* @endverbatim
3840
*
3941
* Fixed BIT STRING with size 12.
4042
*
41-
* Wire Format (12 bits total):
43+
* @par Wire Format (12 bits total):
44+
* @verbatim
4245
* ┌──────────────────────────────────────────────────────────────┐
4346
* │ Bits 0-11 │
4447
* ├──────────────────────────────────────────────────────────────┤
4548
* │ flags[0..11] (12 bits) │
4649
* └──────────────────────────────────────────────────────────────┘
50+
* @endverbatim
4751
*
48-
* Optimization: Single-Read Strategy
49-
* ──────────────────────────────────────────────────────────────────────────────────────────
52+
* @par Optimization: Single-Read Strategy
53+
* @verbatim
5054
* Max wire size = 12 bits ≤ 56-bit READ_BITS limit.
5155
* We read all 12 bits in ONE call, then use bit arithmetic to extract:
5256
* - Flags at positions 0-11
5357
*
5458
* 12-bit read layout (left-justified from bit 0):
5559
* [F0..F11] (12 flag bits, no extension marker)
60+
* @endverbatim
5661
*/
5762
#ifndef J2735_INTERNAL_DE_ALLOWEDMANEUVERS_H
5863
#define J2735_INTERNAL_DE_ALLOWEDMANEUVERS_H
5964

6065
#include "J2735_internal_common.h"
6166

62-
/* ============================================================================================== */
63-
/* Constants */
64-
/* ============================================================================================== */
65-
/**
66-
* @internal
67-
* @brief Root size of AllowedManeuvers in bits.
68-
*/
69-
#define J2735_INTERNAL_ROOT_SIZE_ALLOWED_MANEUVERS 12U
70-
7167
/* ============================================================================================== */
7268
/* INTERNAL: Bit Position Constants */
7369
/* */
7470
/* ASN.1 BIT STRING numbering convention: bit 0 = MSB (leftmost in wire order). */
7571
/* These constants map semantic flag names to their ASN.1 bit positions. */
7672
/* */
77-
/* @note Internal use only. Use the public J2735_ALLOWED_MANEUVERS_GET_*() accessors instead. */
73+
/* @note Internal use only. */
74+
/* Use the public J2735_ALLOWED_MANEUVERS_GET_*() accessors instead. */
7875
/* ============================================================================================== */
7976
#define J2735_INTERNAL_BIT_ALLOWED_MANEUVERS_MANEUVER_STRAIGHT_ALLOWED 0U
8077
#define J2735_INTERNAL_BIT_ALLOWED_MANEUVERS_MANEUVER_LEFT_ALLOWED 1U
@@ -107,7 +104,7 @@
107104
* @note Internal use only. Not part of the public API.
108105
*/
109106
#define J2735_INTERNAL_RAW_READ_ALLOWED_MANEUVERS(buf) \
110-
J2735_READ_BITS((buf), 0U, J2735_INTERNAL_ROOT_SIZE_ALLOWED_MANEUVERS)
107+
J2735_READ_BITS((buf), 0U, J2735_BW_ALLOWED_MANEUVERS)
111108

112109
/* ============================================================================================== */
113110
/* INTERNAL: Extension Bit Check */
@@ -174,7 +171,7 @@
174171
* @param[in] buf Pointer to the start of the AllowedManeuvers UPER encoding (const uint8_t*).
175172
* @return Always 12U.
176173
*/
177-
#define J2735_ALLOWED_MANEUVERS_SIZE(buf) ((void)(buf), J2735_INTERNAL_ROOT_SIZE_ALLOWED_MANEUVERS)
174+
#define J2735_ALLOWED_MANEUVERS_SIZE(buf) ((void)(buf), J2735_BW_ALLOWED_MANEUVERS)
178175

179176
/**
180177
* @brief Get all AllowedManeuvers as a single uint16_t value.

src/J2735_internal_DE_BrakeAppliedStatus.h

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,53 +21,50 @@
2121
* @author Yogev Neumann
2222
* @brief J2735 BrakeAppliedStatus Definition and Access Macros.
2323
*
24+
* @verbatim
2425
* BrakeAppliedStatus ::= BIT STRING {
2526
* unavailable (0),
2627
* leftFront (1),
2728
* leftRear (2),
2829
* rightFront (3),
2930
* rightRear (4)
3031
* } (SIZE (5))
32+
* @endverbatim
3133
*
3234
* Fixed BIT STRING with size 5.
3335
*
34-
* Wire Format (5 bits total):
36+
* @par Wire Format (5 bits total):
37+
* @verbatim
3538
* ┌──────────────────────────────────────────────────────────────┐
3639
* │ Bits 0-4 │
3740
* ├──────────────────────────────────────────────────────────────┤
3841
* │ flags[0..4] (5 bits) │
3942
* └──────────────────────────────────────────────────────────────┘
43+
* @endverbatim
4044
*
41-
* Optimization: Single-Read Strategy
42-
* ──────────────────────────────────────────────────────────────────────────────────────────
45+
* @par Optimization: Single-Read Strategy
46+
* @verbatim
4347
* Max wire size = 5 bits ≤ 56-bit READ_BITS limit.
4448
* We read all 5 bits in ONE call, then use bit arithmetic to extract:
4549
* - Flags at positions 0-4
4650
*
4751
* 5-bit read layout (left-justified from bit 0):
4852
* [F0..F4] (5 flag bits, no extension marker)
53+
* @endverbatim
4954
*/
5055
#ifndef J2735_INTERNAL_DE_BRAKEAPPLIEDSTATUS_H
5156
#define J2735_INTERNAL_DE_BRAKEAPPLIEDSTATUS_H
5257

5358
#include "J2735_internal_common.h"
5459

55-
/* ============================================================================================== */
56-
/* Constants */
57-
/* ============================================================================================== */
58-
/**
59-
* @internal
60-
* @brief Root size of BrakeAppliedStatus in bits.
61-
*/
62-
#define J2735_INTERNAL_ROOT_SIZE_BRAKE_APPLIED_STATUS 5U
63-
6460
/* ============================================================================================== */
6561
/* INTERNAL: Bit Position Constants */
6662
/* */
6763
/* ASN.1 BIT STRING numbering convention: bit 0 = MSB (leftmost in wire order). */
6864
/* These constants map semantic flag names to their ASN.1 bit positions. */
6965
/* */
70-
/* @note Internal use only. Use the public J2735_BRAKE_APPLIED_STATUS_GET_*() accessors instead. */
66+
/* @note Internal use only. */
67+
/* Use the public J2735_BRAKE_APPLIED_STATUS_GET_*() accessors instead. */
7168
/* ============================================================================================== */
7269
#define J2735_INTERNAL_BIT_BRAKE_APPLIED_STATUS_UNAVAILABLE 0U
7370
#define J2735_INTERNAL_BIT_BRAKE_APPLIED_STATUS_LEFT_FRONT 1U
@@ -93,7 +90,7 @@
9390
* @note Internal use only. Not part of the public API.
9491
*/
9592
#define J2735_INTERNAL_RAW_READ_BRAKE_APPLIED_STATUS(buf) \
96-
J2735_READ_BITS((buf), 0U, J2735_INTERNAL_ROOT_SIZE_BRAKE_APPLIED_STATUS)
93+
J2735_READ_BITS((buf), 0U, J2735_BW_BRAKE_APPLIED_STATUS)
9794

9895
/* ============================================================================================== */
9996
/* INTERNAL: Extension Bit Check */
@@ -119,7 +116,7 @@
119116
*
120117
* @param[in] raw5 Value previously returned by J2735_INTERNAL_RAW_READ_BRAKE_APPLIED_STATUS().
121118
* @return Right-aligned flag bits as uint8_t:
122-
* - 5 significant bits (0x0000-0x001F)
119+
* - 5 significant bits (0x00-0x1F)
123120
* @note Internal use only. Use J2735_BRAKE_APPLIED_STATUS_GET() for public API.
124121
*/
125122
#define J2735_INTERNAL_GET_ALL_BRAKE_APPLIED_STATUS(raw5) ((uint8_t)((raw5) & 0x1FU))
@@ -160,8 +157,7 @@
160157
* @param[in] buf Pointer to the start of the BrakeAppliedStatus UPER encoding (const uint8_t*).
161158
* @return Always 5U.
162159
*/
163-
#define J2735_BRAKE_APPLIED_STATUS_SIZE(buf) \
164-
((void)(buf), J2735_INTERNAL_ROOT_SIZE_BRAKE_APPLIED_STATUS)
160+
#define J2735_BRAKE_APPLIED_STATUS_SIZE(buf) ((void)(buf), J2735_BW_BRAKE_APPLIED_STATUS)
165161

166162
/**
167163
* @brief Get all BrakeAppliedStatus as a single uint8_t value.

src/J2735_internal_DE_ExteriorLights.h

Lines changed: 13 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
* @author Yogev Neumann
2222
* @brief J2735 ExteriorLights Definition and Access Macros.
2323
*
24+
* @verbatim
2425
* ExteriorLights ::= BIT STRING {
2526
* lowBeamHeadlightsOn (0),
2627
* highBeamHeadlightsOn (1),
@@ -32,25 +33,30 @@
3233
* fogLightOn (7),
3334
* parkingLightsOn (8)
3435
* } (SIZE (9, ...))
36+
* @endverbatim
3537
*
3638
* Extensible BIT STRING with root size 9 and known extension size 9.
3739
*
38-
* Wire Format (non-extended, 10 bits total):
40+
* @par Wire Format (non-extended, 10 bits total):
41+
* @verbatim
3942
* ┌───────┬──────────────────────────────────────────────────────┐
4043
* │ Bit 0 │ Bits 1-9 │
4144
* ├───────┼──────────────────────────────────────────────────────┤
4245
* │ Ext=0 │ flags[0..8] (9 bits) │
4346
* └───────┴──────────────────────────────────────────────────────┘
47+
* @endverbatim
4448
*
45-
* Wire Format (extended, 17 bits total):
49+
* @par Wire Format (extended, 17 bits total):
50+
* @verbatim
4651
* ┌───────┬────────────────────┬─────────────────────────────────┐
4752
* │ Bit 0 │ Bits 1-7 │ Bits 8-16 │
4853
* ├───────┼────────────────────┼─────────────────────────────────┤
4954
* │ Ext=1 │ nsnnwn=9 (7 bits) │ flags[0..8] (9 bits) │
5055
* └───────┴────────────────────┴─────────────────────────────────┘
56+
* @endverbatim
5157
*
52-
* Optimization: Single-Read Strategy
53-
* ──────────────────────────────────────────────────────────────────────────────────────────
58+
* @par Optimization: Single-Read Strategy
59+
* @verbatim
5460
* Max wire size = 17 bits ≤ 56-bit READ_BITS limit.
5561
* We read all 17 bits in ONE call, then use bit arithmetic to extract:
5662
* - Extension bit at position 16 (MSB of 17-bit value)
@@ -61,6 +67,7 @@
6167
* bit16 15..7 6..0
6268
* Extended: [Ext=1][nsnnwn:7][F0..F8]
6369
* bit16 15..9 8..0
70+
* @endverbatim
6471
*/
6572
#ifndef J2735_INTERNAL_DE_EXTERIORLIGHTS_H
6673
#define J2735_INTERNAL_DE_EXTERIORLIGHTS_H
@@ -102,7 +109,8 @@ _Static_assert(J2735_INTERNAL_MAX_WIRE_BITS_EXTERIOR_LIGHTS ==
102109
/* ASN.1 BIT STRING numbering convention: bit 0 = MSB (leftmost in wire order). */
103110
/* These constants map semantic flag names to their ASN.1 bit positions. */
104111
/* */
105-
/* @note Internal use only. Use the public J2735_EXTERIOR_LIGHTS_GET_*() accessors instead. */
112+
/* @note Internal use only. */
113+
/* Use the public J2735_EXTERIOR_LIGHTS_GET_*() accessors instead. */
106114
/* ============================================================================================== */
107115
#define J2735_INTERNAL_BIT_EXTERIOR_LIGHTS_LOW_BEAM_HEADLIGHTS_ON 0U
108116
#define J2735_INTERNAL_BIT_EXTERIOR_LIGHTS_HIGH_BEAM_HEADLIGHTS_ON 1U

src/J2735_internal_DE_GNSSstatus.h

Lines changed: 13 additions & 17 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121
* @author Yogev Neumann
2222
* @brief J2735 GNSSstatus Definition and Access Macros.
2323
*
24+
* @verbatim
2425
* GNSSstatus ::= BIT STRING {
2526
* unavailable (0),
2627
* isHealthy (1),
@@ -31,46 +32,42 @@
3132
* localCorrectionsPresent (6),
3233
* networkCorrectionsPresent (7)
3334
* } (SIZE (8))
35+
* @endverbatim
3436
*
3537
* Fixed BIT STRING with size 8.
3638
*
37-
* Wire Format (8 bits total):
39+
* @par Wire Format (8 bits total):
40+
* @verbatim
3841
* ┌──────────────────────────────────────────────────────────────┐
3942
* │ Bits 0-7 │
4043
* ├──────────────────────────────────────────────────────────────┤
4144
* │ flags[0..7] (8 bits) │
4245
* └──────────────────────────────────────────────────────────────┘
46+
* @endverbatim
4347
*
44-
* Optimization: Single-Read Strategy
45-
* ──────────────────────────────────────────────────────────────────────────────────────────
48+
* @par Optimization: Single-Read Strategy
49+
* @verbatim
4650
* Max wire size = 8 bits ≤ 56-bit READ_BITS limit.
4751
* We read all 8 bits in ONE call, then use bit arithmetic to extract:
4852
* - Flags at positions 0-7
4953
*
5054
* 8-bit read layout (left-justified from bit 0):
5155
* [F0..F7] (8 flag bits, no extension marker)
56+
* @endverbatim
5257
*/
5358
#ifndef J2735_INTERNAL_DE_GNSSSTATUS_H
5459
#define J2735_INTERNAL_DE_GNSSSTATUS_H
5560

5661
#include "J2735_internal_common.h"
5762

58-
/* ============================================================================================== */
59-
/* Constants */
60-
/* ============================================================================================== */
61-
/**
62-
* @internal
63-
* @brief Root size of GNSSstatus in bits.
64-
*/
65-
#define J2735_INTERNAL_ROOT_SIZE_GNSS_STATUS 8U
66-
6763
/* ============================================================================================== */
6864
/* INTERNAL: Bit Position Constants */
6965
/* */
7066
/* ASN.1 BIT STRING numbering convention: bit 0 = MSB (leftmost in wire order). */
7167
/* These constants map semantic flag names to their ASN.1 bit positions. */
7268
/* */
73-
/* @note Internal use only. Use the public J2735_GNSS_STATUS_GET_*() accessors instead. */
69+
/* @note Internal use only. */
70+
/* Use the public J2735_GNSS_STATUS_GET_*() accessors instead. */
7471
/* ============================================================================================== */
7572
#define J2735_INTERNAL_BIT_GNSS_STATUS_UNAVAILABLE 0U
7673
#define J2735_INTERNAL_BIT_GNSS_STATUS_IS_HEALTHY 1U
@@ -98,8 +95,7 @@
9895
* @return 8-bit value as uint64_t with wire bits left-justified.
9996
* @note Internal use only. Not part of the public API.
10097
*/
101-
#define J2735_INTERNAL_RAW_READ_GNSS_STATUS(buf) \
102-
J2735_READ_BITS((buf), 0U, J2735_INTERNAL_ROOT_SIZE_GNSS_STATUS)
98+
#define J2735_INTERNAL_RAW_READ_GNSS_STATUS(buf) J2735_READ_BITS((buf), 0U, J2735_BW_GNSS_STATUS)
10399

104100
/* ============================================================================================== */
105101
/* INTERNAL: Extension Bit Check */
@@ -125,7 +121,7 @@
125121
*
126122
* @param[in] raw8 Value previously returned by J2735_INTERNAL_RAW_READ_GNSS_STATUS().
127123
* @return Right-aligned flag bits as uint8_t:
128-
* - 8 significant bits (0x0000-0x00FF)
124+
* - 8 significant bits (0x00-0xFF)
129125
* @note Internal use only. Use J2735_GNSS_STATUS_GET() for public API.
130126
*/
131127
#define J2735_INTERNAL_GET_ALL_GNSS_STATUS(raw8) ((uint8_t)((raw8) & 0xFFU))
@@ -166,7 +162,7 @@
166162
* @param[in] buf Pointer to the start of the GNSSstatus UPER encoding (const uint8_t*).
167163
* @return Always 8U.
168164
*/
169-
#define J2735_GNSS_STATUS_SIZE(buf) ((void)(buf), J2735_INTERNAL_ROOT_SIZE_GNSS_STATUS)
165+
#define J2735_GNSS_STATUS_SIZE(buf) ((void)(buf), J2735_BW_GNSS_STATUS)
170166

171167
/**
172168
* @brief Get all GNSSstatus as a single uint8_t value.

0 commit comments

Comments
 (0)