Skip to content

feat(radio): support CRSF Trainer over USB-VCP - #7630

Open
BelixRogner wants to merge 1 commit into
EdgeTX:mainfrom
BelixRogner:feat/crsf-trainer-over-usb-vcp
Open

feat(radio): support CRSF Trainer over USB-VCP#7630
BelixRogner wants to merge 1 commit into
EdgeTX:mainfrom
BelixRogner:feat/crsf-trainer-over-usb-vcp

Conversation

@BelixRogner

Copy link
Copy Markdown

Summary of changes:

SYS → Hardware → Serial Port → USB-VCP can now be set to CRSF Trainer. With the model's
trainer mode set to Master/CRSF, a PC can drive the trainer channels by writing CRSF frames to
the radio's CDC device. The handset then acts as a bridge: commands in over USB-C, out over its
own RF link, with the physical sticks still live so a momentary switch gives an instant override.

This replaces #7615, which did the same thing with SBUS. @3djc and @pfeerick both preferred a
modern checksummed protocol there, and they were right — CRSF turned out to be both a better fit
and less code.

Most of this already existed

  • TRAINER_MODE_CRSF is already in the trainer mode enum.
  • telemetry/crossfire.cpp already decodes CHANNELS_ID frames into trainerInput[] and resets
    the trainer validity timer. It was simply only reachable from a module's telemetry stream.

That decode is extracted here as crossfireProcessChannelsFrame() so the module path and the new
USB path share one decoder, and therefore produce identical channel values and identical freshness
behaviour.

Correction to something I said on #7615

I claimed there that _processFrames() could be reused as-is and that this "needs no new framing
at all". That was wrong. _processFrames() casts its context to etx_module_state_t* and needs a
module index (modulePortGetModule, lastAlive[module], processCrossfireTelemetryFrame), and a
USB-VCP port has no module. The algorithm was already proven; the plumbing was not, so the
assembler in crsf_trainer.cpp is new.

It is still far less awkward than SBUS over CDC: CRSF is self-framing, with a length byte and a
CRC, so no idle-line detection is needed to find frame boundaries — CDC just needs the partial
frame state kept across chunked reads.

Notes for review

  • Offered on USB-VCP only. The mode is driven from a receive callback, and
    stm32_serial_driver.cpp has setReceiveCb = nullptr, so the AUX UARTs cannot drive it today.
    If/when the USART driver grows a receive callback this restriction can simply be dropped.
  • UART_MODE_CRSF_TRAINER is appended to the end of UartModes, so existing persisted 4-bit
    g_eeGeneral.serialPort values keep their meaning. The count is now 12, still inside
    SERIAL_CONF_MODE_MASK.
  • isTrainerModeAvailable() also had to change. It gated TRAINER_MODE_CRSF on an ELRS module

    = 4.0 being enabled, which is correct when the frames come from a module but wrong when they
    arrive on a serial port — there is no module involved, and on a bench the internal RF is usually
    off. It now mirrors what TRAINER_MODE_MASTER_SERIAL does one line above:
    if (serialGetModePort(UART_MODE_CRSF_TRAINER) >= 0) return true;
    Consequence for users: USB-VCP has to be set to CRSF Trainer before Master/CRSF appears in
    the model's trainer mode list, the same ordering the SBUS path already has.

  • The receive callback is released on de-init. Without it, switching USB-VCP away from
    CRSF Trainer to a mode that installs no RX callback of its own would leave the assembler
    attached to the stream. (CLI happens to overwrite the pointer, so it was masked.)
  • A CRC failure drops the whole frame rather than re-syncing byte-by-byte, matching
    _processFrames(). This matters: with a valid address and a sane length, a CRC failure is far
    more likely to be corruption inside a real frame than a false lock, and byte-wise re-sync can
    latch onto a payload byte that happens to equal an address byte. See the test note below.
  • Companion is untouched. Its AuxSerialMode enum already diverges from the radio's (it has no
    SBUS_TRAINER_INV), and its YAML is name-based, so this does not make anything worse — but
    Companion cannot represent the new mode. Happy to follow up separately.

Testing

Unit tests — 16 new cases in radio/src/tests/crsf_trainer.cpp (117 total pass): frames split
at every chunk boundary, back-to-back frames in one packet, garbage prefix, bad CRC, corrupted
payload, corrupted frame dropped with the stream staying aligned, re-sync after truncation, insane
length byte, other frame types consumed but ignored, both address bytes accepted, gating when the
trainer mode is not CRSF, staleness after frames stop, partial-frame drop on port release, and the
two menu-availability checks.

One of those caught a real bug during development. The test channel values put 0xC8 (UART_SYNC)
inside the payload, so an earlier byte-wise re-sync on CRC failure latched onto it, read a bogus
length, and swallowed the following good frame while waiting for it to complete. Hence the
drop-whole-frame behaviour above.

On a RadioMaster TX15 Max (STM32H7), driven by tools/crsf_vcp_test.py:

Check Result
CRSF Trainer offered on USB-VCP pass
Master/CRSF selectable with internal RF off pass
CRSF frames drive the trainer channels pass — 100 Hz sweep, full travel
Mid-stream corruption pass — 59 injections of 1–11 random bytes over 30 s, no visible glitch
Frames stop, port held open pass — channels return to the sticks in ~1 s
USB-VCP back to CLI afterwards pass — prompt, ls / and beep all work

Builds: tx15, tx16s, t12max.

Not verified: ExpressLRS passthrough flashing was not directly exercised (only the CLI it
shares a path with). The momentary-switch override was verified on the SBUS build and not re-run
here — it lives in mixer.cpp and is untouched by this change.

Happy to close #7615 in favour of this, or leave it as @pfeerick suggested. Also happy to rebase
around #4102 whichever way suits.

Allow USB-VCP to be set to CRSF Trainer, so a PC can drive the trainer
channels over the radio's USB-C port. The handset then acts as a bridge:
commands in over USB, out over its own RF link, with the physical sticks
still live so a momentary switch gives an instant override.

Most of this already existed. TRAINER_MODE_CRSF is in the trainer mode
enum, and telemetry/crossfire.cpp already decodes CHANNELS_ID frames
into trainerInput[]; it was simply only reachable from a module's
telemetry stream. That decode is extracted here as
crossfireProcessChannelsFrame() so the module path and the new USB path
share one decoder, and therefore identical scaling and freshness
behaviour.

CRSF suits USB CDC well: it is self-framing, with a length byte and a
CRC per frame, so it needs no idle-line detection to find frame
boundaries. crsf_trainer.cpp assembles frames from the arbitrarily
chunked buffers CDC delivers, and releases the receive callback on
de-init so the next user of the port gets a clean stream.

The mode is offered on USB-VCP only: it is driven from a receive
callback, and the STM32 USART driver has setReceiveCb = nullptr, so the
AUX UARTs cannot drive it today.

UART_MODE_CRSF_TRAINER is appended to the end of UartModes so existing
persisted g_eeGeneral.serialPort values keep their meaning.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011yNWsmBtBtjmHtNSJjuUeA
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant