Skip to content

Add option to generate typed constants from DBC BA_ attributes#175

Open
felixvanoost wants to merge 8 commits into
oxibus:mainfrom
felixvanoost:emit-user-defined-structs
Open

Add option to generate typed constants from DBC BA_ attributes#175
felixvanoost wants to merge 8 commits into
oxibus:mainfrom
felixvanoost:emit-user-defined-structs

Conversation

@felixvanoost

@felixvanoost felixvanoost commented Jul 9, 2026

Copy link
Copy Markdown

Adds an option to generate typed constants from DBC BA_ attributes and place them within the existing generated message types. This is useful for DBCs that define per-message or per-signal metadata, like AUTOSAR E2E protection or SecOC parameters.

Users define a struct that their crate owns and specify where each field should come from by adding one or more AttributeStruct entries to the generator Config. I have created an example to show the syntax in the README.

@codecov

codecov Bot commented Jul 10, 2026

Copy link
Copy Markdown

Codecov Report

❌ Patch coverage is 95.27027% with 7 lines in your changes missing coverage. Please review.

Files with missing lines Patch % Lines
src/lib.rs 95.27% 0 Missing and 7 partials ⚠️

📢 Thoughts on this report? Let us know!

@felixvanoost felixvanoost self-assigned this Jul 10, 2026
@felixvanoost felixvanoost added the enhancement New feature or request label Jul 10, 2026
@nyurik

nyurik commented Jul 13, 2026

Copy link
Copy Markdown
Member

are there any existing examples of this in https://github.com/cantools/cantools in their tests/files?

@felixvanoost

felixvanoost commented Jul 13, 2026

Copy link
Copy Markdown
Author

@nyurik cantools does a limited version of this with the message cycle times, which it extracts from the BA_ attribute GenMsgCycleTime. I am also proposing to match this behaviour in a separate PR, #176, because the message length and cycle time are simple scalar values and more or less universally defined in DBC files.

This PR allows users to extract any arbitrary BA_ attributes and expose them as an arbitrary struct in the generated code, which cantools doesn't do. It's useful for grouping sets of attributes together, e.g. AUTOSAR E2E/SecOC parameters or OEM-specific metadata, in a flexible way.

cantools does have a test fixture attributes.dbc that we can use, so I've added some additional tests based on that. :)

@felixvanoost

Copy link
Copy Markdown
Author

@nyurik I've added some additional test cases using the attributes.dbc fixture. PR should be good to go now!

@felixvanoost felixvanoost enabled auto-merge (squash) July 14, 2026 18:12
@nyurik nyurik requested a review from Copilot July 15, 2026 15:12

Copilot AI left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Pull request overview

Adds a new codegen configuration option to emit typed associated constants on generated message types, where the values are sourced from DBC BA_ attributes (with optional fallback to BA_DEF_DEF_ defaults), derived signal layout fields, and/or literals. This enables exposing per-message/per-signal metadata (e.g., AUTOSAR E2E / SecOC parameters) in a structured way in the generated Rust API.

Changes:

  • Introduce Config::attribute_structs plus supporting public spec types (AttributeStruct, AttributeScope, AttributeField, FieldSource) and generation logic in src/lib.rs.
  • Add an end-to-end example and README documentation demonstrating how users map struct fields to DBC attributes/layout/literals.
  • Add extensive tests covering success cases (scopes, defaults, layouts) and error cases (invalid sources, collisions, missing values).

Reviewed changes

Copilot reviewed 4 out of 4 changed files in this pull request and generated 4 comments.

File Description
tests/attribute_structs.rs New tests validating attribute-driven constant generation and error handling.
src/lib.rs Implements attribute-struct specs, validation, attribute lookup/defaulting, and code emission into message impl blocks.
README.md Documents the new attribute_structs feature and provides usage/example output.
examples/attribute_structs.rs Adds a runnable example showing signal/message-scoped attribute structs.

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

@@ -0,0 +1,82 @@
//! Demonstrates [`Config::attribute_structs`].
//! Emits constants for AUTOSAR E2E / SecOC-style metdata from DBC 'BA_' attributes.
Comment thread src/lib.rs
Comment on lines +510 to +528
ensure!(
!spec.fields.is_empty(),
"attribute_structs: '{}' declares no fields",
spec.const_name
);
if matches!(spec.scope, AttributeScope::Message) {
for field in spec.fields {
ensure!(
!matches!(
field.source,
FieldSource::StartBit | FieldSource::StartByte | FieldSource::BitWidth
),
"attribute_structs: field '{}' of '{}' uses a signal-only source \
(StartBit/StartByte/BitWidth) but the struct has message scope",
field.name,
spec.const_name
);
}
}
Comment thread src/lib.rs
Comment on lines +539 to +550
// Track every constant name already emitted for this message type so that
// duplicates are rejected here instead of producing code that fails to
// compile.
let mut used: BTreeSet<String> = BTreeSet::new();
used.insert("MESSAGE_ID".to_string());
for signal in &msg.signals {
if ValType::from_signal(signal) != ValType::Bool {
let sig = signal.field_name().to_uppercase();
used.insert(format!("{sig}_MIN"));
used.insert(format!("{sig}_MAX"));
}
}
Comment thread src/lib.rs
Comment on lines +1602 to +1607
fn signal_start_byte(signal: &Signal) -> u64 {
let start_bit = match signal.byte_order {
LittleEndian | BigEndian => signal.start_bit,
};
start_bit.checked_div(8).unwrap_or(0)
}
@nyurik

nyurik commented Jul 15, 2026

Copy link
Copy Markdown
Member

@felixvanoost I ran some more AI on this, with some additional thoughts of mine. I asked AI also to consider if any of this functionality should be moved up to can-dbc crate, and it did find a few ones.


Verdict

The direction is sound and worth merging, with a few things to fix first. The feature is genuinely useful, opt-in (zero effect when unused), avoids hard-coding OEM attribute names into the generator, and the tests are thorough. But there are two API-evolvability problems, one real compile-breaking edge case, and three helper functions that belong in can-dbc.

Why it's needed

Automotive DBCs (especially AUTOSAR-flavored ones) carry per-message/per-signal metadata in BA_ attributes — E2E protection data IDs, SecOC freshness value IDs, etc. Code consuming the generated messages needs those values at compile time. The PR lets a user declare a struct their own crate owns and map each field to an attribute, a derived layout fact (start bit/byte, width, message size), or a literal; the generator emits it as an associated const like pub const SPEED_CRC_E2E: data_protection::E2EDataIdInfo = ... inside the message impl.

Could a user do this themselves with can-dbc in their own build.rs? Mostly yes — impl blocks for their own types can live in a separate generated file. But they'd have to replicate dbc-codegen's private signal-renaming rules (field_name()) and message-filtering logic to get consistent names, so integrating it does buy real value. cantools has no equivalent (it only special-cases GenMsgCycleTime, which is what #176 mirrors).

What should move to can-dbc

The three lookup helpers added at lib.rs:1544-1570 are pure DBC queries with zero codegen logic:

  • message_attr(dbc, id, name) — find an assigned BO_ attribute
  • signal_attr(dbc, id, signal, name) — find an assigned SG_ attribute
  • attr_default(dbc, name) — find a BA_DEF_DEF_ default

can-dbc already has exactly this pattern of methods on Dbcmessage_comment(), signal_comment(), signal_by_name(), value_descriptions_for_signal() — but nothing for attributes, which is clearly a gap. Two additional signals that these belong upstream:

  1. PR Generate message size and cycle time consts #176 copy-pastes message_attr and the author explicitly says it "should be stacked on top of Add option to generate typed constants from DBC BA_ attributes #175 as it reuses the message_attr helper." When two PRs in the same repo need the same DBC-query primitive, it's a parser-crate primitive.
  2. The "assigned value, falling back to BA_DEF_DEF_ default" resolution in resolve_field_source is defined DBC semantics, not codegen policy — a Dbc::resolved_message_attribute(...) / resolved_signal_attribute(...) upstream would serve any consumer.

I'd add these as Dbc methods in can-dbc, cut a release, and have both #175 and #176 shrink to pure rendering logic. What should not move: the AttributeStruct/FieldSource spec types and attr_value_literal (Rust literal formatting) — that's codegen policy.

A real bug: ENUM attributes can generate non-compiling code

Verified against the cantools fixture the PR itself uses: BA_ assigns ENUM attributes by integer index (BA_ "GenMsgSendType" BO_ ... 0;) while BA_DEF_DEF_ writes the default as a quoted label (BA_DEF_DEF_ "GenMsgSendType" "NoMsgSendType";). The PR emits whatever can-dbc parsed, so the same struct field gets 0 for a message with an explicit assignment and "NoMsgSendType" for one falling back to the default. One field can't be both u16 and &str — if a DBC has both cases, the generated code won't compile. The test fixture_enum_attribute_is_index_not_label pins the index behavior but nothing exercises the mixed case. The proper fix is resolving enum values through dbc.attribute_definitions (AttributeValueType::Enum(Vec<String>)) to a consistent representation — and that resolution is another good candidate for can-dbc. Related smaller wart: an INT attribute feeding an f32 field emits 373, which is also a type error in Rust; there's no way to request a float-formatted literal.

Internal duplication and smells

  • The collision-tracking used set (lib.rs:536-549) re-derives the names of consts emitted elsewhere — MESSAGE_ID plus {SIG}_MIN/{SIG}_MAX including the ValType::Bool exclusion condition copied from lib.rs:355-363. This is fragile: Generate message size and cycle time consts #176 adds MESSAGE_SIZE and MESSAGE_CYCLE_TIME consts and whoever stacks it must remember to extend this set. Better to record const names at emission time.
  • signal_start_byte lib.rs:1607 contains a no-op match signal.byte_order { LittleEndian | BigEndian => signal.start_bit } and a checked_div(8).unwrap_or(0) that can never fail. It's just signal.start_bit / 8; the intent ("both byte orders deliberately use the raw DBC start bit") should be a comment, not dead code. It does not duplicate le_start_end_bit/be_start_end_bit — those normalize Motorola bit positions for packing, while this intentionally uses raw DBC numbering, and the byte index comes out the same either way.
  • No duplication with main otherwise — this PR is the first consumer of BA_ data in dbc-codegen.

API-evolvability concerns worth settling before merge

  1. AttributeStruct and AttributeField are plain structs with public fields that users construct literally. Unlike the #[non_exhaustive] enums, adding any field later is a breaking change — and follow-ups are easy to imagine (per-field default, doc comment, literal type hint to fix the float issue). Either give them builders (consistent with Config's TypedBuilder) or accept the churn deliberately.
  2. Everything is &'a str / &'a [..] borrows. That matches Config's style, but it means dbc-codegen-cli can never expose this feature from a config file without leaking memory or restructuring. Owned String/Vec here would cost nothing measurable and keep the CLI door open.
  3. require semantics are subtle and now contractual: only an explicit BA_ assignment triggers emission (defaults don't), and there's no "emit for every signal" option. Fine as a v1, but worth documenting explicitly since it can't easily change later.

Suggested path

Before merging the code: (1) land message_attribute/signal_attribute/attribute_default (+ resolved variants) in can-dbc and rebase both PRs on it, (2) resolve the ENUM index-vs-label inconsistency, (3) decide owned-vs-borrowed and builder-vs-literal for the public spec types since those are one-way doors, (4) clean up signal_start_byte and the used-set duplication

@felixvanoost

Copy link
Copy Markdown
Author

@nyurik I've opened a PR to move the helper functions into can-dbc here: oxibus/can-dbc#83. Once that is merged and released I can rebase this PR and resolve the Copilot findings.

nyurik added a commit to oxibus/can-dbc that referenced this pull request Jul 15, 2026
Adds methods to lookup DBC attributes: `message_attribute`,
`signal_attribute`, `attribute_default`, `resolved_message_attribute`
and `resolved_signal_attribute`. These will be used in
oxibus/dbc-codegen#175 and
oxibus/dbc-codegen#176.

---------

Co-authored-by: Yuri Astrakhan <YuriAstrakhan@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

enhancement New feature or request

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants