MessageId Standard vs Extended enum #13
Conversation
In dbc files the information whether a message is an extended frame or standard frame is encoded in bit 31 of the message ID. This is an implementation detail that I believe most users are not familiar with and that I have not seen in the documentation of this crate. Therefore I think it is very unintuitive to expose this to the users of this crate. Therefore I have replaced the previous MessageId struct which only contained the u32 as it was saved in the file with the MessageId enum proposed by schphil. marcelbuesing#10 (comment) The message id is now masked and can be directly compared to the message ids of incoming CAN bus messages. The information whether it's a standard frame or extended frame is intuitively available. This does not only make the usage of the crate more intuitive but is also helpful in case this crate ever wants to support more file formats such as sym files where this information is encoded differently. For more information on dbc file specifics see the DBC_File_Format_Documentation: http://mcu.so/Microcontroller/Automotive/dbc-file-format-documentation_compress.pdf This commit is, of course, a breaking change. But given that this crate is at version 5.0.0 it does not seem like it has a strict policy of avoiding breaking changes. If breaking changes are a concern we could rename the new enum to FrameId, restore the old struct and add both of them to the Message struct. That might confuse users, though, which field to use and would require good documentation.
|
I have tested my changes with |
|
i believe it would be nice to follow the socket can standard here the IDE flag is stored in the 30 bits, RTR at 31 and ERR at 29. But maybe a stupid idea because it would brake backwards compatibility. I believe the simplest solution is to add a simple getter: is_ext and is_std for the MessageId. Edit: |
|
I recently encountered the same issue, and I believe the solution proposed here by @erzoe is ideal. In my opinion, it is a much better approach than the one in #14, which appears to be more of a workaround. Handling Standard and Extended CAN IDs is a fundamental aspect of the CAN standard and should be treated accordingly. |
src/parser.rs
Outdated
| map(complete::u32, MessageId)(s) | ||
| let (s, parsed_value) = complete::u32(s)?; | ||
|
|
||
| if parsed_value > u16::MAX as u32 { |
as requested by marcelbuesing#13 (review)
This is a continuation of #10.
In dbc files the information whether a message is an extended frame or standard frame is encoded in bit 31 of the message ID. This is an implementation detail that I believe most users are not familiar with and that I have not seen in the documentation of this crate. Therefore I think it is very unintuitive to expose this to the users of this crate. Therefore I have replaced the previous MessageId struct which only contained the u32 as it was saved in the file with the MessageId enum proposed by schphil. #10 (comment)
The message id is now masked and can be directly compared to the message ids of incoming CAN bus messages. The information whether it's a standard frame or extended frame is intuitively available. This does not only make the usage of the crate more intuitive but is also helpful in case this crate ever wants to support more file formats such as sym files where this information is encoded differently.
For more information on dbc file specifics see the DBC_File_Format_Documentation: http://mcu.so/Microcontroller/Automotive/dbc-file-format-documentation_compress.pdf
This commit is, of course, a breaking change. But given that this crate is at version 5.0.0 it does not seem like it has a strict policy of avoiding breaking changes. If breaking changes are a concern we could rename the new enum to FrameId, restore the old struct and add both of them to the Message struct. That might confuse users, though, which field to use and would require good documentation.
Thank you for this useful crate.