From c864c8a982f97601f31d400b84ca2548aaf9829d Mon Sep 17 00:00:00 2001 From: Demo for summer'23 <123392361+lure23@users.noreply.github.com> Date: Mon, 24 Jun 2024 16:26:08 +0300 Subject: [PATCH] Fix: allow `defmt-espflash` + `auto` configuration @ esp-println (#1687) * esp-println: Fix 'defmt-espflash,auto' feature combination * formatted according to instructions * restructure, compiles also on 'esp32' * Added CI check for compiling with 'dfm-espflash' feature --------- Co-authored-by: Asko Kauppi --- .github/workflows/ci.yml | 10 +++- esp-println/build.rs | 3 +- esp-println/src/defmt.rs | 8 +-- esp-println/src/lib.rs | 115 ++++++++++++++++++++++++--------------- 4 files changed, 86 insertions(+), 50 deletions(-) diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 7d3b9f5ff..47cc8a0b7 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -207,7 +207,15 @@ jobs: --features=${{ matrix.device.soc }},log \ --target=${{ matrix.device.target }} \ esp-println - + + # So #1678 doesn't reoccur ('defmt-espflash,auto') + - name: Build (with feature 'defmt-espflash') + run: | + cargo xtask build-package \ + --features=${{ matrix.device.soc }},log,defmt-espflash \ + --target=${{ matrix.device.target }} \ + esp-println + extras: runs-on: ubuntu-latest diff --git a/esp-println/build.rs b/esp-println/build.rs index b81e5604b..1ac2f2368 100644 --- a/esp-println/build.rs +++ b/esp-println/build.rs @@ -10,8 +10,7 @@ fn main() { assert_unique_used_features!("jtag-serial", "uart", "auto"); // Ensure that, if the `jtag-serial` communication method feature is enabled, - // either the `esp32c3`, `esp32c6`, `esp32h2`, or `esp32s3` chip feature is - // enabled. + // a compatible chip feature is also enabled. if cfg!(feature = "jtag-serial") && !(cfg!(feature = "esp32c3") || cfg!(feature = "esp32c6") diff --git a/esp-println/src/defmt.rs b/esp-println/src/defmt.rs index f5c8455e9..b790f8591 100644 --- a/esp-println/src/defmt.rs +++ b/esp-println/src/defmt.rs @@ -4,7 +4,7 @@ #[cfg(feature = "critical-section")] use critical_section::RestoreState; -use super::Printer; +use super::PrinterImpl; /// Global logger lock. #[cfg(feature = "critical-section")] @@ -52,7 +52,7 @@ unsafe impl defmt::Logger for Logger { // section. ENCODER.end_frame(do_write); - Printer.flush(); + Self::flush(); #[cfg(feature = "critical-section")] { @@ -74,7 +74,7 @@ unsafe impl defmt::Logger for Logger { } unsafe fn flush() { - Printer.flush(); + PrinterImpl::flush(); } unsafe fn write(bytes: &[u8]) { @@ -85,5 +85,5 @@ unsafe impl defmt::Logger for Logger { } fn do_write(bytes: &[u8]) { - Printer.write_bytes_assume_cs(bytes) + PrinterImpl::write_bytes_assume_cs(bytes) } diff --git a/esp-println/src/lib.rs b/esp-println/src/lib.rs index a31790586..e63ed5aa7 100644 --- a/esp-println/src/lib.rs +++ b/esp-println/src/lib.rs @@ -68,18 +68,6 @@ macro_rules! dbg { }; } -#[cfg(any(feature = "jtag-serial", feature = "auto"))] -pub struct PrinterSerialJtag; - -#[cfg(any(feature = "uart", feature = "auto"))] -pub struct PrinterUart; - -#[cfg(feature = "jtag-serial")] -pub type PrinterImpl = PrinterSerialJtag; - -#[cfg(feature = "uart")] -pub type PrinterImpl = PrinterUart; - pub struct Printer; impl core::fmt::Write for Printer { @@ -90,70 +78,106 @@ impl core::fmt::Write for Printer { } impl Printer { - #[cfg(not(feature = "auto"))] - pub fn write_bytes(bytes: &[u8]) { + fn write_bytes(bytes: &[u8]) { with(|| { PrinterImpl::write_bytes_assume_cs(bytes); PrinterImpl::flush(); }) } +} - #[cfg(feature = "auto")] - pub fn write_bytes(bytes: &[u8]) { - #[cfg(any( - feature = "esp32c3", - feature = "esp32c6", - feature = "esp32h2", - feature = "esp32s3" - ))] - { +#[cfg(feature = "jtag-serial")] +type PrinterImpl = serial_jtag_printer::Printer; + +#[cfg(feature = "uart")] +type PrinterImpl = uart_printer::Printer; + +#[cfg(feature = "auto")] +type PrinterImpl = auto_printer::Printer; + +#[cfg(all( + feature = "auto", + any( + feature = "esp32c3", + feature = "esp32c6", + feature = "esp32h2", + feature = "esp32p4", // as mentioned in 'build.rs' + feature = "esp32s3" + ) +))] +mod auto_printer { + use super::with; + use crate::{ + serial_jtag_printer::Printer as PrinterSerialJtag, + uart_printer::Printer as PrinterUart, + }; + + pub struct Printer; + impl Printer { + fn use_jtag() -> bool { // Decide if serial-jtag is used by checking SOF interrupt flag. // SOF packet is sent by the HOST every 1ms on a full speed bus. // Between two consecutive ticks, there will be at least 1ms (selectable tick // rate range is 1 - 1000Hz). // We don't reset the flag - if it was ever connected we assume serial-jtag is // used - #[cfg(feature = "esp32c3")] const USB_DEVICE_INT_RAW: *const u32 = 0x60043008 as *const u32; #[cfg(feature = "esp32c6")] const USB_DEVICE_INT_RAW: *const u32 = 0x6000f008 as *const u32; #[cfg(feature = "esp32h2")] const USB_DEVICE_INT_RAW: *const u32 = 0x6000f008 as *const u32; + #[cfg(feature = "esp32p4")] + const USB_DEVICE_INT_RAW: *const u32 = unimplemented!(); #[cfg(feature = "esp32s3")] const USB_DEVICE_INT_RAW: *const u32 = 0x60038000 as *const u32; const SOF_INT_MASK: u32 = 0b10; - let is_serial_jtag = - unsafe { (USB_DEVICE_INT_RAW.read_volatile() & SOF_INT_MASK) != 0 }; + unsafe { (USB_DEVICE_INT_RAW.read_volatile() & SOF_INT_MASK) != 0 } + } - if is_serial_jtag { + pub fn write_bytes_assume_cs(bytes: &[u8]) { + if Self::use_jtag() { with(|| { PrinterSerialJtag::write_bytes_assume_cs(bytes); - PrinterSerialJtag::flush(); }) } else { with(|| { PrinterUart::write_bytes_assume_cs(bytes); - PrinterUart::flush(); }) } } - #[cfg(not(any( - feature = "esp32c3", - feature = "esp32c6", - feature = "esp32h2", - feature = "esp32s3" - )))] - with(|| { - PrinterUart::write_bytes_assume_cs(bytes); - PrinterUart::flush(); - }) + pub fn flush() { + if Self::use_jtag() { + with(|| { + PrinterSerialJtag::flush(); + }) + } else { + with(|| { + PrinterUart::flush(); + }) + } + } } } +#[cfg(all( + feature = "auto", + not(any( + feature = "esp32c3", + feature = "esp32c6", + feature = "esp32h2", + feature = "esp32p4", + feature = "esp32s3" + )) +))] +mod auto_printer { + // models that only have UART + pub type Printer = crate::uart_printer::Printer; +} + #[cfg(all( any(feature = "jtag-serial", feature = "auto"), any( @@ -166,6 +190,7 @@ impl Printer { ))] mod serial_jtag_printer { use portable_atomic::{AtomicBool, Ordering}; + pub struct Printer; #[cfg(feature = "esp32c3")] const SERIAL_JTAG_FIFO_REG: usize = 0x6004_3000; @@ -222,7 +247,7 @@ mod serial_jtag_printer { true } - impl super::PrinterSerialJtag { + impl Printer { pub fn write_bytes_assume_cs(bytes: &[u8]) { if fifo_full() { // The FIFO is full. Let's see if we can progress. @@ -265,7 +290,9 @@ mod serial_jtag_printer { #[cfg(all(any(feature = "uart", feature = "auto"), feature = "esp32"))] mod uart_printer { const UART_TX_ONE_CHAR: usize = 0x4000_9200; - impl super::PrinterUart { + + pub struct Printer; + impl Printer { pub fn write_bytes_assume_cs(bytes: &[u8]) { for &b in bytes { unsafe { @@ -282,7 +309,8 @@ mod uart_printer { #[cfg(all(any(feature = "uart", feature = "auto"), feature = "esp32s2"))] mod uart_printer { - impl super::PrinterUart { + pub struct Printer; + impl Printer { pub fn write_bytes_assume_cs(bytes: &[u8]) { // On ESP32-S2 the UART_TX_ONE_CHAR ROM-function seems to have some issues. for chunk in bytes.chunks(64) { @@ -421,7 +449,8 @@ mod uart_printer { } } - impl super::PrinterUart { + pub struct Printer; + impl Printer { pub fn write_bytes_assume_cs(bytes: &[u8]) { for chunk in bytes.chunks(Device::CHUNK_SIZE) { for &b in chunk {