From 26a056097ec4b6a5333a361576b879fb352b4cf9 Mon Sep 17 00:00:00 2001 From: David Tarazi Date: Fri, 31 Jul 2026 15:32:28 -0700 Subject: [PATCH 1/4] adding support for generating enums in cpp --- README.md | 43 +++++++++ dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py | 99 +++++++++++++++++++- dbc_gen_cpp/dbc_gen_cpp/templates/can.hpp.j2 | 20 ++++ test_dbc_gen_cpp/test/FakeVehicle.dbc | 13 +++ test_dbc_gen_cpp/test/test_dbc_cpp.cpp | 56 +++++++++++ 5 files changed, 230 insertions(+), 1 deletion(-) diff --git a/README.md b/README.md index 71c02fa..b8e4698 100644 --- a/README.md +++ b/README.md @@ -39,6 +39,49 @@ my_can_socket->send(frame); ``` +### Signal Value Maps (Enums) + +DBC value tables (`VAL_` lines, and the global `VAL_TABLE_`) map raw signal values +to human-readable states. For every signal that has one, an `enum class` is generated +alongside the message structs, in the library's namespace. + +Enums are named `_` (top-level and prefixed, so the same +signal name in different messages never collides). A matching `to_string()` overload +returns the original DBC label text. + +Signal fields on the message struct stay as raw physical values (`double`) — the enum +is **additive**, so you opt in by casting when you want the named value: + +```c++ +#include "my_can_library_name/my_can_library_name.hpp" + +// Given a DBC message TransmissionStatus with a signal `gear` whose VAL_ table is +// VAL_ gear 0 "Neutral" 1 "Drive" 2 "Reverse" ... ; +my_can_library_name::TransmissionStatus msg{frame}; + +auto gear = static_cast( + static_cast(msg.gear)); + +if (gear == my_can_library_name::TransmissionStatus_gear::REVERSE) { + // ... +} + +// to_string() returns the original label from the DBC, handy for logging. +printf("gear = %s\n", my_can_library_name::to_string(gear)); // e.g. "Reverse" +``` + +Enumerator names come from the DBC label text, uppercased with non-alphanumeric +characters turned into underscores (matching cantools' C `..._CHOICE` macros). A few +labels are adjusted so they remain valid, unique C++ identifiers: + +- duplicate labels are de-duplicated by appending their raw value + (`"Reserved"` at 3 and 4 → `RESERVED_3`, `RESERVED_4`); +- labels starting with a digit get a leading underscore (`"4wd mode"` → `_4WD_MODE`); +- doubled and trailing underscores are collapsed/stripped + (`"Truck system with fault, stop!"` → `TRUCK_SYSTEM_WITH_FAULT_STOP`). + +`to_string()` always returns the unmodified label, regardless of these adjustments. + ### CAN Handler - Receive/Subscribe to CAN Messages A helper class `dbc_gen_cpp::CANHandler` is provided. diff --git a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py index 66c0425..5d54876 100644 --- a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py +++ b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py @@ -2,6 +2,7 @@ # SPDX-License-Identifier: Apache-2.0 import argparse import importlib.resources +import re from pathlib import Path from cantools import database @@ -37,6 +38,94 @@ def parse_j1939_id(frame_id): return result +def _sanitize_identifier(name, fallback='X'): + """Turn arbitrary DBC text into a clean, valid C++ identifier. + + Signal names can carry spaces and punctuation (e.g. cantools surfaces the DBC + ``SystemSignalLongSymbol`` attribute as the signal name: "Accelerator Pedal 1 + Low Idle Switch"), and choice labels are free-form prose. Replace every run of + non-alphanumeric characters with a single underscore, strip leading/trailing + underscores, fall back when nothing is left, and prefix an underscore when the + result would otherwise start with a digit ("1000 ms" -> "_1000_ms"). + """ + name = re.sub(r'[^0-9a-zA-Z]+', '_', name).strip('_') + if not name: + name = fallback + if name[0].isdigit(): + name = '_' + name + return name + + +def _escape_c_string(text): + """Escape a DBC label so it is safe inside a C string literal.""" + return text.replace('\\', '\\\\').replace('"', '\\"') + + +def build_signal_enums(message, cg_message, used_enum_names): + """Build C++ enum descriptors for every signal in a message that has VAL_ choices. + + Each enum is named _ (top-level, prefixed to avoid + collisions across messages) with enumerators derived from the DBC value table. + ``used_enum_names`` is a set shared across the whole database used to keep enum + type names globally unique. + """ + enums = [] + for cg_signal in cg_message.cg_signals: + choices = cg_signal.signal.choices + if not choices: + continue + + # Message/signal names can contain characters that are not valid in an + # identifier (spaces from SystemSignalLongSymbol, etc.), so sanitize both + # parts. Keep the type name globally unique as a final safeguard. + enum_name = _sanitize_identifier( + f'{_sanitize_identifier(message.name)}_{_sanitize_identifier(cg_signal.signal.name)}', fallback='Enum' + ) + while enum_name in used_enum_names: + enum_name += '_' + used_enum_names.add(enum_name) + + # unique_choices gives {raw_int: UNIQUE_UPPER_IDENT}, already de-duplicated + # and matching the identifiers cantools emits for its C ..._CHOICE #defines. + unique = cg_signal.unique_choices + raws = sorted(unique) + + # Sanitize each name into a valid enumerator, re-deduplicating in case two + # names collapse to the same identifier (append the raw value, then '_'). + idents = {} + used = set() + for raw in raws: + ident = _sanitize_identifier(unique[raw], fallback='VALUE') + if ident in used: + ident = f'{ident}_{raw}' if raw >= 0 else f'{ident}_n{-raw}' + while ident in used: + ident += '_' + used.add(ident) + idents[raw] = ident + + # Choices on a scaled/float signal are still integer-raw; guard the type. + if cg_signal.signal.conversion.is_float: + underlying_type = f'int{cg_signal.type_length}_t' + else: + underlying_type = cg_signal.type_name + + enums.append({ + 'name': enum_name, + 'signal_name': cg_signal.signal.name, + 'message_name': message.name, + 'underlying_type': underlying_type, + 'enumerators': [ + { + 'ident': idents[raw], + 'value': raw, + 'label': _escape_c_string(str(choices[raw])), + } + for raw in raws + ], + }) + return enums + + def generate_cpp_source(args): dbase = database.load_file(args.infile) database_name: str = args.database_name or camel_to_snake_case(args.infile.stem) @@ -68,8 +157,11 @@ def generate_cpp_source(args): hpp_template = jinja_env.from_string(hpp_template_src) message_types = [] + signal_enums = [] + used_enum_names = set() for message in dbase.messages: cg_message = CodeGenMessage(message) + signal_enums.extend(build_signal_enums(message, cg_message, used_enum_names)) msg_dict = { 'name': message.name, @@ -89,7 +181,12 @@ def generate_cpp_source(args): if message.protocol == 'j1939': msg_dict['j1939'] = parse_j1939_id(message.frame_id) message_types.append(msg_dict) - hpp_src = hpp_template.render(library_name=database_name, messages=message_types, c_header=filename_h) + hpp_src = hpp_template.render( + library_name=database_name, + messages=message_types, + enums=signal_enums, + c_header=filename_h, + ) with (outdir / filename_hpp).open('w') as f: f.write(hpp_src) diff --git a/dbc_gen_cpp/dbc_gen_cpp/templates/can.hpp.j2 b/dbc_gen_cpp/dbc_gen_cpp/templates/can.hpp.j2 index a196261..ae6e12b 100644 --- a/dbc_gen_cpp/dbc_gen_cpp/templates/can.hpp.j2 +++ b/dbc_gen_cpp/dbc_gen_cpp/templates/can.hpp.j2 @@ -123,5 +123,25 @@ struct {{ message.name }} } }; +{% endfor %} +{% for enum in enums %} +// Value map for {{ enum.message_name }}.{{ enum.signal_name }} +enum class {{ enum.name }} : {{ enum.underlying_type }} +{ + {% for v in enum.enumerators %} + {{ v.ident }} = {{ v.value }}, + {% endfor %} +}; + +inline const char * to_string({{ enum.name }} value) +{ + switch (value) { + {% for v in enum.enumerators %} + case {{ enum.name }}::{{ v.ident }}: return "{{ v.label }}"; + {% endfor %} + default: return "UNKNOWN"; + } +} + {% endfor %} } // namespace {{ library_name }} diff --git a/test_dbc_gen_cpp/test/FakeVehicle.dbc b/test_dbc_gen_cpp/test/FakeVehicle.dbc index fcf16ee..5e423c6 100644 --- a/test_dbc_gen_cpp/test/FakeVehicle.dbc +++ b/test_dbc_gen_cpp/test/FakeVehicle.dbc @@ -61,6 +61,12 @@ BO_ 2566859520 EngineAuxStatus: 8 Vehicle SG_ aux_coolant_temp : 0|8@1+ (1,-40) [-40|210] "degC" Computer SG_ aux_oil_pressure : 8|8@1+ (4,0) [0|1000] "kPa" Computer +BO_ 102 GearStatus: 1 Vehicle + SG_ gear : 0|8@1+ (1,0) [0|255] "" Computer + +BO_ 103 DriveModeStatus: 1 Vehicle + SG_ mode : 0|8@1+ (1,0) [0|255] "" Computer + BA_DEF_ BO_ "VFrameFormat" ENUM "StandardCAN","ExtendedCAN","reserved","J1939PG"; BA_DEF_DEF_ "VFrameFormat" ""; BA_ "VFrameFormat" BO_ 2566859520 3; @@ -71,3 +77,10 @@ SIG_VALTYPE_ 200 drive_angle : 1; CM_ BO_ 2147483848 "CAN ID 200 (0xC8) with extended frame flag (0x80000000) set"; CM_ BO_ 2566859520 "Add one J1939 Message to test having a single J1939 message in a non-J1939 DBC"; + +VAL_ 102 gear 0 "Neutral" 1 "Drive" 2 "Reverse" 3 "Reserved" 4 "Reserved" 5 "4wd mode" 6 "Park!" ; +VAL_ 103 mode 0 "Off" 1 "On" ; + +BA_DEF_ SG_ "SystemSignalLongSymbol" STRING ; +BA_DEF_DEF_ "SystemSignalLongSymbol" ""; +BA_ "SystemSignalLongSymbol" SG_ 103 mode "Drive Mode Select"; diff --git a/test_dbc_gen_cpp/test/test_dbc_cpp.cpp b/test_dbc_gen_cpp/test/test_dbc_cpp.cpp index ad1a1bb..dc649bf 100644 --- a/test_dbc_gen_cpp/test/test_dbc_cpp.cpp +++ b/test_dbc_gen_cpp/test/test_dbc_cpp.cpp @@ -2,6 +2,7 @@ // SPDX-License-Identifier: Apache-2.0 #include +#include #include "dbc_gen_cpp/can_handler.hpp" #include "fake_j1939_can/fake_j1939_can.hpp" @@ -954,3 +955,58 @@ TEST_CASE("CANHandler routes messages with 0 length payload") REQUIRE(callback_invoked == true); } } + +// ============================================================================ +// Signal value-map (enum) generation tests +// ============================================================================ +// GearStatus.gear in FakeVehicle.dbc carries a VAL_ table exercising every +// identifier-sanitization path: normal names, a duplicated label ("Reserved"), +// a leading-digit label ("4wd mode") and a punctuation label ("Park!"). + +TEST_CASE("Generated enum - enumerator values match the DBC value table") +{ + using fake_vehicle_can::GearStatus_gear; + + REQUIRE(static_cast(GearStatus_gear::NEUTRAL) == 0); + REQUIRE(static_cast(GearStatus_gear::DRIVE) == 1); + REQUIRE(static_cast(GearStatus_gear::REVERSE) == 2); + // Duplicated label "Reserved" is de-duplicated by appending the raw value. + REQUIRE(static_cast(GearStatus_gear::RESERVED_3) == 3); + REQUIRE(static_cast(GearStatus_gear::RESERVED_4) == 4); + // Leading-digit label gets an underscore prefix to stay a valid identifier. + REQUIRE(static_cast(GearStatus_gear::_4WD_MODE) == 5); + // Trailing punctuation is stripped. + REQUIRE(static_cast(GearStatus_gear::PARK) == 6); +} + +TEST_CASE("Generated enum - underlying type follows the signal type") +{ + // gear is an 8-bit unsigned signal. + STATIC_REQUIRE(std::is_same_v, uint8_t>); +} + +TEST_CASE("Generated enum - to_string returns the original DBC label") +{ + using fake_vehicle_can::GearStatus_gear; + + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::NEUTRAL), "Neutral") == 0); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::REVERSE), "Reverse") == 0); + // Original label text is preserved verbatim, including punctuation and casing. + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::_4WD_MODE), "4wd mode") == 0); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::PARK), "Park!") == 0); + // Both de-duplicated enumerators keep the same source label. + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::RESERVED_3), "Reserved") == 0); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(GearStatus_gear::RESERVED_4), "Reserved") == 0); +} + +TEST_CASE("Generated enum - signal name with spaces yields a valid enum type") +{ + // DriveModeStatus.mode carries a SystemSignalLongSymbol ("Drive Mode Select"), + // which cantools surfaces as the signal name. The spaces must be sanitized out + // of the generated enum type name. + using fake_vehicle_can::DriveModeStatus_Drive_Mode_Select; + + REQUIRE(static_cast(DriveModeStatus_Drive_Mode_Select::OFF) == 0); + REQUIRE(static_cast(DriveModeStatus_Drive_Mode_Select::ON) == 1); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(DriveModeStatus_Drive_Mode_Select::ON), "On") == 0); +} From 5a31888fa352e6dcda4c677d1537df1f6d00d6fc Mon Sep 17 00:00:00 2001 From: David Tarazi Date: Fri, 31 Jul 2026 16:10:32 -0700 Subject: [PATCH 2/4] fix failure cases, add more tests --- dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py | 87 +++++++++--------- test_dbc_gen_cpp/CMakeLists.txt | 8 ++ test_dbc_gen_cpp/package.xml | 2 + test_dbc_gen_cpp/test/test_enum_generation.py | 88 +++++++++++++++++++ 4 files changed, 138 insertions(+), 47 deletions(-) create mode 100644 test_dbc_gen_cpp/test/test_enum_generation.py diff --git a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py index 5d54876..d82bd9f 100644 --- a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py +++ b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py @@ -39,15 +39,7 @@ def parse_j1939_id(frame_id): def _sanitize_identifier(name, fallback='X'): - """Turn arbitrary DBC text into a clean, valid C++ identifier. - - Signal names can carry spaces and punctuation (e.g. cantools surfaces the DBC - ``SystemSignalLongSymbol`` attribute as the signal name: "Accelerator Pedal 1 - Low Idle Switch"), and choice labels are free-form prose. Replace every run of - non-alphanumeric characters with a single underscore, strip leading/trailing - underscores, fall back when nothing is left, and prefix an underscore when the - result would otherwise start with a digit ("1000 ms" -> "_1000_ms"). - """ + """Turn arbitrary DBC text (names, labels) into a valid C++ identifier.""" name = re.sub(r'[^0-9a-zA-Z]+', '_', name).strip('_') if not name: name = fallback @@ -62,48 +54,49 @@ def _escape_c_string(text): def build_signal_enums(message, cg_message, used_enum_names): - """Build C++ enum descriptors for every signal in a message that has VAL_ choices. - - Each enum is named _ (top-level, prefixed to avoid - collisions across messages) with enumerators derived from the DBC value table. - ``used_enum_names`` is a set shared across the whole database used to keep enum - type names globally unique. - """ + """Build an _ enum descriptor per signal with VAL_ choices.""" enums = [] for cg_signal in cg_message.cg_signals: choices = cg_signal.signal.choices if not choices: continue - # Message/signal names can contain characters that are not valid in an - # identifier (spaces from SystemSignalLongSymbol, etc.), so sanitize both - # parts. Keep the type name globally unique as a final safeguard. + descriptor = f'{message.name}.{cg_signal.signal.name}' enum_name = _sanitize_identifier( f'{_sanitize_identifier(message.name)}_{_sanitize_identifier(cg_signal.signal.name)}', fallback='Enum' ) - while enum_name in used_enum_names: - enum_name += '_' - used_enum_names.add(enum_name) - - # unique_choices gives {raw_int: UNIQUE_UPPER_IDENT}, already de-duplicated - # and matching the identifiers cantools emits for its C ..._CHOICE #defines. - unique = cg_signal.unique_choices - raws = sorted(unique) - - # Sanitize each name into a valid enumerator, re-deduplicating in case two - # names collapse to the same identifier (append the raw value, then '_'). - idents = {} - used = set() - for raw in raws: - ident = _sanitize_identifier(unique[raw], fallback='VALUE') - if ident in used: - ident = f'{ident}_{raw}' if raw >= 0 else f'{ident}_n{-raw}' - while ident in used: - ident += '_' - used.add(ident) - idents[raw] = ident - - # Choices on a scaled/float signal are still integer-raw; guard the type. + # Fail loudly for identical names + if enum_name in used_enum_names: + raise ValueError( + f"Generated enum name '{enum_name}' collides between " + f"'{used_enum_names[enum_name]}' and '{descriptor}'. Rename one of the " + f'DBC signals (or its SystemSignalLongSymbol) so the sanitized ' + f'_ names are unique.' + ) + used_enum_names[enum_name] = descriptor + + # De-duplicated UPPER_SNAKE names, matching cantools' C ..._CHOICE #defines. + choice_name_by_value = cg_signal.unique_choices + sorted_values = sorted(choice_name_by_value) + + enumerator_ident_by_value = {} + used_idents = set() + for raw_value in sorted_values: + enumerator_ident = _sanitize_identifier(choice_name_by_value[raw_value], fallback='VALUE') + if enumerator_ident in used_idents: + # Repeated label (e.g. two "Reserved"): suffix the value to disambiguate. + enumerator_ident = ( + f'{enumerator_ident}_{raw_value}' if raw_value >= 0 else f'{enumerator_ident}_n{-raw_value}' + ) + if enumerator_ident in used_idents: + raise ValueError( + f"Enumerator '{enumerator_ident}' collides in enum '{enum_name}'. Two DBC " + f'choices sanitize to the same C++ identifier; rename one.' + ) + used_idents.add(enumerator_ident) + enumerator_ident_by_value[raw_value] = enumerator_ident + + # Choice values are integer-raw even on scaled/float signals. if cg_signal.signal.conversion.is_float: underlying_type = f'int{cg_signal.type_length}_t' else: @@ -116,11 +109,11 @@ def build_signal_enums(message, cg_message, used_enum_names): 'underlying_type': underlying_type, 'enumerators': [ { - 'ident': idents[raw], - 'value': raw, - 'label': _escape_c_string(str(choices[raw])), + 'ident': enumerator_ident_by_value[raw_value], + 'value': raw_value, + 'label': _escape_c_string(str(choices[raw_value])), } - for raw in raws + for raw_value in sorted_values ], }) return enums @@ -158,7 +151,7 @@ def generate_cpp_source(args): message_types = [] signal_enums = [] - used_enum_names = set() + used_enum_names = {} for message in dbase.messages: cg_message = CodeGenMessage(message) signal_enums.extend(build_signal_enums(message, cg_message, used_enum_names)) diff --git a/test_dbc_gen_cpp/CMakeLists.txt b/test_dbc_gen_cpp/CMakeLists.txt index 5426f04..a087e50 100644 --- a/test_dbc_gen_cpp/CMakeLists.txt +++ b/test_dbc_gen_cpp/CMakeLists.txt @@ -18,8 +18,16 @@ if(BUILD_TESTING) ament_auto_find_test_dependencies() find_package(ament_cmake_test REQUIRED) + find_package(ament_cmake_pytest REQUIRED) find_package(Catch2 REQUIRED) + # Python-level tests for the generator (e.g. the fail-on-ambiguous-name path, + # which cannot be exercised through a generated header). + ament_add_pytest_test(test_enum_generation + test/test_enum_generation.py + WORKING_DIRECTORY ${CMAKE_CURRENT_SOURCE_DIR} + ) + generate_dbc_cpp(fake_vehicle_can DBC ${CMAKE_CURRENT_SOURCE_DIR}/test/FakeVehicle.dbc ) diff --git a/test_dbc_gen_cpp/package.xml b/test_dbc_gen_cpp/package.xml index 548fcd9..fa16f94 100644 --- a/test_dbc_gen_cpp/package.xml +++ b/test_dbc_gen_cpp/package.xml @@ -12,6 +12,8 @@ catch2 dbc_gen_cpp + ament_cmake_pytest + python3-pytest ament_cmake diff --git a/test_dbc_gen_cpp/test/test_enum_generation.py b/test_dbc_gen_cpp/test/test_enum_generation.py new file mode 100644 index 0000000..f0d3f84 --- /dev/null +++ b/test_dbc_gen_cpp/test/test_enum_generation.py @@ -0,0 +1,88 @@ +# SPDX-FileCopyrightText: 2026 Polymath Robotics, Inc. +# SPDX-License-Identifier: Apache-2.0 +"""Unit tests for the enum/value-map generation in dbc_gen_cpp. + +These exercise the Python generation layer directly (the compiled Catch2 tests +cover the emitted C++). In particular they lock in the "fail loudly on an +ambiguous name" behaviour, which cannot be tested through a generated header +because a colliding DBC would simply fail to compile. +""" + +import textwrap + +import pytest +from cantools import database +from cantools.database.can.c_source import CodeGenMessage + +from dbc_gen_cpp.generate_cpp import build_signal_enums + + +def _build_enums(dbc_text, tmp_path): + """Load an inline DBC and run it through build_signal_enums.""" + path = tmp_path / 'in.dbc' + path.write_text(textwrap.dedent(dbc_text)) + dbase = database.load_file(str(path)) + used_enum_names = {} + enums = [] + for message in dbase.messages: + enums.extend(build_signal_enums(message, CodeGenMessage(message), used_enum_names)) + return enums + + +# Message "A_B" signal "C" and message "A" signal "B_C" both sanitize to A_B_C. +COLLIDING_DBC = """\ + VERSION "" + NS_ : + BS_: + BU_: N + BO_ 100 A_B: 1 N + SG_ C : 0|8@1+ (1,0) [0|255] "" N + BO_ 101 A: 1 N + SG_ B_C : 0|8@1+ (1,0) [0|255] "" N + VAL_ 100 C 0 "x" 1 "y" ; + VAL_ 101 B_C 0 "p" 1 "q" ; +""" + +# gear exercises every enumerator path: normal, duplicate label ("Reserved"), +# leading-digit label ("4wd mode") and a trailing-punctuation label ("Park!"). +GEAR_DBC = """\ + VERSION "" + NS_ : + BS_: + BU_: N + BO_ 102 GearStatus: 1 N + SG_ gear : 0|8@1+ (1,0) [0|255] "" N + VAL_ 102 gear 0 "Neutral" 3 "Reserved" 4 "Reserved" 5 "4wd mode" 6 "Park!" ; +""" + + +def test_colliding_enum_names_raise(tmp_path): + """Two signals whose sanitized _ names match must fail loudly.""" + with pytest.raises(ValueError, match='collides') as excinfo: + _build_enums(COLLIDING_DBC, tmp_path) + # The error names both offending signals so the ambiguity is decipherable. + message = str(excinfo.value) + assert 'A_B.C' in message + assert 'A.B_C' in message + + +def test_value_map_generates_enum(tmp_path): + """A normal value table produces one enum with decipherable enumerators.""" + enums = _build_enums(GEAR_DBC, tmp_path) + assert len(enums) == 1 + + enum = enums[0] + assert enum['name'] == 'GearStatus_gear' + assert enum['underlying_type'] == 'uint8_t' + + by_value = {v['value']: v for v in enum['enumerators']} + assert by_value[0]['ident'] == 'NEUTRAL' + # Duplicated "Reserved" label is de-duplicated by raw value (still decipherable). + assert by_value[3]['ident'] == 'RESERVED_3' + assert by_value[4]['ident'] == 'RESERVED_4' + # Leading digit is prefixed to stay a valid identifier. + assert by_value[5]['ident'] == '_4WD_MODE' + # Trailing punctuation is stripped, but to_string keeps the original label. + assert by_value[6]['ident'] == 'PARK' + assert by_value[6]['label'] == 'Park!' + assert by_value[3]['label'] == 'Reserved' From 6d9c11bf047852496678b8a4bcd24a825177f15f Mon Sep 17 00:00:00 2001 From: David Tarazi Date: Fri, 31 Jul 2026 16:14:52 -0700 Subject: [PATCH 3/4] add readme clarification --- README.md | 5 +++-- 1 file changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index b8e4698..c6b40af 100644 --- a/README.md +++ b/README.md @@ -74,8 +74,9 @@ Enumerator names come from the DBC label text, uppercased with non-alphanumeric characters turned into underscores (matching cantools' C `..._CHOICE` macros). A few labels are adjusted so they remain valid, unique C++ identifiers: -- duplicate labels are de-duplicated by appending their raw value - (`"Reserved"` at 3 and 4 → `RESERVED_3`, `RESERVED_4`); +- duplicate labels must become distinct enumerators (C++ forbids repeating a name), + so the raw value is appended: `"Reserved"` at 3 and 4 → `RESERVED_3`, `RESERVED_4` + (cantools does this for its C `..._CHOICE` macros; we keep the same names); - labels starting with a digit get a leading underscore (`"4wd mode"` → `_4WD_MODE`); - doubled and trailing underscores are collapsed/stripped (`"Truck system with fault, stop!"` → `TRUCK_SYSTEM_WITH_FAULT_STOP`). From 1b0fec163b44352469018a3d15c9d43a178f3748 Mon Sep 17 00:00:00 2001 From: David Tarazi Date: Fri, 31 Jul 2026 16:26:07 -0700 Subject: [PATCH 4/4] address float special case and add regression tests --- dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py | 25 +++++++++++----- test_dbc_gen_cpp/test/FakeVehicle.dbc | 9 ++++++ test_dbc_gen_cpp/test/test_dbc_cpp.cpp | 30 +++++++++++++++++-- test_dbc_gen_cpp/test/test_enum_generation.py | 23 +++++++++++++- 4 files changed, 77 insertions(+), 10 deletions(-) diff --git a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py index d82bd9f..8afe165 100644 --- a/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py +++ b/dbc_gen_cpp/dbc_gen_cpp/generate_cpp.py @@ -53,6 +53,23 @@ def _escape_c_string(text): return text.replace('\\', '\\\\').replace('"', '\\"') +def _enum_underlying_type(sorted_values): + """Smallest stdint type spanning the choice values. + + Driven by the values, not the signal's declared type, so float-typed signals + still get an integral base and negatives/high-bit flags always fit. + """ + low, high = sorted_values[0], sorted_values[-1] + signed = low < 0 + for bits in (8, 16, 32, 64): + if signed: + if -(1 << (bits - 1)) <= low and high <= (1 << (bits - 1)) - 1: + return f'int{bits}_t' + elif high <= (1 << bits) - 1: + return f'uint{bits}_t' + return 'int64_t' if signed else 'uint64_t' + + def build_signal_enums(message, cg_message, used_enum_names): """Build an _ enum descriptor per signal with VAL_ choices.""" enums = [] @@ -96,17 +113,11 @@ def build_signal_enums(message, cg_message, used_enum_names): used_idents.add(enumerator_ident) enumerator_ident_by_value[raw_value] = enumerator_ident - # Choice values are integer-raw even on scaled/float signals. - if cg_signal.signal.conversion.is_float: - underlying_type = f'int{cg_signal.type_length}_t' - else: - underlying_type = cg_signal.type_name - enums.append({ 'name': enum_name, 'signal_name': cg_signal.signal.name, 'message_name': message.name, - 'underlying_type': underlying_type, + 'underlying_type': _enum_underlying_type(sorted_values), 'enumerators': [ { 'ident': enumerator_ident_by_value[raw_value], diff --git a/test_dbc_gen_cpp/test/FakeVehicle.dbc b/test_dbc_gen_cpp/test/FakeVehicle.dbc index 5e423c6..0af2b8c 100644 --- a/test_dbc_gen_cpp/test/FakeVehicle.dbc +++ b/test_dbc_gen_cpp/test/FakeVehicle.dbc @@ -67,6 +67,12 @@ BO_ 102 GearStatus: 1 Vehicle BO_ 103 DriveModeStatus: 1 Vehicle SG_ mode : 0|8@1+ (1,0) [0|255] "" Computer +BO_ 104 MotorStatus: 1 Vehicle + SG_ direction : 0|8@1- (1,0) [-1|1] "" Computer + +BO_ 105 SensorFlags: 4 Vehicle + SG_ flags : 0|32@1- (1,0) [0|0] "" Computer + BA_DEF_ BO_ "VFrameFormat" ENUM "StandardCAN","ExtendedCAN","reserved","J1939PG"; BA_DEF_DEF_ "VFrameFormat" ""; BA_ "VFrameFormat" BO_ 2566859520 3; @@ -74,12 +80,15 @@ BA_ "VFrameFormat" BO_ 2566859520 3; SIG_VALTYPE_ 100 speed : 1; SIG_VALTYPE_ 200 drive_speed : 1; SIG_VALTYPE_ 200 drive_angle : 1; +SIG_VALTYPE_ 105 flags : 1; CM_ BO_ 2147483848 "CAN ID 200 (0xC8) with extended frame flag (0x80000000) set"; CM_ BO_ 2566859520 "Add one J1939 Message to test having a single J1939 message in a non-J1939 DBC"; VAL_ 102 gear 0 "Neutral" 1 "Drive" 2 "Reverse" 3 "Reserved" 4 "Reserved" 5 "4wd mode" 6 "Park!" ; VAL_ 103 mode 0 "Off" 1 "On" ; +VAL_ 104 direction -1 "Reverse" 0 "Stopped" 1 "Forward" ; +VAL_ 105 flags 1 "Enabled" 2 "Fault" 2147483648 "Calibrating" ; BA_DEF_ SG_ "SystemSignalLongSymbol" STRING ; BA_DEF_DEF_ "SystemSignalLongSymbol" ""; diff --git a/test_dbc_gen_cpp/test/test_dbc_cpp.cpp b/test_dbc_gen_cpp/test/test_dbc_cpp.cpp index dc649bf..d5e0730 100644 --- a/test_dbc_gen_cpp/test/test_dbc_cpp.cpp +++ b/test_dbc_gen_cpp/test/test_dbc_cpp.cpp @@ -979,9 +979,9 @@ TEST_CASE("Generated enum - enumerator values match the DBC value table") REQUIRE(static_cast(GearStatus_gear::PARK) == 6); } -TEST_CASE("Generated enum - underlying type follows the signal type") +TEST_CASE("Generated enum - underlying type is the smallest that fits the values") { - // gear is an 8-bit unsigned signal. + // gear values are 0..6 -> smallest fitting unsigned type. STATIC_REQUIRE(std::is_same_v, uint8_t>); } @@ -1010,3 +1010,29 @@ TEST_CASE("Generated enum - signal name with spaces yields a valid enum type") REQUIRE(static_cast(DriveModeStatus_Drive_Mode_Select::ON) == 1); REQUIRE(std::strcmp(fake_vehicle_can::to_string(DriveModeStatus_Drive_Mode_Select::ON), "On") == 0); } + +TEST_CASE("Generated enum - negative choice values yield a signed underlying type") +{ + // MotorStatus.direction is a signed 8-bit signal with a -1 choice. + using fake_vehicle_can::MotorStatus_direction; + + STATIC_REQUIRE(std::is_same_v, int8_t>); + REQUIRE(static_cast(MotorStatus_direction::REVERSE) == -1); + REQUIRE(static_cast(MotorStatus_direction::STOPPED) == 0); + REQUIRE(static_cast(MotorStatus_direction::FORWARD) == 1); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(MotorStatus_direction::REVERSE), "Reverse") == 0); +} + +TEST_CASE("Generated enum - float-typed signal with integer flag choices") +{ + // Float-typed signal (SIG_VALTYPE_) with integer bit flags: needs an integral + // base wide enough for a 2^31 flag (uint32_t), never `float`. + using fake_vehicle_can::SensorFlags_flags; + + STATIC_REQUIRE(std::is_integral_v>); + STATIC_REQUIRE(std::is_same_v, uint32_t>); + REQUIRE(static_cast(SensorFlags_flags::ENABLED) == 1u); + REQUIRE(static_cast(SensorFlags_flags::FAULT) == 2u); + REQUIRE(static_cast(SensorFlags_flags::CALIBRATING) == 2147483648u); + REQUIRE(std::strcmp(fake_vehicle_can::to_string(SensorFlags_flags::CALIBRATING), "Calibrating") == 0); +} diff --git a/test_dbc_gen_cpp/test/test_enum_generation.py b/test_dbc_gen_cpp/test/test_enum_generation.py index f0d3f84..904c1ad 100644 --- a/test_dbc_gen_cpp/test/test_enum_generation.py +++ b/test_dbc_gen_cpp/test/test_enum_generation.py @@ -14,7 +14,7 @@ from cantools import database from cantools.database.can.c_source import CodeGenMessage -from dbc_gen_cpp.generate_cpp import build_signal_enums +from dbc_gen_cpp.generate_cpp import _enum_underlying_type, build_signal_enums def _build_enums(dbc_text, tmp_path): @@ -56,6 +56,27 @@ def _build_enums(dbc_text, tmp_path): """ +@pytest.mark.parametrize( + 'values, expected', + [ + ([0, 1, 2, 3], 'uint8_t'), + ([0, 255], 'uint8_t'), + ([0, 256], 'uint16_t'), + ([0, 65535], 'uint16_t'), + ([0, 65536], 'uint32_t'), + ([1, 2, 2147483648], 'uint32_t'), # 2^31 flag: stays unsigned 32-bit + ([0, 4294967296], 'uint64_t'), # 2^32: needs 64-bit + ([-1, 0, 1], 'int8_t'), # any negative -> signed + ([-128, 127], 'int8_t'), + ([-129, 0], 'int16_t'), + ([-1, 2147483647], 'int32_t'), + ], +) +def test_enum_underlying_type_sizing(values, expected): + """The underlying type is the smallest stdint type spanning the choice values.""" + assert _enum_underlying_type(sorted(values)) == expected + + def test_colliding_enum_names_raise(tmp_path): """Two signals whose sanitized _ names match must fail loudly.""" with pytest.raises(ValueError, match='collides') as excinfo: