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 <akauppi@gmail.com>

.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
 

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")

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)
 }

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 {