diff --git a/examples/companion_radio/main.cpp b/examples/companion_radio/main.cpp index 89f0e6cb9f..d6da46f439 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,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 diff --git a/src/helpers/ArduinoSerialInterface.cpp b/src/helpers/ArduinoSerialInterface.cpp index a01fa5866f..079508df4a 100644 --- a/src/helpers/ArduinoSerialInterface.cpp +++ b/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(_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; } } diff --git a/src/helpers/ArduinoSerialInterface.h b/src/helpers/ArduinoSerialInterface.h index c4086353aa..ca6d9f69e0 100644 --- a/src/helpers/ArduinoSerialInterface.h +++ b/src/helpers/ArduinoSerialInterface.h @@ -4,23 +4,45 @@ #include 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;