Conversation
Boards::getManufacturer() (added in EdgeTX#7623) was a hand-maintained Board::Type switch duplicating knowledge the hw_defs/*.json files already encode per board. It had already drifted: BOARD_SENDUWING_H17 and BOARD_RADIOMASTER_GX15 were both missing, silently showing "???" in the firmware picker. Add an "identity" object (manufacturer + model) to the hw_defs JSON schema and populate it for all 57 boards, teach BoardJson to parse it as a new Board::Manufacturer capability, and replace the switch with a one-liner delegating to the existing getCapabilityStr() mechanism (the same pattern already used for CPU/CPUType). No caller changes needed. model is populated now but not yet consumed - Boards::getBoardName() is a near-identical switch that can read it in a follow-up. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
radio/src/CMakeLists.txt already auto-generates RADIO_<FLAVOUR> for every target (uppercased FLAVOUR, "+" -> "P"), unconditionally, after each target's own CMakeLists.txt runs. h17 and stm32h7s78-dk each also manually added the exact same macro (-DRADIO_H17, -DRADIO_H7RS), duplicating it for no reason - every other target already relies solely on the auto-generated one. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every target defined its own -DMANUFACTURER_<NAME> by hand in CMakeLists.txt, duplicating the "identity.manufacturer" field the previous commit added to hw_defs/*.json for Companion. Generate it instead, via a new hal_manufacturer.jinja template wired into the existing AddHWGenTarget codegen pipeline (same mechanism already used for hal_settings.h etc.), and remove all the manual definitions. The generated hal_manufacturer.h is included from each target's hal.h next to hal_settings.h, which every existing #if defined(MANUFACTURER_X) call site already transitively includes via board.h/edgetx.h - matching the established pattern for hal_settings.h itself. Macro names are unchanged (MANUFACTURER_<manufacturer.upper()> exactly matches what was hand-written), so no call site needed updating. Verified by building 3 representative targets end to end (H17/Senduwing standalone target, X9E/FrSky taranis-family target - the widest MANUFACTURER_FRSKY usage across board.cpp/hal.h/gui_common.h/ navigation.h, V16/HelloRadioSky horus-family target) and by test_templates.py (912/912 board x template combinations). Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Every target hand-maintained its own usb_descriptor.h defining USB_NAME/USB_MANUFACTURER/USB_PRODUCT for the USB MSC descriptor - a fourth hand-maintained source of overlapping board identity data, alongside the manufacturer/model switch (companion) and the MANUFACTURER_<NAME> defines (previous commit). Move it into the same hw_defs/*.json "identity" node as three new fields, generate an equivalent usb_descriptor.h via a new hal_usb_descriptor.jinja template, and delete every hand-written usb_descriptor.h outright (rather than just replacing their content) - usbd_storage_msd.cpp is the sole #include site, and the generated file lands in the same build-directory include path the source-directory one used to occupy, so no source changes were needed there at all. usb_manufacturer/usb_product store only the meaningful (right-trimmed) content; the template re-pads to the required 8 bytes at generation time, matching the original files' fixed-width space-padded C literal arrays exactly - verified byte-for-byte against all 57 boards' current values before deleting anything. One deliberate fix, not a byte-preserving transcription: the plain Jumper T12 board had no RADIO_T12 guard in taranis/usb_descriptor.h at all, so it fell through to the FrSky Taranis default and shipped with the wrong manufacturer/product identity - a real, pre-existing bug. Fixed per explicit instruction, matching sibling Jumper boards' style in the same file (JUMPER / "T12" / "Jumper T12"). usb_name is kept as its own explicit field for now, not derived from manufacturer+model, since only 21/57 boards' values are actually equal to that combination today - deriving it now would silently change 36 boards' real shipping USB descriptor name (casing corrections plus structural differences: missing manufacturer prefixes, product-line names like "Taranis"/"Horus" instead of the bare model codes chosen for `model`). Deriving it is a deliberate follow-up, not bundled here. Verified: test_templates.py 969/969 (57 boards x 17 templates), a script cross-checking all 57 boards' generated output byte-for-byte against the values extracted from the original files before deletion, and building H17/X9E/V16 firmware end to end (byte-identical output size to the pre-change builds for H17). stm32h7s78-dk/usb_descriptor.h is intentionally untouched: it has no corresponding hw_defs/h7rs.json, so nothing exists to generate a replacement from, and that target already can't build through the standard AddHWGenTarget pipeline for the same reason (hal_settings.h etc. need the same missing file) - unrelated to this change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
usb_name was carried as its own explicit JSON field in the previous commit specifically to avoid changing any board's shipping USB descriptor string as a side effect of that migration. Only 21/57 boards' usb_name actually equalled "<manufacturer> <model>" though; now that the byte-preserving migration has landed and is separately committed, deriving it is a deliberate follow-up cleanup rather than an accidental side effect. This changes the real generated USB_NAME for 36 boards: casing corrections on ~12 (e.g. "Radiomaster Boxer" -> "RadioMaster Boxer", matching the branding correction already applied to `manufacturer` in the companion commit), and structural differences on the rest - missing manufacturer prefixes (e.g. c14 "C14" -> "iFlight Commando 14"), product-line names replaced by the bare model code chosen for `model` (e.g. x7 "FrSky Taranis" -> "FrSky X7/X7S", x12s "FrSky Horus" -> "FrSky X12S"), and a couple of spacing/abbreviation differences (tlite "Jumper TLite" -> "Jumper T-Lite"). Verified: json_validator.py and test_templates.py (969/969) both pass, derived output spot-checked against the expected manufacturer+model combination for a representative mix of previously-matching and previously-mismatching boards, and H17/X9E/V16 firmware rebuilt clean. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
Spotted while reviewing the migrated data: - c14: usb_manufacturer "IFlight" -> "iFlight" (wrong capitalisation, inconsistent with commando8 and with identity.manufacturer "iFlight") - commando8: usb_manufacturer "iFlight-" -> "iFlight" (stray trailing hyphen); usb_product "Commando" -> "C8", matching c14's "C14" style - v12: usb_manufacturer "HELLO" -> "HRSky", matching v14/v14lcd/v16 (all four are HelloRadioSky boards; v12 was the only one still using the older, inconsistent abbreviation) - zorro: usb_product "RM Zorro" -> "Zorro" Verified: json_validator.py and test_templates.py (969/969) both pass; rebuilt the Zorro target end to end. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Is a new section (identity) and hal file (hal_manufacturer.h) necessary? |
|
It isn't really "hardware" though, is it? - 1) it is not related to CPU / hardware capabilties and 2) it means it is not exposed at the very top of the file. I initially was going to do that when it was only manufacturer, but as soon as the model name and other details made sense to go in also, having that embedded in the middle of file just seems to make no sense at all. |
The manufacturer commit only added hal_manufacturer as a dependency of board/board_bl, but hal_manufacturer.h is #included from every target's hal.h (right next to hal_settings.h) - so anything compiling a file that reaches hal.h needs it, not just those two targets. CI caught four more: stm32_drivers, simu_drivers (used by both the native gtests-radio/tests-radio build and the WASM module build - this is what broke most of the CI matrix), yaml_data (the YAML codegen drift check), and minimal_board_lib. Found by searching for every existing add_dependencies(... hal_settings ...) call, since hal.h always pairs the two - anywhere hal_settings was already listed but hal_manufacturer wasn't is exactly the set of targets this was missing from. Verified locally (the CI failures weren't reproducible target-by-target without this): yaml_data builds clean for x10 (matches the "YAML parsers" drift job), tests-radio builds and passes 130/130 for el18 (matches both the per-flavour "Run tests" jobs and the WASM module build, which share simu_drivers), and a full H17 firmware rebuild still produces byte-identical output. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Please note the reason this is here as a draft is for consideration and discussion, if there is consensus on a different direction or that anything should be implemented differently (once everything works!)... I'm more than happy to make changes to it, abandon it, etc. |
|
The more generated and maintained in radio/src the better for Companion. |
Summary
While reviewing #7623,
Boards::getManufacturer()was added for companion - a hand-maintainedBoard::Typeswitch, which had knowledge which really should have been present in thehw_defs/*.jsonfiles already. It had already drifted (BOARD_SENDUWING_H17andBOARD_RADIOMASTER_GX15both fell through to "???").So rather than just patching that switch, moved board identity into the JSON source of truth already shared by firmware codegen and Companion's runtime parser, and did the same for two more places that carried the same duplicated data:
Boards::getManufacturer()'s ~75-line switch → reads a newidentity.manufacturer/identity.modelobject inhw_defs/*.jsonviaBoardJson, one-line delegate to the existinggetCapabilityStr()mechanism (same pattern asCPU/CPUType).-DMANUFACTURER_<NAME>→ generated from the same JSON field via a newhal_manufacturer.jinjatemplate in the existingAddHWGenTargetcodegen pipeline.usb_descriptor.h(USB_NAME/USB_MANUFACTURER/USB_PRODUCT) → generated fromidentitytoo;usbd_storage_msd.cppis the only#includesite, so the 13 hand-written files were deleted outright rather than just having their content replaced.h17,stm32h7s78-dk) manually re-definedRADIO_<FLAVOUR>, whichradio/src/CMakeLists.txtalready auto-generates for every target unconditionally.fw.json(the web flasher's own manufacturer list) was deliberately not touched - its board subset mirrorsnightly.yml's CI publish matrix rather thanhw_defs, and its display names use marketing conventions ("Horus X10", "QX7 Access") that don't derive cleanly from anything in this JSON. Folding it in would mean either adding yet another hand-curated field or silently renaming boards in the web flasher UI, so left it alone.Test plan
radio/util/hw_defs/json_validator.pypasses for all 57hw_defs/*.jsonfilesradio/util/hw_defs/test_templates.pypasses (57 boards × 17 templates = 969 combinations)gtests-companionbuilds and passes, including newboardjson_manufacturer_test.cpp(asserts every registered board resolves to a real, non-"???" manufacturer; would have caught the H17/GX15 gaps)H17(Senduwing, standalone),X9E(FrSky,taranisshared dir - widestMANUFACTURER_FRSKYusage),V16(HelloRadioSky,horusshared dir),Zorro(Radiomaster)🤖 Generated with Claude Code