Companion USB interface: report real connection state, add flow control 馃馃 - #3214
Open
notbucki wants to merge 2 commits into
Open
Companion USB interface: report real connection state, add flow control 馃馃#3214notbucki wants to merge 2 commits into
notbucki wants to merge 2 commits into
Conversation
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 <noreply@anthropic.com>
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 <noreply@anthropic.com>
notbucki
changed the title
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Two related fixes for the companion USB interface, found while running the
*_companion_radio_usb/ multi-interface builds on a small fleet of devices.1. The device always believes a client is connected
ArduinoSerialInterface::isConnected()returnstrueunconditionally, so from boot onwards the companion thinks somebody is attached to the USB port. That leaks into two places:MyMeshonly calls_ui->notify()while no client is connected, so new message notifications (display, buzzer) never fire on a USB companion build.UITaskshows< Connected >in place of the BLE pairing PIN. On a build that exposes both BLE and USB this means a freshly flashed device cannot be paired at all - the PIN is simply never shown.The commit adds an optional
setConnectedCheck()hook and wires it up per platform:ARDUINO_USB_MODE=0)(bool)Serial= real DTR, ie. the host has the port openARDUINO_USB_MODE=1)The USB-Serial-JTAG peripheral has no DTR concept at all -
HWCDC::isCDC_Connected()is driven by SOF frames plus an IN-EMPTY interrupt, so it goes true as soon as the cable sits in a powered port, whether or not anything opened the port. There is no register that exposes the CDC line state, so the only honest signal left is "a client has actually sent us a frame recently" (USB_CLIENT_IDLE_TIMEOUT, 10 minutes, overridable per build).2. No flow control on the USB write path
CMD_GET_CONTACTSstreams up toMAX_CONTACTSframes back to back (~150 bytes each, so ~50 KB with the companion default of 350).MyMesh::checkSerialInterface()does gate this on!_serial->isWriteBusy(), butArduinoSerialInterface::isWriteBusy()returnsfalseunconditionally, andwriteFrame()callswrite()without ever looking atavailableForWrite().On ESP32 that meets a 256 byte TX ring: when the host stalls for longer than the write timeout, HWCDC latches itself disconnected and
flushTXBuffer()silently discards queued bytes, ie. it tears a frame in half. The framing is a length prefix with no checksum and no resync marker, so the client stays desynchronised for the rest of the session. In practice this shows up as "the app disconnects while syncing contacts", and only on devices that already have a decent contact list - a fresh device sends a few hundred bytes and never hits it.The commit makes flow control an opt-in feature of
ArduinoSerialInterface(thePrintdefault foravailableForWrite()is 0, so it must not be on by default for arbitrary streams) and enables it for the companion USB interface: report busy until a whole frame fits, and drop a frame as a unit rather than tearing it. It also gives HWCDC a larger TX buffer and a short write timeout, the same reasoning as the existingkiss_modemtuning in #2819.Note the ordering: while no client is connected, writes are dropped and
isWriteBusy()returns false. Otherwise a port that nobody drains keeps its buffer full forever, and the busy flag would stall the paced streams on all interfaces, including BLE.Testing
heltec_v4_r8_companion_radio_usbandGAT562_30S_Mesh_Kit_companion_radio_usb(ESP32-S3 HWCDC and nRF52 TinyUSB).The two commits are independent in spirit but the second one uses the connection state from the first, so they are submitted together. Happy to split or rework either of them.
Disclosure: this pull request was prepared by an AI agent (Claude) working together with the repository owner of the fork, who runs the affected devices. The bugs were found in daily use on real hardware, the analysis and patches are the agent's work and were reviewed and tested by a human before submission. Marked with 馃馃 per CONTRIBUTING.md. Happy to answer any questions or rework the patches.