From a4f732478a58144f7e3d93b6547a2235b6309daf Mon Sep 17 00:00:00 2001 From: agessaman Date: Fri, 29 May 2026 23:45:40 -0700 Subject: [PATCH 1/3] Implement non-blocking write mechanism in KissModem to prevent TX buffer stalls. Introduce frame write management and update related methods for improved handling of serial communication. Adjust platform configurations for USB-CDC to enhance reliability on ESP32. --- examples/kiss_modem/KissModem.cpp | 42 +++++++++++++++++++++++------- examples/kiss_modem/KissModem.h | 9 +++++++ examples/kiss_modem/main.cpp | 5 ++++ variants/heltec_v4/platformio.ini | 10 +++++++ variants/station_g2/platformio.ini | 10 +++++++ 5 files changed, 67 insertions(+), 9 deletions(-) diff --git a/examples/kiss_modem/KissModem.cpp b/examples/kiss_modem/KissModem.cpp index eeab1501d5..7be9c5319e 100644 --- a/examples/kiss_modem/KissModem.cpp +++ b/examples/kiss_modem/KissModem.cpp @@ -22,6 +22,7 @@ KissModem::KissModem(Stream& serial, mesh::LocalIdentity& identity, mesh::RNG& r _getStatsCallback = nullptr; _config = {0, 0, 0, 0, 0}; _signal_report_enabled = true; + _tx_write_aborted = false; } void KissModem::begin() { @@ -32,35 +33,58 @@ void KissModem::begin() { _tx_state = TX_IDLE; } +void KissModem::beginFrameWrite() { + _tx_write_aborted = false; +} + +void KissModem::rawWrite(uint8_t b) { + /* Once a frame has been aborted, swallow the rest of it so we never emit a + truncated KISS frame to the host. */ + if (_tx_write_aborted) return; + + /* Strictly non-blocking: if there is no room in the TX buffer, drop the rest + of this frame rather than wait. A stalled/absent USB-CDC host must never + stall loop() (any wait here starves the RX read loop and lets the TinyUSB + RX path overflow). Only write() when there is space, so write() can never + enter its internal busy-wait either. */ + if (_serial.availableForWrite() <= 0) { + _tx_write_aborted = true; + return; + } + _serial.write(b); +} + void KissModem::writeByte(uint8_t b) { if (b == KISS_FEND) { - _serial.write(KISS_FESC); - _serial.write(KISS_TFEND); + rawWrite(KISS_FESC); + rawWrite(KISS_TFEND); } else if (b == KISS_FESC) { - _serial.write(KISS_FESC); - _serial.write(KISS_TFESC); + rawWrite(KISS_FESC); + rawWrite(KISS_TFESC); } else { - _serial.write(b); + rawWrite(b); } } void KissModem::writeFrame(uint8_t type, const uint8_t* data, uint16_t len) { - _serial.write(KISS_FEND); + beginFrameWrite(); + rawWrite(KISS_FEND); writeByte(type); for (uint16_t i = 0; i < len; i++) { writeByte(data[i]); } - _serial.write(KISS_FEND); + rawWrite(KISS_FEND); } void KissModem::writeHardwareFrame(uint8_t sub_cmd, const uint8_t* data, uint16_t len) { - _serial.write(KISS_FEND); + beginFrameWrite(); + rawWrite(KISS_FEND); writeByte(KISS_CMD_SETHARDWARE); writeByte(sub_cmd); for (uint16_t i = 0; i < len; i++) { writeByte(data[i]); } - _serial.write(KISS_FEND); + rawWrite(KISS_FEND); } void KissModem::writeHardwareError(uint8_t error_code) { diff --git a/examples/kiss_modem/KissModem.h b/examples/kiss_modem/KissModem.h index bbe99d6de4..f80bec33f2 100644 --- a/examples/kiss_modem/KissModem.h +++ b/examples/kiss_modem/KissModem.h @@ -28,6 +28,11 @@ #define KISS_DEFAULT_SLOTTIME 10 #define KISS_TX_TIMEOUT_FACTOR 3/2 // 1.5x estimated airtime +/* Max time (ms) to wait for serial TX buffer space before dropping a frame. + Prevents a stalled/absent USB-CDC host from blocking the single-threaded loop() + indefinitely (the ESP32 USB-CDC failure mode; UART transports never fill up). */ +#define KISS_WRITE_TIMEOUT_MS 50 + #define HW_CMD_GET_IDENTITY 0x01 #define HW_CMD_GET_RANDOM 0x02 #define HW_CMD_VERIFY_SIGNATURE 0x03 @@ -131,6 +136,10 @@ class KissModem { RadioConfig _config; bool _signal_report_enabled; + bool _tx_write_aborted; // set when the current frame is dropped (no TX buffer space) + + void beginFrameWrite(); // reset per-frame abort state + void rawWrite(uint8_t b); // non-blocking write: drops the frame rather than stalling loop() void writeByte(uint8_t b); void writeFrame(uint8_t type, const uint8_t* data, uint16_t len); void writeHardwareFrame(uint8_t sub_cmd, const uint8_t* data, uint16_t len); diff --git a/examples/kiss_modem/main.cpp b/examples/kiss_modem/main.cpp index 7fbcaed127..c1ec1f805e 100644 --- a/examples/kiss_modem/main.cpp +++ b/examples/kiss_modem/main.cpp @@ -108,6 +108,11 @@ void setup() { modem = new KissModem(Serial1, identity, rng, radio_driver, board, sensors); #else Serial.begin(115200); +#if defined(ESP32) + /* Bound USB-CDC transmit so write() drops instead of blocking forever when the + host read side stalls; otherwise the single-threaded modem loop wedges. */ + Serial.setTxTimeoutMs(KISS_WRITE_TIMEOUT_MS); +#endif uint32_t start = millis(); while (!Serial && millis() - start < 3000) delay(10); delay(100); diff --git a/variants/heltec_v4/platformio.ini b/variants/heltec_v4/platformio.ini index fabf38272d..4b6185162b 100644 --- a/variants/heltec_v4/platformio.ini +++ b/variants/heltec_v4/platformio.ini @@ -434,3 +434,13 @@ lib_deps = extends = Heltec_lora32_v4 build_src_filter = ${Heltec_lora32_v4.build_src_filter} +<../examples/kiss_modem/> +; Use the USB-Serial-JTAG peripheral (HWCDC) instead of TinyUSB CDC. The TinyUSB +; USBCDC path wedges permanently under TX backpressure on ESP32-S3 (write() busy- +; spins while "connected", RX events post to a 5-deep queue with portMAX_DELAY), +; leaving the modem unresponsive across host restarts. HWCDC bounds its writes and +; posts RX from ISR, so it does not hang. build_unflags strips the board default +; (=0); a bare -U is unreliable here because SCons reorders it after the -D defines. +build_unflags = -DARDUINO_USB_MODE=0 +build_flags = + ${Heltec_lora32_v4.build_flags} + -DARDUINO_USB_MODE=1 diff --git a/variants/station_g2/platformio.ini b/variants/station_g2/platformio.ini index 6432b52386..c10e19337b 100644 --- a/variants/station_g2/platformio.ini +++ b/variants/station_g2/platformio.ini @@ -243,3 +243,13 @@ lib_deps = extends = Station_G2 build_src_filter = ${Station_G2.build_src_filter} +<../examples/kiss_modem/> +; Use the USB-Serial-JTAG peripheral (HWCDC) instead of TinyUSB CDC. The TinyUSB +; USBCDC path wedges permanently under TX backpressure on ESP32-S3 (write() busy- +; spins while "connected", RX events post to a 5-deep queue with portMAX_DELAY), +; leaving the modem unresponsive across host restarts. HWCDC bounds its writes and +; posts RX from ISR, so it does not hang. build_unflags strips the board default +; (=0); a bare -U is unreliable here because SCons reorders it after the -D defines. +build_unflags = -DARDUINO_USB_MODE=0 +build_flags = + ${Station_G2.build_flags} + -DARDUINO_USB_MODE=1 From bd89519bb5b4bdc0d2441ac40fe4c42443a27aa5 Mon Sep 17 00:00:00 2001 From: agessaman Date: Sat, 30 May 2026 08:13:47 -0700 Subject: [PATCH 2/3] Refine comments in KissModem to clarify non-blocking write behavior and timeout handling for USB-CDC. Enhance documentation for better understanding of frame management and serial communication flow. --- examples/kiss_modem/KissModem.cpp | 14 +++++++------- examples/kiss_modem/KissModem.h | 6 +++--- examples/kiss_modem/main.cpp | 5 +++-- 3 files changed, 13 insertions(+), 12 deletions(-) diff --git a/examples/kiss_modem/KissModem.cpp b/examples/kiss_modem/KissModem.cpp index 7be9c5319e..19be5d412f 100644 --- a/examples/kiss_modem/KissModem.cpp +++ b/examples/kiss_modem/KissModem.cpp @@ -38,15 +38,15 @@ void KissModem::beginFrameWrite() { } void KissModem::rawWrite(uint8_t b) { - /* Once a frame has been aborted, swallow the rest of it so we never emit a - truncated KISS frame to the host. */ + /* A frame is sent all-or-nothing; once we start dropping, swallow the rest so + we never emit a truncated KISS frame. */ if (_tx_write_aborted) return; - /* Strictly non-blocking: if there is no room in the TX buffer, drop the rest - of this frame rather than wait. A stalled/absent USB-CDC host must never - stall loop() (any wait here starves the RX read loop and lets the TinyUSB - RX path overflow). Only write() when there is space, so write() can never - enter its internal busy-wait either. */ + /* Non-blocking: if the TX buffer is full, drop this frame instead of waiting. + loop() is single-threaded and also services RX and the radio, so it must not + stall on a host that has stopped reading. Writing only when space is free + keeps the underlying write() from blocking; a dropped reply is harmless as + the host retries. */ if (_serial.availableForWrite() <= 0) { _tx_write_aborted = true; return; diff --git a/examples/kiss_modem/KissModem.h b/examples/kiss_modem/KissModem.h index f80bec33f2..dedd51b73e 100644 --- a/examples/kiss_modem/KissModem.h +++ b/examples/kiss_modem/KissModem.h @@ -28,9 +28,9 @@ #define KISS_DEFAULT_SLOTTIME 10 #define KISS_TX_TIMEOUT_FACTOR 3/2 // 1.5x estimated airtime -/* Max time (ms) to wait for serial TX buffer space before dropping a frame. - Prevents a stalled/absent USB-CDC host from blocking the single-threaded loop() - indefinitely (the ESP32 USB-CDC failure mode; UART transports never fill up). */ +/* Upper bound (ms) on how long a serial write may wait for the host to drain the + TX buffer. Keeps a stalled USB-CDC host from freezing the single-threaded loop(); + UART transports drain via FIFO and never reach this. */ #define KISS_WRITE_TIMEOUT_MS 50 #define HW_CMD_GET_IDENTITY 0x01 diff --git a/examples/kiss_modem/main.cpp b/examples/kiss_modem/main.cpp index c1ec1f805e..eca2f42ef3 100644 --- a/examples/kiss_modem/main.cpp +++ b/examples/kiss_modem/main.cpp @@ -109,8 +109,9 @@ void setup() { #else Serial.begin(115200); #if defined(ESP32) - /* Bound USB-CDC transmit so write() drops instead of blocking forever when the - host read side stalls; otherwise the single-threaded modem loop wedges. */ + /* Cap how long a USB-CDC write blocks waiting for the host to drain the TX + buffer. loop() is single-threaded, so an unbounded wait when the host stalls + would also freeze RX and radio servicing. */ Serial.setTxTimeoutMs(KISS_WRITE_TIMEOUT_MS); #endif uint32_t start = millis(); From f1f9bd668f39991c711a28a6060e24475a1a9476 Mon Sep 17 00:00:00 2001 From: agessaman Date: Mon, 1 Jun 2026 20:42:11 -0700 Subject: [PATCH 3/3] Simplify KISS write path with all-or-nothing frame writes Pre-compute the escaped frame length and write only when the whole frame fits the TX buffer (canWriteFrame), replacing the per-byte non-blocking abort. This can't emit a truncated frame and lets the write helpers drop the _tx_write_aborted/beginFrameWrite/rawWrite machinery. Enlarge the HWCDC TX ring to 1024B (KISS_TX_BUFFER_SIZE); the 256B default is smaller than a max-size DATA frame, so all-or-nothing would otherwise drop every full-size RX packet. --- build.sh | 2 +- examples/kiss_modem/KissModem.cpp | 73 ++++++++++++++++--------------- examples/kiss_modem/KissModem.h | 15 ++++--- examples/kiss_modem/main.cpp | 6 +-- 4 files changed, 49 insertions(+), 47 deletions(-) diff --git a/build.sh b/build.sh index 313c4c47a0..23e31ab25c 100755 --- a/build.sh +++ b/build.sh @@ -93,7 +93,7 @@ get_pio_envs_ending_with_string() { # $1 should be the environment name get_platform_for_env() { local env_name=$1 - echo "$PIO_CONFIG_JSON" | python3 -c " + printf '%s' "$PIO_CONFIG_JSON" | python3 -c " import sys, json, re data = json.load(sys.stdin) for section, options in data: diff --git a/examples/kiss_modem/KissModem.cpp b/examples/kiss_modem/KissModem.cpp index 19be5d412f..6aef46e2c1 100644 --- a/examples/kiss_modem/KissModem.cpp +++ b/examples/kiss_modem/KissModem.cpp @@ -22,7 +22,6 @@ KissModem::KissModem(Stream& serial, mesh::LocalIdentity& identity, mesh::RNG& r _getStatsCallback = nullptr; _config = {0, 0, 0, 0, 0}; _signal_report_enabled = true; - _tx_write_aborted = false; } void KissModem::begin() { @@ -33,58 +32,60 @@ void KissModem::begin() { _tx_state = TX_IDLE; } -void KissModem::beginFrameWrite() { - _tx_write_aborted = false; +size_t KissModem::escapedLength(uint8_t b) const { + return (b == KISS_FEND || b == KISS_FESC) ? 2 : 1; } -void KissModem::rawWrite(uint8_t b) { - /* A frame is sent all-or-nothing; once we start dropping, swallow the rest so - we never emit a truncated KISS frame. */ - if (_tx_write_aborted) return; +size_t KissModem::escapedLength(const uint8_t* data, size_t len) const { + size_t total = 0; + for (size_t i = 0; i < len; i++) { + total += escapedLength(data[i]); + } + return total; +} - /* Non-blocking: if the TX buffer is full, drop this frame instead of waiting. - loop() is single-threaded and also services RX and the radio, so it must not - stall on a host that has stopped reading. Writing only when space is free - keeps the underlying write() from blocking; a dropped reply is harmless as - the host retries. */ - if (_serial.availableForWrite() <= 0) { - _tx_write_aborted = true; - return; +bool KissModem::canWriteFrame(size_t total_len) const { + int available = _serial.availableForWrite(); + return available > 0 && (size_t)available >= total_len; +} + +void KissModem::writeEscapedFrame(const uint8_t* prefix, size_t prefix_len, const uint8_t* data, uint16_t len) { + // All-or-nothing: only write if the whole escaped frame fits, so loop() never blocks and frames are never truncated + size_t total_len = 2; // frame delimiters + total_len += escapedLength(prefix, prefix_len); + total_len += escapedLength(data, len); + if (!canWriteFrame(total_len)) return; + + _serial.write(KISS_FEND); + for (size_t i = 0; i < prefix_len; i++) { + writeByte(prefix[i]); } - _serial.write(b); + for (uint16_t i = 0; i < len; i++) { + writeByte(data[i]); + } + _serial.write(KISS_FEND); } void KissModem::writeByte(uint8_t b) { if (b == KISS_FEND) { - rawWrite(KISS_FESC); - rawWrite(KISS_TFEND); + _serial.write(KISS_FESC); + _serial.write(KISS_TFEND); } else if (b == KISS_FESC) { - rawWrite(KISS_FESC); - rawWrite(KISS_TFESC); + _serial.write(KISS_FESC); + _serial.write(KISS_TFESC); } else { - rawWrite(b); + _serial.write(b); } } void KissModem::writeFrame(uint8_t type, const uint8_t* data, uint16_t len) { - beginFrameWrite(); - rawWrite(KISS_FEND); - writeByte(type); - for (uint16_t i = 0; i < len; i++) { - writeByte(data[i]); - } - rawWrite(KISS_FEND); + uint8_t prefix[] = { type }; + writeEscapedFrame(prefix, sizeof(prefix), data, len); } void KissModem::writeHardwareFrame(uint8_t sub_cmd, const uint8_t* data, uint16_t len) { - beginFrameWrite(); - rawWrite(KISS_FEND); - writeByte(KISS_CMD_SETHARDWARE); - writeByte(sub_cmd); - for (uint16_t i = 0; i < len; i++) { - writeByte(data[i]); - } - rawWrite(KISS_FEND); + uint8_t prefix[] = { KISS_CMD_SETHARDWARE, sub_cmd }; + writeEscapedFrame(prefix, sizeof(prefix), data, len); } void KissModem::writeHardwareError(uint8_t error_code) { diff --git a/examples/kiss_modem/KissModem.h b/examples/kiss_modem/KissModem.h index dedd51b73e..8f19c213ed 100644 --- a/examples/kiss_modem/KissModem.h +++ b/examples/kiss_modem/KissModem.h @@ -28,11 +28,12 @@ #define KISS_DEFAULT_SLOTTIME 10 #define KISS_TX_TIMEOUT_FACTOR 3/2 // 1.5x estimated airtime -/* Upper bound (ms) on how long a serial write may wait for the host to drain the - TX buffer. Keeps a stalled USB-CDC host from freezing the single-threaded loop(); - UART transports drain via FIFO and never reach this. */ +// Max ms a USB-CDC write may block on a stalled host, so loop() never freezes (UART drains via FIFO, never hits this) #define KISS_WRITE_TIMEOUT_MS 50 +// Must fit a full escaped DATA frame (~514B) for all-or-nothing writes; 256B default drops max-size RX frames +#define KISS_TX_BUFFER_SIZE 1024 + #define HW_CMD_GET_IDENTITY 0x01 #define HW_CMD_GET_RANDOM 0x02 #define HW_CMD_VERIFY_SIGNATURE 0x03 @@ -136,10 +137,10 @@ class KissModem { RadioConfig _config; bool _signal_report_enabled; - bool _tx_write_aborted; // set when the current frame is dropped (no TX buffer space) - - void beginFrameWrite(); // reset per-frame abort state - void rawWrite(uint8_t b); // non-blocking write: drops the frame rather than stalling loop() + size_t escapedLength(uint8_t b) const; + size_t escapedLength(const uint8_t* data, size_t len) const; + bool canWriteFrame(size_t total_len) const; // true only if the whole frame fits the TX buffer now + void writeEscapedFrame(const uint8_t* prefix, size_t prefix_len, const uint8_t* data, uint16_t len); void writeByte(uint8_t b); void writeFrame(uint8_t type, const uint8_t* data, uint16_t len); void writeHardwareFrame(uint8_t sub_cmd, const uint8_t* data, uint16_t len); diff --git a/examples/kiss_modem/main.cpp b/examples/kiss_modem/main.cpp index eca2f42ef3..1f4d4b2b6f 100644 --- a/examples/kiss_modem/main.cpp +++ b/examples/kiss_modem/main.cpp @@ -107,11 +107,11 @@ void setup() { #endif modem = new KissModem(Serial1, identity, rng, radio_driver, board, sensors); #else +#if defined(ESP32) && (ARDUINO_USB_MODE == 1) + Serial.setTxBufferSize(KISS_TX_BUFFER_SIZE); // HWCDC ring must fit a whole KISS frame; set before begin() +#endif Serial.begin(115200); #if defined(ESP32) - /* Cap how long a USB-CDC write blocks waiting for the host to drain the TX - buffer. loop() is single-threaded, so an unbounded wait when the host stalls - would also freeze RX and radio servicing. */ Serial.setTxTimeoutMs(KISS_WRITE_TIMEOUT_MS); #endif uint32_t start = millis();