Companion USB interface: report real connection state, add flow control 🤖🤖

examples/companion_radio/main.cpp

@@ -49,6 +49,11 @@ MultiSerialInterface interface_manager;
 #if defined(ENABLE_USB_INTERFACE)
   #include <helpers/ArduinoSerialInterface.h>
   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,29 @@ 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
+  // 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
 

src/helpers/ArduinoSerialInterface.cpp

@@ -13,11 +13,17 @@ 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
 }
 
 bool ArduinoSerialInterface::isWriteBusy() const {
+  if (_flow_ctl && isConnected()) {
+    return const_cast<Stream*>(_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;
 }
 
@@ -26,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] = '>';
@@ -65,6 +82,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;
         }
     }

src/helpers/ArduinoSerialInterface.h

@@ -4,23 +4,45 @@
 #include <Arduino.h>
 
 class ArduinoSerialInterface : public BaseSerialInterface {
+public:
+  // reports whether a client currently has this stream open (see setConnectedCheck)
+  typedef bool (*ConnectedCheck)();
+
+private:
   bool _isEnabled;
+  bool _flow_ctl;
   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; _flow_ctl = 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; }
+
+  // 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;