Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

defmt-print: Recover from decoding-errors #598

Merged
merged 11 commits into from
Oct 1, 2021
25 changes: 20 additions & 5 deletions decoder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -119,7 +119,8 @@ struct BitflagsKey {
}

#[derive(Copy, Clone, Debug, PartialEq)]
enum Encoding {
#[non_exhaustive]
pub enum Encoding {
Urhengulas marked this conversation as resolved.
Show resolved Hide resolved
Raw,
Rzcobs,
}
Expand All @@ -136,6 +137,15 @@ impl FromStr for Encoding {
}
}

impl Encoding {
pub const fn can_recover(&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 {
Expand Down Expand Up @@ -205,14 +215,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::<LE>()? as u64;

Expand Down Expand Up @@ -252,6 +263,10 @@ impl Table {
Encoding::Rzcobs => Box::new(stream::Rzcobs::new(self)),
}
}

pub fn encoding(&self) -> Encoding {
self.encoding
}
}

// NOTE follows `parser::Type`
Expand Down
85 changes: 44 additions & 41 deletions print/src/main.rs
Original file line number Diff line number Diff line change
@@ -1,11 +1,11 @@
use std::{
env, fs,
io::{self, Read},
path::PathBuf,
path::{Path, PathBuf},
};

use anyhow::anyhow;
use defmt_decoder::Table;
use defmt_decoder::{DecodeError, Frame, Locations, Table};
use structopt::StructOpt;

/// Prints defmt-encoded logs to stdout
Expand Down Expand Up @@ -51,58 +51,61 @@ fn main() -> anyhow::Result<()> {
};

let mut buf = [0; READ_BUFFER_SIZE];
let mut frames = vec![];
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 push it to the decoder
let n = stdin.read(&mut buf)?;
stream_decoder.received(&buf[..n]);

frames.extend_from_slice(&buf[..n]);

// decode the received data
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(&current_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());
}

// Forward the defmt frame to our logger.
defmt_decoder::log::log_defmt(
&frame,
file.as_deref(),
line,
mod_path.as_deref(),
);

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);
return Err(defmt_decoder::DecodeError::Malformed.into());
}
match stream_decoder.decode() {
Ok(frame) => forward_to_logger(&frame, location_info(&locs, &frame, &current_dir)),
Err(DecodeError::UnexpectedEof) => break,
Err(DecodeError::Malformed) => match table.encoding().recoverable() {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

(probably down to personal preference but if boolean is more common than match boolean IMO)

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think it is more common to use if. But match saves one line 😋

Urhengulas marked this conversation as resolved.
Show resolved Hide resolved
// 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,
},
}
}
}
}

type LocationInfo = (Option<String>, Option<u32>, Option<String>);

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<Locations>, 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
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(&current_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.
Expand Down