Preserve the first HWCDC console session after flashing

.github/workflows/run-unit-tests.yml

@@ -282,6 +282,7 @@ jobs:
           python3 -B test/test_esp32_tinyusb_nonblocking.py
           python3 -B test/test_esp32_usb_session_fix.py
           python3 -B test/test_hwcdc_tx_backport.py -v
+          python3 -B test/test_hwcdc_startup_session.py -v
           python3 -B test/test_esp32_hwcdc_recipes.py -v
           python3 -B test/test_hwcdc_role_transport.py -v
           python3 -B test/test_hwcdc_write_capacity.py -v

scripts/esp32_usb_session_fix.py

@@ -106,7 +106,7 @@ def replace_source(build_env, node):
 # HWCDC fixes are deliberately limited to the installed Arduino2.0.17 C3/S3
 # native Serial/JTAG driver. Other versions retain their own framework driver.
 PINNED_HWCDC_SHA256 = "d0a8ca606c2729c8522a041113285dbf27033c22a5a6af8649a7305ffe84c449"
-PATCHED_HWCDC_SHA256 = "7429655b063022e42c677dc79ecf0aab78acf33bba21a8cdb822435400c39719"
+PATCHED_HWCDC_SHA256 = "0969a94ae32edcfeeedb3daab19e5697e6c7647dc31578cdb0c3a0cb29e7c6d6"
 HWCDC_S3_PHY_GUARD = "#if CONFIG_IDF_TARGET_ESP32S3 && ARDUINO_USB_MODE && ARDUINO_USB_CDC_ON_BOOT\n"
 HWCDC_S3_PHY_HELPERS = HWCDC_S3_PHY_GUARD + r'''#include "soc/usb_wrap_struct.h"
 #include "soc/rtc_cntl_struct.h"
@@ -146,6 +146,7 @@ HWCDC_S3_PHY_CONFIG = r"""    // TinyUSB's RTC PHY selection survives a software
 """
 HWCDC_TX_SUPPORT = r"""
 // MeshCore pinned HWCDC TX suffix/interrupt backport (upstream #12606).
+extern "C" bool meshEsp32HwcdcShouldReportBusReset() __attribute__((weak));
 static uint8_t mesh_hwcdc_tx_stash[64] = {0};
 static size_t mesh_hwcdc_tx_stash_len = 0;
 static bool mesh_hwcdc_tx_allowed = true;
@@ -320,6 +321,13 @@ def patched_hwcdc_source(source):
                             '        portENTER_CRITICAL_ISR(&mesh_hwcdc_tx_mux);\n'
                             '        mesh_hwcdc_tx_stash_len = 0;\n        mesh_hwcdc_fifo_pending = false;\n'
                             '        portEXIT_CRITICAL_ISR(&mesh_hwcdc_tx_mux);\n        connected = false;\n    }\n\n//    if (usbjtag_intr_status', 1)
+    source = source.replace(
+        '        arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken);',
+        '        // Classify at capture: setup-time enumeration must not become an\n'
+        '        // active-session reset if the framework event task runs later.\n'
+        '        if (!meshEsp32HwcdcShouldReportBusReset || meshEsp32HwcdcShouldReportBusReset()) {\n'
+        '            arduino_hw_cdc_event_post(ARDUINO_HW_CDC_EVENTS, ARDUINO_HW_CDC_BUS_RESET_EVENT, &event, sizeof(arduino_hw_cdc_event_data_t), &xTaskWoken);\n'
+        '        }', 1)
     source = source.replace('    if(buffer == NULL) {\n',
                             '    if(buffer == NULL) {\n        mesh_hwcdc_clear_tx_stash();\n', 1)
     source = source.replace('size_t HWCDC::setTxBufferSize(size_t tx_queue_len){\n',

src/helpers/UsbLogging.cpp

@@ -386,6 +386,13 @@ static std::atomic<uint32_t> esp32_hwcdc_access_generation{0};
 static std::atomic<uint32_t> esp32_hwcdc_allowed_generation{0};
 static std::atomic<uint32_t> esp32_hwcdc_bus_reset_generation{0};
 static UsbSelfResetBurstGuard esp32_hwcdc_self_reset_guard;
+// Cold-start queues contain only this boot's diagnostics, never a previous
+// application session. The host can enumerate and submit its first command
+// while setup() is still initializing the radio/filesystem. Do not turn that
+// initial enumeration into a second PHY detach which invalidates its new port.
+// TX/RX event delivery must not end startup: boot diagnostics and a queued
+// first command are normal before the protocol owner begins its service loop.
+static std::atomic<bool> esp32_hwcdc_startup_pending{true};
 // HWCDC::write() retains bytes in its software ring when the framework's raw
 // five-millisecond SOF detector happens to report false. In that path the core
 // does not leave SERIAL_IN_EMPTY enabled, so a valid Companion response can
@@ -415,6 +422,7 @@ static void handleEsp32HwcdcEvent(void*, esp_event_base_t, int32_t event_id,
                                   void*) {
   if (event_id == ARDUINO_HW_CDC_BUS_RESET_EVENT) {
     clearUsbLoggingClientActivity();
+    if (esp32_hwcdc_startup_pending.load(std::memory_order_acquire)) return;
     // A host may issue several reset requests while enumerating the clean
     // transport. Suppress the complete burst; post-clean traffic below ends
     // the exemption before any later active-session reset can be ignored.
@@ -1554,6 +1562,10 @@ bool takeUsbTerminalSessionReset() {
   primary_usb_terminal_taken_reset_generation = reset_generation;
   return true;
 #elif MESH_ESP32_HWCDC_SESSION_GUARD
+  // Every HWCDC protocol owner checks for a boundary before processing input.
+  // After this first service pass, resets retain the ordinary stale-session
+  // quarantine even if no host has submitted a command yet.
+  esp32_hwcdc_startup_pending.store(false, std::memory_order_release);
   const uint32_t reset_generation =
       esp32_hwcdc_bus_reset_generation.load(std::memory_order_acquire);
   if (reset_generation == esp32_hwcdc_taken_bus_reset_generation) {
@@ -1607,6 +1619,7 @@ bool tryCompleteUsbTerminalSessionReset() {
       == primaryUsbSessionGeneration()
       && !nrf52_usb_reenumeration_pending.load(std::memory_order_acquire);
 #elif MESH_ESP32_HWCDC_SESSION_GUARD
+  esp32_hwcdc_startup_pending.store(false, std::memory_order_release);
   if (!esp32_hwcdc_cleanup_pending
       && esp32_hwcdc_allowed_generation.load(std::memory_order_acquire)
           == esp32_hwcdc_access_generation.load(std::memory_order_acquire)) {
@@ -1786,6 +1799,15 @@ bool isUsbLoggingTransportRecoveryPending() {
 
 }  // namespace mesh
 
+#if MESH_ESP32_HWCDC_SESSION_GUARD && MESH_HWCDC_PINNED_TX_BACKPORT
+// Capture startup at the actual hardware reset, not when the finite framework
+// event queue eventually delivers it. A delayed initial enumeration event
+// must not be reclassified as an active-session reset after loop() has begun.
+extern "C" bool meshEsp32HwcdcShouldReportBusReset() {
+  return !mesh::esp32_hwcdc_startup_pending.load(std::memory_order_acquire);
+}
+#endif
+
 #if defined(NRF52_PLATFORM) && defined(USE_TINYUSB)
 extern "C" void meshTinyUsbLoggingTxComplete(uint8_t instance) {
   if (instance == (mesh::hasDedicatedUsbLoggingPort() ? 1 : 0))

src/helpers/UsbLogging.h

@@ -160,6 +160,8 @@ uint32_t usbTerminalDroppedBytes();
 // CDC0 DTR-low on TinyUSB, or a hardware CDC bus reset on ESP32. This is
 // independent of polling current line/SOF state; ESP32 retains that poll as a
 // fallback because its bundled framework event queue is finite.
+// The first HWCDC call ends cold-start enumeration before protocol input is
+// processed. Enumeration during setup preserves the first host's queued RX.
 bool takeUsbTerminalSessionReset();
 // After the application has reset its protocol state, atomically purge any
 // CDC0 writer that raced the close callback and reopen the producer gate. A

test/test_hwcdc_startup_session.py

@@ -0,0 +1,253 @@
+#!/usr/bin/env python3
+"""Execute initial HWCDC enumeration with the real ISR and session cleanup.
+
+The RTOS event queue, USB pads and RX/ring queues are peripheral stubs. The
+pinned SDK ISR, MeshCore event handler, session gate and cleanup are production
+code. This specifically tests the gap between Serial.begin() and the first
+application command service, including delayed framework event delivery.
+"""
+from pathlib import Path
+import re
+import subprocess
+import tempfile
+import unittest
+
+import test_hwcdc_tx_backport as backport
+
+PATCHED = backport.PATCHED
+body = backport.body
+
+ROOT = Path(__file__).resolve().parents[1]
+
+EXTRA = r'''
+#include <helpers/UsbAsciiBinarySwitch.h>
+#include <iostream>
+#include <string>
+#define MESH_ESP32_HWCDC_SESSION_GUARD 1
+#define MESH_HWCDC_PINNED_TX_BACKPORT 1
+#define portENTER_CRITICAL(m) portENTER_CRITICAL_SAFE(m)
+#define portEXIT_CRITICAL(m) portEXIT_CRITICAL_SAFE(m)
+using esp_event_base_t = int;
+static bool xPortInIsrContext() { return false; }
+static uint32_t clock_ms;
+static uint32_t millis() { return clock_ms; }
+static void delay(uint32_t ms) { clock_ms += ms; }
+static bool pads_enabled = true;
+static unsigned phy_detaches = 0;
+static std::deque<uint8_t> host_rx;
+static bool detachEsp32HwcdcPads() {
+  ++phy_detaches;
+  const bool previous = pads_enabled;
+  pads_enabled = plugged = false;
+  return previous;
+}
+static void restoreEsp32HwcdcPads(bool enabled) {
+  pads_enabled = plugged = enabled;
+}
+static void setPlatformDebugOutputEnabled(bool) {}
+static bool isUsbDebugLoggingEnabled() { return false; }
+static void clearUsbLoggingClientActivity() {}
+static void noteUsbLoggingTxComplete() {}
+static std::atomic<uint32_t> esp32_hwcdc_access_generation{0};
+static std::atomic<uint32_t> esp32_hwcdc_allowed_generation{0};
+static std::atomic<uint32_t> esp32_hwcdc_bus_reset_generation{0};
+static std::atomic<bool> esp32_hwcdc_rx_queue_ready{true};
+static uint32_t esp32_hwcdc_taken_bus_reset_generation = 0;
+static mesh::UsbSelfResetBurstGuard esp32_hwcdc_self_reset_guard;
+@STARTUP_STATE@
+static std::atomic<bool> esp32_hwcdc_tx_kick_pending{false};
+static std::atomic<bool> esp32_hwcdc_tx_primed{false};
+static portMUX_TYPE esp32_hwcdc_session_mux;
+static std::atomic<size_t> esp32_hwcdc_tx_buffer_capacity{4096};
+static bool esp32_hwcdc_cleanup_pending = false;
+static bool esp32_hwcdc_restore_pad_enabled = false;
+static uint32_t esp32_hwcdc_cleanup_generation = 0;
+static std::atomic<uint32_t> usb_logging_tx_progress{0};
+static std::atomic<bool> usb_logging_tx_waiting{false};
+struct SerialMock {
+  unsigned flush_calls = 0;
+  int availableForWrite() {
+    unsigned waiting = 0;
+    vRingbufferGetInfo(&ring, nullptr, nullptr, nullptr, nullptr, &waiting);
+    return 4096 - waiting;
+  }
+  void flush() {
+    ++flush_calls;
+    if (!ring.data.empty()) ring.data.pop_front();
+  }
+  int read() {
+    if (host_rx.empty()) return -1;
+    const int byte = host_rx.front(); host_rx.pop_front(); return byte;
+  }
+  bool isPlugged() { return plugged; }
+  operator bool() { return connected && plugged; }
+  void setDebugOutput(bool) {}
+} Serial;
+struct Exclusive {
+  bool tryRunExclusive(void (*function)(void*), void* opaque) {
+    function(opaque); return true;
+  }
+} guarded_esp32_hwcdc_port;
+struct Esp32HwcdcPurgeResult { bool tx_empty = false; };
+struct UsbLoggingObservation {
+  uint32_t tx_progress = 0;
+  bool supported = false, host_connected = false, reader_connected = false;
+  bool pending = false;
+};
+@FUNCTIONS@
+@OWNER_RESET_HOOK@
+static void deliverEvents() {
+  while (!queued_events.empty()) {
+    const int event = queued_events.front(); queued_events.pop_front();
+    handleEsp32HwcdcEvent(nullptr, 0, event, nullptr);
+  }
+}
+static void enumerate() {
+  intr_status = USB_SERIAL_JTAG_INTR_BUS_RESET;
+  hw_cdc_isr_handler(nullptr);
+}
+static void sendFirstCommand() {
+  host_rx = {'v', 'e', 'r', '\r'};
+  queued_events.push_back(ARDUINO_HW_CDC_RX_EVENT);
+}
+static void firstService() {
+  // These are the calls the real role makes before processing commands.
+  assert(!takeUsbTerminalSessionReset());
+  assert(tryCompleteUsbTerminalSessionReset());
+}
+static void requireFirstCommand() {
+  assert(phy_detaches == 0 && pads_enabled && canAccessEsp32Hwcdc(nullptr));
+  assert((host_rx == std::deque<uint8_t>{'v', 'e', 'r', '\r'}));
+}
+int main(int argc, char** argv) {
+  assert(argc == 2);
+  const std::string name = argv[1];
+  reset(); plugged = true;
+  if (name == "early") {
+    enumerate(); sendFirstCommand(); deliverEvents(); firstService();
+    requireFirstCommand();
+  } else if (name == "delayed") {
+    enumerate(); sendFirstCommand(); firstService(); deliverEvents();
+    assert(tryCompleteUsbTerminalSessionReset()); requireFirstCommand();
+  } else if (name == "boot_tx") {
+    // Real ISR stages a partial boot diagnostic before the host enumerates.
+    ring.data.push_back({'F', 'i', 'r', 'm', 'w', 'a', 'r', 'e', ':', '\r', '\n'});
+    pulse(4); assert(mesh_hwcdc_tx_stash_len != 0 && mesh_hwcdc_fifo_pending);
+    deliverEvents(); // Boot TX is not application protocol readiness.
+    enumerate();
+    assert(mesh_hwcdc_tx_stash_len == 0 && !mesh_hwcdc_fifo_pending);
+    sendFirstCommand(); firstService(); deliverEvents();
+    assert(tryCompleteUsbTerminalSessionReset()); requireFirstCommand();
+  } else if (name == "boot_rx") {
+    // A browser can submit input before setup finishes. An RX event alone
+    // cannot make its subsequent enumeration trigger another PHY detach.
+    sendFirstCommand(); deliverEvents(); enumerate();
+    firstService(); deliverEvents();
+    assert(tryCompleteUsbTerminalSessionReset()); requireFirstCommand();
+  } else if (name == "active") {
+    firstService(); assert(meshEsp32HwcdcShouldReportBusReset());
+    ring.data.push_back({'o', 'l', 'd', '\r', '\n'});
+    host_rx = {'o', 'l', 'd', '\r'};
+    enumerate(); deliverEvents();
+    assert(!canAccessEsp32Hwcdc(nullptr));
+    assert(takeUsbTerminalSessionReset());
+    assert(tryCompleteUsbTerminalSessionReset());
+    assert(phy_detaches == 1 && host_rx.empty() && ring.data.empty());
+    assert(canAccessEsp32Hwcdc(nullptr) && mesh_hwcdc_tx_allowed);
+  } else { assert(false && "Unknown startup case"); }
+  std::cout << "PASS " << name << " phy_detaches=" << phy_detaches
+            << " retained_rx=" << host_rx.size() << '\n';
+}
+'''
+
+
+def harness(*, remove_boot_gate=False, remove_owner_hook=False,
+            boot_traffic_arms=False, keep_startup_forever=False):
+    source = (ROOT / "src/helpers/UsbLogging.cpp").read_text()
+    signatures = (
+        "static bool canAccessEsp32Hwcdc(void*)",
+        "static void handleEsp32HwcdcEvent(",
+        "static void serviceEsp32HwcdcTxKickExclusive(",
+        "static void purgeEsp32HwcdcQueues(",
+        "bool resetUsbCompanionTransport(",
+        "UsbLoggingObservation observeUsbLoggingTransport(",
+        "bool takeUsbTerminalSessionReset()",
+        "bool tryCompleteUsbTerminalSessionReset()",
+    )
+    functions = "\n".join(body(source, signature) for signature in signatures)
+    owner = body(source, 'extern "C" bool meshEsp32HwcdcShouldReportBusReset()')
+    # The extracted application methods live in a flat test shell. Only their
+    # namespace qualification changes; state and the actual owner hook remain.
+    owner = owner.replace("mesh::esp32_hwcdc_startup_pending", "esp32_hwcdc_startup_pending")
+    state = re.search(r"^static std::atomic<bool> esp32_hwcdc_startup_pending.*;",
+                      source, re.MULTILINE)
+    assert state is not None
+    sdk = PATCHED
+    if remove_boot_gate:
+        functions = functions.replace(
+            "    if (esp32_hwcdc_startup_pending.load(std::memory_order_acquire)) return;\n", "")
+        owner = owner.replace("!esp32_hwcdc_startup_pending.load(std::memory_order_acquire)", "true")
+    if remove_owner_hook:
+        owner = owner.replace("!esp32_hwcdc_startup_pending.load(std::memory_order_acquire)", "true")
+    if boot_traffic_arms:
+        functions = functions.replace(
+            "    esp32_hwcdc_self_reset_guard.notePostCleanActivity();",
+            "    esp32_hwcdc_startup_pending.store(false, std::memory_order_release);\n"
+            "    esp32_hwcdc_self_reset_guard.notePostCleanActivity();")
+    if keep_startup_forever:
+        functions = functions.replace(
+            "  esp32_hwcdc_startup_pending.store(false, std::memory_order_release);", "")
+    prefix = backport.BackportTest().harness(sdk).split("int main(){", 1)[0]
+    prefix = prefix.replace("static std::vector<unsigned> event_lengths;",
+                            "static std::vector<unsigned> event_lengths;\n"
+                            "static std::deque<int> queued_events;")
+    prefix = prefix.replace(
+        " if(event==ARDUINO_HW_CDC_TX_EVENT)event_lengths.push_back(data->tx.len);",
+        " queued_events.push_back(event);\n"
+        " if(event==ARDUINO_HW_CDC_TX_EVENT)event_lengths.push_back(data->tx.len);")
+    return prefix + EXTRA.replace("@STARTUP_STATE@", state[0]).replace(
+        "@FUNCTIONS@", functions).replace("@OWNER_RESET_HOOK@", owner)
+
+
+class HwcdcStartupSessionTests(unittest.TestCase):
+    def run_case(self, case, *, expect_success=True, **controls):
+        with tempfile.TemporaryDirectory(prefix="hwcdc-startup-") as directory:
+            directory = Path(directory)
+            cpp, binary = directory / "startup.cpp", directory / "startup"
+            cpp.write_text(harness(**controls), encoding="ascii")
+            compiled = subprocess.run([
+                "g++", "-std=c++17", "-pthread", "-Wall", "-Wextra",
+                "-Wno-unused-parameter", "-Wno-sign-compare",
+                "-fsanitize=address,undefined", "-fno-sanitize-recover=all",
+                "-fno-pie", "-no-pie", "-I" + str(ROOT / "src"),
+                str(cpp), "-o", str(binary)], capture_output=True, text=True, timeout=60)
+            self.assertEqual(compiled.returncode, 0, compiled.stdout + compiled.stderr)
+            result = subprocess.run([str(binary), case], capture_output=True,
+                                    text=True, timeout=10)
+            if expect_success:
+                self.assertEqual(result.returncode, 0, result.stdout + result.stderr)
+            else:
+                self.assertNotEqual(result.returncode, 0, "Negative control did not fail")
+
+    def test_initial_enumeration_retains_command_and_active_reset_still_purges(self):
+        for case in ("early", "delayed", "boot_tx", "boot_rx", "active"):
+            with self.subTest(case=case):
+                self.run_case(case)
+
+    def test_original_boot_gate_failure_negative_control(self):
+        self.run_case("early", expect_success=False, remove_boot_gate=True)
+
+    def test_delayed_owner_event_negative_control(self):
+        self.run_case("delayed", expect_success=False, remove_owner_hook=True)
+
+    def test_boot_traffic_cannot_end_startup_negative_controls(self):
+        for case in ("boot_tx", "boot_rx"):
+            with self.subTest(case=case):
+                self.run_case(case, expect_success=False, boot_traffic_arms=True)
+
+    def test_post_ready_quarantine_remains_required_negative_control(self):
+        self.run_case("active", expect_success=False, keep_startup_forever=True)
+
+
+if __name__ == "__main__":
+    unittest.main(verbosity=2)

test/test_hwcdc_tx_backport.py

@@ -395,6 +395,7 @@ static void noteUsbLoggingTxComplete(){}
 static std::atomic<uint32_t> esp32_hwcdc_access_generation{0},esp32_hwcdc_allowed_generation{0};
 static std::atomic<uint32_t> esp32_hwcdc_bus_reset_generation{0};
 static mesh::UsbSelfResetBurstGuard esp32_hwcdc_self_reset_guard;
+static std::atomic<bool> esp32_hwcdc_startup_pending{false}; // post-setup runtime fixture
 static std::atomic<bool> esp32_hwcdc_tx_kick_pending{false},esp32_hwcdc_tx_primed{false};
 static portMUX_TYPE esp32_hwcdc_session_mux;
 static std::atomic<size_t> esp32_hwcdc_tx_buffer_capacity{4096};