Repository: jmagnuson/canparse (language: rust)

Issue:
Implement DbcLibrary
As touched upon in #10, a generalized `DbcLibrary` is needed to give access to Messages/Signals without the J1939-specific data massaging done in `PgnLibrary`.

Another nice thing to implement would be `impl From<DbcLibrary> for PgnLibrary`, but isn't strictly necessary.


Discussion / hints:

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 0c2ee3e..4a99960 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -7,9 +7,13 @@ and this project adheres to [Semantic Versioning](http://semver.org).
 
 ## [Unreleased]
 
+### Added
+- Implement `DbcLibrary`
+
 ### Changed
 - Update and clean out old deps
 - Replace regex parsing with `nom` to handle strings with newlines
+- Convert message/PGN ID from `String` to `u32`
 
 ## [0.1.3] - 2018-02-09
 
diff --git a/benches/pgnlibrary.rs b/benches/pgnlibrary.rs
index 18e3378..91652b0 100644
--- a/benches/pgnlibrary.rs
+++ b/benches/pgnlibrary.rs
@@ -16,7 +16,7 @@ use socketcan::CANFrame;
 lazy_static!{
 
 static ref SPNDEF: SpnDefinition =
-    SpnDefinition::new("Engine_Speed".to_string(), 190, "2364539904".to_string(),
+    SpnDefinition::new("Engine_Speed".to_string(), 190, 2364539904,
         "A description for Engine speed.".to_string(),
         24, 16, true, false, 0.125, 0.0, 0.0, 8031.88, "rpm".to_string()
     );
diff --git a/src/dbc/library.rs b/src/dbc/library.rs
new file mode 100644
index 0000000..685b64b
--- /dev/null
+++ b/src/dbc/library.rs
@@ -0,0 +1,436 @@
+use ::dbc;
+use std::collections::HashMap;
+
+/// Trait for converting `Entry` values into a library's own entries.
+pub trait FromDbc {
+    type Err;
+
+    /// Converts an `Entity` value from scratch.
+    fn from_entry(entry: dbc::Entry) -> Result<Self, Self::Err>
+        where
+            Self: Sized;
+
+    /// Merges the given `Entity` with a `mut` version of the library's entity.  Useful for when
+    /// multiple `Entry` types contribute to various attributes within the same destination.
+    fn merge_entry(&mut self, entry: dbc::Entry) -> Result<(), Self::Err>;
+}
+
+
+type SignalAttribute = String;
+#[derive(Clone, Debug, Default, PartialEq)]
+pub struct Signal {
+    /// e.g., {"SPN", "190"}
+    /// BA_ "SPN" SG_ 2364540158 EngSpeed 190;
+    /// BA_ "SigType" SG_ 2364540158 EngSpeed 1;
+    attributes: HashMap<String, SignalAttribute>,
+    /// e.g., "Actual engine speed which is calculated over a minimum
+    /// crankshaft angle of 720 degrees divided by the number of cylinders."
+    description: Option<String>,
+    /// e.g, SG_ EngSpeed : 24|16@1+ (0.125,0) [0|8031.875] "rpm" Vector__XXX
+    definition: Option<dbc::SignalDefinition>, // FIXME: hate that this has to be Option
+
+    /// Only applicable for enum types
+    /// e.g., VAL_ 2364540158 ActlEngPrcntTrqueHighResolution 8 "1111NotAvailable" 7 "0875" 1 "0125" 0 "0000" ;
+    value_definition: Option<dbc::ValueDefinition>,
+}
+
+type MessageAttribute = String;
+
+#[derive(Clone, Debug, Default)]
+pub struct Message {
+    name: String,
+    message_len: u32,
+    sending_node: String,
+
+    /// e.g., BA_ "VFrameFormat" BO_ 2364540158 3;
+    attributes: HashMap<String, MessageAttribute>,
+    /// e.g., CM_ BO_ 2364540158 "Electronic Engine Controller 1";
+    description: Option<String>,
+    signals: HashMap<String, Signal>,
+}
+
+impl FromDbc for Message {
+    type Err = ();
+
+    fn from_entry(entry: dbc::Entry) -> Result<Self, Self::Err> where
+        Self: Sized {
+        use dbc::Entry;
+
+        match entry {
+            Entry::MessageDefinition(dbc::MessageDefinition {
+                                         id: _id,
+                                         name,
+                                         message_len,
+                                         sending_node,
+                                     }) => {
+                Ok(Message {
+                    name: name,
+                    message_len: message_len,
+                    sending_node: sending_node,
+                    .. Default::default()
+                })
+            },
+            Entry::MessageDescription(dbc::MessageDescription {
+                                          id: _id,
+                                          signal_name: _signal_name,
+                                          description,
+                                      }) => {
+                Ok(Message {
+                    description: Some(description),
+                    .. Default::default()
+                })
+            },
+            Entry::MessageAttribute(dbc::MessageAttribute {
+                                        name,
+                                        signal_name: _signal_name,
+                                        id: _id,
+                                        value,
+                                    }) => {
+                let mut attributes = HashMap::new();
+                attributes.insert(name, value);
+
+                Ok(Message {
+                    attributes: attributes,
+                    .. Default::default()
+                })
+            },
+            // TODO: Need to propogate Signal FromDbc in here..maybe, or just search in DbcLibrary
+            _ => Err(())
+        }
+    }
+
+    fn merge_entry(&mut self, entry: dbc::Entry) -> Result<(), Self::Err> {
+        use dbc::Entry;
+
+        match entry {
+            Entry::MessageDefinition(
+                dbc::MessageDefinition {
+                    id: _id,
+                    name,
+                    message_len,
+                    sending_node,
+                }) => {
+                self.name = name;
+                self.message_len = message_len;
+                self.sending_node = sending_node;
+                Ok(())
+            },
+            Entry::MessageDescription(
+                dbc::MessageDescription {
+                    id: _id,
+                    signal_name: _signal_name,
+                    description,
+                }) => {
+                self.description = Some(description);
+                Ok(())
+            },
+            Entry::MessageAttribute(
+                dbc::MessageAttribute {
+                    name,
+                    signal_name: _signal_name,
+                    id: _id,
+                    value,
+                }) => {
+                if let Some(_previous_value) = self.attributes.insert(name, value) {
+                    // TODO: Warn that we somehow already had an existing entry
+                }
+                Ok(())
+            },
+            Entry::SignalDefinition(inner) => {
+                if self.signals.contains_key(&inner.name) {
+                    (*self.signals.get_mut(&inner.name).expect("Already checked for Signal key"))
+                        .merge_entry(Entry::SignalDefinition(inner))
+                } else {
+                    let name = inner.name.clone();
+                    let signal = Signal::from_entry(Entry::SignalDefinition(inner))?;
+                    self.signals.insert(name, signal);
+                    Ok(())
+                }
+            },
+            Entry::SignalDescription(inner) => {
+                if self.signals.contains_key(&inner.signal_name) {
+                    (*self.signals.get_mut(&inner.signal_name).expect("Already checked for Signal key"))
+                        .merge_entry(Entry::SignalDescription(inner))
+                } else {
+                    let name = inner.signal_name.clone();
+                    let signal = Signal::from_entry(Entry::SignalDescription(inner))?;
+                    self.signals.insert(name, signal);
+                    Ok(())
+                }
+            },
+            Entry::SignalAttribute(inner) => {
+                if self.signals.contains_key(&inner.signal_name) {
+                    (*self.signals.get_mut(&inner.signal_name).expect("Already checked for Signal key"))
+                        .merge_entry(Entry::SignalAttribute(inner))
+                } else {
+                    let name = inner.signal_name.clone();
+                    let signal = Signal::from_entry(Entry::SignalAttribute(inner))?;
+                    self.signals.insert(name, signal);
+                    Ok(())
+                }
+            },
+            _ => Err(())
+        }
+    }
+}
+
+impl FromDbc for Signal {
+    type Err = ();
+
+    fn from_entry(entry: dbc::Entry) -> Result<Self, Self::Err> where
+        Self: Sized {
+        use dbc::Entry;
+        match entry {
+            Entry::SignalDefinition(definition) => {
+                Ok(Signal {
+                    attributes: HashMap::new(),
+                    description: None,
+                    definition: Some(definition),
+                    value_definition: None,
+                })
+            },
+            Entry::SignalDescription(
+                dbc::SignalDescription {
+                    id: _id,
+                    signal_name: _signal_name,
+                    description,
+                }) => {
+                Ok(Signal {
+                    attributes: HashMap::new(),
+                    description: Some(description),
+                    definition: None,
+                    value_definition: None,
+                })
+            },
+            Entry::SignalAttribute(dbc::SignalAttribute {
+                                       name,
+                                       id: _id,
+                                       signal_name: _signal_name,
+                                       value,
+                                   }) => {
+                let mut attributes = HashMap::new();
+                attributes.insert(name, value);
+                Ok(Signal {
+                    attributes,
+                    description: None,
+                    definition: None,
+                    value_definition: None
+                })
+            },
+            _ => Err(()),
+        }
+    }
+
+    fn merge_entry(&mut self, entry: dbc::Entry) -> Result<(), Self::Err> {
+        use dbc::Entry;
+        match entry {
+            Entry::SignalDefinition(definition) => {
+                self.definition = Some(definition);
+                Ok(())
+            },
+            Entry::SignalDescription(
+                dbc::SignalDescription {
+                    id: _id,
+                    signal_name: _signal_name,
+                    description,
+                }) => {
+                self.description = Some(description);
+                Ok(())
+            },
+            Entry::SignalAttribute(dbc::SignalAttribute {
+                                       name,
+                                       id: _id,
+                                       signal_name: _signal_name,
+                                       value,
+                                   }) => {
+                if let Some(_previous_value) = self.attributes.insert(name, value) {
+                    // TODO: Warn that we somehow already had an existing entry
+                }
+                Ok(())
+            },
+            _ => Err(()),
+        }
+    }
+}
+
+/// A struct that represents a CANdb file, and provides APIs for interacting
+/// with CAN messages and signals.
+#[derive(Clone, Debug, Default)]
+pub struct DbcLibrary {
+    last_id: Option<u32>,
+    messages: HashMap<u32, Message>,
+}
+
+use std::fs::File;
+use std::io;
+use std::io::prelude::*;
+use encoding::{DecoderTrap, Encoding};
+use encoding::all::ISO_8859_1;
+use std::path::Path;
+use super::{nom as nomparse, *};
+use nom;
+
+impl DbcLibrary {
+    /// Creates a new `DbcLibrary` instance given an existing lookup table.
+    pub fn new(messages: HashMap<u32, Message>) -> Self {
+        DbcLibrary {
+            last_id: None,
+            messages,
+        }
+    }
+
+    /// Convenience function for loading an entire DBC file into a returned `DbcLibrary`.  This
+    /// function ignores unparseable lines as well as `Entry` variants which don't apply to
+    /// `DbcLibrary` (such as `Entry::Version`).  Fails on `io::Error`.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// use canparse::dbc::DbcLibrary;
+    ///
+    /// let lib: DbcLibrary = DbcLibrary::from_dbc_file("./tests/data/sample.dbc").unwrap();
+    ///
+    /// ```
+    pub fn from_dbc_file<P>(path: P) -> io::Result<Self>
+        where
+            P: AsRef<Path>,
+    {
+        Self::from_encoded_dbc_file(path, ISO_8859_1)
+    }
+
+    #[doc(hidden)]
+    pub fn from_encoded_dbc_file<P, E>(path: P, encoding: &E) -> io::Result<Self>
+        where
+            P: AsRef<Path>,
+            E: Encoding,
+    {
+        let mut lib = DbcLibrary::default();
+
+        let data = File::open(path)
+            .and_then(|mut f| {
+                let mut contents: Vec<u8> = Vec::new();
+                f.read_to_end(&mut contents).map(|_bytes_read| contents)
+            })
+            .and_then(|mut contents| {
+                encoding
+                    .decode(contents.as_slice(), DecoderTrap::Replace)
+                    .map_err(|e| io::Error::new(io::ErrorKind::Other, e))
+            })?;
+
+        let mut i = data.as_str();
+        while !i.is_empty() {
+            match nomparse::entry(i) {
+                Ok((new_i, entry)) => {
+                    if let Err(_e) = lib.add_entry(entry) {
+                        // TODO: Handle add_entry error
+                    }
+                    i = new_i;
+                }
+                // FIXME: handling `IResult::Err`s could be better
+                Err(nom::Err::Incomplete(_)) => {
+                    break;
+                }
+                Err(_) => {
+                    i = &i[1..];
+                }
+            }
+        }
+
+        Ok(lib)
+    }
+}
+
+impl DbcLibrary {
+    pub fn add_entry(&mut self, entry: Entry) -> Result<(), String> {
+        let _id: u32 = *match entry {
+            Entry::MessageDefinition(dbc::MessageDefinition { ref id, .. }) => id,
+            Entry::MessageDescription(dbc::MessageDescription { ref id, .. }) => id,
+            Entry::MessageAttribute(dbc::MessageAttribute { ref id, .. }) => id,
+            Entry::SignalDefinition(..) => {
+                // no id, and by definition must follow MessageDefinition
+                if let Some(last_id) = self.last_id.as_ref() {
+                    last_id
+                } else {
+                    return Err("Tried to add SignalDefinition without last ID.".to_string());
+                }
+            }
+            Entry::SignalDescription(dbc::SignalDescription { ref id, .. }) => id,
+            Entry::SignalAttribute(dbc::SignalAttribute { ref id, .. }) => id,
+            _ => {
+                return Err(format!("Unsupported entry: {}.", entry).to_string());
+            }
+        };
+
+        self.messages.entry(_id)
+            .and_modify(|cur_entry| cur_entry.merge_entry(entry.clone())
+                .unwrap_or_else(|_| panic!("Already checked for Signal key: {:?}", entry))
+            ).or_insert_with(|| Message::from_entry(entry.clone())
+                .unwrap_or_else(|_| panic!("Some inserted a Signal for empty key: {:?}", _id)));
+
+        self.last_id = Some(_id);
+        Ok(())
+    }
+}
+
+#[cfg(test)]
+mod tests {
+
+    use dbc::{Entry, SignalDefinition, Version};
+    use super::{DbcLibrary};
+
+    lazy_static! {
+        static ref DBCLIB_EMPTY: DbcLibrary = DbcLibrary::default();
+        static ref DBCLIB_ONE: DbcLibrary = DbcLibrary::from_dbc_file("./tests/data/sample.dbc")
+            .expect("Failed to create DbcLibrary from file");
+        static ref SIGNALDEF: SignalDefinition = SignalDefinition {
+            name: "Engine_Speed".to_string(),
+            start_bit: 24,
+            bit_len: 16,
+            little_endian: true,
+            signed: false,
+            scale: 0.125,
+            offset: 0.0,
+            min_value: 0.0,
+            max_value: 8031.88,
+            units: "rpm".to_string(),
+            receiving_node: "Vector__XXX".to_string()
+        };
+        static ref SIGNALDEF_BE: SignalDefinition = {
+            let mut _spndef = SIGNALDEF.clone();
+            _spndef.little_endian = false;
+            _spndef
+        };
+        static ref MSG: [u8; 8] = [0x11, 0x22, 0x33, 0x44, 0x55, 0x66, 0x77, 0x88];
+        static ref MSG_BE: [u8; 8] = [0x88, 0x77, 0x66, 0x55, 0x44, 0x33, 0x22, 0x11];
+    }
+
+    #[test]
+    fn default_pgnlibrary() {
+        assert_eq!(DBCLIB_EMPTY.messages.len(), 0);
+    }
+
+    #[test]
+    fn get_spndefinition() {
+        assert_eq!(
+            *DBCLIB_ONE
+                .messages
+                .get(&2364539904)
+                .expect("failed to get DbcDefinition from DbcLibrary")
+                .signals
+                .get("Engine_Speed")
+                .expect("failed to get Signal from DbcDefinition")
+                .definition.as_ref()
+                .expect("failed to get SignalDefinition from DbcDefinition")
+            ,*SIGNALDEF
+        );
+    }
+
+    #[test]
+    fn unsupported_entry() {
+        let mut dbclib: DbcLibrary = DbcLibrary::default();
+        let unsupported = Entry::Version(Version("Don't care about version entry".to_string()));
+        let res = dbclib.add_entry(unsupported);
+
+        assert!(res.is_err(), "Unsupported entry: Version".to_string());
+    }
+}
diff --git a/src/dbc.rs b/src/dbc/mod.rs
similarity index 58%
rename from src/dbc.rs
rename to src/dbc/mod.rs
index 5835c09..8200845 100644
--- a/src/dbc.rs
+++ b/src/dbc/mod.rs
@@ -2,45 +2,47 @@
 
 #![allow(non_upper_case_globals)]
 
-use enum_primitive::FromPrimitive;
-use regex::{Regex, RegexSet};
-use rustc_serialize::{Decodable, Decoder};
-use std;
 use std::error::Error;
 use std::fmt;
 use std::fmt::{Display, Formatter};
 use std::str::FromStr;
 
-#[derive(Debug, PartialEq, Clone)]
+pub mod library;
+pub mod nom;
+
+pub use self::library::DbcLibrary;
+
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct Version(pub String);
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct BusConfiguration(pub f32);
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct MessageDefinition {
-    pub id: String,
+    pub id: u32,
     pub name: String,
     pub message_len: u32,
     pub sending_node: String,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct MessageDescription {
-    pub id: String,
+    pub id: u32,
+    // TODO: Remove this
     pub signal_name: String,
     pub description: String,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct MessageAttribute {
     pub name: String,
-    pub id: String,
+    pub id: u32,
     pub signal_name: String,
     pub value: String,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub struct SignalDefinition {
     pub name: String,
     pub start_bit: usize,
@@ -55,23 +57,23 @@ pub struct SignalDefinition {
     pub receiving_node: String,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct SignalDescription {
-    pub id: String,
+    pub id: u32,
     pub signal_name: String,
     pub description: String,
 }
 
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct SignalAttribute {
     pub name: String,
-    pub id: String,
+    pub id: u32,
     pub signal_name: String,
     pub value: String,
 }
 
 /// Composed DBC entry.
-#[derive(Debug, PartialEq, Clone)]
+#[derive(Debug, Clone, PartialEq)]
 pub enum Entry {
     /// `VERSION`
     Version(Version),
@@ -138,7 +140,7 @@ impl Display for Entry {
 
 /// Internal type for DBC `Entry` line.
 enum_from_primitive! {
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub enum EntryType {
     Version = 0,
 
@@ -151,10 +153,12 @@ pub enum EntryType {
     MessageDefinition,
     MessageDescription,
     MessageAttribute,
+//    MessageAttributeDefinition,
 
     SignalDefinition,
     SignalDescription,
     SignalAttribute,
+//    SignalAttributeDefinition,
 
     // AttributeDefinition,
     // AttributeDefault,
@@ -182,242 +186,8 @@ impl Display for EntryType {
     }
 }
 
-pub mod nom {
-    use super::*;
-    use nom::{alphanumeric, anychar, digit, float, line_ending, space, space0};
-    use std::str::FromStr;
-
-    // TODO: convert `tag!(" ")` to `space`
-
-    named! {
-        quoted_str<&str, String>,
-        map!(
-            delimited!(
-                tag!("\""),
-                escaped_transform!(
-                    none_of!("\\\""),
-                    '\\',
-                    alt!(
-                        tag!("\\") => { |_| "\\" }
-                      | tag!("\"") => { |_| "\"" }
-                    )),
-                tag!("\"")),
-            |data| data)
-    }
-
-    named!(pub entry<&str, Entry>, alt!(
-        version                => { |r| Entry::Version(r) } |
-        bus_configuration      => { |r| Entry::BusConfiguration(r) } |
-        message_definition     => { |r| Entry::MessageDefinition(r) } |
-        message_description    => { |r| Entry::MessageDescription(r) } |
-        message_attribute      => { |r| Entry::MessageAttribute(r) } |
-        signal_definition      => { |r| Entry::SignalDefinition(r) } |
-        signal_description     => { |r| Entry::SignalDescription(r) } |
-        signal_attribute       => { |r| Entry::SignalAttribute(r) } |
-        unknown                => { |r| Entry::Unknown(r) }
-    ));
-
-    named!(pub unknown<&str, String>,
-        do_parse!(
-            // FIXME: many0!(quoted_str) >> line_ending
-            data: take_until_either!("\r\n") >>
-            line_ending >>
-            ( data.to_string() )
-        )
-    );
-
-    named!(pub version<&str, Version>,
-        do_parse!(
-            tag!("VERSION")   >>
-            tag!(" ")   >>
-            data: quoted_str >>
-            line_ending >>
-            ( Version(data) )
-        )
-    );
-
-    named!(pub bus_configuration<&str, BusConfiguration>,
-        do_parse!(
-            tag!("BS_:")   >>
-            tag!(" ")   >>
-            data: map_res!(
-                take_until_either!("\r\n"),
-                FromStr::from_str) >>
-            line_ending >>
-            ( BusConfiguration(data) )
-        )
-    );
-
-    // FIXME: `space` isn't really correct since there should only be ONE (probably need alt)
-    named!(pub message_definition<&str, MessageDefinition>,
-        do_parse!(
-            tag!("BO_")   >>
-            space >>
-            id: digit >>
-            space >>
-            name: alphanumeric >>
-            space0 >>
-            tag!(":")   >>
-            space >>
-            len: map_res!(
-                digit,
-                FromStr::from_str) >>
-            space >>
-            sending_node: take_until_either!(" \t\r\n") >>
-            space0 >>
-            line_ending >>
-            ( MessageDefinition {
-                id: id.to_string(),
-                name: name.to_string(),
-                message_len: len,
-                sending_node: sending_node.to_string(),
-            } )
-        )
-    );
-
-    named!(pub message_description<&str, MessageDescription>,
-        do_parse!(
-            tag!("CM_")   >>
-            space >>
-            tag!("BO_")   >>
-            space >>
-            id: digit >>
-            space >>
-            description: quoted_str >>
-            tag!(";") >>
-            line_ending >>
-            ( MessageDescription {
-                id: id.to_string(),
-                signal_name: "".to_string(),
-                description: description.to_string(),
-            } )
-        )
-    );
-
-    named!(pub message_attribute<&str, MessageAttribute>,
-        do_parse!(
-            tag!("BA_")   >>
-            space >>
-            name: quoted_str >>
-            space >>
-            tag!("BO_")   >>
-            space >>
-            id: digit >>
-            space >>
-            value: digit >>
-            tag!(";") >>
-            line_ending >>
-            ( MessageAttribute {
-                name: name.to_string(),
-                signal_name: "".to_string(),
-                id: id.to_string(),
-                value: value.to_string()
-            } )
-        )
-    );
-
-    named!(pub signal_definition<&str, SignalDefinition>,
-        do_parse!(
-            space >>
-            tag!("SG_") >>
-            space >>
-            name: take_until_either!(" \t") >>
-            space >>
-            tag!(":") >>
-            space >>
-            start_bit: map_res!(
-                digit,
-                FromStr::from_str) >>
-            tag!("|") >>
-            bit_len: map_res!(
-                digit,
-                FromStr::from_str) >>
-            tag!("@") >>
-            little_endian: map!(digit, |d| d == "1") >>
-            signed: alt!(
-                tag!("+") => { |_| false } |
-                tag!("-") => { |_| true } ) >>
-            space >>
-            tag!("(") >>
-            scale: float >>
-            tag!(",") >>
-            offset: float >>
-            tag!(")") >>
-            space >>
-            tag!("[") >>
-            min_value: float >>
-            tag!("|") >>
-            max_value: float >>
-            tag!("]") >>
-            space >>
-            units: quoted_str >>
-            space >>
-            receiving_node: take_until_either!(" \t\r\n") >>
-            line_ending >>
-            ( SignalDefinition {
-                name: name.to_string(),
-                start_bit: start_bit,
-                bit_len: bit_len,
-                little_endian: little_endian,
-                signed: signed,
-                scale: scale,
-                offset: offset,
-                min_value: min_value,
-                max_value: max_value,
-                units: units.to_string(),
-                receiving_node: receiving_node.to_string(),
-            } )
-        )
-    );
-
-    named!(pub signal_description<&str, SignalDescription>,
-        do_parse!(
-            tag!("CM_")   >>
-            space >>
-            tag!("SG_")   >>
-            space >>
-            id: digit >>
-            space >>
-            signal_name: take_until_either!(" \t") >>
-            space >>
-            description: quoted_str >>
-            tag!(";") >>
-            line_ending >>
-            ( SignalDescription {
-                id: id.to_string(),
-                signal_name: signal_name.to_string(),
-                description: description.to_string()
-            } )
-        )
-    );
-
-    named!(pub signal_attribute<&str, SignalAttribute>,
-        do_parse!(
-            tag!("BA_")   >>
-            space >>
-            name: quoted_str >>
-            space >>
-            tag!("SG_")   >>
-            space >>
-            id: digit >>
-            space >>
-            signal_name: take_until_either!(" \t") >>
-            space >>
-            value: digit >>
-            tag!(";") >>
-            line_ending >>
-            ( SignalAttribute {
-                name: name.to_string(),
-                id: id.to_string(),
-                signal_name: signal_name.to_string(),
-                value: value.to_string()
-            } )
-        )
-    );
-}
-
 /// Error returned on failure to parse DBC `Entry`.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 pub struct ParseEntryError {
     kind: EntryErrorKind,
 }
@@ -451,7 +221,7 @@ impl Error for ParseEntryError {
 }
 
 /// Internal type DBC `Entry` parsing error.
-#[derive(Debug, Clone, PartialEq, Eq)]
+#[derive(Debug, Clone, Eq, PartialEq)]
 enum EntryErrorKind {
     /// Could not find a regex match for input
     RegexNoMatch,
@@ -505,6 +275,30 @@ impl FromStr for Entry {
     }
 }
 
+/// Probably some spec to determine a type when generating structs
+/// Here an enum will be dispatched instead (e.g., VAL_)
+#[derive(Clone, Debug, Default, Eq, PartialEq)]
+pub struct ValueDefinition {
+    values: Vec<String>,
+}
+
+pub enum AttributeType {
+    /// Integer type with min/max values
+    Int {
+        min: i32,
+        max: i32,
+    },
+    /// Float type with min/max values
+    Float {
+        min: f32,
+        max: f32,
+    },
+    /// String type
+    String,
+    /// Enum type, represented as a vector of `String`s
+    Enum(Vec<String>),
+}
+
 #[cfg(test)]
 mod tests {
     use std::str::FromStr;
@@ -570,7 +364,7 @@ mod tests {
         MessageDefinition,
         "BO_ 2364539904 EEC1 : 8 Vector__XXX\n",
         MessageDefinition {
-            id: "2364539904".to_string(),
+            id: 2364539904,
             name: "EEC1".to_string(),
             message_len: 8,
             sending_node: "Vector__XXX".to_string()
@@ -582,7 +376,7 @@ mod tests {
         MessageDescription,
         "CM_ BO_ 2364539904 \"Engine Controller\";\n",
         MessageDescription {
-            id: "2364539904".to_string(),
+            id: 2364539904,
             signal_name: "".to_string(),
             description: "Engine Controller".to_string()
         }
@@ -595,7 +389,7 @@ mod tests {
         MessageAttribute {
             name: "SingleFrame".to_string(),
             signal_name: "".to_string(),
-            id: "2364539904".to_string(),
+            id: 2364539904,
             value: "0".to_string()
         }
     );
@@ -624,7 +418,7 @@ mod tests {
         SignalDescription,
         "CM_ SG_ 2364539904 Engine_Speed \"A description for Engine speed.\";\n",
         SignalDescription {
-            id: "2364539904".to_string(),
+            id: 2364539904,
             signal_name: "Engine_Speed".to_string(),
             description: "A description for Engine speed.".to_string()
         }
@@ -636,7 +430,7 @@ mod tests {
         "BA_ \"SPN\" SG_ 2364539904 Engine_Speed 190;\n",
         SignalAttribute {
             name: "SPN".to_string(),
-            id: "2364539904".to_string(),
+            id: 2364539904,
             signal_name: "Engine_Speed".to_string(),
             value: "190".to_string()
         }
@@ -650,7 +444,7 @@ mod tests {
              \r \
              line description for Engine torque.\";\n",
             SignalDescription {
-                id: "2364539904".to_string(),
+                id: 2364539904,
                 signal_name: "Actual_Engine___Percent_Torque_High_Resolution".to_string(),
                 description: "A multi- \r \
                               \r \
diff --git a/src/dbc/nom.rs b/src/dbc/nom.rs
new file mode 100644
index 0000000..3f25298
--- /dev/null
+++ b/src/dbc/nom.rs
@@ -0,0 +1,243 @@
+//! Nom-based parsers for Entry types
+
+use super::*;
+use nom::{alphanumeric, digit, float, line_ending, space, space0};
+use std::str::FromStr;
+
+// TODO: convert `tag!(" ")` to `space`
+
+named! {
+    quoted_str<&str, String>,
+    map!(
+        delimited!(
+            tag!("\""),
+            escaped_transform!(
+                none_of!("\\\""),
+                '\\',
+                alt!(
+                    tag!("\\") => { |_| "\\" }
+                  | tag!("\"") => { |_| "\"" }
+                )),
+            tag!("\"")),
+        |data| data)
+}
+
+named!(pub entry<&str, Entry>, alt!(
+    version                => { |r| Entry::Version(r) } |
+    bus_configuration      => { |r| Entry::BusConfiguration(r) } |
+    message_definition     => { |r| Entry::MessageDefinition(r) } |
+    message_description    => { |r| Entry::MessageDescription(r) } |
+    message_attribute      => { |r| Entry::MessageAttribute(r) } |
+    signal_definition      => { |r| Entry::SignalDefinition(r) } |
+    signal_description     => { |r| Entry::SignalDescription(r) } |
+    signal_attribute       => { |r| Entry::SignalAttribute(r) } |
+    unknown                => { |r| Entry::Unknown(r) }
+));
+
+named!(pub unknown<&str, String>,
+    do_parse!(
+        // FIXME: many0!(quoted_str) >> line_ending
+        data: take_until_either!("\r\n") >>
+        line_ending >>
+        ( data.to_string() )
+    )
+);
+
+named!(pub version<&str, Version>,
+    do_parse!(
+        tag!("VERSION")   >>
+        tag!(" ")   >>
+        data: quoted_str >>
+        line_ending >>
+        ( Version(data) )
+    )
+);
+
+named!(pub bus_configuration<&str, BusConfiguration>,
+    do_parse!(
+        tag!("BS_:")   >>
+        tag!(" ")   >>
+        data: map_res!(
+            take_until_either!("\r\n"),
+            FromStr::from_str) >>
+        line_ending >>
+        ( BusConfiguration(data) )
+    )
+);
+
+// FIXME: `space` isn't really correct since there should only be ONE (probably need alt)
+named!(pub message_definition<&str, MessageDefinition>,
+    do_parse!(
+        tag!("BO_")   >>
+        space >>
+        id: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        name: alphanumeric >>
+        space0 >>
+        tag!(":")   >>
+        space >>
+        len: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        sending_node: take_until_either!(" \t\r\n") >>
+        space0 >>
+        line_ending >>
+        ( MessageDefinition {
+            id: id,
+            name: name.to_string(),
+            message_len: len,
+            sending_node: sending_node.to_string(),
+        } )
+    )
+);
+
+named!(pub message_description<&str, MessageDescription>,
+    do_parse!(
+        tag!("CM_")   >>
+        space >>
+        tag!("BO_")   >>
+        space >>
+        id: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        description: quoted_str >>
+        tag!(";") >>
+        line_ending >>
+        ( MessageDescription {
+            id: id,
+            signal_name: "".to_string(),
+            description: description.to_string(),
+        } )
+    )
+);
+
+named!(pub message_attribute<&str, MessageAttribute>,
+    do_parse!(
+        tag!("BA_")   >>
+        space >>
+        name: quoted_str >>
+        space >>
+        tag!("BO_")   >>
+        space >>
+        id: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        value: digit >>
+        tag!(";") >>
+        line_ending >>
+        ( MessageAttribute {
+            name: name.to_string(),
+            signal_name: "".to_string(),
+            id: id,
+            value: value.to_string()
+        } )
+    )
+);
+
+named!(pub signal_definition<&str, SignalDefinition>,
+    do_parse!(
+        space >>
+        tag!("SG_") >>
+        space >>
+        name: take_until_either!(" \t") >>
+        space >>
+        tag!(":") >>
+        space >>
+        start_bit: map_res!(
+            digit,
+            FromStr::from_str) >>
+        tag!("|") >>
+        bit_len: map_res!(
+            digit,
+            FromStr::from_str) >>
+        tag!("@") >>
+        little_endian: map!(digit, |d| d == "1") >>
+        signed: alt!(
+            tag!("+") => { |_| false } |
+            tag!("-") => { |_| true } ) >>
+        space >>
+        tag!("(") >>
+        scale: float >>
+        tag!(",") >>
+        offset: float >>
+        tag!(")") >>
+        space >>
+        tag!("[") >>
+        min_value: float >>
+        tag!("|") >>
+        max_value: float >>
+        tag!("]") >>
+        space >>
+        units: quoted_str >>
+        space >>
+        receiving_node: take_until_either!(" \t\r\n") >>
+        line_ending >>
+        ( SignalDefinition {
+            name: name.to_string(),
+            start_bit: start_bit,
+            bit_len: bit_len,
+            little_endian: little_endian,
+            signed: signed,
+            scale: scale,
+            offset: offset,
+            min_value: min_value,
+            max_value: max_value,
+            units: units.to_string(),
+            receiving_node: receiving_node.to_string(),
+        } )
+    )
+);
+
+named!(pub signal_description<&str, SignalDescription>,
+    do_parse!(
+        tag!("CM_")   >>
+        space >>
+        tag!("SG_")   >>
+        space >>
+        id: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        signal_name: take_until_either!(" \t") >>
+        space >>
+        description: quoted_str >>
+        tag!(";") >>
+        line_ending >>
+        ( SignalDescription {
+            id: id,
+            signal_name: signal_name.to_string(),
+            description: description.to_string()
+        } )
+    )
+);
+
+named!(pub signal_attribute<&str, SignalAttribute>,
+    do_parse!(
+        tag!("BA_")   >>
+        space >>
+        name: quoted_str >>
+        space >>
+        tag!("SG_")   >>
+        space >>
+        id: map_res!(
+            digit,
+            FromStr::from_str) >>
+        space >>
+        signal_name: take_until_either!(" \t") >>
+        space >>
+        value: digit >>
+        tag!(";") >>
+        line_ending >>
+        ( SignalAttribute {
+            name: name.to_string(),
+            id: id,
+            signal_name: signal_name.to_string(),
+            value: value.to_string()
+        } )
+    )
+);
diff --git a/src/lib.rs b/src/lib.rs
index 0d2b689..a3a9a2c 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -34,11 +34,7 @@
 //! }
 //! ```
 
-#![cfg_attr(feature = "cargo-clippy", allow(redundant_field_names))]
-#![allow(non_snake_case, non_camel_case_types)]
-#![allow(unused_variables, dead_code)]
-#![allow(unused_imports, unused_mut)]
-#![allow(plugin_as_library)]
+#![cfg_attr(feature = "cargo-clippy", allow(clippy::redundant_field_names))]
 #![crate_name = "canparse"]
 
 extern crate encoding;
diff --git a/src/pgn.rs b/src/pgn.rs
index 4e4fa14..b2cfc77 100644
--- a/src/pgn.rs
+++ b/src/pgn.rs
@@ -5,7 +5,6 @@ use dbc::{nom as nomparse, *};
 use encoding::all::ISO_8859_1;
 use encoding::{CodecError, DecoderTrap, Encoding};
 use nom;
-use regex::Regex;
 use std::collections::HashMap;
 use std::error::Error;
 use std::fmt;
@@ -13,7 +12,6 @@ use std::fmt::{Display, Formatter};
 use std::fs::File;
 use std::io;
 use std::io::prelude::*;
-use std::io::BufReader;
 use std::marker::Sized;
 use std::path::Path;
 use std::str::FromStr;
@@ -158,24 +156,23 @@ impl PgnLibrary {
     /// }
     /// ```
     pub fn add_entry(&mut self, entry: Entry) -> Result<(), String> {
-        let _id: u32 = match entry {
-            Entry::MessageDefinition(MessageDefinition { ref id, .. }) => id.parse(),
-            Entry::MessageDescription(MessageDescription { ref id, .. }) => id.parse(),
-            Entry::MessageAttribute(MessageAttribute { ref id, .. }) => id.parse(),
+        let _id: u32 = *match entry {
+            Entry::MessageDefinition(MessageDefinition { ref id, .. }) => id,
+            Entry::MessageDescription(MessageDescription { ref id, .. }) => id,
+            Entry::MessageAttribute(MessageAttribute { ref id, .. }) => id,
             Entry::SignalDefinition(..) => {
                 // no id, and by definition must follow MessageDefinition
                 if self.last_id == 0 {
                     return Err("Tried to add SignalDefinition without last ID.".to_string());
                 }
-                Ok(self.last_id)
+                &self.last_id
             }
-            Entry::SignalDescription(SignalDescription { ref id, .. }) => id.parse(),
-            Entry::SignalAttribute(SignalAttribute { ref id, .. }) => id.parse(),
+            Entry::SignalDescription(SignalDescription { ref id, .. }) => id,
+            Entry::SignalAttribute(SignalAttribute { ref id, .. }) => id,
             _ => {
                 return Err(format!("Unsupported entry: {}.", entry).to_string());
             }
-        }
-        .unwrap();
+        };
 
         // CanId{ DP, PF, PS, SA } => Pgn{ PF, PS }
         let pgn = (_id >> 8) & 0x1FFFF;
@@ -348,7 +345,7 @@ impl FromDbc for PgnDefinition {
                 message_len,
                 sending_node,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 Ok(PgnDefinition::new(
                     pgn,
@@ -364,7 +361,7 @@ impl FromDbc for PgnDefinition {
                 signal_name,
                 description,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 Ok(PgnDefinition::new(
                     pgn,
@@ -381,7 +378,7 @@ impl FromDbc for PgnDefinition {
                 id,
                 value,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 Ok(PgnDefinition::new(
                     pgn,
@@ -404,7 +401,7 @@ impl FromDbc for PgnDefinition {
                 message_len,
                 sending_node,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 self.pgn = pgn;
                 self.pgn_long = pgn_long;
@@ -416,7 +413,7 @@ impl FromDbc for PgnDefinition {
                 signal_name,
                 description,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 self.pgn = pgn;
                 self.pgn_long = pgn_long;
@@ -429,7 +426,7 @@ impl FromDbc for PgnDefinition {
                 id,
                 value,
             }) => {
-                let pgn_long = id.parse::<u32>().unwrap();
+                let pgn_long = id;
                 let pgn = pgn_long & 0x1FFFF;
                 self.pgn = pgn;
                 self.pgn_long = pgn_long;
@@ -488,7 +485,7 @@ impl FromDbc for PgnDefinition {
 pub struct SpnDefinition {
     name: String,
     pub number: usize,
-    id: String,
+    id: u32,
     description: String,
     start_bit: usize,
     bit_len: usize,
@@ -571,7 +568,7 @@ impl SpnDefinition {
     pub fn new(
         name: String,
         number: usize,
-        id: String,
+        id: u32,
         description: String,
         start_bit: usize,
         bit_len: usize,
@@ -783,7 +780,7 @@ impl From<SignalDefinition> for SpnDefinition {
         SpnDefinition::new(
             name,
             0,
-            "".to_string(),
+            0, // TODO: Some()?
             "".to_string(),
             start_bit,
             bit_len,
@@ -862,7 +859,7 @@ mod tests {
         static ref SPNDEF: SpnDefinition = SpnDefinition::new(
             "Engine_Speed".to_string(),
             190,
-            "2364539904".to_string(),
+            2364539904,
             "A description for Engine speed.".to_string(),
             24,
             16,


Test patch: