fix(radio): correct swapped SBUS Trainer / SBUS Trn Inv. labels - #7616
fix(radio): correct swapped SBUS Trainer / SBUS Trn Inv. labels#7616BelixRogner wants to merge 1 commit into
Conversation
The two SBUS trainer entries in SYS -> Hardware -> Serial Port are swapped: selecting "SBUS Trainer" configures UART_MODE_SBUS_TRAINER_INV (inverted polarity) and selecting "SBUS Trn Inv." configures UART_MODE_SBUS_TRAINER (normal polarity). SA11() expands to s##_1 ... s##_11, so array index 0 is TR_AUX_SERIAL_MODES_1. That puts "SBUS Trn Inv." at index 3, which is UART_MODE_SBUS_TRAINER, and "SBUS Trainer" at index 4, which is UART_MODE_SBUS_TRAINER_INV. The Choice widget indexes the array directly by enum value, so the mislabelling reaches the screen. All 19 language files share the same ordering. Swap the two label values rather than reordering the enum: g_eeGeneral.serialPort persists the mode as a raw 4-bit value, so changing the enum would silently repoint existing configurations at a different mode. No string grows, so there is no new truncation risk on small screens. Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com> Claude-Session: https://claude.ai/code/session_011yNWsmBtBtjmHtNSJjuUeA
I'm not sure this is actually correct, as IIRC SBUS is a "inverted" signal by default on the handset electronics... thus the weird NORMAL to INV semantics... Regardless, @3djc is more conversant with this... if radio and companion disagree on the same setting, this needs to be fixed regardless, but which side is correct as far as the end user is concerned? |
|
Good question — I went back through the history, and it turns out not to be a polarity-semantics issue at all. It's an off-by-one introduced when the second mode was added. Before #7133 the labels were correct: #define TR_AUX_SERIAL_MODES_4 "SBUS Trainer" // index 3 = UART_MODE_SBUS_TRAINER
#define TR_AUX_SERIAL_MODES_5 "Lua" // index 4 = UART_MODE_LUAb3d36eb (#7133, "non inverted SBUS trainer support on serial inputs on H5/H7") then inserted the new entry at different positions in the two lists:
UART_MODE_SBUS_TRAINER,
+ UART_MODE_SBUS_TRAINER_INV,
UART_MODE_LUA,
-#define TR_AUX_SERIAL_MODES_4 "SBUS Trainer"
-#define TR_AUX_SERIAL_MODES_5 "Lua"
+#define TR_AUX_SERIAL_MODES_4 TR("SBUS Trn Inv.",TR("SBUS Trn Inv.","SBUS Trainer Inv."))
+#define TR_AUX_SERIAL_MODES_5 "SBUS Trainer"
+#define TR_AUX_SERIAL_MODES_6 "LUA"Everything from
That also explains the disagreement with Companion: Companion wasn't touched by #7133, so it still reflects the original mapping ( Verified on a TX15 Max. Happy to defer to @3djc if the intent in #7133 was actually to relabel the pre-existing mode as well as add the new one — in that case the fix would instead be to swap the enum, though that would need a storage conversion since |
|
I will do my best to have a look before the end of the week |
|
Possibly the 3.0 version of #7142 wasn't done? |
|
The analysis is wrong. Tested a TX15 max with those settings:
And it works perfectly. My guess is there has been confusion in the understanding of: In H7 terms, this means:
So for H7 radio, all is fine. But investigating the issue, two side issues appeared:
For radio side, I think this PR is not correct, I will issue one fixing the issues above. @pfeerick could you test the Companion side ? |
|
You're right, thanks for testing it properly. I was reasoning from internal consistency between the enum name and the label array, and never accounted for the physical layer — that normal SBUS is inverted serial, so MCU-side inversion is exactly what an end user with a standard receiver needs on H7. Under that reading the labels are describing the signal the user supplies rather than the polarity register, and they're correct. The #7133 relabel was deliberate, not an off-by-one. My history argument showed the enum and label insertions landed at different positions, which is true, but that doesn't establish the pre-#7133 labelling was still correct once a second mode existed — and it isn't. @pfeerick called this in the first reply and I talked past it. Apologies for the noise. Closing. Glad it at least surfaced the F4 static-inverter labelling and the H5 driver inversion gap — happy to test whatever you put up for those. |



Summary of changes:
The
SBUS TrainerandSBUS Trn Inv.entries inSYS → Hardware → Serial Portare swapped:picking "SBUS Trainer" actually selects
UART_MODE_SBUS_TRAINER_INV(inverted polarity), andpicking "SBUS Trn Inv." selects
UART_MODE_SBUS_TRAINER(normal polarity).Why
The mode enum (
radio/src/dataconstants.h) is:The polarity semantics are unambiguous in
serial.cpp:UART_MODE_SBUS_TRAINERleavesparams.polarityat the default, andUART_MODE_SBUS_TRAINER_INVsetsparams.polarity = ETX_Pol_Inverted.The label array is built with
SA11(s)→s##_1, s##_2, … s##_11(
radio/src/translations/translation_def.h), so array index 0 isTR_AUX_SERIAL_MODES_1and:TR_AUX_SERIAL_MODES_4UART_MODE_SBUS_TRAINER(normal)TR_AUX_SERIAL_MODES_5UART_MODE_SBUS_TRAINER_INV(inverted)Choice(box, rect_t{}, STR_AUX_SERIAL_MODES, 0, UART_MODE_MAX, …)ingui/colorlcd/radio/hw_serial.cppindexes the array directly by enum value, so the mislabellingreaches the screen. The same ordering is present in all 19
i18n/*.hfiles, so this is systematicrather than one bad translation.
Corroboration: Companion maps the mode to its label by enum name rather than by array position
(
companion/src/firmwares/generalsettings.cpp), and returns"SBUS Trainer"forAUX_SERIAL_SBUS_TRAINER— i.e. Companion and the radio currently disagree about what mode 3 iscalled.
Impact
Anyone configuring an SBUS trainer input picks the opposite polarity to the one they intended,
which presents as "trainer just doesn't work" with no diagnostic. It may be a contributing factor
in the confusion in #6384, where the discussion turned on which polarity AUX1 actually needs.
Note also that on STM32F4 targets
UART_MODE_SBUS_TRAINER_INVis hidden(
gui_common.cpp), so F4 users are currently offered a single SBUS option labelled"SBUS Trn Inv." which is in fact the normal-polarity mode.
The fix
Swap the two label values in all 19
i18n/*.hfiles. Deliberately not reordering the enum:g_eeGeneral.serialPortpersists these as raw 4-bit values, so changing the enum would silentlyrepoint existing radio configurations at a different mode.
No string is made longer by the swap, so there is no new truncation risk on small screens; the
existing
TR()short/long variants travel with their strings.Testing
tools/commit-tests.shfortx15(colour) andt12max(monochrome) — builds clean, tests pass.reads "SBUS Trainer" rather than "SBUS Trainer Inv.".
Out of scope
Companion's
AuxSerialModeenum has noSBUS_TRAINER_INVentry at all, so its values divergefrom the radio's from index 4 onward. YAML round-trips are name-based (
uartModeLut) and so arenot corrupted by this, but Companion cannot represent the inverted mode. Left for a separate PR.