Logging cleanup and fix for RTT

firmware/.vscode/launch.json

@@ -9,9 +9,9 @@
       "chip": "esp32s3",
       "coreConfigs": [
         {
-          "programBinary": "target/xtensa-esp32s3-none-elf/debug/acid-firmware"
+          "programBinary": "target/xtensa-esp32s3-none-elf/debug/acid-firmware",
-        }
+        },
-      ]
+      ],
     },
     {
       "name": "probe-rs release restart",
@@ -21,9 +21,9 @@
       "chip": "esp32s3",
       "coreConfigs": [
         {
-          "programBinary": "target/xtensa-esp32s3-none-elf/release/acid-firmware"
+          "programBinary": "target/xtensa-esp32s3-none-elf/release/acid-firmware",
-        }
+        },
-      ]
+      ],
     },
     {
       "preLaunchTask": "rust: cargo build",
@@ -37,9 +37,9 @@
       "chip": "esp32s3",
       "coreConfigs": [
         {
-          "programBinary": "target/xtensa-esp32s3-none-elf/debug/acid-firmware"
+          "programBinary": "target/xtensa-esp32s3-none-elf/debug/acid-firmware",
-        }
+        },
-      ]
+      ],
     },
     {
       "preLaunchTask": "rust: cargo build --release",
@@ -53,9 +53,9 @@
       "chip": "esp32s3",
       "coreConfigs": [
         {
-          "programBinary": "target/xtensa-esp32s3-none-elf/release/acid-firmware"
+          "programBinary": "target/xtensa-esp32s3-none-elf/release/acid-firmware",
-        }
+        },
-      ]
+      ],
-    }
+    },
-  ]
+  ],
 }

firmware/.vscode/settings.json

@@ -0,0 +1,4 @@
+{
+    "rust-analyzer.cargo.noDefaultFeatures": true,
+    "rust-analyzer.cargo.features": ["probe"],
+}

firmware/.vscode/tasks.json

@@ -4,9 +4,12 @@
 		{
 			"label": "rust: cargo build",
 			"type": "cargo",
+			"options": {
+				"cwd": "${workspaceFolder}/acid-firmware"
+			},
 			"command": "build",
 			"args": [
-				"--no-default-features", "--features=probe,info"
+				"--no-default-features", "--features=probe"
 			],
 			"problemMatcher": [
 				"$rustc"
@@ -19,9 +22,12 @@
 		{
 			"label": "rust: cargo build --release",
 			"type": "cargo",
+			"options": {
+				"cwd": "${workspaceFolder}/acid-firmware"
+			},
 			"command": "build",
 			"args": [
-				"--release", "--no-default-features", "--features=probe,info"
+				"--release", "--no-default-features", "--features=probe"
 			],
 			"problemMatcher": [
 				"$rustc"

firmware/Cargo.lock

@@ -22,7 +22,7 @@ dependencies = [
  "embassy-sync 0.7.2",
  "embassy-time",
  "embedded-cli",
- "embedded-io 0.7.1",
+ "embedded-io 0.6.1",
  "embedded-storage-async",
  "embuild",
  "enumset",

firmware/acid-firmware/.cargo/config.toml

@@ -1,6 +1,6 @@
 [target.'cfg(all(any(target_arch = "riscv32", target_arch = "xtensa"), target_os = "none"))']
-runner = "espflash flash --monitor"
+# runner = "espflash flash --monitor"
-# runner = "probe-rs run --chip esp32s3 --preverify"
+runner = "probe-rs run --chip esp32s3 --preverify"
 
 [build]
 target = "xtensa-esp32s3-none-elf"

firmware/acid-firmware/Cargo.toml

@@ -95,7 +95,7 @@ i-slint-core = { version = "1.14.1", git = "https://github.com/Limeth/slint", re
 
 # Crates for serial UART CLI
 embedded-cli = { version = "0.2.1", default-features = false, features = ["help", "macros"] }
-embedded-io = "0.7"
+embedded-io = "0.6"
 mutually_exclusive_features = "0.1.0"
 
 [dependencies.ekv]

firmware/acid-firmware/src/console.rs

@@ -1,12 +1,14 @@
-
 use core::fmt::Write;
 
-use embedded_cli::cli::CliBuilder;
 use embedded_cli::Command;
-use esp_hal::{Async, uart::{TxError, UartRx}};
+use embedded_cli::cli::CliBuilder;
-use log::{info, error};
+use esp_hal::{
+    Async,
+    uart::{TxError, UartRx},
+};
+use log::{error, info};
 
-use crate::logging::with_uart_tx;
+use crate::logging::uart::with_uart_tx;
 
 struct Writer;
 
@@ -16,26 +18,21 @@ impl embedded_io::ErrorType for Writer {
 
 impl embedded_io::Write for Writer {
     fn write(&mut self, buf: &[u8]) -> Result<usize, Self::Error> {
-        with_uart_tx(|_, uart| {
+        with_uart_tx(|_, uart| uart.write(buf))
-            uart.write(buf)
-        })
     }
 
     fn flush(&mut self) -> Result<(), Self::Error> {
-        with_uart_tx(|_, uart| {
+        with_uart_tx(|_, uart| uart.flush())
-            uart.flush()
-        })
     }
 }
 
 #[derive(Command)]
-enum Base/*<'a>*/ {
+enum Base /*<'a>*/ {
     // /// Say hello to World or someone else
     // Hello {
     //     /// To whom to say hello (World by default)
     //     name: Option<&'a str>,
     // },
-
     /// Display the version of the firmware.
     Version,
 
@@ -66,23 +63,33 @@ pub async fn run_console(mut uart_rx: UartRx<'_, Async>) {
 
         for &byte in &buf[0..len] {
             cli.process_byte::<Base, _>(
-                byte, 
+                byte,
                 &mut Base::processor(|cli, command| {
                     match command {
                         // Base::Hello { name } => {
                         //     write!(cli.writer(), "Hello, {}", name.unwrap_or("World"))?;
                         // }
                         Base::Version => {
-                            cli.writer().write_fmt(format_args!("{} - {} - {}", env!("CARGO_PKG_NAME"), env!("CARGO_PKG_VERSION"), env!("GIT_COMMIT"))).unwrap();
+                            cli.writer()
+                                .write_fmt(format_args!(
+                                    "{} - {} - {}",
+                                    env!("CARGO_PKG_NAME"),
+                                    env!("CARGO_PKG_VERSION"),
+                                    env!("GIT_COMMIT")
+                                ))
+                                .unwrap();
                         }
                         Base::Reset => {
-                            cli.writer().write_str("Performing software reset.").unwrap();
+                            cli.writer()
+                                .write_str("Performing software reset.")
+                                .unwrap();
                             esp_hal::system::software_reset();
                         }
                     }
                     Ok(())
                 }),
-            ).unwrap();
+            )
+            .unwrap();
         }
     }
 }

firmware/acid-firmware/src/logging.rs

@@ -1,8 +1,10 @@
-use core::cell::RefCell;
 use core::fmt::Write;
+use core::{cell::RefCell, fmt::Arguments};
 
 use critical_section::{CriticalSection, Mutex};
 use esp_hal::Blocking;
+use esp_hal::clock::CpuClock;
+use esp_hal::psram::{FlashFreq, PsramConfig, PsramSize, SpiRamFreq, SpiTimingConfigCoreClock};
 use esp_hal::uart::UartTx;
 use log::{LevelFilter, Log};
 
@@ -19,46 +21,15 @@ pub const LOG_LEVEL_FILTER: LevelFilter = {
         } else if string.eq_ignore_ascii_case("TRACE") {
             LevelFilter::Trace
         } else {
-            panic!("Unknown `ESP_LOG` value. Only `ERROR`, `WARN`, `INFO`, `DEBUG`, `TRACE`, or `OFF` may be used.");
+            panic!(
+                "Unknown `ESP_LOG` value. Only `ERROR`, `WARN`, `INFO`, `DEBUG`, `TRACE`, or `OFF` may be used."
+            );
         }
     } else {
         LevelFilter::Off
     }
 };
 
-static ALT_LOGGER_UART: Mutex<RefCell<Option<UartTx<'static, Blocking>>>> =
-    Mutex::new(RefCell::new(None));
-
-pub fn with_uart_tx<R>(
-    f: impl FnOnce(CriticalSection<'_>, &'_ mut UartTx<'static, Blocking>) -> R,
-) -> R {
-    critical_section::with(|cs| {
-        let mut uart = ALT_LOGGER_UART.borrow(cs).borrow_mut();
-        let uart = uart.as_mut().unwrap();
-
-        (f)(cs, uart)
-    })
-}
-
-struct AlternativeLogger;
-
-impl Log for AlternativeLogger {
-    #[allow(unused)]
-    fn enabled(&self, _: &log::Metadata) -> bool {
-        // Filtered by `log` already
-        true
-    }
-
-    #[allow(unused)]
-    fn log(&self, record: &log::Record) {
-        with_uart_tx(|cs, uart| {
-            print_log_record(uart, record);
-        })
-    }
-
-    fn flush(&self) {}
-}
-
 const RESET: &str = "\u{001B}[0m";
 const RED: &str = "\u{001B}[31m";
 const GREEN: &str = "\u{001B}[32m";
@@ -66,32 +37,10 @@ const YELLOW: &str = "\u{001B}[33m";
 const BLUE: &str = "\u{001B}[34m";
 const CYAN: &str = "\u{001B}[35m";
 
-#[cfg(feature = "rtt-log")]
+fn with_formatted_log_record<R>(
-#[allow(unused)]
+    record: &log::Record,
-use ::rtt_target::{rprint as print, rprintln as println};
+    callback: impl FnOnce(Arguments<'_>) -> R,
-
+) -> R {
-#[cfg(feature = "alt-log")]
-#[allow(unused)]
-macro_rules! println {
-    () => {{
-        do_print(Default::default());
-    }};
-
-    ($($arg:tt)*) => {{
-        do_print(::core::format_args!($($arg)*));
-    }};
-}
-
-#[allow(unused)]
-fn do_print(args: core::fmt::Arguments<'_>) {
-    with_uart_tx(|_, uart| {
-        uart.write_fmt(args).unwrap();
-        uart.write_str("\n").unwrap();
-        uart.flush().unwrap();
-    })
-}
-
-fn print_log_record(uart: &mut UartTx<'_, Blocking>, record: &log::Record) {
     let color = match record.level() {
         log::Level::Error => RED,
         log::Level::Warn => YELLOW,
@@ -99,53 +48,155 @@ fn print_log_record(uart: &mut UartTx<'_, Blocking>, record: &log::Record) {
         log::Level::Debug => BLUE,
         log::Level::Trace => CYAN,
     };
-    let reset = RESET;
     let args = format_args!(
         "{}{:>5} - {}{}\n",
         color,
         record.level(),
         record.args(),
-        reset
+        RESET
     );
-    uart.write_fmt(args).unwrap();
+    (callback)(args)
-    uart.flush().unwrap();
 }
 
+/// The default USB logger.
+#[cfg(feature = "usb-log")]
+pub mod usb {
+    use super::*;
+
+    pub fn setup_logging() -> impl Future<Output = ()> {
+        esp_println::logger::init_logger(LOG_LEVEL_FILTER);
+        log::info!("Logger initialized!");
+        async {}
+    }
+}
+
+/// Alternative logger via UART.
+#[cfg(feature = "alt-log")]
+pub mod uart {
+    use super::*;
+    use crate::console;
+    use core::{cell::RefCell, fmt::Write};
+    use critical_section::{CriticalSection, Mutex};
+    use esp_hal::{
+        Blocking,
+        gpio::interconnect::{PeripheralInput, PeripheralOutput},
+        peripherals::UART2,
+        uart::{Uart, UartTx},
+    };
+    use log::{Log, info};
+
+    static ALT_LOGGER_UART: Mutex<RefCell<Option<UartTx<'static, Blocking>>>> =
+        Mutex::new(RefCell::new(None));
+
+    pub fn with_uart_tx<R>(
+        f: impl FnOnce(CriticalSection<'_>, &'_ mut UartTx<'static, Blocking>) -> R,
+    ) -> R {
+        critical_section::with(|cs| {
+            let mut uart = ALT_LOGGER_UART.borrow(cs).borrow_mut();
+            let uart = uart.as_mut().unwrap();
+
+            (f)(cs, uart)
+        })
+    }
+
+    #[allow(unused)]
+    macro_rules! println {
+        // TODO: I don't think this is necessary. Consider removing.
+        // () => {{
+        //     do_print(Default::default());
+        // }};
+
+        ($($arg:tt)*) => {{
+            do_print(::core::format_args!($($arg)*));
+        }};
+    }
+
+    #[allow(unused)]
+    fn do_print(args: core::fmt::Arguments<'_>) {
+        with_uart_tx(|_, uart| {
+            uart.write_fmt(format_args!("{}\n", args)).unwrap();
+            uart.flush().unwrap();
+        })
+    }
+
+    struct UartLogger;
+
+    impl Log for UartLogger {
+        #[allow(unused)]
+        fn enabled(&self, _: &log::Metadata) -> bool {
+            // Filtered by `log` already
+            true
+        }
+
+        #[allow(unused)]
+        fn log(&self, record: &log::Record) {
+            with_uart_tx(|cs, uart| {
+                with_formatted_log_record(record, |args| uart.write_fmt(args)).unwrap();
+                uart.flush().unwrap();
+            })
+        }
+
+        fn flush(&self) {}
+    }
+
+    #[panic_handler]
+    fn panic_handler(info: &core::panic::PanicInfo) -> ! {
+        use super::{RED, RESET};
+        use esp_backtrace::Backtrace;
+
+        println!("{RED}");
+        println!("=============== CUSTOM PANIC HANDLER ==============");
+        println!("{info}{RESET}");
+        println!("");
+        println!("Backtrace:");
+        println!("");
+
+        let backtrace = Backtrace::capture();
+
+        for frame in backtrace.frames() {
+            println!("0x{:x}", frame.program_counter());
+        }
+
+        loop {}
+    }
+
+    pub fn setup_logging(
+        uart: impl esp_hal::uart::Instance + 'static,
+        tx: impl PeripheralOutput<'static>,
+        rx: impl PeripheralInput<'static>,
+    ) -> impl Future<Output = ()> {
+        let (uart_rx, uart_tx) = Uart::new(uart, Default::default())
+            .unwrap()
+            .with_tx(tx)
+            .with_rx(rx)
+            .split();
+
+        critical_section::with(|cs| {
+            *ALT_LOGGER_UART.borrow(cs).borrow_mut() = Some(uart_tx);
+        });
+
+        unsafe {
+            log::set_logger_racy(&UartLogger).unwrap();
+            log::set_max_level_racy(LOG_LEVEL_FILTER);
+        }
+
+        info!("Logger initialized!");
+        console::run_console(uart_rx.into_async())
+    }
+}
+
+/// Logging via RTT for probe-rs.
 #[cfg(feature = "rtt-log")]
-use panic_rtt_target as _;
+pub mod rtt {
+    use super::*;
+    #[allow(unused)]
+    use ::rtt_target::{rprint as print, rprintln as println};
+    use panic_rtt_target as _; // Use the RTT panic handler.
+    use rtt_target::ChannelMode;
 
-#[cfg(feature = "alt-log")]
+    pub fn setup_logging() -> impl Future<Output = ()> {
-#[panic_handler]
+        rtt_target::rtt_init_log!(LOG_LEVEL_FILTER, ChannelMode::BlockIfFull);
-fn panic_handler(info: &core::panic::PanicInfo) -> ! {
+        log::info!("Logger initialized!");
-    use esp_backtrace::Backtrace;
+        async {}
-
-    println!("{RED}");
-    println!("=============== CUSTOM PANIC HANDLER ==============");
-    println!("{info}{RESET}");
-    println!("");
-    println!("Backtrace:");
-    println!("");
-
-    let backtrace = Backtrace::capture();
-
-    for frame in backtrace.frames() {
-        println!("0x{:x}", frame.program_counter());
-    }
-
-    loop {}
-}
-
-#[cfg(feature = "alt-log")]
-pub fn setup_alternative_logging(
-    alt_uart: UartTx<'static, Blocking>,
-    level_filter: log::LevelFilter,
-) {
-    critical_section::with(|cs| {
-        *ALT_LOGGER_UART.borrow(cs).borrow_mut() = Some(alt_uart);
-    });
-
-    unsafe {
-        log::set_logger_racy(&AlternativeLogger).unwrap();
-        log::set_max_level_racy(level_filter);
     }
 }

firmware/acid-firmware/src/main.rs

@@ -117,15 +117,6 @@ static SIGNAL_UI_RENDER: Signal<CriticalSectionRawMutex, ()> = Signal::new();
 
 #[esp_rtos::main]
 async fn main(_spawner: Spawner) {
-    #[cfg(feature = "usb-log")]
-    {
-        esp_println::logger::init_logger(LOG_LEVEL_FILTER);
-        info!("Logger initialized!");
-    }
-
-    #[cfg(feature = "rtt-log")]
-    rtt_target::rtt_init_log!(LOG_LEVEL_FILTER);
-
     let config = esp_hal::Config::default()
         .with_cpu_clock(CpuClock::max())
         .with_psram(PsramConfig {
@@ -135,24 +126,14 @@ async fn main(_spawner: Spawner) {
             ram_frequency: SpiRamFreq::Freq80m,
         });
     let peripherals: esp_hal::peripherals::Peripherals = esp_hal::init(config);
-    info!("System initialized!");
 
+    #[cfg(feature = "usb-log")]
+    let console_task = logging::usb::setup_logging();
     #[cfg(feature = "alt-log")]
-    let alt_uart_rx_task = {
+    let console_task =
-        use esp_hal::uart::Uart;
+        logging::uart::setup_logging(peripherals.UART2, peripherals.GPIO12, peripherals.GPIO5);
-
+    #[cfg(feature = "rtt-log")]
-        let (uart_rx, uart_tx) = Uart::new(peripherals.UART2, Default::default())
+    let console_task = logging::rtt::setup_logging();
-            .unwrap()
-            .with_tx(peripherals.GPIO12)
-            .with_rx(peripherals.GPIO5)
-            .split();
-        logging::setup_alternative_logging(uart_tx, LOG_LEVEL_FILTER);
-        info!("Logger initialized!");
-        console::run_console(uart_rx.into_async())
-    };
-
-    #[cfg(not(feature = "alt-log"))]
-    let alt_uart_rx_task = async {};
 
     // Use the internal DRAM as the heap.
     // Memory reclaimed from the esp-idf bootloader.
@@ -444,7 +425,7 @@ async fn main(_spawner: Spawner) {
         ),
         create_hid_report_interceptor(),
         user_controller.event_loop(),
-        alt_uart_rx_task
+        console_task
     ]
     .await;
 }