From d132d23139eb854f34ebec94213b4aeb68d72c1f Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 13:15:15 +0200 Subject: [PATCH 01/11] `defmt-print`: Extract `fn obtain_location_info` --- print/src/main.rs | 47 +++++++++++++++++++++++++++++------------------ 1 file changed, 29 insertions(+), 18 deletions(-) diff --git a/print/src/main.rs b/print/src/main.rs index 5e9e0a60..a64a94a6 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -5,7 +5,7 @@ use std::{ }; use anyhow::anyhow; -use defmt_decoder::Table; +use defmt_decoder::{Frame, Locations, Table}; use structopt::StructOpt; /// Prints defmt-encoded logs to stdout @@ -56,30 +56,16 @@ fn main() -> anyhow::Result<()> { let current_dir = env::current_dir()?; let stdin = io::stdin(); let mut stdin = stdin.lock(); + loop { + // read from stdin and add it the the frames let n = stdin.read(&mut buf)?; - frames.extend_from_slice(&buf[..n]); loop { match table.decode(&frames) { Ok((frame, consumed)) => { - // NOTE(`[]` indexing) all indices in `table` have already been - // verified to exist in the `locs` map - let loc = locs.as_ref().map(|locs| &locs[&frame.index()]); - - let (mut file, mut line, mut mod_path) = (None, None, None); - if let Some(loc) = loc { - let relpath = if let Ok(relpath) = loc.file.strip_prefix(¤t_dir) { - relpath - } else { - // not relative; use full path - &loc.file - }; - file = Some(relpath.display().to_string()); - line = Some(loc.line as u32); - mod_path = Some(loc.module.clone()); - } + let (file, line, mod_path) = obtain_location_info(&locs, &frame, ¤t_dir); // Forward the defmt frame to our logger. defmt_decoder::log::log_defmt( @@ -103,6 +89,31 @@ fn main() -> anyhow::Result<()> { } } +fn obtain_location_info( + locs: &Option, + frame: &Frame, + current_dir: &PathBuf, +) -> (Option, Option, Option) { + let (mut file, mut line, mut mod_path) = (None, None, None); + + // NOTE(`[]` indexing) all indices in `table` have been verified to exist in the `locs` map + let loc = locs.as_ref().map(|locs| &locs[&frame.index()]); + + if let Some(loc) = loc { + let relpath = if let Ok(relpath) = loc.file.strip_prefix(¤t_dir) { + relpath + } else { + // not relative; use full path + &loc.file + }; + file = Some(relpath.display().to_string()); + line = Some(loc.line as u32); + mod_path = Some(loc.module.clone()); + } + + (file, line, mod_path) +} + /// Report version from Cargo.toml _(e.g. "0.1.4")_ and supported `defmt`-versions. /// /// Used by `--version` flag. From 13e368f3477e50a1eed4bc70ead07f9620b64c5a Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 13:15:53 +0200 Subject: [PATCH 02/11] `defmt-decoder`: Fix doc-comment for `Table::decode` --- decoder/src/lib.rs | 9 +++++---- 1 file changed, 5 insertions(+), 4 deletions(-) diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index 16ffdabf..077d124b 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -205,14 +205,15 @@ impl Table { elf2table::get_locations(elf, self) } - /// decode the data sent by the device using the previosuly stored metadata + /// Decode the data sent by the device using the previously stored metadata. /// - /// * bytes: contains the data sent by the device that logs. - /// contains the [log string index, timestamp, optional fmt string args] + /// * `bytes` + /// * contains the data sent by the device that logs. + /// * contains the [log string index, timestamp, optional fmt string args] pub fn decode<'t>( &'t self, mut bytes: &[u8], - ) -> Result<(Frame<'t>, /*consumed: */ usize), DecodeError> { + ) -> Result<(Frame<'t>, /* consumed: */ usize), DecodeError> { let len = bytes.len(); let index = bytes.read_u16::()? as u64; From 67323bedf9434c7805a48c613118b2e3f6cd1255 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 13:37:25 +0200 Subject: [PATCH 03/11] `defmt-print`: Add `fn forward_defmt_frame_to_logger` --- print/src/main.rs | 20 ++++++++++---------- 1 file changed, 10 insertions(+), 10 deletions(-) diff --git a/print/src/main.rs b/print/src/main.rs index a64a94a6..87b17668 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -65,15 +65,8 @@ fn main() -> anyhow::Result<()> { loop { match table.decode(&frames) { Ok((frame, consumed)) => { - let (file, line, mod_path) = obtain_location_info(&locs, &frame, ¤t_dir); - - // Forward the defmt frame to our logger. - defmt_decoder::log::log_defmt( - &frame, - file.as_deref(), - line, - mod_path.as_deref(), - ); + let location_info = obtain_location_info(&locs, &frame, ¤t_dir); + forward_defmt_frame_to_logger(&frame, location_info); let num_frames = frames.len(); frames.rotate_left(consumed); @@ -89,11 +82,13 @@ fn main() -> anyhow::Result<()> { } } +type LocationInfo = (Option, Option, Option); + fn obtain_location_info( locs: &Option, frame: &Frame, current_dir: &PathBuf, -) -> (Option, Option, Option) { +) -> LocationInfo { let (mut file, mut line, mut mod_path) = (None, None, None); // NOTE(`[]` indexing) all indices in `table` have been verified to exist in the `locs` map @@ -114,6 +109,11 @@ fn obtain_location_info( (file, line, mod_path) } +fn forward_defmt_frame_to_logger(frame: &Frame, location_info: LocationInfo) { + let (file, line, mod_path) = location_info; + defmt_decoder::log::log_defmt(&frame, file.as_deref(), line, mod_path.as_deref()); +} + /// Report version from Cargo.toml _(e.g. "0.1.4")_ and supported `defmt`-versions. /// /// Used by `--version` flag. From c4495b784172c91b6d1da33dca23e24a0160b41c Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 13:39:52 +0200 Subject: [PATCH 04/11] `defmt-print`: Switch to stream-decoder --- print/src/main.rs | 14 +++++--------- 1 file changed, 5 insertions(+), 9 deletions(-) diff --git a/print/src/main.rs b/print/src/main.rs index 87b17668..5d490a3b 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -51,7 +51,7 @@ fn main() -> anyhow::Result<()> { }; let mut buf = [0; READ_BUFFER_SIZE]; - let mut frames = vec![]; + let mut stream = table.new_stream_decoder(); let current_dir = env::current_dir()?; let stdin = io::stdin(); @@ -60,21 +60,17 @@ fn main() -> anyhow::Result<()> { loop { // read from stdin and add it the the frames let n = stdin.read(&mut buf)?; - frames.extend_from_slice(&buf[..n]); + stream.received(&buf[..n]); loop { - match table.decode(&frames) { - Ok((frame, consumed)) => { + match stream.decode() { + Ok(frame) => { let location_info = obtain_location_info(&locs, &frame, ¤t_dir); forward_defmt_frame_to_logger(&frame, location_info); - - let num_frames = frames.len(); - frames.rotate_left(consumed); - frames.truncate(num_frames - consumed); } Err(defmt_decoder::DecodeError::UnexpectedEof) => break, Err(defmt_decoder::DecodeError::Malformed) => { - log::error!("failed to decode defmt data: {:x?}", frames); + log::error!("failed to decode defmt data"); return Err(defmt_decoder::DecodeError::Malformed.into()); } } From 46e9fa832153850aa7afdc457b844d10033fc0ca Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 14:07:58 +0200 Subject: [PATCH 05/11] `defmt-print`: Implement recovery from decoding-errors --- decoder/src/lib.rs | 6 +++++- print/src/main.rs | 24 ++++++++++++++---------- 2 files changed, 19 insertions(+), 11 deletions(-) diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index 077d124b..dbb93359 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -119,7 +119,7 @@ struct BitflagsKey { } #[derive(Copy, Clone, Debug, PartialEq)] -enum Encoding { +pub enum Encoding { Raw, Rzcobs, } @@ -253,6 +253,10 @@ impl Table { Encoding::Rzcobs => Box::new(stream::Rzcobs::new(self)), } } + + pub fn encoding(&self) -> Encoding { + self.encoding + } } // NOTE follows `parser::Type` diff --git a/print/src/main.rs b/print/src/main.rs index 5d490a3b..67b9d804 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -5,7 +5,7 @@ use std::{ }; use anyhow::anyhow; -use defmt_decoder::{Frame, Locations, Table}; +use defmt_decoder::{DecodeError, Encoding, Frame, Locations, Table}; use structopt::StructOpt; /// Prints defmt-encoded logs to stdout @@ -51,28 +51,32 @@ fn main() -> anyhow::Result<()> { }; let mut buf = [0; READ_BUFFER_SIZE]; - let mut stream = table.new_stream_decoder(); + let mut stream_decoder = table.new_stream_decoder(); let current_dir = env::current_dir()?; let stdin = io::stdin(); let mut stdin = stdin.lock(); loop { - // read from stdin and add it the the frames + // read from stdin and push it to the decoder let n = stdin.read(&mut buf)?; - stream.received(&buf[..n]); + stream_decoder.received(&buf[..n]); + // decode the received data loop { - match stream.decode() { + match stream_decoder.decode() { Ok(frame) => { let location_info = obtain_location_info(&locs, &frame, ¤t_dir); forward_defmt_frame_to_logger(&frame, location_info); } - Err(defmt_decoder::DecodeError::UnexpectedEof) => break, - Err(defmt_decoder::DecodeError::Malformed) => { - log::error!("failed to decode defmt data"); - return Err(defmt_decoder::DecodeError::Malformed.into()); - } + Err(DecodeError::UnexpectedEof) => break, + Err(DecodeError::Malformed) => match table.encoding() { + // raw encoding doesn't allow for recovery. therefore we abort. + Encoding::Raw => return Err(DecodeError::Malformed.into()), + // rzcobs encoding allows recovery from decoding-errors. therefore we stop + // decoding the current, corrupted, data and continue with new one + Encoding::Rzcobs => break, + }, } } } From 066a9e59b05051853d9a3e7e366c1e960eb43591 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 15:05:07 +0200 Subject: [PATCH 06/11] `defmt-print`: Refactor --- print/src/main.rs | 13 +++---------- 1 file changed, 3 insertions(+), 10 deletions(-) diff --git a/print/src/main.rs b/print/src/main.rs index 67b9d804..9b60d46a 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -65,10 +65,7 @@ fn main() -> anyhow::Result<()> { // decode the received data loop { match stream_decoder.decode() { - Ok(frame) => { - let location_info = obtain_location_info(&locs, &frame, ¤t_dir); - forward_defmt_frame_to_logger(&frame, location_info); - } + Ok(frame) => forward_to_logger(&frame, location_info(&locs, &frame, ¤t_dir)), Err(DecodeError::UnexpectedEof) => break, Err(DecodeError::Malformed) => match table.encoding() { // raw encoding doesn't allow for recovery. therefore we abort. @@ -84,11 +81,7 @@ fn main() -> anyhow::Result<()> { type LocationInfo = (Option, Option, Option); -fn obtain_location_info( - locs: &Option, - frame: &Frame, - current_dir: &PathBuf, -) -> LocationInfo { +fn location_info(locs: &Option, frame: &Frame, current_dir: &PathBuf) -> LocationInfo { let (mut file, mut line, mut mod_path) = (None, None, None); // NOTE(`[]` indexing) all indices in `table` have been verified to exist in the `locs` map @@ -109,7 +102,7 @@ fn obtain_location_info( (file, line, mod_path) } -fn forward_defmt_frame_to_logger(frame: &Frame, location_info: LocationInfo) { +fn forward_to_logger(frame: &Frame, location_info: LocationInfo) { let (file, line, mod_path) = location_info; defmt_decoder::log::log_defmt(&frame, file.as_deref(), line, mod_path.as_deref()); } From d42de4e158e4cdb2df8d14b1778d34718193746f Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 15:53:49 +0200 Subject: [PATCH 07/11] Satisfy clippy --- print/src/main.rs | 14 +++++++------- 1 file changed, 7 insertions(+), 7 deletions(-) diff --git a/print/src/main.rs b/print/src/main.rs index 9b60d46a..19129ea5 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -1,7 +1,7 @@ use std::{ env, fs, io::{self, Read}, - path::PathBuf, + path::{Path, PathBuf}, }; use anyhow::anyhow; @@ -81,7 +81,12 @@ fn main() -> anyhow::Result<()> { type LocationInfo = (Option, Option, Option); -fn location_info(locs: &Option, frame: &Frame, current_dir: &PathBuf) -> LocationInfo { +fn forward_to_logger(frame: &Frame, location_info: LocationInfo) { + let (file, line, mod_path) = location_info; + defmt_decoder::log::log_defmt(frame, file.as_deref(), line, mod_path.as_deref()); +} + +fn location_info(locs: &Option, frame: &Frame, current_dir: &Path) -> LocationInfo { let (mut file, mut line, mut mod_path) = (None, None, None); // NOTE(`[]` indexing) all indices in `table` have been verified to exist in the `locs` map @@ -102,11 +107,6 @@ fn location_info(locs: &Option, frame: &Frame, current_dir: &PathBuf) (file, line, mod_path) } -fn forward_to_logger(frame: &Frame, location_info: LocationInfo) { - let (file, line, mod_path) = location_info; - defmt_decoder::log::log_defmt(&frame, file.as_deref(), line, mod_path.as_deref()); -} - /// Report version from Cargo.toml _(e.g. "0.1.4")_ and supported `defmt`-versions. /// /// Used by `--version` flag. From 3f7644185f224ff2691022b054f76e3e6e312c58 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 16:48:51 +0200 Subject: [PATCH 08/11] `defmt-decoder`: Make `enum Encoding` _non-exhaustive_ and add method `recoverable() -> bool` --- decoder/src/lib.rs | 10 ++++++++++ print/src/main.rs | 11 +++++------ 2 files changed, 15 insertions(+), 6 deletions(-) diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index dbb93359..e4300a56 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -119,6 +119,7 @@ struct BitflagsKey { } #[derive(Copy, Clone, Debug, PartialEq)] +#[non_exhaustive] pub enum Encoding { Raw, Rzcobs, @@ -136,6 +137,15 @@ impl FromStr for Encoding { } } +impl Encoding { + pub const fn recoverable(&self) -> bool { + match self { + Encoding::Raw => false, + Encoding::Rzcobs => true, + } + } +} + /// Internal table that holds log levels and maps format strings to indices #[derive(Debug, PartialEq)] pub struct Table { diff --git a/print/src/main.rs b/print/src/main.rs index 19129ea5..b79d5b1c 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -67,12 +67,11 @@ fn main() -> anyhow::Result<()> { match stream_decoder.decode() { Ok(frame) => forward_to_logger(&frame, location_info(&locs, &frame, ¤t_dir)), Err(DecodeError::UnexpectedEof) => break, - Err(DecodeError::Malformed) => match table.encoding() { - // raw encoding doesn't allow for recovery. therefore we abort. - Encoding::Raw => return Err(DecodeError::Malformed.into()), - // rzcobs encoding allows recovery from decoding-errors. therefore we stop - // decoding the current, corrupted, data and continue with new one - Encoding::Rzcobs => break, + Err(DecodeError::Malformed) => match table.encoding().recoverable() { + // if recovery is impossible, abort + false => return Err(DecodeError::Malformed.into()), + // if recovery is possible, skip the current frame and continue with new data + true => continue, }, } } From c4ebd14ccd1a44eb6cc5dc5182c8a4ad6d024b28 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 17:05:36 +0200 Subject: [PATCH 09/11] `defmt-print`: Rm unused import --- print/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/print/src/main.rs b/print/src/main.rs index b79d5b1c..e83ba113 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -5,7 +5,7 @@ use std::{ }; use anyhow::anyhow; -use defmt_decoder::{DecodeError, Encoding, Frame, Locations, Table}; +use defmt_decoder::{DecodeError, Frame, Locations, Table}; use structopt::StructOpt; /// Prints defmt-encoded logs to stdout From 7d69036de29f6c9839921061d6cd9e10ae307555 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 17:42:14 +0200 Subject: [PATCH 10/11] Update decoder/src/lib.rs --- decoder/src/lib.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/decoder/src/lib.rs b/decoder/src/lib.rs index e4300a56..b7ae9d6b 100644 --- a/decoder/src/lib.rs +++ b/decoder/src/lib.rs @@ -138,7 +138,7 @@ impl FromStr for Encoding { } impl Encoding { - pub const fn recoverable(&self) -> bool { + pub const fn can_recover(&self) -> bool { match self { Encoding::Raw => false, Encoding::Rzcobs => true, From 941789756ee9ed3b66d8193d1f941bbac3f0c606 Mon Sep 17 00:00:00 2001 From: Johann Hemmann Date: Fri, 1 Oct 2021 17:45:38 +0200 Subject: [PATCH 11/11] Update print/src/main.rs --- print/src/main.rs | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/print/src/main.rs b/print/src/main.rs index e83ba113..650f4f84 100644 --- a/print/src/main.rs +++ b/print/src/main.rs @@ -67,7 +67,7 @@ fn main() -> anyhow::Result<()> { match stream_decoder.decode() { Ok(frame) => forward_to_logger(&frame, location_info(&locs, &frame, ¤t_dir)), Err(DecodeError::UnexpectedEof) => break, - Err(DecodeError::Malformed) => match table.encoding().recoverable() { + Err(DecodeError::Malformed) => match table.encoding().can_recover() { // if recovery is impossible, abort false => return Err(DecodeError::Malformed.into()), // if recovery is possible, skip the current frame and continue with new data