feat: add HRS V14 LCD variant - 3.0 - #7480
Conversation
|
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:
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (1)
📝 WalkthroughWalkthroughAdds HelloRadioSky V14LCD as a supported EdgeTX board, including hardware mappings, companion metadata, firmware registration, build tooling, target-specific HAL/LCD settings, USB identity, and CI build targets. ChangesHelloRadioSky V14LCD target support
Estimated code review effort: 3 (Moderate) | ~25 minutes Sequence Diagram(s)sequenceDiagram
participant BuildWorkflow
participant FirmwareBuilder
participant CMake
participant TaranisHAL
participant USBDescriptor
BuildWorkflow->>FirmwareBuilder: selects v14lcd
FirmwareBuilder->>CMake: configures PCB=X7 and PCBREV=V14LCD
CMake->>TaranisHAL: selects FLAVOUR=v14lcd and RADIO_V14LCD
TaranisHAL->>USBDescriptor: selects V14LCD device identity
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 |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 `@tools/generate-hw-defs.sh`:
- Line 12: The FLAVOR variable assignment on line 12 uses unquoted parameter
expansion which can trigger glob expansion if FLAVOR is externally set with
wildcard characters. Add double quotes around the entire parameter expansion in
the FLAVOR assignment, changing ${FLAVOR:=...} to "${FLAVOR:=...}" to prevent
the shell from interpreting glob characters in the default value list.
🪄 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: 898acf04-b42b-43ca-a5f7-5fc43216f238
📒 Files selected for processing (13)
fw.jsonradio/src/boards/hw_defs/v14lcd.jsonradio/src/gui/navigation/navigation.hradio/src/targets/taranis/CMakeLists.txtradio/src/targets/taranis/hal.hradio/src/targets/taranis/lcd_driver_spi.cppradio/src/targets/taranis/usb_descriptor.hradio/util/build-firmware.pyradio/util/hw_defs/legacy_names.pytools/boards.pytools/build-common.shtools/build-wasm-modules.shtools/generate-hw-defs.sh
|
How is it intended? The V14 LCD FW here (v14-1f865aa) is still only for OLED, not LCD, Have tested! |
Necessary bits to actually build the firmware, and companion side support are missing and will be added shortly. |
3199bab to
371ccc1
Compare
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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/targets/taranis/hal.h`:
- Line 43: Update the ROTARY_ENCODER_POSITION macro in hal.h to wrap the entire
shifted-and-masked expression in outer parentheses, ensuring comparisons and
other surrounding operators evaluate the macro as a single value.
🪄 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: affc841c-5406-449e-affd-8fed4761260f
📒 Files selected for processing (16)
companion/src/firmwares/boards.cppcompanion/src/firmwares/boards.hcompanion/src/firmwares/opentx/opentxinterface.cppfw.jsonradio/src/boards/hw_defs/v14lcd.jsonradio/src/gui/navigation/navigation.hradio/src/targets/taranis/CMakeLists.txtradio/src/targets/taranis/hal.hradio/src/targets/taranis/lcd_driver_spi.cppradio/src/targets/taranis/usb_descriptor.hradio/util/build-firmware.pyradio/util/hw_defs/legacy_names.pytools/boards.pytools/build-common.shtools/build-wasm-modules.shtools/generate-hw-defs.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/boards.py
- tools/build-common.sh
- radio/util/build-firmware.py
- fw.json
- tools/build-wasm-modules.sh
- radio/src/targets/taranis/usb_descriptor.h
- radio/src/gui/navigation/navigation.h
- radio/src/targets/taranis/lcd_driver_spi.cpp
There was a problem hiding this comment.
Caution
Inline review comments failed to post. This is likely due to GitHub's internal server error or limits when posting large numbers of comments. If you are seeing this consistently it is likely a permissions issue. Please check "Moderation" -> "Code review limits" under your organization settings.
Actionable comments posted: 1
🤖 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/targets/taranis/hal.h`:
- Line 43: Update the ROTARY_ENCODER_POSITION macro in hal.h to wrap the entire
shifted-and-masked expression in outer parentheses, ensuring comparisons and
other surrounding operators evaluate the macro as a single value.
🪄 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: affc841c-5406-449e-affd-8fed4761260f
📒 Files selected for processing (16)
companion/src/firmwares/boards.cppcompanion/src/firmwares/boards.hcompanion/src/firmwares/opentx/opentxinterface.cppfw.jsonradio/src/boards/hw_defs/v14lcd.jsonradio/src/gui/navigation/navigation.hradio/src/targets/taranis/CMakeLists.txtradio/src/targets/taranis/hal.hradio/src/targets/taranis/lcd_driver_spi.cppradio/src/targets/taranis/usb_descriptor.hradio/util/build-firmware.pyradio/util/hw_defs/legacy_names.pytools/boards.pytools/build-common.shtools/build-wasm-modules.shtools/generate-hw-defs.sh
🚧 Files skipped from review as they are similar to previous changes (8)
- tools/boards.py
- tools/build-common.sh
- radio/util/build-firmware.py
- fw.json
- tools/build-wasm-modules.sh
- radio/src/targets/taranis/usb_descriptor.h
- radio/src/gui/navigation/navigation.h
- radio/src/targets/taranis/lcd_driver_spi.cpp
🛑 Comments failed to post (1)
radio/src/targets/taranis/hal.h (1)
43-43: 🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash # Description: Search for remaining function-like calls to ROTARY_ENCODER_POSITION. rg -n 'ROTARY_ENCODER_POSITION\s*\('Repository: EdgeTX/edgetx
Length of output: 151
🏁 Script executed:
#!/bin/bash set -euo pipefail printf '--- hal.h excerpt ---\n' sed -n '35,50p' radio/src/targets/taranis/hal.h printf '\n--- usages of ROTARY_ENCODER_POSITION ---\n' rg -n '\bROTARY_ENCODER_POSITION\b' radio/src -g '!**/*build*' -g '!**/*dist*'Repository: EdgeTX/edgetx
Length of output: 242
🏁 Script executed:
#!/bin/bash set -euo pipefail echo '--- hal.h excerpt ---' sed -n '35,50p' radio/src/targets/taranis/hal.h echo echo '--- usages of ROTARY_ENCODER_POSITION ---' rg -n '\bROTARY_ENCODER_POSITION\b' radio/srcRepository: EdgeTX/edgetx
Length of output: 1961
Wrap
ROTARY_ENCODER_POSITIONin outer parenthesesradio/src/targets/taranis/hal.h:43— the macro can bind incorrectly in expressions likeROTARY_ENCODER_POSITION == 1; use((ROTARY_ENCODER_GPIO->IDR >> 12) & 0x03).🤖 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/taranis/hal.h` at line 43, Update the ROTARY_ENCODER_POSITION macro in hal.h to wrap the entire shifted-and-masked expression in outer parentheses, ensuring comparisons and other surrounding operators evaluate the macro as a single value.
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 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 @.github/workflows/build_fw.yml:
- Line 90: Update the firmware build group at .github/workflows/build_fw.yml
lines 90-90 to retain v12 while including v14lcd alongside v14 and v16; make the
same change to the WASM module group at .github/workflows/companion.yml lines
56-56, preserving CI coverage for v12 in both workflows.
🪄 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: 7bc825bf-4949-4d5e-b59f-dd46378a6671
📒 Files selected for processing (3)
.github/workflows/build_fw.yml.github/workflows/companion.yml.github/workflows/nightly.yml
Wire up BOARD_HELLORADIOSKY_V14LCD in companion (board enum, FourCC, EEPROM/flash size, board name, default internal module, battery range, and firmware registration) so Companion can load the V14LCD hardware definition and target it as a firmware/simulator option, mirroring the existing V14 board wiring. Reference: EdgeTX#7477
c265037 to
5a003c4
Compare
Thanks, let's hope it's ready soon; I've been waiting seven months for the relevant firmware for my V14 LCD now... |
Just merged into the main line, still waiting for buddy support |
|
Automatically available with the nightly after merge... https://buddy.edgetx.org/#/flash?version=nightly&source=releases&filters=includePrereleases&target=v14lcd 2.12 version will be in 2.12.3 |
4 participants
added V14LCD target
Summary by CodeRabbit
New Features
Hardware Configuration
Documentation