From 26bd20663e08e3445542897da550eba8311e362e Mon Sep 17 00:00:00 2001 From: Peter Feerick Date: Sun, 30 Aug 2026 18:55:03 +1000 Subject: [PATCH] fix(x10express): restore RADIO_X10EXPRESS guards, de-dupe RADIO_FLAVOR, add ci check to prevent future occurances (#7725) Co-authored-by: Peter Feerick <5500713+pfeerick@users.noreply.github.com> Co-authored-by: Claude Opus 5 --- .github/workflows/codegen_drift.yml | 11 +- .github/workflows/validate_radio_macros.yml | 23 ++++ Justfile | 37 +++++- .../src/boards/generic_stm32/module_ports.cpp | 2 +- radio/src/targets/c14/CMakeLists.txt | 2 +- radio/src/targets/gx15/CMakeLists.txt | 1 - radio/src/targets/horus/board.h | 2 +- radio/src/targets/horus/hal.h | 2 +- radio/src/targets/pa01/CMakeLists.txt | 1 + radio/src/targets/pl18/CMakeLists.txt | 1 + radio/src/targets/st16/CMakeLists.txt | 1 + radio/src/targets/t15pro/CMakeLists.txt | 2 +- radio/src/targets/t22/CMakeLists.txt | 2 +- radio/src/targets/tx16smk3/CMakeLists.txt | 1 - radio/src/targets/v12/CMakeLists.txt | 1 - tools/check_radio_macros.py | 109 ++++++++++++++++++ 16 files changed, 178 insertions(+), 20 deletions(-) create mode 100644 .github/workflows/validate_radio_macros.yml create mode 100755 tools/check_radio_macros.py diff --git a/.github/workflows/codegen_drift.yml b/.github/workflows/codegen_drift.yml index 9b696d63f..1efd3e518 100644 --- a/.github/workflows/codegen_drift.yml +++ b/.github/workflows/codegen_drift.yml @@ -74,13 +74,14 @@ jobs: wget -qO- "https://github.com/casey/just/releases/download/${JUST_VERSION}/just-${JUST_VERSION}-x86_64-unknown-linux-musl.tar.gz" \ | tar xz -C /usr/local/bin just - # A Justfile change cannot cause drift, but it can rename a recipe out - # from under the jobs below, so check they still resolve rather than - # regenerating everything. - - name: Check the recipes this workflow calls still resolve + # A Justfile change cannot cause drift, but it can break the file, and + # parsing validates every recipe rather than only the ones below. Fails + # here in seconds instead of in the container jobs; the list is derived, + # so adding a recipe needs no change. + - name: Validate the Justfile run: | just --list >/dev/null - for recipe in cfn-sort gen-fonts gen-yaml gen-radios gen-simstr; do + for recipe in $(just --summary); do just --show "${recipe}" >/dev/null done diff --git a/.github/workflows/validate_radio_macros.yml b/.github/workflows/validate_radio_macros.yml new file mode 100644 index 000000000..46ae0669e --- /dev/null +++ b/.github/workflows/validate_radio_macros.yml @@ -0,0 +1,23 @@ +--- +name: Validate radio macros +on: + push: + branches: + - 'main' + - '[0-9]+.[0-9]+' + pull_request: + workflow_dispatch: + +# No 'paths' filter: any CMakeLists or source edit can orphan a macro, and #6437 +# touched neither hw_defs nor codegen. Pure stdlib, runs in about a second. + +jobs: + validate: + name: Radio macros + runs-on: ubuntu-latest + steps: + - name: Check out the repo + uses: actions/checkout@v7 + + - name: Check every referenced RADIO_* macro can be defined + run: python3 tools/check_radio_macros.py diff --git a/Justfile b/Justfile index 92bcbfd10..41b361469 100644 --- a/Justfile +++ b/Justfile @@ -1,11 +1,12 @@ -# EdgeTX code generation helpers. +# EdgeTX code generation and check helpers. # -# These recipes wrap existing scripts that regenerate files COMMITTED to git. -# After running one, review and commit the result. +# The codegen recipes wrap existing scripts that regenerate files COMMITTED to +# git. After running one, review and commit the result. The check recipes are +# read-only and mirror what CI runs; they regenerate nothing. # -# The codegen recipes run on the host and need the tools installed locally. The -# docker- variants run the same scripts in the dev container instead, which -# already has them. CI uses the host recipes, as its jobs run in that image. +# Both run on the host and need the tools installed locally. The docker- +# variants run the same scripts in the dev container instead, which already has +# them. CI uses the host recipes, as its jobs run in that image. IMAGE := "ghcr.io/edgetx/edgetx-dev:latest" @@ -54,6 +55,23 @@ gen-simstr: [group('codegen')] codegen: gen-fonts cfn-sort gen-yaml gen-radios gen-simstr +# Needs: python3 only. +[doc('Check every referenced RADIO_* macro can be defined')] +[group('checks')] +check-radio-macros: + python3 tools/check_radio_macros.py + +# Needs: the Python packages in radio/util/hw_defs/requirements.txt. +[doc('Validate the hw_defs JSON and that every template renders for every board')] +[group('checks')] +check-hw-defs: + cd radio/util/hw_defs && python3 json_validator.py ../../src/boards/hw_defs + cd radio/util/hw_defs && python3 test_templates.py ../../src/boards/hw_defs + +[doc('Run the RADIO_* macro and hw_defs checks')] +[group('checks')] +check: check-radio-macros check-hw-defs + [doc('Regenerate the LVGL fonts in the dev container')] [group('codegen (docker)')] docker-gen-fonts: @@ -86,3 +104,10 @@ docker-gen-simstr: [doc('Regenerate everything in the dev container')] [group('codegen (docker)')] docker-codegen: docker-gen-fonts docker-cfn-sort docker-gen-yaml docker-gen-radios docker-gen-simstr + +[doc('Run the checks in the dev container')] +[group('checks (docker)')] +docker-check: + {{ _docker }} python3 tools/check_radio_macros.py + {{ _docker }} sh -c 'cd radio/util/hw_defs && python3 json_validator.py ../../src/boards/hw_defs' + {{ _docker }} sh -c 'cd radio/util/hw_defs && python3 test_templates.py ../../src/boards/hw_defs' diff --git a/radio/src/boards/generic_stm32/module_ports.cpp b/radio/src/boards/generic_stm32/module_ports.cpp index 9fd2e7638..981da7141 100644 --- a/radio/src/boards/generic_stm32/module_ports.cpp +++ b/radio/src/boards/generic_stm32/module_ports.cpp @@ -604,7 +604,7 @@ const etx_module_t _sport_module = { #endif // SPORT_UPDATE_PWR_GPIO -#if !defined(RADIO_X10E) +#if !defined(RADIO_X10EXPRESS) uint32_t __pxx1_get_inverter_comp() { return 1; } #endif diff --git a/radio/src/targets/c14/CMakeLists.txt b/radio/src/targets/c14/CMakeLists.txt index 293577c9a..e2e244b10 100644 --- a/radio/src/targets/c14/CMakeLists.txt +++ b/radio/src/targets/c14/CMakeLists.txt @@ -56,7 +56,7 @@ add_definitions(-DFIRMWARE_FORMAT_UF2) add_definitions(-DUSE_CUSTOM_EXTI_IRQ) set(FLAVOUR c14) -add_definitions(-DRADIO_C14) +add_definitions(-DMANUFACTURER_IFLIGHT) # Defines internal modules for PL18 via UART7 # set(INTERNAL_MODULES MULTI CACHE STRING "Internal modules") diff --git a/radio/src/targets/gx15/CMakeLists.txt b/radio/src/targets/gx15/CMakeLists.txt index bbc00294a..fdbaf77b1 100644 --- a/radio/src/targets/gx15/CMakeLists.txt +++ b/radio/src/targets/gx15/CMakeLists.txt @@ -50,7 +50,6 @@ add_definitions(-DUSB_CHARGER) add_definitions(-DUSB_CHARGE_CONTROL) set(FLAVOUR gx15) -add_definitions(-DRADIO_GX15) add_definitions(-DMANUFACTURER_RADIOMASTER) # defines existing internal modules diff --git a/radio/src/targets/horus/board.h b/radio/src/targets/horus/board.h index 0a5bbb22e..19544888f 100644 --- a/radio/src/targets/horus/board.h +++ b/radio/src/targets/horus/board.h @@ -211,7 +211,7 @@ void lcdCopy(void * dest, void * src); #define BACKLIGHT_FORCED_ON BACKLIGHT_LEVEL_MAX + 1 #if defined(PCBX12S) #define BACKLIGHT_LEVEL_MIN 5 -#elif defined(RADIO_FAMILY_T16) || defined(RADIO_X10E) +#elif defined(RADIO_FAMILY_T16) || defined(RADIO_X10EXPRESS) #define BACKLIGHT_LEVEL_MIN 1 #else #define BACKLIGHT_LEVEL_MIN 46 diff --git a/radio/src/targets/horus/hal.h b/radio/src/targets/horus/hal.h index d31f34d17..38bb95fcf 100644 --- a/radio/src/targets/horus/hal.h +++ b/radio/src/targets/horus/hal.h @@ -62,7 +62,7 @@ #elif defined(RADIO_V16) #define ADC_VREF_PREC2 330 #elif defined(PCBX10) - #if defined(RADIO_X10) || defined(RADIO_X10E) + #if defined(RADIO_X10) || defined(RADIO_X10EXPRESS) #define PWM_STICKS #define PWM_TIMER TIM5 #define PWM_TIMER_FREQ (PERI1_FREQUENCY * TIMER_MULT_APB1) diff --git a/radio/src/targets/pa01/CMakeLists.txt b/radio/src/targets/pa01/CMakeLists.txt index d24112cbf..89c785762 100644 --- a/radio/src/targets/pa01/CMakeLists.txt +++ b/radio/src/targets/pa01/CMakeLists.txt @@ -47,6 +47,7 @@ add_definitions(-DFIRMWARE_FORMAT_UF2) add_definitions(-DUSE_CUSTOM_EXTI_IRQ) set(FLAVOUR pa01) +add_definitions(-DMANUFACTURER_FLYSKY) # Defines internal modules for PL18 via UART7 set(INTERNAL_MODULES AFHDS3;CRSF CACHE STRING "Internal modules") diff --git a/radio/src/targets/pl18/CMakeLists.txt b/radio/src/targets/pl18/CMakeLists.txt index 82d491f2c..db75d9119 100644 --- a/radio/src/targets/pl18/CMakeLists.txt +++ b/radio/src/targets/pl18/CMakeLists.txt @@ -40,6 +40,7 @@ set(PCB_RF_BAUD 921600 CACHE STRING "INTERNAL_MODULE_BAUDRATE: ${RF_BAUD_RATE}") set_property(CACHE PCB_RF_BAUD PROPERTY STRINGS ${RF_BAUD_RATE}) add_definitions(-DPCBPL18 -DPCBFLYSKY) +add_definitions(-DMANUFACTURER_FLYSKY) add_definitions(-DBATTERY_CHARGE) add_definitions(-DSTM32_SUPPORT_32BIT_TIMERS) diff --git a/radio/src/targets/st16/CMakeLists.txt b/radio/src/targets/st16/CMakeLists.txt index 9d0a56ab2..b4882c78b 100644 --- a/radio/src/targets/st16/CMakeLists.txt +++ b/radio/src/targets/st16/CMakeLists.txt @@ -48,6 +48,7 @@ add_definitions(-DFIRMWARE_FORMAT_UF2) add_definitions(-DUSE_CUSTOM_EXTI_IRQ) set(FLAVOUR st16) +add_definitions(-DMANUFACTURER_FLYSKY) # Defines internal modules for PL18 via UART7 # set(INTERNAL_MODULES MULTI CACHE STRING "Internal modules") diff --git a/radio/src/targets/t15pro/CMakeLists.txt b/radio/src/targets/t15pro/CMakeLists.txt index b3ac10623..4d8f5ffc5 100644 --- a/radio/src/targets/t15pro/CMakeLists.txt +++ b/radio/src/targets/t15pro/CMakeLists.txt @@ -47,7 +47,7 @@ add_definitions(-DUSE_CUSTOM_EXTI_IRQ) add_definitions(-DUSB_CHARGER) set(FLAVOUR t15pro) -add_definitions(-DRADIO_T15PRO) +add_definitions(-DMANUFACTURER_JUMPER) # defines existing internal modules set(INTERNAL_MODULES CRSF CACHE STRING "Internal modules") diff --git a/radio/src/targets/t22/CMakeLists.txt b/radio/src/targets/t22/CMakeLists.txt index 2429ee4a6..e4e050e35 100644 --- a/radio/src/targets/t22/CMakeLists.txt +++ b/radio/src/targets/t22/CMakeLists.txt @@ -47,7 +47,7 @@ add_definitions(-DUSE_CUSTOM_EXTI_IRQ) add_definitions(-DUSB_CHARGER) set(FLAVOUR t22) -add_definitions(-DRADIO_T22) +add_definitions(-DMANUFACTURER_JUMPER) # defines existing internal modules set(INTERNAL_MODULES CRSF CACHE STRING "Internal modules") diff --git a/radio/src/targets/tx16smk3/CMakeLists.txt b/radio/src/targets/tx16smk3/CMakeLists.txt index 27364de70..56cad4346 100644 --- a/radio/src/targets/tx16smk3/CMakeLists.txt +++ b/radio/src/targets/tx16smk3/CMakeLists.txt @@ -68,7 +68,6 @@ add_definitions(-DUSB_CHARGE_CONTROL) add_definitions(-DLUMINOSITY_SENSOR) set(FLAVOUR tx16smk3) -add_definitions(-DRADIO_TX16SMK3) add_definitions(-DMANUFACTURER_RADIOMASTER) # defines existing internal modules diff --git a/radio/src/targets/v12/CMakeLists.txt b/radio/src/targets/v12/CMakeLists.txt index fd23dff60..c5595217a 100644 --- a/radio/src/targets/v12/CMakeLists.txt +++ b/radio/src/targets/v12/CMakeLists.txt @@ -47,7 +47,6 @@ add_definitions(-DUSE_CUSTOM_EXTI_IRQ) add_definitions(-DUSB_CHARGER) set(FLAVOUR v12) -add_definitions(-DRADIO_V12) add_definitions(-DMANUFACTURER_HELLORADIOSKY) # defines existing internal modules diff --git a/tools/check_radio_macros.py b/tools/check_radio_macros.py new file mode 100755 index 000000000..4d3279136 --- /dev/null +++ b/tools/check_radio_macros.py @@ -0,0 +1,109 @@ +#!/usr/bin/env python3 +"""Detect RADIO_* macros that are referenced but can never be defined. + +These exist only as CMake -D flags, so a stale '#if defined(RADIO_FOO)' is +silently always-false. Both sets are derived from the tree. +""" +import re +import sys +from pathlib import Path + +ROOT = Path(__file__).resolve().parents[1] +SRC = ROOT / "radio" / "src" +TARGETS = SRC / "targets" + +TOKEN = r"RADIO_[A-Z0-9_]+" +SRC_EXT = {".c", ".cpp", ".h", ".hpp", ".inc"} +# dot-dirs skipped too: a stray git worktree must not widen the definable set +SKIP_DIRS = {"thirdparty", "build"} + + +def skipped(path): + return any(p in SKIP_DIRS or p.startswith(".") for p in path.parts) + + +def flavour_macro(flavour): + # mirrors radio/src/CMakeLists.txt: TOUPPER then '+' -> 'P' + return "RADIO_" + flavour.upper().replace("+", "P") + + +def definable(): + """Every RADIO_* the build can possibly define.""" + names, why = set(), {} + + def add(name, reason): + names.add(name) + why.setdefault(name, reason) + + for cml in TARGETS.glob("*/CMakeLists.txt"): + text = cml.read_text(errors="ignore") + for flav in re.findall(r"set\s*\(\s*FLAVOUR\s+([A-Za-z0-9+._-]+)\s*\)", text): + add(flavour_macro(flav), f"FLAVOUR {flav} ({cml.relative_to(ROOT)})") + + # hand-written -DRADIO_* anywhere in the CMake layer + cmake_files = [] + for base in (ROOT / "radio", ROOT / "cmake"): + cmake_files += list(base.rglob("CMakeLists.txt")) + list(base.rglob("*.cmake")) + for cml in cmake_files: + if skipped(cml.relative_to(ROOT)): + continue + for name in re.findall(r"-D(" + TOKEN + ")", cml.read_text(errors="ignore")): + add(name, f"add_definitions ({cml.relative_to(ROOT)})") + + # plain #define in sources + for f in walk_sources(): + for name in re.findall(r"^\s*#\s*define\s+(" + TOKEN + ")", f.read_text(errors="ignore"), re.M): + add(name, f"#define ({f.relative_to(ROOT)})") + + return names, why + + +def walk_sources(): + for f in SRC.rglob("*"): + if f.suffix in SRC_EXT and f.is_file() and not skipped(f.relative_to(ROOT)): + yield f + + +def referenced(): + """Every RADIO_* the tree tests, with locations.""" + hits = {} + cond = re.compile(r"^\s*#\s*(?:if|elif|ifdef|ifndef)\b(.*)$") + for f in walk_sources(): + for n, line in enumerate(f.read_text(errors="ignore").splitlines(), 1): + m = cond.match(line) + if m: + for name in re.findall(TOKEN, m.group(1)): + hits.setdefault(name, []).append(f"{f.relative_to(ROOT)}:{n}") + + # hw_defs codegen reads these back out of the preprocessed dump + for f in (ROOT / "radio" / "util").rglob("*.py"): + if skipped(f.relative_to(ROOT)): + continue + for n, line in enumerate(f.read_text(errors="ignore").splitlines(), 1): + if "hw_defs" in line: + for name in re.findall(r"['\"](" + TOKEN + r")['\"]", line): + hits.setdefault(name, []).append(f"{f.relative_to(ROOT)}:{n}") + return hits + + +def main(): + can, _ = definable() + used = referenced() + orphans = {k: v for k, v in used.items() if k not in can} + + print(f"{len(can)} definable RADIO_* macros, {len(used)} referenced") + if not orphans: + print("OK: every referenced RADIO_* macro can be defined.") + return 0 + + print(f"\nERROR: {len(orphans)} RADIO_* macro(s) referenced but never definable:\n", file=sys.stderr) + for name in sorted(orphans): + print(f" {name}", file=sys.stderr) + for loc in orphans[name]: + print(f" {loc}", file=sys.stderr) + print("\nEither the name is stale, or its target no longer defines it.", file=sys.stderr) + return 1 + + +if __name__ == "__main__": + sys.exit(main())