search

technocreatives / dbc-codegen

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

Allow specifying signal enmum default variant name #32

Closed marcelbuesing closed 3 years ago

marcelbuesing commented 3 years ago

I noticed it seems to occur quite frequently that there are naming collisions with existing value descriptions. I assume it would be actually better to also have some other default string but I don't really have a good idea here for a new name that may not collide. E.g. "Other" - Variant

killercup commented 3 years ago

Thanks! I appreciate the work a lot, I'd however like to not introduce options for as long as possible, so we only have one configuration to test.

Is this something you'd use a lot? An alternative simpler way would be to treat "Other" as a keyword to automatically rename a given "Other" type to "XOther".

marcelbuesing commented 3 years ago

Sure I would be fine with that, it's probably a lot more unlikely than Other, to a point where it really becomes an edge case. Will update this accordingly.

killercup commented 3 years ago

Ah sorry, i meant the other way around, adding Other to this list to treat our catch-all variant as a keyword. I'd rather have it stand out than look like an escaped name from the input if that makes sense. Maybe even go with _Other (and add the allow lint attr if necessary)?

marcelbuesing commented 3 years ago

Ah sorry, i meant the other way around, adding Other to this list to treat our catch-all variant as a keyword. I'd rather have it stand out than look like an escaped name from the input if that makes sense. Maybe even go with _Other (and add the allow lint attr if necessary)?

Sure just changed it. I think the downside here is that you may accidentally use the wrong enum variant. So do you suggest to use _Other for the dbc Other variant or for the default variant that is currently named Other? From my perspective it might more sense to name the current default variant _Other and _other as keyword (which i think is pretty unlikely). Then one can still prefix that with the current X.

killercup commented 3 years ago

From my perspective it might more sense to name the current default variant _Other

Precisely! Didn't want to bother you doing another round, so I updated it and merged it :)

marcelbuesing commented 3 years ago

Thanks. Looks good =)!