Repository: jmagnuson/canparse (language: rust)

Issue:
Canparse incompatible with ISO-8859-1 DBC files
I ran into some issues using canparse. After some debugging I found out that it can't parse dbc files in iso-8859-1 encoding. You get this kind of encoding as soon as you use characters like ° (for temperature).

Line 65 in pgn.rs gives an Err(..) back when an not UFT-8 is included in the read line


Discussion / hints:
Interesting, thanks for the notice-- I have only ever worked in UTF-8 but agree that other encodings could definitely be supported as well.
To convert it to UTF-8 helped. But then the program bugged out on a later stage in the file. I line 68 in pgn.rs with Err` value: ParseIntError { kind: InvalidDigit }'

The responsible line is the middle one. 
BA_ "VNSigAllNodes" SG_ 160 StWhlAngChksm 1;
BA_ "SignalLongName" SG_ 160 StWhlAngChksm "Steering Wheel Angle Checksum";
BA_ "SignalType" SG_ 160 StWhlAngChksm "UNM";

When is parse it it finds a entry which looks like this an crashes: 
SignalDefinition(SignalDefinition{start_bit:140737331213568, bit_len:33, little_endian:false, signed:true, scale:0.0000000000000000000000000000000000000000000126116862, offset:0, min_value:0.0000000000000000000000000000000000000000000140129846, max_value:0, units:None, receiving_node:None})

Did you ever tryout your parser with a dbc file created with Vector CANDB++ ?
Howdy! Just ran into the same issue attempting to load a DBC which was created by a German company and so this crate was unable to parse it, failing with the same error (not utf-8) due to some identifiers and string content containing umlauts and so on.
Encoding of this file is reported as: `ISO-8859 text, with very long lines, with CRLF line terminators`.

Worth noting this was also created by the Vector tools.
So the ISO-8859-1 issue should be a pretty quick fix, I can get that out soon.

The `ParseIntError` may require some rework and possible API breakage, due to how the library is currently abstracted. But I've wanted to make this crate more generic anyway, so some amount of breakage is inevitable. Thanks for the reports!

Fix patch:
diff --git a/CHANGELOG.md b/CHANGELOG.md
index 876d670..466ebff 100644
--- a/CHANGELOG.md
+++ b/CHANGELOG.md
@@ -11,6 +11,10 @@ and this project adheres to [Semantic Versioning](http://semver.org).
 - Change `get_spn` to take `&str` input
 - Speed up parsing PGN from `&[u8]`
 
+### Fixed
+- Explicitly parse file passed into `PgnLibrary::from_dbc_file` using
+  ISO-8859-1 codec
+
 ## [0.1.2] - 2018-06-09
 
 ### Changed
diff --git a/Cargo.toml b/Cargo.toml
index 219815f..d593b02 100644
--- a/Cargo.toml
+++ b/Cargo.toml
@@ -21,6 +21,7 @@ use-socketcan = ["socketcan"]
 rustc-serialize = "0.3.24"
 regex = "0.2.2"
 lazy_static = "0.2.8"
+encoding = "0.2"
 enum_primitive = "0.1.1"
 num = "0.1.37"
 byteorder = "1.0.0"
diff --git a/src/lib.rs b/src/lib.rs
index 54e1e9f..4c24c9b 100644
--- a/src/lib.rs
+++ b/src/lib.rs
@@ -43,6 +43,7 @@
 
 #![crate_name = "canparse"]
 
+extern crate encoding;
 #[macro_use] extern crate enum_primitive;
 #[macro_use] extern crate lazy_static;
 extern crate num;
diff --git a/src/pgn.rs b/src/pgn.rs
index 4deff1f..d92b827 100644
--- a/src/pgn.rs
+++ b/src/pgn.rs
@@ -1,5 +1,7 @@
 //! Signal processing using PGN/SPN definitions.
 
+use encoding::{CodecError, DecoderTrap, Encoding};
+use encoding::all::ISO_8859_1;
 use regex::Regex;
 use std::collections::HashMap;
 use std::error::Error;
@@ -45,7 +47,7 @@ impl PgnLibrary {
     }
 
     /// Convenience function for loading an entire DBC file into a returned `PgnLibrary`.  This
-    /// function ignores unparsable lines as well as `Entry` variants which don't apply to
+    /// function ignores unparseable lines as well as `Entry` variants which don't apply to
     /// `PgnLibrary` (such as `Entry::Version`).  Fails on `io::Error`.
     ///
     /// # Example
@@ -56,16 +58,57 @@ impl PgnLibrary {
     /// let lib: PgnLibrary = PgnLibrary::from_dbc_file("./tests/data/sample.dbc").unwrap();
     ///
     /// ```
-    pub fn from_dbc_file<P: AsRef<Path>>(path: P) -> io::Result<Self> {
-        let mut lib = PgnLibrary::default();
+    pub fn from_dbc_file<P>(path: P) -> io::Result<Self>
+    where
+        P: AsRef<Path>,
+    {
+        Self::from_encoded_dbc_file(path, ISO_8859_1)
+    }
 
-        let f = File::open(path)?;
-        let f = BufReader::new(f);
+    /// Convenience function for loading an entire DBC file into a returned `PgnLibrary`, using
+    /// a specified `Encoding` codec. This function ignores unparseable lines as well as `Entry`
+    /// variants which don't apply to `PgnLibrary` (such as `Entry::Version`).
+    ///
+    /// This function is currently considered unstable and subject to change or removal.
+    ///
+    /// # Example
+    ///
+    /// ```rust
+    /// extern crate canparse;
+    /// extern crate encoding;
+    ///
+    /// use canparse::pgn::PgnLibrary;
+    /// use encoding::Encoding;
+    /// use encoding::all::ISO_8859_1;
+    ///
+    /// let lib: PgnLibrary = PgnLibrary::from_encoded_dbc_file("./tests/data/sample.dbc",
+    ///                                                         ISO_8859_1).unwrap();
+    ///
+    /// ```
+    #[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 = PgnLibrary::default();
 
-        for l in f.lines() {
-            let line = l?;
-            if let Ok(entry) = Entry::from_str(line.as_str()) {
-                lib.add_entry(entry).ok();
+        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))
+            })?;
+
+        for (idx, line) in data.lines().enumerate() {
+            if let Ok(entry) = Entry::from_str(line) {
+                if let Err(_e) = lib.add_entry(entry) {
+                    // TODO: Handle add_entry error
+                }
             }
         }
 
@@ -87,11 +130,13 @@ impl PgnLibrary {
     ///
     /// let mut lib = PgnLibrary::new( HashMap::default() );
     ///
-    /// let br = include_bytes!("../tests/data/sample.dbc");
+    /// // File is ISO-8859-1 and needs to be converted before iterating
+    /// // with `lines`. `PgnLibrary::from_dbc_file` does this for you.
+    /// let data: String = include_bytes!("../tests/data/sample.dbc")
+    ///     .iter().map(|b| *b as char).collect();
     ///
-    /// for l in br.lines() {
-    ///     let line = l.unwrap();
-    ///     if let Some(entry) = Entry::from_str(line.as_str()).ok() {
+    /// for line in data.lines() {
+    ///     if let Some(entry) = Entry::from_str(line).ok() {
     ///         lib.add_entry(entry).ok();
     ///     }
     /// }


Test patch:
diff --git a/tests/data/sample.dbc b/tests/data/sample.dbc
index 536da87..5f9028c 100644
--- a/tests/data/sample.dbc
+++ b/tests/data/sample.dbc
@@ -1,4 +1,4 @@
-VERSION "A version string"
+VERSION "A version string�"
 
 BO_ 2364539904 EEC1 : 8 Vector__XXX
 CM_ BO_ 2364539904 "Engine Controller";
diff --git a/tests/pgnlibrary.rs b/tests/pgnlibrary.rs
index df49083..b2c30af 100644
--- a/tests/pgnlibrary.rs
+++ b/tests/pgnlibrary.rs
@@ -5,18 +5,17 @@ use canparse::dbc::Entry;
 use canparse::pgn::{PgnLibrary, SpnDefinition, ParseMessage};
 use std::str::FromStr;
 use std::collections::HashMap;
-use std::io::BufRead;
 
 #[test]
 fn pgnlib_build_parse() {
     let mut lib = PgnLibrary::new( HashMap::default() );
 
-    let br = include_bytes!("./data/sample.dbc");
+    let data: String = include_bytes!("./data/sample.dbc")
+        .iter().map(|b| *b as char).collect();
 
     // Parse db lines into PgnLibrary
-    for l in br.lines() {
-        let line = l.unwrap();
-        if let Ok(entry) = Entry::from_str(line.as_str()) {
+    for line in data.lines() {
+        if let Ok(entry) = Entry::from_str(&line) {
             lib.add_entry(entry).ok();
         }
     }