feat: Support of new radio DRO1 by DumboRC - #6076
Conversation
|
Are there any informations about this radio available? |
…ardware internal or external module
|
@FreedomLJS Are there any further infos about the DR01 (tech specs, release date, ...)? |
|
Since it is a F4 radio, it cannot be merged anymore anyway |
Ok, thanks for that info! |
|
Note Reviews pausedIt looks like this branch is under active development. To avoid overwhelming you with review comments due to an influx of new commits, CodeRabbit has automatically paused this review. You can configure this behavior by changing the Use the following commands to manage reviews:
Use the checkboxes below for quick actions:
📝 WalkthroughWalkthroughThis PR adds DumboRC DRO1 support across companion firmware, radio target configuration, build tooling, and CI workflows. It also introduces a DRO1 hardware definition and updates several radio runtime conditionals and UI paths for the new target. ChangesDumboRC DRO1 Board Integration
Estimated code review effort: 4 (Complex) | ~60 minutes Possibly related PRs
Suggested labels: 🚥 Pre-merge checks | ✅ 3 | ❌ 2❌ Failed checks (2 warnings)
✅ Passed checks (3 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
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. Comment |
There was a problem hiding this comment.
Actionable comments posted: 5
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (1)
radio/src/targets/horus/usb_descriptor.h (1)
52-59:⚠️ Potential issue | 🟠 Major | ⚡ Quick winDRO1 USB descriptors are shadowed by the earlier
PCBX10branch.Because
#elif defined(PCBX10)is evaluated before#elif defined(RADIO_DRO1), DRO1 builds that also definePCBX10will expose X10 USB strings instead of DumboRC DRO1 strings.Suggested fix
-#elif defined(PCBX10) - `#define` USB_NAME "FrSky X10" - `#define` USB_MANUFACTURER 'F', 'r', 'S', 'k', 'y', ' ', ' ', ' ' /* 8 bytes */ - `#define` USB_PRODUCT 'X', '1', '0', ' ', ' ', ' ', ' ', ' ' /* 8 Bytes */ -#elif defined(RADIO_DRO1) +#elif defined(RADIO_DRO1) `#define` USB_NAME "DumboRC DRO1" `#define` USB_MANUFACTURER 'D', 'U', 'M', 'B', 'O', 'R', 'C', ' ' /* 8 bytes */ `#define` USB_PRODUCT 'D', 'R', 'O', '1', ' ', ' ', ' ', ' ' /* 8 Bytes */ +#elif defined(PCBX10) + `#define` USB_NAME "FrSky X10" + `#define` USB_MANUFACTURER 'F', 'r', 'S', 'k', 'y', ' ', ' ', ' ' /* 8 bytes */ + `#define` USB_PRODUCT 'X', '1', '0', ' ', ' ', ' ', ' ', ' ' /* 8 Bytes */🤖 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/targets/horus/usb_descriptor.h` around lines 52 - 59, The RADIO_DRO1 USB descriptors are being shadowed by the earlier PCBX10 branch so builds that define both PCBX10 and RADIO_DRO1 get the X10 strings; change the conditional ordering or make the branches independent: ensure the RADIO_DRO1 block (defines USB_NAME, USB_MANUFACTURER, USB_PRODUCT) is evaluated before the PCBX10 block (or convert both to separate `#if` ... `#endif` blocks) so RADIO_DRO1 builds receive the DumboRC DRO1 strings instead of the FrSky X10 strings; update the placement of the RADIO_DRO1/PCBX10 conditionals around the USB_* defines accordingly.
🤖 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/boards/hw_defs/dro1.json`:
- Around line 137-143: EXT3 and EXT4 entries currently reuse the same
ADC/GPIO/pin/channel as RH/RV, making them electrical aliases; update the "EXT3"
and "EXT4" objects so their "adc", "gpio", "pin" and "channel" fields point to
unique ADC/GPIO pins (not the same values used by "RH" or "RV"/stick axes) and
keep "inverted" as required, and apply the same fix to the other duplicated
blocks noted (the other EXT3/EXT4 and RH/RV occurrences); locate entries by the
"name":"EXT3"/"EXT4" and "name":"RH"/"RV" symbols and replace the duplicated
mapping values with the correct distinct ADC channel and GPIO pin identifiers.
In `@radio/src/targets/horus/hal.h`:
- Around line 133-134: PCBREV_VALUE() is defined as the braced-init-list "{0}"
for RADIO_V16 / RADIO_DRO1 but is used as an rvalue in an assignment
(hardwareOptions.pcbrev = PCBREV_VALUE(); in pwr_driver.cpp) where a scalar is
expected; change the macro for RADIO_DRO1/RADIO_V16 from "{0}" to a plain scalar
(e.g., 0 or (0)) so PCBREV_VALUE() expands to a valid numeric expression for
assignment to the bit-field hardwareOptions.pcbrev.
In `@tools/build-dumborc.py`:
- Around line 71-73: Validate that each requested board and translation exists
before calling build: check entries in args.boards against the boards mapping
keys (the variable boards) and args.translations against the translations list,
and if any unknown values are present raise an argparse.ArgumentError or call
parser.error with a clear message listing the invalid items; then iterate only
over the validated lists and call build(board, translation, args.srcdir). Locate
this in the loop using the variables boards, translations, args.boards,
args.translations and the build() call and add the membership checks and error
reporting there.
- Around line 38-54: The current build flow uses os.system and continues even if
cmake/make fail, then may try to copy a missing artifact and skip cleanup; wrap
the build and copy steps in a try/finally where the finally always restores cwd
and calls shutil.rmtree(path), and make the build commands fail-fast by
replacing os.system(...) calls (or checking their return codes) with
subprocess.run(..., check=True) or explicitly raising on non-zero return so that
the shutil.copy of "%s/arm-none-eabi/firmware.bin" and the loop that creates
filename only run when builds succeed; ensure you reference and preserve cwd,
path, command, the make invocation, shutil.copy and shutil.rmtree when
implementing the change.
- Around line 40-43: The code currently builds a shell command string using
cmake_options, translation, and srcdir and calls os.system, which is vulnerable
to injection; replace the string interpolation and os.system calls with
subprocess.run using an argument list (e.g., subprocess.run(["cmake"] +
cmake_options.split() + ["-DTRANSLATIONS="+translation, "-DDUMBORC_RELEASE=YES",
srcdir], check=True) and subprocess.run(["make", "firmware", "-j6"],
check=True)), avoid shell=True, and keep or log a safe printable representation
of the command instead of executing the formatted shell string.
---
Outside diff comments:
In `@radio/src/targets/horus/usb_descriptor.h`:
- Around line 52-59: The RADIO_DRO1 USB descriptors are being shadowed by the
earlier PCBX10 branch so builds that define both PCBX10 and RADIO_DRO1 get the
X10 strings; change the conditional ordering or make the branches independent:
ensure the RADIO_DRO1 block (defines USB_NAME, USB_MANUFACTURER, USB_PRODUCT) is
evaluated before the PCBX10 block (or convert both to separate `#if` ... `#endif`
blocks) so RADIO_DRO1 builds receive the DumboRC DRO1 strings instead of the
FrSky X10 strings; update the placement of the RADIO_DRO1/PCBX10 conditionals
around the USB_* defines accordingly.
🪄 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: aecfdc5c-ce6e-493e-9d0b-4e5afe5b43ff
📒 Files selected for processing (32)
.github/ISSUE_TEMPLATE/bug-report.yml.github/workflows/build_fw.yml.github/workflows/nightly.ymlcompanion/src/firmwares/boardjson.cppcompanion/src/firmwares/boards.cppcompanion/src/firmwares/boards.hcompanion/src/firmwares/opentx/opentxinterface.cppfw.jsonradio/src/CMakeLists.txtradio/src/boards/hw_defs/dro1.jsonradio/src/edgetx.cppradio/src/gui/colorlcd/fonts.cppradio/src/gui/colorlcd/model/model_setup.cppradio/src/gui/colorlcd/module/module_setup.cppradio/src/stamp.cppradio/src/targets/horus/CMakeLists.txtradio/src/targets/horus/board.hradio/src/targets/horus/hal.hradio/src/targets/horus/lcd_driver.cppradio/src/targets/horus/tp_gt911.cppradio/src/targets/horus/usb_descriptor.hradio/src/tasks.cppradio/src/telemetry/frsky.hradio/src/tests/module_ports.cppradio/util/build-firmware.pyradio/util/fwoptions.pyradio/util/hw_defs/legacy_names.pytools/boards.pytools/build-common.shtools/build-dumborc.pytools/generate-hw-defs.shtools/generate-yaml.sh
There was a problem hiding this comment.
🧹 Nitpick comments (1)
radio/src/gui/colorlcd/model/model_setup.cpp (1)
459-459: 💤 Low valueConsider removing the "FIX :" comment prefix.
The "FIX :" prefix is non-standard. Standard practice uses "NOTE:", "TODO:", or plain descriptive text. Since the guard itself is correct, the comment could simply read:
/* Valid only if HARDWARE_INTERNAL_MODULE is defined */🤖 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/gui/colorlcd/model/model_setup.cpp` at line 459, Remove the non-standard "FIX :" prefix from the comment on line 459 in the file radio/src/gui/colorlcd/model/model_setup.cpp. The comment should simply read "/* Valid only if HARDWARE_INTERNAL_MODULE is defined */" without the "FIX :" prefix to follow standard commenting conventions.
🤖 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.
Nitpick comments:
In `@radio/src/gui/colorlcd/model/model_setup.cpp`:
- Line 459: Remove the non-standard "FIX :" prefix from the comment on line 459
in the file radio/src/gui/colorlcd/model/model_setup.cpp. The comment should
simply read "/* Valid only if HARDWARE_INTERNAL_MODULE is defined */" without
the "FIX :" prefix to follow standard commenting conventions.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro Plus
Run ID: 7a0c4f6c-eaa1-40f0-8812-2be8d34cb685
📒 Files selected for processing (3)
radio/src/edgetx.cppradio/src/gui/colorlcd/model/model_setup.cppradio/src/gui/colorlcd/module/module_setup.cpp
🚧 Files skipped from review as they are similar to previous changes (2)
- radio/src/edgetx.cpp
- radio/src/gui/colorlcd/module/module_setup.cpp
This thread was created on april 2025 and maybe the initial radio project differs from the current radio form factor which looks very much like a bad copy of the TX16 MK3 (january 2026) in other words you won't miss anything as far as build quality goes.... |
@dsolaray |
Many years and long track record in this hobby. Did you use to work for Radiomaster ?
I have several times visited this outdated website which contains chinenglish wording or confusing spec. tables. Does the support email work now ?
There's life beyond the FPV world, so you are actually developing more clones for an already overly saturated market.
This is nothing but a clone of the TX15 MAX with missing top sliders and features the same low-resolution screen as the V16. The Gemini module is not much relevant for LOS flying
Yet you have to hire western located users @frankiearzu for the receivers firmware development
We hope you do better next time and come out more appealing products for helis and fixed wings. |
|
This is not a place for this kind of discussion |
DRO1 is a new product from DumboRC, which is committed to creating the best user experience for users.
Summary by CodeRabbit
Release Notes