Skip to content

Add virtual inputs and virtual switches - #5885

Open
wimalopaan wants to merge 18 commits into
EdgeTX:mainfrom
wimalopaan:wmvcontrols
Open

Add virtual inputs and virtual switches #5885
wimalopaan wants to merge 18 commits into
EdgeTX:mainfrom
wimalopaan:wmvcontrols

Conversation

@wimalopaan

@wimalopaan wimalopaan commented Feb 8, 2025

Copy link
Copy Markdown
Contributor

This PR adds virtual inputs (Vin1 ... Vin16) and virtual switches (V01 ... V64) and neccessary LUA support functions

  • setVirtualInput()
  • setVirtualSwitch()
  • getVirtualSwitch()

A LUA widget lvglControls (see: https://github.com/wimalopaan/LUA/tree/main/WIDGETS/lvglControls ) is provided for demonstration a simple use case: extending the number of physical inputs and physical switches of the radio via a widget that provides some virtual touch controls for these virtual inputs and virtual switches.

Another purpose is to extend the amount of physical knobs, etc. of the radio by connecting some external device via serial (AUX1, AUX2) to the radio, and installing a custom widget, that reads from the serial and uses the above functions to set the virtual inputs and virtual switches.

A proposal for such a serial protocol can be found: https://github.com/wimalopaan/Electronics?tab=readme-ov-file#hwext

Rationale:
In the good old days there were radios like the FrSky X9E that provide a large amount of potentiometers and switches. This type of radio was/is very popular for controlling crawler/ships- or other functional-models. The number of these physical items is still limited and due to their physical nature, the labels for all these elements are also handwritten on paper.
With the advent of the lvgl LUA support it is possible to create virtual controls that perfectly fit into the UI look-and-feel of EdgeTx and also are not very compute-intensive. This opens the way to use widgets to provide arbitrary virtual controls and switches with customisable labes and colors. It also opens the way to use widgets to read from serials like AUX1/AUX2 and get controls data from some external devices (e.g. a µC with some physical potiometers, switches, incrementals, ... connected).

Alternative:
There is a cumbersome alternative way to achieve a similar but very limited result: a widget could use the function shmSet() to transport up to 16 values, to a mixer script which could pick them up via shmGet() and simple return them (only up to 6). These output values of the mixer script could then be used as mixer-inputs. There is no such a way for switches.

Example Widget:

lvgl1
lvgl2
lvgl3
lvgl4

Summary by CodeRabbit

New Features

  • Added support for virtual controls and virtual switches on color LCD displays
  • New Lua API functions to manage virtual controls and switches: activateVirtualSwitch, activateVirtualInput, getVirtualSwitch, setVirtualSwitch, getVirtualInput, setVirtualInput
  • Enhanced configuration support with new syntax for virtual inputs and switches in model files
  • Expanded language support across all translation files for virtual controls features

@3djc

3djc commented Feb 8, 2025

Copy link
Copy Markdown
Collaborator

Please do not add something like this on bw, it will make scrolling a nightmare! (And likely Colors too)

@wimalopaan

Copy link
Copy Markdown
Contributor Author

Please do not add something like this on bw, it will make scrolling a nightmare! (And likely Colors too)

On BW it should be not available (need to check COLORLCD)

On COLORLCD: why do you think scrolling would be an issue?

@wimalopaan

Copy link
Copy Markdown
Contributor Author

I added the possibility to explicitly activate a virtual control (input, switch). If no LUA script activates some virtual controls, they do not show up at all.
A LUA script can activate some of them by using

  • activateVirtualInput()
  • activateVirtualSwitch()

@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 4 times, most recently from ba8f0ba to d62a841 Compare February 11, 2025 11:04
@wimalopaan
wimalopaan marked this pull request as ready for review February 12, 2025 12:13
@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 3 times, most recently from 0c8ed57 to 7f3a149 Compare February 15, 2025 10:12
@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 2 times, most recently from e837be6 to f296d20 Compare March 6, 2025 17:13
@wimalopaan

Copy link
Copy Markdown
Contributor Author

The complete API for the virtual controls (virtual inputs, virtual switches) now comprises the following LUA functions:

  • setVirtualInput()
  • getVirtualInput()
  • activateVirtualInput()
  • setVirtualSwitch()
  • getVirtualSwitch()
  • activateVirtualSwitch()

@wimalopaan

Copy link
Copy Markdown
Contributor Author

@elecpower What does the label "needs: companion" mean?

@pfeerick

pfeerick commented Mar 31, 2025 via email

Copy link
Copy Markdown
Member

@wimalopaan

Copy link
Copy Markdown
Contributor Author

Ok, think I need some advice here ;-)

In firmware/simulator a widget activates virtual sources / switches. This is to prevent littering the sources lists / switches lists with unused virtual controls. The question is now how to emulate this activation process, since widget installation in simulation started from companion is not persistent. So, this activation hast to be done by other means. Maybe an extra dialog for the model config? But that wouldn't exist in firmware/simulator.

@elecpower

Copy link
Copy Markdown
Collaborator

As a guide Companion:

  • must be able read and write the yml files without data loss or corruption (not negotiable);
  • update radio and model settings editing to support maintenance of the feature as radio firmware (unless performed using lua as it is not currently implemented in Companion or reliant on radio firmware runtime environment eg receiver capabilities or custom hardware mods)
  • update two-way interfacing between simulator and libsimulators including "faking" the runtime environment for the libsimulator

@elecpower

Copy link
Copy Markdown
Collaborator

Also populate lookup lists eg sources and switches

@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 3 times, most recently from d7202a2 to 39a7816 Compare January 11, 2026 06:55
@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 4 times, most recently from 3195023 to 0e8d54b Compare January 25, 2026 06:38
@wimalopaan
wimalopaan force-pushed the wmvcontrols branch 3 times, most recently from 5048a24 to e2734bf Compare February 1, 2026 13:54
@coderabbitai

coderabbitai Bot commented Jun 14, 2026

Copy link
Copy Markdown

Review Change Stack

📝 Walkthrough

Walkthrough

Adds a new VCONTROLS build option (default ON) that introduces virtual inputs and virtual switches as a Lua-scriptable subsystem. State is stored in global arrays and bitmasks defined in vcontrols.cpp, consumed by the mixer, switch evaluator, GUI availability checks, YAML persistence, and a new set of Lua API functions. Translations for all 20 locales are included.

Changes

Virtual Controls Feature

Layer / File(s) Summary
Build option, data contracts, and vcontrols state module
radio/src/CMakeLists.txt, radio/src/dataconstants.h, radio/src/vcontrols.h, radio/src/vcontrols.cpp
VCONTROLS CMake option (default ON) gates -DVCONTROLS and vcontrols.cpp. dataconstants.h adds MAX_VIRTUAL_INPUTS, MAX_VIRTUAL_SWITCHES, SWSRC_FIRST/LAST_VIRTUAL_SWITCH, SW_VIRTUAL, MIXSRC_FIRST/LAST_VCONTROL, and SRC_VCONTROL. vcontrols.cpp defines global state arrays (virtualInputs, virtualSwitches, activeVirtualInputs, activeVirtualSwitches) and resetVirtualControls().
Runtime engine: mixer, switch evaluation, and model reset
radio/src/mixer.cpp, radio/src/switches.cpp, radio/src/storage/storage_common.cpp
_getValue returns virtualInputs[] for the VCONTROL source range. getSwitch() evaluates virtualSwitches bitmask for virtual switch sources. storage_common.cpp calls resetVirtualControls() on model post-load.
GUI availability checks and toolbar buttons
radio/src/gui/gui_common.cpp, radio/src/gui/colorlcd/controls/sourcechoice.cpp, radio/src/gui/colorlcd/controls/switchchoice.cpp
gui_common.cpp adds isSourceVControlAvailable() and isVSwitchAvailable() registered in sourceChecks/switchChecks. sourcechoice.cpp adds a "VI" toolbar button; switchchoice.cpp guards the existing "VS" button under VCONTROLS.
YAML persistence and string rendering
radio/src/storage/yaml/yaml_datastructs_funcs.cpp, radio/src/strhelpers.cpp
YAML read/write extended to parse/emit vin(n) for mix sources and Vn for switch sources. getSourceString() and getSwitchPositionName() render STR_VC/STR_VSW for virtual sources.
Lua API for virtual switches and inputs
radio/src/lua/api_general.cpp, radio/src/lua/api_colorlcd_lvgl.cpp
Six new Lua functions (activateVirtualSwitch, activateVirtualInput, getVirtualSwitch, setVirtualSwitch, getVirtualInput, setVirtualInput) with bounds/clamping, registered under VCONTROLS && COLORLCD. SW_VIRTUAL Lua constant guarded by same flags.
Translations (all locales)
radio/src/translations/string_list.h, radio/src/translations/sim_string_list.h, radio/src/translations/i18n/*
TR_MENU_VIRTUAL_SWITCHES, TR_MENU_VCONTROL, TR_VC, and TR_VSW added across all 20 locale headers. string_list.h registers identifiers under #if defined(VCONTROLS); sim_string_list.h adds simulator aliases.

Sequence Diagram(s)

sequenceDiagram
  participant LuaScript
  participant api_general
  participant vcontrols
  participant switches_cpp as switches.cpp
  participant mixer_cpp as mixer.cpp

  rect rgba(100, 149, 237, 0.5)
    Note over LuaScript,vcontrols: Lua activates a virtual switch
    LuaScript->>api_general: setVirtualSwitch(sw, true)
    api_general->>vcontrols: virtualSwitches |= (1 << sw)
  end

  rect rgba(144, 238, 144, 0.5)
    Note over switches_cpp,vcontrols: Firmware evaluates switch state
    switches_cpp->>vcontrols: read virtualSwitches bitmask
    switches_cpp-->>switches_cpp: return bit for SWSRC_FIRST_VIRTUAL_SWITCH+sw
  end

  rect rgba(255, 200, 100, 0.5)
    Note over LuaScript,vcontrols: Lua sets a virtual input value
    LuaScript->>api_general: setVirtualInput(ch, value)
    api_general->>vcontrols: virtualInputs[ch] = clamp(value)
  end

  rect rgba(200, 150, 255, 0.5)
    Note over mixer_cpp,vcontrols: Firmware reads virtual input in mix
    mixer_cpp->>vcontrols: read virtualInputs[i - MIXSRC_FIRST_VCONTROL]
    mixer_cpp-->>mixer_cpp: return value to _getValue caller
  end
Loading

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~25 minutes

Suggested labels

enhancement ✨, translation, color, UX-UI

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 10.71% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title clearly summarizes the main change: adding virtual inputs and virtual switches functionality to the codebase.
Description check ✅ Passed The description comprehensively covers the feature purpose, LUA functions, use cases, rationale, and includes visual examples. It exceeds template requirements by providing detailed context and demonstrating the feature.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
🧪 Generate unit tests (beta)
  • Create PR with unit tests

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

Comment @coderabbitai help to get the list of available commands and usage tips.

@coderabbitai coderabbitai Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Actionable comments posted: 6

🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

Inline comments:
In `@radio/src/switches.cpp`:
- Around line 767-772: The virtualSwitches 64-bit variable is accessed without
mutex protection on 32-bit targets, creating a race condition between the mixer
task reading it via getSwitch() and Lua scripts writing it via
setVirtualSwitch() and getVirtualSwitch(). To fix this, create a dedicated mutex
for virtualSwitches access, then wrap the bitwise read operation (result =
virtualSwitches & mask) in the virtual switch handling block within the
VCONTROLS and COLORLCD preprocessor guard with mutex lock/unlock, and similarly
protect all write accesses to virtualSwitches in the Lua API functions to ensure
atomic 64-bit operations on 32-bit targets.

In `@radio/src/translations/i18n/cn.h`:
- Around line 97-98: The virtual-control menu labels in the locale files are
using English placeholder strings instead of proper locale-specific
translations. In radio/src/translations/i18n/cn.h at lines 97-98, replace the
English strings in the TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL macro
definitions with their corresponding Chinese translations. Similarly, in
radio/src/translations/i18n/cz.h at lines 666-667, replace those same English
strings with their corresponding Czech translations for the same two macros.
Ensure each locale file contains the appropriate translated text for its
language rather than English fallbacks.

In `@radio/src/translations/i18n/fi.h`:
- Around line 1252-1253: The TR_VC macro definition is inconsistent across
locales and uses "VIn" instead of "VI" to match the VIxx/VSxx naming convention.
Update TR_VC in radio/src/translations/i18n/fi.h at lines 1252-1253 by changing
the value from "VIn" to "VI". Apply the same change to
radio/src/translations/i18n/fr.h at lines 1254-1255 and
radio/src/translations/i18n/he.h at lines 1253-1254, ensuring the TR_VC macro
uses "VI" as its value in all three locale files.

In `@radio/src/translations/i18n/it.h`:
- Around line 1248-1251: The VCONTROLS locale strings are mixing English
placeholders with localized text and the TR_VC definition uses "VIn" instead of
the agreed "VI" prefix format. Fix this across all four locale files: In
radio/src/translations/i18n/it.h (lines 1248-1251), localize the strings for
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Italian, and change TR_VC from
"VIn" to "VI". In radio/src/translations/i18n/jp.h (lines 1247-1250), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Japanese, and align TR_VC to
use "VI". In radio/src/translations/i18n/ko.h (lines 1294-1297), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Korean, and align TR_VC to use
"VI". In radio/src/translations/i18n/nl.h (lines 1251-1254), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Dutch, and align TR_VC to use
"VI". Ensure all four files follow the consistent VIxx/VSxx naming convention
with properly localized UI text rather than English placeholders.

In `@radio/src/translations/i18n/pl.h`:
- Around line 1250-1251: The TR_VC macro definition uses "VIn" but should use
"VI" to comply with the VIxx/VSxx naming convention. Fix this issue in three
translation files: In radio/src/translations/i18n/pl.h at line 1250, change the
TR_VC definition from "VIn" to "VI" (leave TR_VSW as "VS"); in
radio/src/translations/i18n/pt.h at line 1251, make the same change to TR_VC
from "VIn" to "VI"; and in radio/src/translations/i18n/ru.h at line 1252, apply
the identical fix to TR_VC, changing it from "VIn" to "VI". Keep TR_VSW
unchanged as "VS" in all three files.

In `@radio/src/translations/i18n/se.h`:
- Line 1265: The macro TR_VC is currently set to "VIn" in three translation
files, which conflicts with the VIxx naming convention. Update the TR_VC macro
definition in all three locations: in radio/src/translations/i18n/se.h at line
1265, change TR_VC from "VIn" to "VI"; in radio/src/translations/i18n/tw.h at
line 1248, change TR_VC from "VIn" to "VI"; and in
radio/src/translations/i18n/ua.h at line 1251, change TR_VC from "VIn" to "VI".
This will ensure all three locales conform to the documented VIxx/VSxx naming
convention and prevent label rendering conflicts.
🪄 Autofix (Beta)

Fix all unresolved CodeRabbit comments on this PR:

  • Push a commit to this branch (recommended)
  • Create a new PR with the fixes

ℹ️ Review info
⚙️ Run configuration

Configuration used: Organization UI

Review profile: CHILL

Plan: Pro Plus

Run ID: ea325f41-f9b0-4ded-80b4-975f40a0d73e

📥 Commits

Reviewing files that changed from the base of the PR and between 15921e1 and 028e4b9.

📒 Files selected for processing (35)
  • radio/src/CMakeLists.txt
  • radio/src/dataconstants.h
  • radio/src/gui/colorlcd/controls/sourcechoice.cpp
  • radio/src/gui/colorlcd/controls/switchchoice.cpp
  • radio/src/gui/gui_common.cpp
  • radio/src/lua/api_colorlcd_lvgl.cpp
  • radio/src/lua/api_general.cpp
  • radio/src/mixer.cpp
  • radio/src/storage/storage_common.cpp
  • radio/src/storage/yaml/yaml_datastructs_funcs.cpp
  • radio/src/strhelpers.cpp
  • radio/src/switches.cpp
  • radio/src/translations/i18n/cn.h
  • radio/src/translations/i18n/cz.h
  • radio/src/translations/i18n/da.h
  • radio/src/translations/i18n/de.h
  • radio/src/translations/i18n/en.h
  • radio/src/translations/i18n/es.h
  • radio/src/translations/i18n/fi.h
  • radio/src/translations/i18n/fr.h
  • radio/src/translations/i18n/he.h
  • radio/src/translations/i18n/it.h
  • radio/src/translations/i18n/jp.h
  • radio/src/translations/i18n/ko.h
  • radio/src/translations/i18n/nl.h
  • radio/src/translations/i18n/pl.h
  • radio/src/translations/i18n/pt.h
  • radio/src/translations/i18n/ru.h
  • radio/src/translations/i18n/se.h
  • radio/src/translations/i18n/tw.h
  • radio/src/translations/i18n/ua.h
  • radio/src/translations/sim_string_list.h
  • radio/src/translations/string_list.h
  • radio/src/vcontrols.cpp
  • radio/src/vcontrols.h

Comment thread radio/src/switches.cpp
Comment on lines +767 to +772
#if defined(VCONTROLS) && defined(COLORLCD)
else if ((cs_idx >= SWSRC_FIRST_VIRTUAL_SWITCH) && (cs_idx <= SWSRC_LAST_VIRTUAL_SWITCH)) {
cs_idx -= SWSRC_FIRST_VIRTUAL_SWITCH;
const uint64_t mask = (uint64_t{1} << cs_idx);
result = virtualSwitches & mask;
}

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟠 Major

🧩 Analysis chain

🏁 Script executed:

#!/bin/bash
set -euo pipefail

echo "== Locate virtual switch state definitions =="
fd -i "vcontrols.*" radio/src | xargs -r rg -n -C3 "virtualSwitches|activeVirtualSwitches|virtualInputs|resetVirtualControls"

echo
echo "== Locate all virtualSwitches reads/writes =="
rg -n -C3 "\bvirtualSwitches\b" radio/src

echo
echo "== Locate Lua API setters/getters/activators for virtual switches =="
rg -n -C4 "setVirtualSwitch|getVirtualSwitch|activateVirtualSwitch" radio/src/lua

echo
echo "== Locate task/lock/critical-section usage near these callsites =="
rg -n -C3 "mutex|lock|critical|ATOMIC|atomic|taskENTER_CRITICAL|taskEXIT_CRITICAL|Suspend|Resume" radio/src

Repository: EdgeTX/edgetx

Length of output: 50372


🏁 Script executed:

# Check the function context around switches.cpp:767-772
sed -n '750,800p' radio/src/switches.cpp

# Check what function contains this code
rg -n "^[a-zA-Z_].*\{" radio/src/switches.cpp | head -20

# Check the exact line range with function names
awk '/^[a-zA-Z_][a-zA-Z0-9_:*\s]*\(.*\)\s*\{/ {func=$0; line=NR} NR==767 || NR==768 || NR==769 || NR==770 || NR==771 || NR==772 {print line": "func; print NR": "$0}' radio/src/switches.cpp

Repository: EdgeTX/edgetx

Length of output: 2021


🏁 Script executed:

# Examine Lua API function definitions to see if they're protected
sed -n '2515,2545p' radio/src/lua/api_general.cpp

# Check if these functions are wrapped with lua_lock
rg -B5 -A15 "luaGetVirtualSwitch|luaSetVirtualSwitch" radio/src/lua/api_general.cpp | head -60

Repository: EdgeTX/edgetx

Length of output: 2525


🏁 Script executed:

# Check if the switches.cpp code is in mixer context
rg -n "mixerTaskLock|mixerTask" radio/src/switches.cpp

Repository: EdgeTX/edgetx

Length of output: 98


🏁 Script executed:

# Find the function containing switches.cpp line 767
sed -n '700,800p' radio/src/switches.cpp | grep -B 50 "SWSRC_FIRST_VIRTUAL_SWITCH" | head -60

Repository: EdgeTX/edgetx

Length of output: 1945


🏁 Script executed:

# Search backwards from line 767 to find function name
awk 'NR<=767' radio/src/switches.cpp | tac | grep -n "^[a-zA-Z_].*\(.*\).*\{" | head -1

Repository: EdgeTX/edgetx

Length of output: 76


🏁 Script executed:

# Get function signature more reliably
sed -n '700,775p' radio/src/switches.cpp

Repository: EdgeTX/edgetx

Length of output: 2586


🏁 Script executed:

# Check if this function is called from mixer code
rg -n "getSwitch" radio/src/tasks/mixer_task.cpp | head -20

Repository: EdgeTX/edgetx

Length of output: 109


🏁 Script executed:

# Find the getSwitch function definition
rg -n "^bool getSwitch|^uint8_t getSwitch|^int getSwitch" radio/src/switches.cpp | head -5

Repository: EdgeTX/edgetx

Length of output: 186


🏁 Script executed:

# Get more context around the virtual switch read to identify the function
sed -n '600,680p' radio/src/switches.cpp | tail -30

Repository: EdgeTX/edgetx

Length of output: 1104


🏁 Script executed:

# Check getSwitchesPosition calls and context
rg -n "getSwitchesPosition" radio/src/

Repository: EdgeTX/edgetx

Length of output: 270


🏁 Script executed:

# Check if getSwitch is called from mixer and whether mixer is locked
rg -n -B3 -A3 "getSwitch\(" radio/src/mixer.cpp | head -40

Repository: EdgeTX/edgetx

Length of output: 1021


🏁 Script executed:

# Check what tasks call Lua script execution
rg -n "lua_resume|luaScript|LUA_TASK" radio/src/lua/interface.cpp | head -20

Repository: EdgeTX/edgetx

Length of output: 1366


🏁 Script executed:

# Check if Lua script execution is guarded by any lock
rg -n -B5 "lua_resume" radio/src/lua/interface.cpp | head -50

Repository: EdgeTX/edgetx

Length of output: 531


🏁 Script executed:

# Check the mixer task main loop to see the locking pattern
sed -n '150,250p' radio/src/tasks/mixer_task.cpp

Repository: EdgeTX/edgetx

Length of output: 2530


🏁 Script executed:

# Check if luaDoGc or script execution holds any locks
rg -n "luaDoGc|luaTaskRun" radio/src/lua/interface.cpp

Repository: EdgeTX/edgetx

Length of output: 235


🏁 Script executed:

# Check getSwitchesPosition to see if it's called with mixer locked
sed -n '235,250p' radio/src/tasks/mixer_task.cpp

Repository: EdgeTX/edgetx

Length of output: 465


🏁 Script executed:

# Verify the actual task execution flow - check where lua_resume is called from
rg -n "luaTaskRun|luaTask" radio/src/lua/interface.cpp | head -10

Repository: EdgeTX/edgetx

Length of output: 95


🏁 Script executed:

# Check if Lua execution task holds any mutex
rg -n -B10 "lua_resume" radio/src/lua/interface.cpp | grep -E "mutex|lock|critical" | head -10

Repository: EdgeTX/edgetx

Length of output: 39


🏁 Script executed:

# Check task priorities and which task runs Lua
rg -n "LUA_TASK_PRIO|UI_TASK_PRIO|MIXER_TASK_PRIO" radio/src/

Repository: EdgeTX/edgetx

Length of output: 254


🏁 Script executed:

# Look at what the menu/UI task does with Lua
sed -n '1100,1180p' radio/src/lua/interface.cpp

Repository: EdgeTX/edgetx

Length of output: 2851


🏁 Script executed:

# Identify what task runs luaTask
rg -n "luaTask" radio/src/tasks.cpp | head -20

Repository: EdgeTX/edgetx

Length of output: 39


🏁 Script executed:

# Check if UI task holds mixerTaskLock when calling luaTask
sed -n '1,100p' radio/src/tasks.cpp | grep -A 30 "menuTask\|uiTask"

Repository: EdgeTX/edgetx

Length of output: 39


🏁 Script executed:

# Verify mixer lock only protects the mixer code, not Lua
rg -n -A 20 "bool mixerTaskRunning" radio/src/tasks/mixer_task.cpp | head -40

Repository: EdgeTX/edgetx

Length of output: 564


🏁 Script executed:

# Check if there's any atomicity guarantee for 64-bit ops on 32-bit targets
fd -i "atomic\|lock" radio/src/hal | head -20

Repository: EdgeTX/edgetx

Length of output: 39


🏁 Script executed:

# Check doMixerCalculations to confirm mixer lock scope
sed -n '195,260p' radio/src/tasks/mixer_task.cpp

Repository: EdgeTX/edgetx

Length of output: 1465


🏁 Script executed:

# Verify getSwitch is only called from within mixer lock
rg -n "getSwitch" radio/src/mixer.cpp | head -5

Repository: EdgeTX/edgetx

Length of output: 375


🏁 Script executed:

# Check the actual task definitions to confirm Lua runs in separate task
grep -n "luaTask\|menuTask\|uiTask" radio/src/tasks.h

Repository: EdgeTX/edgetx

Length of output: 39


🏁 Script executed:

# Confirm virtualSwitches has no volatile or atomic qualifier
rg -n "volatile.*virtualSwitches|atomic.*virtualSwitches" radio/src/

Repository: EdgeTX/edgetx

Length of output: 39


Guard virtualSwitches 64-bit access with mutex to prevent torn reads on 32-bit targets.

virtualSwitches is read by the mixer task (via getSwitch() at line 771) and written by Lua scripts (via setVirtualSwitch()/getVirtualSwitch() at lua/api_general.cpp:2538,2541). On 32-bit targets, concurrent 64-bit accesses without synchronization can cause torn reads or writes, transiently corrupting switch state. The mixer task holds mixerTaskLock but Lua scripts access this variable unsynchronized. Consider protecting all virtualSwitches accesses with a dedicated mutex or ensuring single-threaded access.

🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/switches.cpp` around lines 767 - 772, The virtualSwitches 64-bit
variable is accessed without mutex protection on 32-bit targets, creating a race
condition between the mixer task reading it via getSwitch() and Lua scripts
writing it via setVirtualSwitch() and getVirtualSwitch(). To fix this, create a
dedicated mutex for virtualSwitches access, then wrap the bitwise read operation
(result = virtualSwitches & mask) in the virtual switch handling block within
the VCONTROLS and COLORLCD preprocessor guard with mutex lock/unlock, and
similarly protect all write accesses to virtualSwitches in the Lua API functions
to ensure atomic 64-bit operations on 32-bit targets.

Comment on lines +97 to +98
#define TR_MENU_VIRTUAL_SWITCHES CHAR_SWITCH "Virt. switches"
#define TR_MENU_VCONTROL "Virtuals"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

New virtual-control menu labels are not localized in non-English locale files.
The shared root cause is adding English placeholders instead of locale-specific strings.

  • radio/src/translations/i18n/cn.h#L97-L98: replace "Virt. switches" and "Virtuals" with Chinese translations.
  • radio/src/translations/i18n/cz.h#L666-L667: replace "Virt. switches" and "Virtuals" with Czech translations.
📍 Affects 2 files
  • radio/src/translations/i18n/cn.h#L97-L98 (this comment)
  • radio/src/translations/i18n/cz.h#L666-L667
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/translations/i18n/cn.h` around lines 97 - 98, The virtual-control
menu labels in the locale files are using English placeholder strings instead of
proper locale-specific translations. In radio/src/translations/i18n/cn.h at
lines 97-98, replace the English strings in the TR_MENU_VIRTUAL_SWITCHES and
TR_MENU_VCONTROL macro definitions with their corresponding Chinese
translations. Similarly, in radio/src/translations/i18n/cz.h at lines 666-667,
replace those same English strings with their corresponding Czech translations
for the same two macros. Ensure each locale file contains the appropriate
translated text for its language rather than English fallbacks.

Comment on lines +1252 to +1253
#define TR_VC "VIn"
#define TR_VSW "VS"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Virtual input prefix is inconsistent with the VIxx contract across locales.
All affected locales currently define TR_VC as "VIn", which renders VIn* labels and diverges from the VIxx/VSxx naming convention for this feature.

  • radio/src/translations/i18n/fi.h#L1252-L1253: change TR_VC from "VIn" to "VI".
  • radio/src/translations/i18n/fr.h#L1254-L1255: change TR_VC from "VIn" to "VI".
  • radio/src/translations/i18n/he.h#L1253-L1254: change TR_VC from "VIn" to "VI".
📍 Affects 3 files
  • radio/src/translations/i18n/fi.h#L1252-L1253 (this comment)
  • radio/src/translations/i18n/fr.h#L1254-L1255
  • radio/src/translations/i18n/he.h#L1253-L1254
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/translations/i18n/fi.h` around lines 1252 - 1253, The TR_VC macro
definition is inconsistent across locales and uses "VIn" instead of "VI" to
match the VIxx/VSxx naming convention. Update TR_VC in
radio/src/translations/i18n/fi.h at lines 1252-1253 by changing the value from
"VIn" to "VI". Apply the same change to radio/src/translations/i18n/fr.h at
lines 1254-1255 and radio/src/translations/i18n/he.h at lines 1253-1254,
ensuring the TR_VC macro uses "VI" as its value in all three locale files.

Comment on lines +1248 to +1251
#define TR_MENU_VIRTUAL_SWITCHES CHAR_SWITCH "Virt. switches"
#define TR_MENU_VCONTROL "Virtuals"
#define TR_VC "VIn"
#define TR_VSW "VS"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Normalize VCONTROLS locale strings to the agreed VIxx/VSxx naming and avoid English placeholders in localized files.

The new keys currently mix localized and English UI text, and TR_VC uses VIn instead of the agreed VI prefix for VIxx. This causes inconsistent labels across locales.

  • radio/src/translations/i18n/it.h#L1248-L1251: localize TR_MENU_VIRTUAL_SWITCHES / TR_MENU_VCONTROL; set TR_VC to the VI prefix format used by UI naming (VIxx).
  • radio/src/translations/i18n/jp.h#L1247-L1250: localize TR_MENU_VIRTUAL_SWITCHES / TR_MENU_VCONTROL; align TR_VC with VIxx.
  • radio/src/translations/i18n/ko.h#L1294-L1297: localize TR_MENU_VIRTUAL_SWITCHES / TR_MENU_VCONTROL; align TR_VC with VIxx.
  • radio/src/translations/i18n/nl.h#L1251-L1254: localize TR_MENU_VIRTUAL_SWITCHES / TR_MENU_VCONTROL; align TR_VC with VIxx.
📍 Affects 4 files
  • radio/src/translations/i18n/it.h#L1248-L1251 (this comment)
  • radio/src/translations/i18n/jp.h#L1247-L1250
  • radio/src/translations/i18n/ko.h#L1294-L1297
  • radio/src/translations/i18n/nl.h#L1251-L1254
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/translations/i18n/it.h` around lines 1248 - 1251, The VCONTROLS
locale strings are mixing English placeholders with localized text and the TR_VC
definition uses "VIn" instead of the agreed "VI" prefix format. Fix this across
all four locale files: In radio/src/translations/i18n/it.h (lines 1248-1251),
localize the strings for TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to
Italian, and change TR_VC from "VIn" to "VI". In
radio/src/translations/i18n/jp.h (lines 1247-1250), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Japanese, and align TR_VC to
use "VI". In radio/src/translations/i18n/ko.h (lines 1294-1297), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Korean, and align TR_VC to use
"VI". In radio/src/translations/i18n/nl.h (lines 1251-1254), localize
TR_MENU_VIRTUAL_SWITCHES and TR_MENU_VCONTROL to Dutch, and align TR_VC to use
"VI". Ensure all four files follow the consistent VIxx/VSxx naming convention
with properly localized UI text rather than English placeholders.

Comment on lines +1250 to +1251
#define TR_VC "VIn"
#define TR_VSW "VS"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align virtual input prefix with the VIxx naming contract.

The new TR_VC values use VIn, which will render labels as VIn… instead of the agreed VI… format. This conflicts with the VIxx/VSxx convention described for this feature.

  • radio/src/translations/i18n/pl.h#L1250-L1251: On Line [1250], change TR_VC from "VIn" to "VI" (keep TR_VSW as "VS").
  • radio/src/translations/i18n/pt.h#L1251-L1252: On Line [1251], change TR_VC from "VIn" to "VI" (keep TR_VSW as "VS").
  • radio/src/translations/i18n/ru.h#L1252-L1253: On Line [1252], change TR_VC from "VIn" to "VI" (keep TR_VSW as "VS").
📍 Affects 3 files
  • radio/src/translations/i18n/pl.h#L1250-L1251 (this comment)
  • radio/src/translations/i18n/pt.h#L1251-L1252
  • radio/src/translations/i18n/ru.h#L1252-L1253
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/translations/i18n/pl.h` around lines 1250 - 1251, The TR_VC macro
definition uses "VIn" but should use "VI" to comply with the VIxx/VSxx naming
convention. Fix this issue in three translation files: In
radio/src/translations/i18n/pl.h at line 1250, change the TR_VC definition from
"VIn" to "VI" (leave TR_VSW as "VS"); in radio/src/translations/i18n/pt.h at
line 1251, make the same change to TR_VC from "VIn" to "VI"; and in
radio/src/translations/i18n/ru.h at line 1252, apply the identical fix to TR_VC,
changing it from "VIn" to "VI". Keep TR_VSW unchanged as "VS" in all three
files.

#define TR_MENU_LOGICAL_SWITCHES CHAR_SWITCH "Logiska brytare"
#define TR_MENU_VIRTUAL_SWITCHES CHAR_SWITCH "Virt. switches"
#define TR_MENU_VCONTROL "Virtuals"
#define TR_VC "VIn"

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue | 🟡 Minor | ⚡ Quick win

Align TR_VC with the VIxx naming contract.

These locales currently set TR_VC to "VIn", which renders labels like VIn1 and conflicts with the VIxx/VSxx convention documented for this feature.

  • radio/src/translations/i18n/se.h#L1265-L1265: change TR_VC from "VIn" to "VI".
  • radio/src/translations/i18n/tw.h#L1248-L1248: change TR_VC from "VIn" to "VI".
  • radio/src/translations/i18n/ua.h#L1251-L1251: change TR_VC from "VIn" to "VI".
📍 Affects 3 files
  • radio/src/translations/i18n/se.h#L1265-L1265 (this comment)
  • radio/src/translations/i18n/tw.h#L1248-L1248
  • radio/src/translations/i18n/ua.h#L1251-L1251
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.

In `@radio/src/translations/i18n/se.h` at line 1265, The macro TR_VC is currently
set to "VIn" in three translation files, which conflicts with the VIxx naming
convention. Update the TR_VC macro definition in all three locations: in
radio/src/translations/i18n/se.h at line 1265, change TR_VC from "VIn" to "VI";
in radio/src/translations/i18n/tw.h at line 1248, change TR_VC from "VIn" to
"VI"; and in radio/src/translations/i18n/ua.h at line 1251, change TR_VC from
"VIn" to "VI". This will ensure all three locales conform to the documented
VIxx/VSxx naming convention and prevent label rendering conflicts.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

8 participants