Skip to content

Commit

Permalink
MessageId Standard vs Extended enum (#13)
Browse files Browse the repository at this point in the history 
* MessageId Standard vs Extended enum

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.

* changed check extended id vs standard id

as requested by #13 (review)
  • Loading branch information
@erzoe
erzoe authored Feb 8, 2024
1 parent 7a35920 commit dc85420
Show file tree
Hide file tree
Showing 2 changed files with 35 additions and 25 deletions.
29 changes: 16 additions & 13 deletions src/lib.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -153,7 +153,7 @@ SIG_VALTYPE_ 2000 Signal_8 : 1;
fn lookup_signal_comment() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let comment = dbc_content
.signal_comment(MessageId(1840), "Signal_4")
.signal_comment(MessageId::Standard(1840), "Signal_4")
.expect("Signal comment missing");
assert_eq!(
"asaklfjlsdfjlsdfgls\nHH?=(%)/&KKDKFSDKFKDFKSDFKSDFNKCnvsdcvsvxkcv",
Expand All @@ -164,31 +164,31 @@ SIG_VALTYPE_ 2000 Signal_8 : 1;
#[test]
fn lookup_signal_comment_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let comment = dbc_content.signal_comment(MessageId(1840), "Signal_2");
let comment = dbc_content.signal_comment(MessageId::Standard(1840), "Signal_2");
assert_eq!(None, comment);
}

#[test]
fn lookup_message_comment() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let comment = dbc_content
.message_comment(MessageId(1840))
.message_comment(MessageId::Standard(1840))
.expect("Message comment missing");
assert_eq!("Some Message comment", comment);
}

#[test]
fn lookup_message_comment_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let comment = dbc_content.message_comment(MessageId(2000));
let comment = dbc_content.message_comment(MessageId::Standard(2000));
assert_eq!(None, comment);
}

#[test]
fn lookup_value_descriptions_for_signal() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let val_descriptions = dbc_content
.value_descriptions_for_signal(MessageId(2000), "Signal_3")
.value_descriptions_for_signal(MessageId::Standard(2000), "Signal_3")
.expect("Message comment missing");

let exp = vec![ValDescription {
Expand All @@ -202,15 +202,15 @@ SIG_VALTYPE_ 2000 Signal_8 : 1;
fn lookup_value_descriptions_for_signal_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let val_descriptions =
dbc_content.value_descriptions_for_signal(MessageId(2000), "Signal_2");
dbc_content.value_descriptions_for_signal(MessageId::Standard(2000), "Signal_2");
assert_eq!(None, val_descriptions);
}

#[test]
fn lookup_extended_value_type_for_signal() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let extended_value_type =
dbc_content.extended_value_type_for_signal(MessageId(2000), "Signal_8");
dbc_content.extended_value_type_for_signal(MessageId::Standard(2000), "Signal_8");
assert_eq!(
extended_value_type,
Some(&SignalExtendedValueType::IEEEfloat32Bit)
Expand All @@ -221,28 +221,28 @@ SIG_VALTYPE_ 2000 Signal_8 : 1;
fn lookup_extended_value_type_for_signal_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let extended_value_type =
dbc_content.extended_value_type_for_signal(MessageId(2000), "Signal_1");
dbc_content.extended_value_type_for_signal(MessageId::Standard(2000), "Signal_1");
assert_eq!(extended_value_type, None);
}

#[test]
fn lookup_signal_by_name() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let signal = dbc_content.signal_by_name(MessageId(2000), "Signal_8");
let signal = dbc_content.signal_by_name(MessageId::Standard(2000), "Signal_8");
assert!(signal.is_some());
}

#[test]
fn lookup_signal_by_name_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let signal = dbc_content.signal_by_name(MessageId(2000), "Signal_25");
let signal = dbc_content.signal_by_name(MessageId::Standard(2000), "Signal_25");
assert_eq!(signal, None);
}

#[test]
fn lookup_multiplex_indicator_switch() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let multiplexor_switch = dbc_content.message_multiplexor_switch(MessageId(3040));
let multiplexor_switch = dbc_content.message_multiplexor_switch(MessageId::Standard(3040));
assert!(multiplexor_switch.is_ok());
assert!(multiplexor_switch.as_ref().unwrap().is_some());
assert_eq!(multiplexor_switch.unwrap().unwrap().name(), "Switch");
Expand All @@ -251,7 +251,7 @@ SIG_VALTYPE_ 2000 Signal_8 : 1;
#[test]
fn lookup_multiplex_indicator_switch_none_when_missing() {
let dbc_content = DBC::try_from(SAMPLE_DBC).expect("Failed to parse DBC");
let multiplexor_switch = dbc_content.message_multiplexor_switch(MessageId(1840));
let multiplexor_switch = dbc_content.message_multiplexor_switch(MessageId::Standard(1840));
assert!(multiplexor_switch.unwrap().is_none());
}
}
Expand Down Expand Up @@ -298,7 +298,10 @@ pub struct Signal {
/// Must be unique in DBC file.
#[derive(Copy, Clone, Debug, PartialEq)]
#[cfg_attr(feature = "serde", derive(Serialize))]
pub struct MessageId(pub u32);
pub enum MessageId {
Standard(u16),
Extended(u32),
}

#[derive(Clone, Debug, PartialEq)]
#[cfg_attr(feature = "serde", derive(Serialize))]
Expand Down
31 changes: 19 additions & 12 deletions src/parser.rs
View file
Original file line number Diff line number Diff line change
Expand Up @@ -124,7 +124,7 @@ mod tests {
#[test]
fn signal_comment_test() {
let def1 = "CM_ SG_ 193 KLU_R_X \"This is a signal comment test\";\n";
let message_id = MessageId(193);
let message_id = MessageId::Standard(193);
let comment1 = Comment::Signal {
message_id,
signal_name: "KLU_R_X".to_string(),
Expand All @@ -137,7 +137,7 @@ mod tests {
#[test]
fn message_definition_comment_test() {
let def1 = "CM_ BO_ 34544 \"Some Message comment\";\n";
let message_id = MessageId(34544);
let message_id = MessageId::Standard(34544);
let comment1 = Comment::Message {
message_id,
comment: "Some Message comment".to_string(),
Expand Down Expand Up @@ -172,7 +172,7 @@ mod tests {
#[test]
fn value_description_for_signal_test() {
let def1 = "VAL_ 837 UF_HZ_OI 255 \"NOP\";\n";
let message_id = MessageId(837);
let message_id = MessageId::Standard(837);
let signal_name = "UF_HZ_OI".to_string();
let val_descriptions = vec![ValDescription {
a: 255.0,
Expand Down Expand Up @@ -243,7 +243,7 @@ mod tests {
fn message_definition_attribute_value_test() {
let def = "BA_ \"AttrName\" BO_ 298 13;\n";
let attribute_value = AttributeValuedForObjectType::MessageDefinitionAttributeValue(
MessageId(298),
MessageId::Standard(298),
Some(AttributeValue::AttributeValueF64(13.0)),
);
let attr_val_exp = AttributeValueForObject {
Expand All @@ -258,7 +258,7 @@ mod tests {
fn signal_attribute_value_test() {
let def = "BA_ \"AttrName\" SG_ 198 SGName 13;\n";
let attribute_value = AttributeValuedForObjectType::SignalAttributeValue(
MessageId(198),
MessageId::Standard(198),
"SGName".to_string(),
AttributeValue::AttributeValueF64(13.0),
);
Expand Down Expand Up @@ -377,7 +377,7 @@ mod tests {
let def = "SIG_GROUP_ 23 X_3290 1 : A_b XY_Z;\n";

let exp = SignalGroups {
message_id: MessageId(23),
message_id: MessageId::Standard(23),
signal_group_name: "X_3290".to_string(),
repetitions: 1,
signal_names: vec!["A_b".to_string(), "XY_Z".to_string()],
Expand Down Expand Up @@ -442,7 +442,7 @@ mod tests {
fn message_transmitters_test() {
let def = "BO_TX_BU_ 12345 : XZY,ABC;\n";
let exp = MessageTransmitter {
message_id: MessageId(12345),
message_id: MessageId::Standard(12345),
transmitter: vec![
Transmitter::NodeName("XZY".to_string()),
Transmitter::NodeName("ABC".to_string()),
Expand Down Expand Up @@ -502,7 +502,7 @@ mod tests {
// simple mapping
let def = "SG_MUL_VAL_ 2147483650 muxed_A_1 MUX_A 1-1;\n";
let exp = ExtendedMultiplex {
message_id: MessageId(2147483650),
message_id: MessageId::Extended(2),
signal_name: "muxed_A_1".to_string(),
multiplexor_signal_name: "MUX_A".to_string(),
mappings: vec![ExtendedMultiplexMapping {
Expand All @@ -516,7 +516,7 @@ mod tests {
// range mapping
let def = "SG_MUL_VAL_ 2147483650 muxed_A_1 MUX_A 1568-2568;\n";
let exp = ExtendedMultiplex {
message_id: MessageId(2147483650),
message_id: MessageId::Extended(2),
signal_name: "muxed_A_1".to_string(),
multiplexor_signal_name: "MUX_A".to_string(),
mappings: vec![ExtendedMultiplexMapping {
Expand All @@ -530,7 +530,7 @@ mod tests {
// multiple mappings
let def = "SG_MUL_VAL_ 2147483650 muxed_B_5 MUX_B 5-5, 16-24;\n";
let exp = ExtendedMultiplex {
message_id: MessageId(2147483650),
message_id: MessageId::Extended(2),
signal_name: "muxed_B_5".to_string(),
multiplexor_signal_name: "MUX_B".to_string(),
mappings: vec![
Expand All @@ -552,7 +552,7 @@ mod tests {
fn sig_val_type_test() {
let def = "SIG_VALTYPE_ 2000 Signal_8 : 1;\n";
let exp = SignalExtendedValueTypeList {
message_id: MessageId(2000),
message_id: MessageId::Standard(2000),
signal_name: "Signal_8".to_string(),
signal_extended_value_type: SignalExtendedValueType::IEEEfloat32Bit,
};
Expand Down Expand Up @@ -687,7 +687,14 @@ fn byte_order(s: &str) -> IResult<&str, ByteOrder> {
}

fn message_id(s: &str) -> IResult<&str, MessageId> {
map(complete::u32, MessageId)(s)
let (s, parsed_value) = complete::u32(s)?;

if parsed_value & (1 << 31) != 0 {
let extended_value = parsed_value & u32::from(u16::MAX);
Ok((s, MessageId::Extended(extended_value & 0x1FFFFFFF)))
} else {
Ok((s, MessageId::Standard(parsed_value as u16)))
}
}

fn signed(s: &str) -> IResult<&str, ValueType> {
Expand Down

0 comments on commit dc85420

Please sign in to comment.