Skip to content

feat(radio): support SBUS Trainer over USB-VCP - #7615

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

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

Conversation

@BelixRogner

@BelixRogner BelixRogner commented Aug 4, 2026

Copy link
Copy Markdown

Summary of changes:

SYS → Hardware → Serial Port → USB-VCP can now be set to SBUS Trainer. With
MDL → Setup → Trainer Mode = Master/Serial, a PC can drive the trainer channels by writing
standard SBUS frames to the radio's CDC device — no USB-serial dongle, and no AUX UART wiring.

Use cases: hardware-in-the-loop testing, simulators, head trackers and robotics. Trainer mode is
the right injection point for these because the physical sticks stay live and a momentary switch
gives an instant, hardware-level override — verified below.

This is what the serial refactor (#1089 / discussion #1101) was meant to enable: port →
application mapping is already user-selectable, and SBUS Trainer was simply excluded from the
VCP list. SpaceMouse serial input is the existing precedent for an external device over serial
driving control inputs.

Why a new framer

The AUX SBUS path finds frame boundaries with the USART idle-line interrupt
(drv->setIdleCbsbusFrameReceived, which requires the RX buffer to hold exactly 25 bytes at
that moment). USB CDC has no idle line — bytes arrive in arbitrarily chunked packets, so a frame
may be split across packets and several frames may arrive in one.

sbusStream* is therefore a byte-stream state machine: sync on 0x0F, accumulate 25 bytes,
validate the end byte, and on failure drop a single byte and re-sync on the next start byte
within the retained buffer rather than discarding it (a valid frame may well have started
inside it).

The AUX path is untouched — it still uses the idle callback, and both paths share the existing
sbusProcessFrame() decoder, so trainer freshness (trainerResetTimer() /
trainerInputValidityTimer) and link-loss fallback behave identically for both.

Notes for review

  • serialSetCallBacks() picks the framer only when a port has no setIdleCb but does have
    setReceiveCb, so behaviour on every hardware UART is unchanged.
  • The de-init branch calls sbusStreamStop(), which releases the CDC RX callback. Without this,
    switching USB-VCP away from SBUS Trainer to a mode that installs no RX callback of its own
    would leave the framer attached to the stream. (CLI happens to overwrite the pointer, so it was
    masked, but it should not be relied on.)
  • USB CDC carries no line polarity, so both SBUS trainer modes would behave identically on
    VCP. Only UART_MODE_SBUS_TRAINER_INV is offered there, as that is the one presented to the
    user as plain SBUS (normal SBUS is inverted serial, so the MCU-inverting mode is the one
    that reads "SBUS Trainer"). The other is hidden so there are not two identical entries.
  • Baud rate is likewise meaningless over CDC, and usbSerialInit() already ignores the requested
    etx_serial_init params, so nothing was needed there. Note serialSetupPort() must keep
    setting a non-zero baudrate for UART_MODE_SBUS_TRAINER, since serialInit() bails out early
    on params.baudrate == 0.
  • One shared-behaviour change, flagged deliberately: the end-byte check was widened from
    == 0x00 to {0x00, 0x04, 0x14, 0x24} (sbusIsEndByte()), which some implementations use to
    carry frame flags. This is a widening only — nothing that works today stops working — but it
    does apply to the AUX path too, for consistency between the two. Happy to restrict it to the
    VCP framer if preferred.

Relationship to #4102

#4102 (draft) touches the same four files. It does not implement trainer over VCP — it
extends the exclusion, adding UART_MODE_IBUS_TRAINER to the same VCP blocklist this PR removes
UART_MODE_SBUS_TRAINER from, and it renames serialGetModePorthasSerialMode.

The two are on different axes (protocol variety on UARTs vs. the VCP transport) and are
complementary, but they conflict textually in isSerialModeAvailable(). Happy to rebase whichever
lands second.

Testing

Unit tests — 13 new cases in radio/src/tests/sbus_trainer.cpp (114 total pass):
frames split at every chunk boundary from 1..25 bytes, back-to-back frames in one packet, garbage
prefix, invalid end byte, re-sync after truncation, re-sync onto a start byte held inside the
buffer, end-byte variants, failsafe/frame-lost rejection, gating when trainer mode is not
Master/Serial, staleness after frames stop, partial-frame drop on port release, and a check that
the menu option is offered on VCP but the inverted variant is not.

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

Check Result
SBUS Trainer offered on USB-VCP pass
SBUS frames drive the trainer channels pass
Full ±100% travel, correct scaling pass
Momentary-switch override pass — released = sticks resume instantly, while frames still arrive at 100 Hz
Frames stop, port held open pass — channels return to sticks in ~1 s
Mid-stream corruption pass — 59 injections of 1–11 random bytes over 30 s at 100 Hz, no visible glitch
USB-VCP back to CLI afterwards pass — prompt, help, ls /, beep all work

Builds: tx15, tx16s, t12max.

Not verified: ExpressLRS passthrough flashing was not directly exercised (only the CLI it
shares a path with), and all-16-channel ordering was not checked, since the trainer block in
mixer.cpp applies to sticks only unless the Trainer special function is set to Chans.

Allow USB-VCP to be set to SBUS Trainer, so a PC can drive the trainer
channels over the radio's USB-C port without a USB-serial dongle or AUX
UART wiring. Useful for hardware-in-the-loop testing, simulators, head
trackers and robotics, where trainer mode is the right injection point
because the physical sticks stay live and a momentary switch gives an
instant override.

The AUX SBUS path finds frame boundaries with the USART idle-line
interrupt, which USB CDC does not have: bytes arrive in arbitrarily
chunked packets, so a frame may be split across packets and several
frames may arrive in one. Add a byte-stream framer that syncs on 0x0F,
accumulates 25 bytes and validates the end byte, dropping a single byte
and re-syncing within the buffer on failure rather than discarding it.

Both paths share the existing sbusProcessFrame() decoder, so trainer
freshness and link-loss fallback behave identically to AUX.
serialSetCallBacks() selects the framer only for ports that have no
setIdleCb but do have setReceiveCb, leaving every hardware UART
unchanged, and releases the CDC RX callback on de-init so the next user
of the port gets a clean stream.

USB CDC carries no line polarity, so the two SBUS trainer modes would
behave identically on VCP. Only UART_MODE_SBUS_TRAINER_INV is offered
there, as that is the one presented to the user as plain SBUS: normal
SBUS is inverted serial, so the MCU-inverting mode is the one that
reads "SBUS Trainer". Baud rate likewise does not apply, and
usbSerialInit() already ignores the requested params.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_011yNWsmBtBtjmHtNSJjuUeA
@BelixRogner
BelixRogner force-pushed the feat/sbus-trainer-over-usb-vcp branch from 97ce55d to b134be0 Compare August 6, 2026 09:55
@BelixRogner

Copy link
Copy Markdown
Author

Updated following @3djc's correction in #7616.

USB CDC carries no line polarity, so both SBUS trainer modes behave identically on VCP — usbSerialInit() ignores the requested etx_serial_init params entirely. The choice between them on this port is therefore purely about which name the user sees.

Previously this PR exposed UART_MODE_SBUS_TRAINER and hid ..._INV, which meant the only option on USB-VCP displayed as "SBUS Trn Inv." — confusing, since a PC writing SBUS frames to a CDC endpoint has no wire polarity at all. Now inverted: UART_MODE_SBUS_TRAINER_INV is offered on VCP (reading "SBUS Trainer") and the other is hidden. No behavioural change, just the label the user ends up with.

Also removed the "unrelated bug" note from the description above, since that turned out to be my misreading rather than a bug.

Unit test updated accordingly; 114 tests pass, tx15 builds clean.

@3djc

3djc commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Why SBUS, since this is not SBUS to start with since polarity is wrong ? Wouldn't be much more appropriate to have a proper serial handler that doesn't carry the limits of SBUS, and simply transmit numerical channel value in a serial frame ???

@BelixRogner

Copy link
Copy Markdown
Author

Fair point on the naming — over CDC there's no polarity, baud or parity, so what's actually being carried is SBUS-framed channel data rather than SBUS.

The reason for reusing it is tooling rather than the wire format: simulators, HIL rigs and head trackers already emit SBUS frames, so this works with what people already have without anyone having to write a sender. It also keeps the radio side down to a framer plus the existing sbusProcessFrame() — no new decode path, and no changes to the trainer subsystem.

On the limits specifically, I don't think they bind here:

  • MAX_TRAINER_CHANNELS is 16 and trainerInput is int16_t[16], so 16 channels is the trainer subsystem's own ceiling, not something SBUS is imposing.
  • The 11-bit resolution maps essentially 1:1 onto the final channel value: (raw - 992) * 5 / 8 gives ±512, then × studWeight / 50 in mixer.cpp gives ±RESX. Nothing is lost in the conversion.
  • USB CDC provides CRC and retransmission at the link layer, so SBUS having no checksum — a real weakness on a UART — costs much less over this transport.

That said, I've no attachment to the format. If you'd prefer a native frame — plain int16 channel values, explicit channel count, sequence number — I'm happy to add it as a separate serial mode. Would you want that to replace this, or sit alongside it?

My instinct is that a properly designed HIL protocol also wants telemetry going back to the PC, which is a considerably bigger feature than this PR, whereas SBUS-in is useful on its own today. But if you'd rather have the native format first, say the word and I'll put it together.

@3djc

3djc commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Maybe you can help me understand by clarifying your target use case ?

@BelixRogner

Copy link
Copy Markdown
Author

Sure. The use case is programmatic control: a PC generates channel values, and the handset acts as a bridge — taking commands in over USB-C and passing them out over its own RF link to the receiver. The radio keeps doing what it already does well (RF link, failsafes, model setup, mixes) and the PC simply supplies stick positions.

Going through the handset rather than driving an RF module directly is the deliberate part. Trainer mode keeps the physical sticks live, so a momentary switch gives an instant hardware-level override: release it and control returns on the next mixer cycle, even while the PC is still streaming frames at full rate. That property is the reason to inject here rather than anywhere else in the chain.

Why USB rather than AUX: on the TX15 Max the AUX connectors are internal JST plugs, so using them means opening the case (plus the UART mapping oddity in #6569). The USB-C port is already on the outside of the handset and already enumerates as CDC — so for a lot of setups this removes a USB-serial dongle and some wiring for something the radio can already do on its own.

Scope-wise this is one-directional channel injection, deliberately the same shape as the existing serial trainer rather than a full HIL protocol. Tested at 100 Hz sustained, and works at 150 Hz.

@3djc

3djc commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

Sorry I have been unclear. I got that part reading your PR, what I don't is what readily available pc software will transmit channel data through usb in sbus format ?

@BelixRogner

Copy link
Copy Markdown
Author

Fair — you're right, and I overstated that.

There are plenty of SBUS encoder libraries (mostly written for driving flight controllers from SBCs over a UART), but there's no established PC application that emits SBUS over USB-CDC, precisely because no radio has accepted it until now. What feeds SBUS trainer today is hardware: another handset, a receiver, a head-tracker module. So "works with existing tooling" doesn't really hold up — anyone using this writes a sender either way.

With that argument gone, the only honest one left for SBUS here is implementation economy: no new decode path, it reuses sbusProcessFrame() and the existing trainer freshness handling. That's a reason to keep a diff small, not a reason to pick a wire format.

Which makes me think the better answer is neither SBUS nor a new bespoke frame, but CRSF over VCP. It has a CRC, extensible frame types, it's bidirectional by design (so telemetry back to the PC later rather than bolting on a second protocol), and real tooling already exists for it. It also looks like where things are heading anyway — #4102 lists Resolves #1607 (CRSF input for Trainer mode).

Worth noting the VCP-specific parts of this PR are protocol-agnostic: USB CDC gives no idle line, so any protocol needs stream framing rather than idle-based frame boundaries, and the port needs its RX callback released on de-init so the next user of the port gets a clean stream. Those apply whatever the payload turns out to be.

So — would you rather this became "serial trainer over VCP" sitting on top of #4102's protocol handlers, or a standalone native frame as you originally suggested? Happy to do either. I'd lean towards the former, since it doesn't add another protocol to maintain.

@3djc

3djc commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

CRSF would indeed be a likely more suitable choice, SBUS is quite aged, and it shows

@BelixRogner

Copy link
Copy Markdown
Author

Agreed. And having looked at what's already in the tree, CRSF isn't just more suitable here — it's less work.

_processFrames() in pulses/crossfire.cpp is already a proper byte-stream parser: length-driven from p_buf[1] + 2, CRC-checked, it holds a partial frame until more bytes arrive, defragments across reads, and resyncs a byte at a time on a bad header. It never relied on the idle line. So over CDC it needs no new framing at all — which was the entire awkward part of doing this with SBUS.

The trainer side already exists too: TRAINER_MODE_CRSF plus the CHANNELS_ID handler in crossfire.cpp fills trainerInput[] and resets the freshness timer.

So this reduces to a serial mode that feeds VCP RX bytes into the existing parser — no new protocol, no new decoder, no changes to the trainer subsystem. It also wouldn't need to wait on #4102, since the CRSF trainer decode is already in main.

Bonus over SBUS: a CRC on every frame, and the same port could carry telemetry back out later without introducing a second protocol.

Shall I close this one and open a fresh PR for CRSF over VCP, or convert this one? Happy to do the work either way.

@3djc

3djc commented Aug 6, 2026

Copy link
Copy Markdown
Collaborator

@pfeerick (our PR master), your take ?

@pfeerick

pfeerick commented Aug 8, 2026

Copy link
Copy Markdown
Member

I have nothing against this PR generally... but given we don't have unlimited resources, we do need to keep it to things are are likely to be used... and given

but there's no established PC application that emits SBUS over USB-CDC

even if it is a chicken and the egg situation like

precisely because no radio has accepted it until now.

I think if we were going to get something established/implemented, a modern serial protocol like CRSF/ELRS is indeed the way to do it. Plus, this is still here if someone does come up with a need for it in the future.

@3djc

3djc commented Aug 8, 2026

Copy link
Copy Markdown
Collaborator

@pfeerick the question is reuse this PR for a CRSF rework, or close this one and create a CRSF one

@pfeerick

pfeerick commented Aug 8, 2026

Copy link
Copy Markdown
Member

Leave this SBUS focused / intact, new PR for CRSF.

@BelixRogner

Copy link
Copy Markdown
Author

Superseded by #7630, which does this with CRSF instead — thanks both for steering it there, it turned out to be a better fit and less code than the SBUS version.

Happy to close this one, or leave it open as @pfeerick suggested in case the SBUS variant is ever wanted.

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.

3 participants