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

Streaming core::fmt adapters #340

Merged
merged 3 commits into from
Jan 13, 2021
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion Cargo.toml
Original file line number Diff line number Diff line change
Expand Up @@ -19,7 +19,6 @@ unstable-test = ["defmt-macros/unstable-test"]

[dependencies]
defmt-macros = { path = "macros", version = "0.1.1" }
heapless = "0.5.6"

[dev-dependencies]
rustc_version = "0.3.0"
Expand Down
19 changes: 18 additions & 1 deletion decoder/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -349,6 +349,9 @@ enum Arg<'t> {
Slice(Vec<u8>),
/// Char
Char(char),

/// `fmt::Debug` / `fmt::Display` formatted on-target.
Preformatted(String),
}

#[derive(Debug, PartialEq)]
Expand Down Expand Up @@ -873,6 +876,20 @@ impl<'t, 'b> Decoder<'t, 'b> {
let c = std::char::from_u32(data).ok_or(DecodeError::Malformed)?;
args.push(Arg::Char(c));
}
Type::Debug | Type::Display => {
// UTF-8 stream without a prefix length, terminated with `0xFF`.
Copy link
Member

Choose a reason for hiding this comment

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

"isn't a 0xFF delimiter problematic with characters like BOM / U+FEFF?"
"no, because UTF-8 encoding never uses the 0xff value; each byte (after encoding) contains at least one zero bit"
case in point, BOM is encoded as [239, 187, 191]


let end = self
.bytes
.iter()
.position(|b| *b == 0xff)
.ok_or(DecodeError::UnexpectedEof)?;
let data = core::str::from_utf8(&self.bytes[..end])
.map_err(|_| DecodeError::Malformed)?;
self.bytes = &self.bytes[end + 1..];

args.push(Arg::Preformatted(data.into()));
}
}
}

Expand Down Expand Up @@ -1029,7 +1046,7 @@ fn format_args_real(
}
}
Arg::Ixx(x) => format_i128(*x as i128, hint, &mut buf)?,
Arg::Str(x) => format_str(x, hint, &mut buf)?,
Arg::Str(x) | Arg::Preformatted(x) => format_str(x, hint, &mut buf)?,
Arg::IStr(x) => format_str(x, hint, &mut buf)?,
Arg::Format { format, args } => buf.push_str(&format_args(format, args, hint)),
Arg::FormatSlice { elements } => {
Expand Down
12 changes: 6 additions & 6 deletions firmware/qemu/src/bin/log.rs
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@ use core::{
};
use cortex_m_rt::entry;
use cortex_m_semihosting::debug;
use defmt::{consts, Debug2Format, Display2Format, Format, Formatter};
use defmt::{Debug2Format, Display2Format, Format, Formatter};

use defmt_semihosting as _; // global logger

Expand Down Expand Up @@ -454,10 +454,10 @@ fn main() -> ! {
}

let s = S { x: -1, y: 2 };
defmt::info!("{=?}", Debug2Format::<consts::U128>(&s));
defmt::info!("{=?}", Debug2Format::<consts::U128>(&Some(s)));
defmt::info!("{=?}", Debug2Format::<consts::U128>(&[s, s]));
defmt::info!("{=?}", Debug2Format::<consts::U128>(&[Some(s), None]));
defmt::info!("{}", Debug2Format(&s));
defmt::info!("{}", Debug2Format(&Some(s)));
defmt::info!("{}", Debug2Format(&[s, s]));
defmt::info!("{}", Debug2Format(&[Some(s), None]));
}

{
Expand All @@ -481,7 +481,7 @@ fn main() -> ! {
port: 8888,
};

defmt::info!("{=?}", Display2Format::<consts::U32>(&addr));
defmt::info!("{=?}", Display2Format(&addr));
}

defmt::info!(
Expand Down
6 changes: 6 additions & 0 deletions macros/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -1150,6 +1150,12 @@ impl Codegen {
defmt_parser::Type::Usize => {
exprs.push(quote!(_fmt_.usize(#arg)));
}
defmt_parser::Type::Debug => {
exprs.push(quote!(_fmt_.debug(#arg)));
}
defmt_parser::Type::Display => {
exprs.push(quote!(_fmt_.display(#arg)));
}
defmt_parser::Type::BitField(_) => {
let all_bitfields = parsed_params.iter().filter(|param| param.index == i);
let (smallest_bit_index, largest_bit_index) =
Expand Down
4 changes: 4 additions & 0 deletions parser/src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -71,6 +71,8 @@ pub enum Type {
Format, // "{=?}" OR "{}"
FormatSlice, // "{=[?]}"
FormatArray(usize), // FIXME: This `usize` is not the target's `usize`; use `u64` instead?
Debug,
Display,
I8,
I16,
I32,
Expand Down Expand Up @@ -217,6 +219,8 @@ fn parse_param(mut input: &str, mode: ParserMode) -> Result<Param, Cow<'static,
"bool" => Type::Bool,
"str" => Type::Str,
"istr" => Type::IStr,
"__internal_Debug" => Type::Debug,
"__internal_Display" => Type::Display,
"[u8]" => Type::U8Slice,
"?" => Type::Format,
"[?]" => Type::FormatSlice,
Expand Down
167 changes: 44 additions & 123 deletions src/lib.rs
Original file line number Diff line number Diff line change
Expand Up @@ -16,10 +16,11 @@
#[cfg(feature = "alloc")]
extern crate alloc;

use core::{fmt, marker::PhantomData, mem::MaybeUninit, ptr::NonNull};
#[cfg(feature = "unstable-test")]
use crate as defmt;

pub use heapless::consts;
use heapless::{ArrayLength, String};
use core::fmt::Write as _;
use core::{fmt, mem::MaybeUninit, ptr::NonNull};

#[doc(hidden)]
pub mod export;
Expand Down Expand Up @@ -566,6 +567,18 @@ impl InternalFormatter {
}
}

/// Implementation detail
pub fn debug(&mut self, val: &dyn core::fmt::Debug) {
core::write!(FmtWrite { fmt: self }, "{:?}", val).ok();
self.write(&[0xff]);
}

/// Implementation detail
pub fn display(&mut self, val: &dyn core::fmt::Display) {
core::write!(FmtWrite { fmt: self }, "{}", val).ok();
self.write(&[0xff]);
}

/// The last pass in a formatting run: clean up & flush leftovers
pub fn finalize(&mut self) {
if self.bools_left < MAX_NUM_BOOL_FLAGS {
Expand All @@ -588,6 +601,17 @@ impl InternalFormatter {
}
}

struct FmtWrite<'a> {
fmt: &'a mut InternalFormatter,
}

impl fmt::Write for FmtWrite<'_> {
fn write_str(&mut self, s: &str) -> fmt::Result {
self.fmt.write(s.as_bytes());
Ok(())
}
}

// these need to be in a separate module or `unreachable!` will end up calling `defmt::panic` and
// this will not compile
// (using `core::unreachable!` instead of `unreachable!` doesn't help)
Expand Down Expand Up @@ -675,143 +699,40 @@ fn default_panic() -> ! {
core::panic!()
}

/// An "adapter" type to feed `Debug` values into defmt macros, which expect `defmt::Format` values
///
/// This adapter disables compression! You should prefer `defmt::Format` over `Debug` whenever
/// possible
///
/// This adapter works by formatting the `Debug` value into a stack-allocated buffer. You need to
/// specify how large that buffer is. If you pick a size that's too small an *incomplete* string
/// will be transmitted.
///
/// The size of the stack-allocated array is specified using one of the type-level integers in the
/// `consts` module. Example:
///
/// ```
/// use defmt::{consts, Debug2Format};
/// An "adapter" type to feed `Debug` values into defmt macros, which expect `defmt::Format` values.
///
/// #[derive(Debug)]
/// struct S { x: i8, y: i16 }
/// This adapter disables compression and uses the `core::fmt` code on-device! You should prefer
/// `defmt::Format` over `Debug` whenever possible.
///
/// defmt::info!("{:?}", Debug2Format::<consts::U32>(&S { x: 1, y: 2 }));
/// // ^^^^^^^^^^^ size = 32 bytes
/// // -> 0.000000 INFO S { x: 1, y: 2 }
/// ```
pub struct Debug2Format<'a, N>
where
N: ArrayLength<u8>,
{
// we don't store a heapless String inline because rustc is not great at optimizing moves of
// large arrays and that can result in double stack usage
_capacity: PhantomData<N>,

// we use a trait object here to avoid adding a second type paratemer to `Debug2Format`. if
// `Debug2Format` has 2 type parameter then users need to add an underscore type parameter to
// the turbofish: `Debug2Format::<U64, _>(value)`
value: &'a dyn fmt::Debug,
}
/// Note that this always uses `{:?}` to format the contained value, meaning that any provided defmt
/// display hints will be ignored.
pub struct Debug2Format<'a, T: fmt::Debug + ?Sized>(pub &'a T);

impl<N> Format for Debug2Format<'_, N>
where
N: ArrayLength<u8>,
{
impl<T: fmt::Debug + ?Sized> Format for Debug2Format<'_, T> {
fn format(&self, fmt: Formatter) {
use core::fmt::Write as _;

// `Debug` format into a stack buffer; if the formatted string doesn't fit discard the
// excess
let mut buf = String::<N>::new();
core::write!(buf, "{:?}", self.value).ok();

// tell defmt we are formatting a `str` value
if fmt.inner.needs_tag() {
let t = impls::str_tag();
let t = defmt_macros::internp!("{=__internal_Debug}");
fmt.inner.u8(&t);
}
fmt.inner.str(&buf);
}
}

/// `Debug2Format` constructor
#[allow(non_snake_case)]
pub fn Debug2Format<N>(value: &dyn fmt::Debug) -> Debug2Format<N>
where
N: ArrayLength<u8>,
{
Debug2Format {
_capacity: PhantomData,
value,
fmt.inner.debug(&self.0);
}
}

/// An "adapter" type to feed `Display` values into defmt macros, which expect `defmt::Format` values.
///
/// This adapter disables compression! You should prefer `defmt::Format` over `Display` whenever
/// possible.
///
/// This adapter works by formatting the `Display` value into a stack-allocated buffer. You need to
/// specify how large that buffer is. If you pick a size that's too small an *incomplete* string
/// will be transmitted.
///
/// The size of the stack-allocated array is specified using one of the type-level integers in the
/// `consts` module. Example:
///
/// ```
/// use core::fmt;
/// use defmt::{consts, Display2Format};
///
/// struct SocketAddr {
/// ip: [u8; 4],
/// port: u16,
/// }
///
/// impl fmt::Display for SocketAddr {
/// fn fmt(&self, f: &mut fmt::Formatter) -> fmt::Result {
/// write!(f, "{}.{}.{}.{}:{}", self.ip[0], self.ip[1], self.ip[2], self.ip[3], self.port)
/// }
/// }
/// This adapter disables compression and uses the `core::fmt` code on-device! You should prefer
/// `defmt::Format` over `Display` whenever possible.
///
/// let addr = SocketAddr { ip: [127, 0, 0, 1], port: 8888 };
/// defmt::info!("{:?}", Display2Format::<consts::U32>(&addr));
/// // ^^^^^^^^^^^ size = 32 bytes
/// // -> 0.000000 INFO 127.0.0.1:8888
/// ```
pub struct Display2Format<'a, N>
where
N: ArrayLength<u8>,
{
_capacity: PhantomData<N>,

value: &'a dyn fmt::Display,
}
/// Note that this always uses `{}` to format the contained value, meaning that any provided defmt
/// display hints will be ignored.
pub struct Display2Format<'a, T: fmt::Display + ?Sized>(pub &'a T);

impl<N> Format for Display2Format<'_, N>
where
N: ArrayLength<u8>,
{
impl<T: fmt::Display + ?Sized> Format for Display2Format<'_, T> {
fn format(&self, fmt: Formatter) {
use core::fmt::Write as _;

let mut buf = String::<N>::new();
core::write!(buf, "{}", self.value).ok();

// tell defmt we are formatting a `str` value
if fmt.inner.needs_tag() {
let t = impls::str_tag();
let t = defmt_macros::internp!("{=__internal_Display}");
fmt.inner.u8(&t);
}
fmt.inner.str(&buf);
}
}

/// `Display2Format` constructor.
#[allow(non_snake_case)]
pub fn Display2Format<N>(value: &dyn fmt::Display) -> Display2Format<N>
where
N: ArrayLength<u8>,
{
Display2Format {
_capacity: PhantomData,
value,
fmt.inner.display(&self.0);
}
}
13 changes: 12 additions & 1 deletion tests/encode.rs
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,10 @@
//
// - the mocked index is 7 bits so its LEB128 encoding is the input byte

use defmt::{export::fetch_string_index, write, Format, Formatter, InternalFormatter};
use defmt::{
export::fetch_string_index, write, Debug2Format, Display2Format, Format, Formatter,
InternalFormatter,
};

// Increase the 7-bit mocked interned index
fn inc(index: u8, n: u8) -> u8 {
Expand Down Expand Up @@ -1024,3 +1027,11 @@ fn derive_str() {
],
);
}

#[test]
fn core_fmt_adapters() {
let index = fetch_string_index();
check_format_implementation(&Debug2Format(&123u8), &[index, b'1', b'2', b'3', 0xff]);
let index = fetch_string_index();
check_format_implementation(&Display2Format(&123u8), &[index, b'1', b'2', b'3', 0xff]);
}