technocreatives / dbc-codegen

Generate Rust structs for messages from a dbc (CAN bus definition) file.
Apache License 2.0
43 stars 23 forks source link

Prepend X to enum fields that start with a numeric #9

Closed andresv closed 3 years ago

andresv commented 3 years ago

I have a dbc file which contains:

VAL_ 1162 TSGN3 0 "none" 1 "speed sign" 2 "0 unlimited" 7 "unlimited" 16 "highway" 17 "no highway" 18 "motorway" 19 "no motorway" 20 "in city" 21 "outside city" 22 "pedestrian area" 23 "no pedestrian area" 65 "no overtaking left" 66 "no overtaking right" 67 "overtaking allowed again" 129 "no entry";

Currently it generates:

#[derive(Clone, Copy, PartialEq)]
#[cfg_attr(feature = "debug", derive(Debug))]
pub enum Rsa2Tsgn3 {
    None,
    SpeedSign,
    0Unlimited,
    Unlimited,
    Highway,
    NoHighway,
...

Notice 0Unlimited which is not allowed in Rust.

This PR prepends X to such enum fields. So it becomes:

#[derive(Clone, Copy, PartialEq)]
#[cfg_attr(feature = "debug", derive(Debug))]
pub enum Rsa2Tsgn3 {
    None,
    SpeedSign,
    X0Unlimited,
    Unlimited,
    Highway,
    NoHighway,
...
killercup commented 3 years ago

Thanks for the quick fix! Any reason you didn't go for raw identifiers?

Would be amazing to have a test for this too :)

andresv commented 3 years ago

rust-analyzer did not like them: Screen Shot 2021-03-09 at 11 43 45

I guess they are not allowed in enums. Also I think it is more nicer to do

let r = Rsa2Tsgn3::X0Unlimited;

than

let r = Rsa2Tsgn3::#r0Unlimited;

At some point it should be probably documented in readme.

Test is coming up.

andresv commented 3 years ago

For testing it would be best to add such field to example.dbc file. There is also example.kdc what do to with that?

killercup commented 3 years ago

Ah, interesting. Seems like raw identifiers are only allowing Rust keywords, so maybe we can actually combine the approaches to allow variants called type and similar?

Regarding tests, the KCD is to allow testing against node-socketcan. You can build it using the python tool canmatrix, see here for how to install it with kcd support. I only do this once a year so if you get it to work feel free to add a line to one of the readmes :)

Thanks so much for contributing!

andresv commented 3 years ago

How do I actually run those tests (will add this to readme).

killercup commented 3 years ago

IIRC I only did the node tests manually -- for example to get lines like this.

Don't worry about it for now :)

andresv commented 3 years ago

It seems I do not understand cargo workspaces correctly. I tried:

cargo build --bin testing
cargo build --bin testing/rust_integration
cargo build --bin rust_integration

# nope
cargo test
# nope
cargo test --test testing
killercup commented 3 years ago

Give cargo test --all a try :)

(You can cheat by looking at what is run on CI)

andresv commented 3 years ago

This change should be now covered with a test. There isn't explicit test, but CI can check that indeed it builds so messages.rs must contain valid Rust code.