reimplement log-logging feature

.github/workflows/codecov.yml

@@ -44,7 +44,7 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --lib --no-fail-fast --all-features
+          args: --lib --no-fail-fast
         env:
           CARGO_INCREMENTAL: "0"
           RUSTFLAGS: "-Zprofile -Ccodegen-units=1 -Copt-level=0 -Clink-dead-code -Coverflow-checks=off -Cpanic=unwind -Zpanic_abort_tests"

.github/workflows/lint.yml

@@ -89,4 +89,4 @@ jobs:
         uses: actions-rs/clippy-check@v1
         with:
           token: ${{ secrets.GITHUB_TOKEN }}
-          args: --all-features -- ${{ env.CLIPPY_PARAMS }}
+          args: -- ${{ env.CLIPPY_PARAMS }}

.github/workflows/test.yml

@@ -30,19 +30,67 @@ jobs:
           target: thumbv7m-none-eabi
           override: true
 
-      - name: Build (native)
+      - name: Build (native - default features)
         uses: actions-rs/cargo@v1
         with:
           command: build
-          args: --all --all-features
+          args: --all
 
-      - name: Build (ARM)
+      - name: Build (native - no logging)
         uses: actions-rs/cargo@v1
         with:
           command: build
-          # every feature except std. Can't specify --features in the root so
-          # directly specify just the atat crate
-          args: --manifest-path=atat/Cargo.toml
+          args: --all --features "derive, std"
+
+      - name: Build (native - log logging)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all --features "derive, std, log"
+
+      - name: Build (native - defmt logging)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all 
+            --features "
+                derive
+                std
+                defmt-default
+                defmt-trace
+                defmt-debug
+                defmt-info
+                defmt-warn
+                defmt-error"
+
+      - name: Build (ARM - default features)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all
+            --target thumbv7m-none-eabi
+
+      - name: Build (ARM - no logging)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all
+            --target thumbv7m-none-eabi
+            --features "derive"
+
+      - name: Build (ARM - log logging)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all
+            --target thumbv7m-none-eabi
+            --features "derive, log"
+
+      - name: Build (ARM - defmt logging)
+        uses: actions-rs/cargo@v1
+        with:
+          command: build
+          args: --all
             --target thumbv7m-none-eabi
             --features "
               derive
@@ -57,10 +105,10 @@ jobs:
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --lib --all-features
+          args: --lib
       # TODO: Change this to a single --all test, when the examples work
       - name: Doctests
         uses: actions-rs/cargo@v1
         with:
           command: test
-          args: --doc --all-features
+          args: --doc

README.md

@@ -41,7 +41,7 @@ This crate attempts to work from these AT best practices:
 
 ## Tests
 
-> The crate is covered by tests. These tests can be run by `cargo test --tests --all-features`, and are run by the CI on every push.
+> The crate is covered by tests. These tests can be run by `cargo test --tests`, and are run by the CI on every push.
 
 
 ## Examples
@@ -96,6 +96,7 @@ The following dependent crates provide platform-agnostic device drivers built on
 ## Features
 
  - `derive`: Enabled by default. Re-exports `atat_derive` to allow deriving `Atat__` traits.
+ - `log`: Disabled by default. Enable log statements on various log levels to aid debugging. Powered by `log`.
  - `defmt-default`: Disabled by default. Enable log statements at INFO, or TRACE, level and up, to aid debugging. Powered by `defmt`.
  - `defmt-trace`: Disabled by default. Enable log statements at TRACE level and up, to aid debugging. Powered by `defmt`.
  - `defmt-debug`: Disabled by default. Enable log statements at DEBUG level and up, to aid debugging. Powered by `defmt`.

atat/Cargo.toml

@@ -26,7 +26,8 @@ atat_derive = { path = "../atat_derive", version = "^0.10.1-alpha.0", optional =
 serde = { version = "^1", default-features = false }
 typenum = "^1"
 
-defmt = { version = "^0.2" }
+log = { version = "^0.4", default-features = false, optional = true }
+defmt = { version = "^0.2", optional = true }
 
 [dev-dependencies]
 cortex-m = "0.7.1"
@@ -41,9 +42,9 @@ derive = ["atat_derive"]
 
 std = ["serde_at/std"]
 
-defmt-default = []
-defmt-trace = []
-defmt-debug = []
-defmt-info = []
-defmt-warn = []
-defmt-error = []
+defmt-default = ["defmt"]
+defmt-trace = ["defmt"]
+defmt-debug = ["defmt"]
+defmt-info = ["defmt"]
+defmt-warn = ["defmt"]
+defmt-error = ["defmt"]

atat/src/client.rs

@@ -1,5 +1,7 @@
 use embedded_hal::{serial, timer::CountDown};
 
+use crate::atat_log;
+use crate::helpers::LossyStr;
 use crate::queues::{ComProducer, ResConsumer, UrcConsumer, UrcItem};
 use crate::traits::{AtatClient, AtatCmd, AtatUrc};
 use crate::{error::Error, queues::ResCapacity};
@@ -92,7 +94,8 @@ where
             if cmd.force_receive_state() && self.com_p.enqueue(Command::ForceReceiveState).is_err()
             {
                 // TODO: Consider how to act in this situation.
-                defmt::error!(
+                atat_log!(
+                    error,
                     "Failed to signal parser to force state transition to 'ReceivingResponse'!"
                 );
             }
@@ -104,9 +107,10 @@ where
             let cmd_buf = cmd.as_bytes();
 
             if cmd_buf.len() < 50 {
-                defmt::debug!("Sending command: \"{=[u8]:a}\"", &cmd_buf);
+                atat_log!(debug, "Sending command: \"{:?}\"", LossyStr(&cmd_buf));
             } else {
-                defmt::debug!(
+                atat_log!(
+                    debug,
                     "Sending command with too long payload ({} bytes) to log!",
                     cmd_buf.len()
                 );
@@ -142,7 +146,7 @@ where
                     return;
                 }
             } else {
-                defmt::error!("Parsing URC FAILED: {=[u8]:a}", urc)
+                atat_log!(error, "Parsing URC FAILED: {:?}", LossyStr(urc));
             }
             unsafe { self.urc_c.dequeue_unchecked() };
         }
@@ -160,7 +164,7 @@ where
                         Ok(r)
                     } else {
                         // FIXME: Is this correct?
-                        defmt::error!("Is this correct?! WouldBlock");
+                        atat_log!(error, "Is this correct?! WouldBlock");
                         Err(nb::Error::WouldBlock)
                     }
                 })
@@ -175,7 +179,7 @@ where
                 // Tell the parser to reset to initial state due to timeout
                 if self.com_p.enqueue(Command::Reset).is_err() {
                     // TODO: Consider how to act in this situation.
-                    defmt::error!("Failed to signal parser to clear buffer on timeout!");
+                    atat_log!(error, "Failed to signal parser to clear buffer on timeout!");
                 }
                 return Err(nb::Error::Other(Error::Timeout));
             }
@@ -190,7 +194,7 @@ where
     fn reset(&mut self) {
         if self.com_p.enqueue(Command::Reset).is_err() {
             // TODO: Consider how to act in this situation.
-            defmt::error!("Failed to signal ingress manager to reset!");
+            atat_log!(error, "Failed to signal ingress manager to reset!");
         }
 
         for _ in 0..ResCapacity::USIZE {
@@ -256,7 +260,7 @@ mod test {
         }
     }
 
-    #[derive(Debug, defmt::Format, PartialEq, Eq)]
+    #[derive(Debug, PartialEq, Eq)]
     pub enum InnerError {
         Test,
     }

atat/src/digest.rs

@@ -1,5 +1,6 @@
 use crate::{
-    helpers::{get_line, SliceExt},
+    atat_log,
+    helpers::{get_line, LossyStr, SliceExt},
     urc_matcher::{UrcMatcher, UrcMatcherResult},
     InternalError,
 };
@@ -32,7 +33,8 @@ pub enum DigestResult<L: ArrayLength<u8>> {
 
 /// State of the `DefaultDigester`, used to distiguish URCs from solicited
 /// responses
-#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq, defmt::Format)]
+#[derive(Debug, Copy, Clone, Hash, PartialEq, Eq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 enum State {
     Idle,
     ReceivingResponse,
@@ -86,7 +88,7 @@ impl Digester for DefaultDigester {
         }
 
         if !buf.is_empty() {
-            defmt::trace!("Digest {} / {=[u8]:a}", self.state, &buf);
+            atat_log!(trace, "Digest {:?} / {:?}", self.state, LossyStr(buf));
         }
 
         match self.state {
@@ -106,7 +108,7 @@ impl Digester for DefaultDigester {
                     {
                         self.state = State::ReceivingResponse;
                         self.buf_incomplete = false;
-                        defmt::trace!("Switching to state ReceivingResponse");
+                        atat_log!(trace, "Switching to state ReceivingResponse");
                     }
 
                 // Handle URCs
@@ -139,7 +141,8 @@ impl Digester for DefaultDigester {
                 // with "AT" or "+") can be ignored. Clear the buffer, but only if we can
                 // ensure that we don't accidentally break a valid response.
                 } else if self.buf_incomplete || buf.len() > 2 {
-                    defmt::error!(
+                    atat_log!(
+                        error,
                         "Clearing buffer with invalid response (incomplete: {}, buflen: {})",
                         self.buf_incomplete,
                         buf.len()
@@ -160,10 +163,10 @@ impl Digester for DefaultDigester {
                     );
 
                     if let Some(r) = removed {
-                        defmt::debug!("Cleared partial buffer, removed {=[u8]:a}", &r);
+                        atat_log!(debug, "Cleared partial buffer, removed {:?}", LossyStr(&r));
                     } else {
                         buf.clear();
-                        defmt::debug!("Cleared partial buffer, removed everything");
+                        atat_log!(debug, "Cleared partial buffer, removed everything");
                     }
 
                     // If the buffer wasn't cleared completely, that means that
@@ -241,7 +244,7 @@ impl Digester for DefaultDigester {
                     return DigestResult::None;
                 };
 
-                defmt::trace!("Switching to state Idle");
+                atat_log!(trace, "Switching to state Idle");
                 self.state = State::Idle;
                 return DigestResult::Response(resp);
             }

atat/src/error.rs

@@ -21,6 +21,7 @@ pub enum InternalError {
     Error(Vec<u8, consts::U85>),
 }
 
+#[cfg(feature = "defmt")]
 impl defmt::Format for InternalError {
     fn format(&self, f: defmt::Formatter) {
         match self {
@@ -37,11 +38,9 @@ impl defmt::Format for InternalError {
 }
 
 /// Errors returned by the crate
-#[derive(Clone, Debug, PartialEq, defmt::Format)]
-pub enum Error<E = GenericError>
-where
-    E: defmt::Format,
-{
+#[derive(Clone, Debug, PartialEq)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
+pub enum Error<E = GenericError> {
     /// Serial read error
     Read,
     /// Serial write error
@@ -62,7 +61,7 @@ where
 
 impl<E> From<&InternalError> for Error<E>
 where
-    E: core::str::FromStr + defmt::Format,
+    E: core::str::FromStr,
 {
     fn from(ie: &InternalError) -> Self {
         match ie {
@@ -85,7 +84,8 @@ where
     }
 }
 
-#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord, defmt::Format)]
+#[derive(Debug, Clone, Copy, PartialEq, Eq, PartialOrd, Ord)]
+#[cfg_attr(feature = "defmt", derive(defmt::Format))]
 pub struct GenericError;
 
 impl core::str::FromStr for GenericError {

atat/src/helpers.rs

@@ -111,6 +111,54 @@ pub fn get_line<L: ArrayLength<u8>, I: ArrayLength<u8>>(
     }
 }
 
+#[cfg(feature = "log")]
+#[macro_export]
+macro_rules! atat_log {
+    ($level:ident, $($arg:expr),*) => {
+        log::$level!($($arg),*);
+    }
+}
+#[cfg(all(feature = "defmt", not(feature = "log")))]
+#[macro_export]
+macro_rules! atat_log {
+    ($level:ident, $($arg:expr),*) => {
+        defmt::$level!($($arg),*);
+    }
+}
+#[cfg(not(any(feature = "defmt", feature = "log")))]
+#[macro_export]
+macro_rules! atat_log {
+    ($level:ident, $($arg:expr),*) => {
+        {
+            $( let _ = $arg; )*
+            ()
+        }
+
+    }
+}
+#[cfg(all(feature = "defmt", feature = "log"))]
+compile_error!("You must enable at most one of the following features: defmt-*, log");
+
+/// Wrapper for a byte-slice that formats it as a string if possible and as
+/// bytes otherwise.
+pub struct LossyStr<'a>(pub &'a [u8]);
+
+impl<'a> core::fmt::Debug for LossyStr<'a> {
+    fn fmt(&self, f: &mut core::fmt::Formatter<'_>) -> core::fmt::Result {
+        match core::str::from_utf8(self.0) {
+            Ok(s) => write!(f, "{:?}", s),
+            Err(_) => write!(f, "{:?}", self.0),
+        }
+    }
+}
+
+#[cfg(feature = "defmt")]
+impl<'a> defmt::Format for LossyStr<'a> {
+    fn format(&self, fmt: defmt::Formatter) {
+        defmt::write!(fmt, "{=[u8]:a}", self.0)
+    }
+}
+
 #[cfg(test)]
 mod test {
     use super::*;