Extended IDs from DBC file are not masked before code generation

[dlips]

Hello, I discovered a problem with the code generation for messages with extended CAN IDs. The dbc format specification describes in section 8, Message Definitions, that if the most significant bit is set to 1 then the CAN ID is an extended ID. The concrete message ID is then obtained by masking the entry in the dbc file (ID_IN_DBC & 0x1FFFFFFF). Otherwise one obtains an Id which does not fit in 29-Bits.

The code generated with the CLI tool (dbc-codegen-cli 0.3.0), just takes the message id from the dbc file to define the MESSAGE_ID constant and uses it in the match statement for decoding messages. This results in the usage of wrong CAN Ids

EDIT: Example for message definition which results in faulty code generation

BO_ 2566834936 M1: 8 Vector__XXX
 SG_ Sig_A : 0|2@1+ (1,0) [0|3] "" Vector__XXX
 SG_ Sig_B : 2|2@1+ (1,0) [0|3] "" Vector__XXX
 SG_ Sig_C : 4|2@1+ (1,0) [0|3] "" Vector__XXX
 SG_ Sig_D : 6|2@1+ (1,0) [0|3] "" Vector__XXX
 SG_ Sig_E : 23|19@0+ (1,0) [0|524287] "" Vector__XXX
 SG_ Sig_F : 32|5@1+ (1,0) [0|31] "" Vector__XXX
 SG_ Sig_G : 40|7@1+ (1,0) [0|127] "" Vector__XXX
 SG_ Sig_H : 47|1@1+ (1,0) [0|1] "" Vector__XXX

[dlips]

I looked into the source code, and the problem is that Message::message_id() from the can_dbc crate returns the message id as defined in the dbc file, without recognizing the meaning of the most significant bit of the message id indicating a standard or extended id.

So the problem can be solved by masking the return value of message_id() at the right places. Additionally I would propose to introduce an additional generated constant IS_EXTENDED: bool to indicate if a message has an extended id or not. This is for example also done by the C code generation from the python canutils.

Should I create a PR for this?

[hulthe]

Good catch. Looks like can_dbc very recently changed their message ID type to solve this. May be time to update?

PRs are welcome :)

[dlips]

Damn, they changed it one day after I checked the crate to understand what's going on :)

Ok, but with the change of the message id to an enum, there comes up the question of how to deal with it in the generated code.

1. Use the enum in the generated code -> this introduces a new dependency of the generated code on the can_dbc crate
2. Re-export the type from can_dbc in the dbc-codegen lib -> introduces a new dependency of the generated code on the dbc_codegen crate
3. Create an equivalent enum in the generated code -> no new dependency, but can introduces some disambiguity in the generated code if one generates two codefiles from two dbc files and uses them in the same project. I don't know how common this is, but I can imagine cases where this could be done.
4. Just use the raw ID as before and mask it accordingly -> Problematic because StandardId(12) is different from ExtendedId(12) but would result in the same u32.
5. Use the Id types from the embedded-hal crate -> Introduces a new dependency of the generated code. These types are commonly use in the rust CAN ecosystem, e.g. by the socketcan-rs crate.

Any thoughts on how to resolve this?

[kilpkonn]

I'd personally opt for 5 as this is a common dependency anyways, and in addition to Id type the Frame trait could be implemented automatically for all the generated messages.