[felixvanoost]

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 Report

✅ All modified and coverable lines are covered by tests.

📢 Thoughts on this report? Let us know!

[nyurik]

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

[felixvanoost]

@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]

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

[Copilot]

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.

[nyurik]

@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 Dbc — message_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 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]

@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.

[felixvanoost]

@nyurik I've updated can-dbc to v10.0.0 and removed the helper methods that were previously in this PR. I also resolved the issues that Copilot found. Let me know if there's anything else!

[nyurik]

overall seems good to go. One minor question about example - not a blocker though

[nyurik]

P.S. I disabled auto-merge to let you figure out what you want to do with the example, and once done, feel free to merge.