From ab492ba187917e68d91083fba748a7fea68b88b4 Mon Sep 17 00:00:00 2001 From: root Date: Fri, 14 Aug 2026 20:13:36 +0200 Subject: [PATCH 1/2] Report the real USB client connection state in companion builds ArduinoSerialInterface::isConnected() always returned true, so any companion build with a USB interface believes a client is attached from boot onwards. Two user visible effects: - new message notifications never fire, because MyMesh only notifies the UI (display, buzzer) while no client is connected - on builds that also expose BLE the home screen shows '< Connected >' instead of the BLE pairing PIN, so a freshly flashed device cannot be paired at all Add an optional connection check callback and wire it up per platform: native USB-CDC (TinyUSB on nRF52/RP2040/ESP32 with USB_MODE=0) exposes real DTR through (bool)Serial. The ESP32 USB-Serial-JTAG peripheral has no DTR concept - it reports 'connected' as soon as the host enumerated the device - so fall back to frame activity there. Plain UARTs keep the previous assume-connected behaviour. Co-Authored-By: Claude Fable 5 --- examples/companion_radio/main.cpp | 21 +++++++++++++++++++++ src/helpers/ArduinoSerialInterface.cpp | 4 +++- src/helpers/ArduinoSerialInterface.h | 23 +++++++++++++++++++---- 3 files changed, 43 insertions(+), 5 deletions(-) diff --git a/examples/companion_radio/main.cpp b/examples/companion_radio/main.cpp index 89f0e6cb9f..6b421550f9 100644 --- a/examples/companion_radio/main.cpp +++ b/examples/companion_radio/main.cpp @@ -49,6 +49,11 @@ MultiSerialInterface interface_manager; #if defined(ENABLE_USB_INTERFACE) #include ArduinoSerialInterface usb_serial_interface; + #ifndef USB_CLIENT_IDLE_TIMEOUT + // how long a USB client is still considered present after its last frame, + // for targets which cannot report DTR (see setConnectedCheck below) + #define USB_CLIENT_IDLE_TIMEOUT (10*60*1000UL) + #endif #endif // include ethernet interface @@ -213,6 +218,22 @@ void setup() { // add usb interface #if defined(ENABLE_USB_INTERFACE) usb_serial_interface.begin(Serial); +#if defined(ESP32) && defined(ARDUINO_USB_MODE) && ARDUINO_USB_MODE == 1 + // The ESP32 USB-Serial-JTAG peripheral (HWCDC) has no DTR concept at all: + // (bool)Serial only tells us the host has enumerated the device, which is + // already true when the cable is plugged into a powered port. Fall back to + // activity: a real client has to send us frames. + usb_serial_interface.setConnectedCheck([]() { + uint32_t last = usb_serial_interface.getLastFrameMillis(); + return (bool)Serial && last != 0 && (millis() - last) < USB_CLIENT_IDLE_TIMEOUT; + }); +#elif (defined(ESP32) && defined(ARDUINO_USB_CDC_ON_BOOT) && ARDUINO_USB_CDC_ON_BOOT) \ + || defined(NRF52_PLATFORM) || defined(RP2040_PLATFORM) + // native USB-CDC (TinyUSB): (bool)Serial reflects DTR, ie. the host really + // has the port open. A classic ESP32 behind a UART bridge has no such + // signal and keeps the assume-connected default. + usb_serial_interface.setConnectedCheck([]() { return (bool)Serial; }); +#endif interface_manager.addInterface(InterfaceType::USB, &usb_serial_interface); #endif diff --git a/src/helpers/ArduinoSerialInterface.cpp b/src/helpers/ArduinoSerialInterface.cpp index a01fa5866f..317e8395c3 100644 --- a/src/helpers/ArduinoSerialInterface.cpp +++ b/src/helpers/ArduinoSerialInterface.cpp @@ -13,7 +13,8 @@ void ArduinoSerialInterface::disable() { _isEnabled = false; } -bool ArduinoSerialInterface::isConnected() const { +bool ArduinoSerialInterface::isConnected() const { + if (_conn_check) return _conn_check(); return true; // no way of knowing, so assume yes } @@ -65,6 +66,7 @@ size_t ArduinoSerialInterface::checkRecvFrame(uint8_t dest[]) { if (_frame_len > MAX_FRAME_SIZE) _frame_len = MAX_FRAME_SIZE; // truncate memcpy(dest, rx_buf, _frame_len); _state = RECV_STATE_IDLE; // reset state, for next frame + _last_frame_ms = millis(); // a real client is talking to us return _frame_len; } } diff --git a/src/helpers/ArduinoSerialInterface.h b/src/helpers/ArduinoSerialInterface.h index c4086353aa..ecc05976a7 100644 --- a/src/helpers/ArduinoSerialInterface.h +++ b/src/helpers/ArduinoSerialInterface.h @@ -4,23 +4,38 @@ #include class ArduinoSerialInterface : public BaseSerialInterface { +public: + // reports whether a client currently has this stream open (see setConnectedCheck) + typedef bool (*ConnectedCheck)(); + +private: bool _isEnabled; uint8_t _state; uint16_t _frame_len; uint16_t rx_len; + uint32_t _last_frame_ms; Stream* _serial; + ConnectedCheck _conn_check; uint8_t rx_buf[MAX_FRAME_SIZE]; public: - ArduinoSerialInterface() { _isEnabled = false; _state = 0; } + ArduinoSerialInterface() { _isEnabled = false; _state = 0; _last_frame_ms = 0; _conn_check = NULL; } - void begin(Stream& serial) { - _serial = &serial; + void begin(Stream& serial) { + _serial = &serial; #ifdef RAK_4631 pinMode(WB_IO2, OUTPUT); - #endif + #endif } + // Optional: let the target report the real link state, e.g. USB-CDC DTR. + // Without it isConnected() assumes true, as a plain UART has no way of knowing. + void setConnectedCheck(ConnectedCheck fn) { _conn_check = fn; } + + // millis() of the last completely received frame, 0 if none since boot. + // Useful as an activity-based connection check where no DTR state exists. + uint32_t getLastFrameMillis() const { return _last_frame_ms; } + // BaseSerialInterface methods void enable() override; void disable() override; From a7724b9ec6dc67db7fcbae4579dc8e32a347eecf Mon Sep 17 00:00:00 2001 From: root Date: Fri, 14 Aug 2026 20:16:20 +0200 Subject: [PATCH 2/2] Add flow control to the companion USB interface The contact sync streams up to MAX_CONTACTS frames back to back, but ArduinoSerialInterface never checked whether the stream could take them: isWriteBusy() returned false unconditionally, so MyMesh's pacing gate had no effect, and writeFrame() called write() without looking at availableForWrite(). On ESP32 (HWCDC, 256 byte TX ring) a host that stalls for a moment makes the driver drop queued bytes silently, which tears a frame in half - and since the framing is length prefixed with no checksum and no resync marker, the client stays desynchronised for the rest of the session. Reported as 'the app disconnects while syncing contacts' on devices with a large contact list. Add opt-in flow control: report busy until a whole frame fits, and drop frames as a unit instead of tearing them. Enable it for the companion USB interface and give HWCDC a bigger TX buffer with a short write timeout, mirroring what kiss_modem already does. Co-Authored-By: Claude Fable 5 --- examples/companion_radio/main.cpp | 7 +++++++ src/helpers/ArduinoSerialInterface.cpp | 16 ++++++++++++++++ src/helpers/ArduinoSerialInterface.h | 9 ++++++++- 3 files changed, 31 insertions(+), 1 deletion(-) diff --git a/examples/companion_radio/main.cpp b/examples/companion_radio/main.cpp index 6b421550f9..d6da46f439 100644 --- a/examples/companion_radio/main.cpp +++ b/examples/companion_radio/main.cpp @@ -218,7 +218,14 @@ void setup() { // add usb interface #if defined(ENABLE_USB_INTERFACE) usb_serial_interface.begin(Serial); + // keep frames intact and pace the contact stream when the host is slow + usb_serial_interface.enableFlowControl(true); #if defined(ESP32) && defined(ARDUINO_USB_MODE) && ARDUINO_USB_MODE == 1 + // a 256 byte TX buffer overflows during a contact sync, and write() blocks + // up to tx_timeout_ms per call against a stalled host (same reasoning as + // the kiss_modem tuning) + Serial.setTxBufferSize(4096); + Serial.setTxTimeoutMs(5); // The ESP32 USB-Serial-JTAG peripheral (HWCDC) has no DTR concept at all: // (bool)Serial only tells us the host has enumerated the device, which is // already true when the cable is plugged into a powered port. Fall back to diff --git a/src/helpers/ArduinoSerialInterface.cpp b/src/helpers/ArduinoSerialInterface.cpp index 317e8395c3..079508df4a 100644 --- a/src/helpers/ArduinoSerialInterface.cpp +++ b/src/helpers/ArduinoSerialInterface.cpp @@ -19,6 +19,11 @@ bool ArduinoSerialInterface::isConnected() const { } bool ArduinoSerialInterface::isWriteBusy() const { + if (_flow_ctl && isConnected()) { + return const_cast(_serial)->availableForWrite() < (int)(MAX_FRAME_SIZE + 3); + } + // while nobody drains the port the TX buffer stays full, so never report + // busy in that case: it would stall the paced streams on all interfaces return false; } @@ -27,6 +32,17 @@ size_t ArduinoSerialInterface::writeFrame(const uint8_t src[], size_t len) { // frame is too big! return 0; } + if (_flow_ctl) { + if (!isConnected()) { + return len; // nobody is listening, drop instead of filling the TX buffer + } + if (_serial->availableForWrite() < (int)(len + 3)) { + // a short write would tear the length prefixed framing, and as there is + // neither a checksum nor a resync marker the receiver would stay out of + // sync forever - so drop the whole frame instead + return 0; + } + } uint8_t hdr[3]; hdr[0] = '>'; diff --git a/src/helpers/ArduinoSerialInterface.h b/src/helpers/ArduinoSerialInterface.h index ecc05976a7..ca6d9f69e0 100644 --- a/src/helpers/ArduinoSerialInterface.h +++ b/src/helpers/ArduinoSerialInterface.h @@ -10,6 +10,7 @@ class ArduinoSerialInterface : public BaseSerialInterface { private: bool _isEnabled; + bool _flow_ctl; uint8_t _state; uint16_t _frame_len; uint16_t rx_len; @@ -19,7 +20,7 @@ class ArduinoSerialInterface : public BaseSerialInterface { uint8_t rx_buf[MAX_FRAME_SIZE]; public: - ArduinoSerialInterface() { _isEnabled = false; _state = 0; _last_frame_ms = 0; _conn_check = NULL; } + ArduinoSerialInterface() { _isEnabled = false; _flow_ctl = false; _state = 0; _last_frame_ms = 0; _conn_check = NULL; } void begin(Stream& serial) { _serial = &serial; @@ -36,6 +37,12 @@ class ArduinoSerialInterface : public BaseSerialInterface { // Useful as an activity-based connection check where no DTR state exists. uint32_t getLastFrameMillis() const { return _last_frame_ms; } + // Optional: only hand a frame to the stream when it fits into the TX buffer + // as a whole, and report busy while it does not, so bulk streams get paced. + // Only enable this for streams which really implement availableForWrite() + // (USB-CDC does, the Print default returns 0). + void enableFlowControl(bool enable) { _flow_ctl = enable; } + // BaseSerialInterface methods void enable() override; void disable() override;