feat(v16): add CI1302 voice control (VGR/VFL, motion control, menu navigation) - #7501
Open
helloradiosky wants to merge 3 commits into
Open
feat(v16): add CI1302 voice control (VGR/VFL, motion control, menu navigation)#7501helloradiosky wants to merge 3 commits into
helloradiosky wants to merge 3 commits into
Conversation
|
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)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThe PR adds ChangesVoice control sensor integration
Estimated code review effort🎯 5 (Critical) | ⏱️ ~90+ minutes Suggested labels
Suggested reviewers
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 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: 5
🧹 Nitpick comments (3)
radio/src/drivers/CI1302.cpp (2)
244-252: 📐 Maintainability & Code Quality | 🔵 Trivial | ⚡ Quick winDuplicate command-byte macros.
VOICE_CMD_TELEMETRY…VOICE_CMD_SYSTEM_MENUduplicate the values already defined at Lines 59, 73, 52, 53, 54, 47, 48, 49, 50 (VOICE_Telemetry, etc.). Two parallel sets risk drifting out of sync. Consider reusing the original macros invoiceControlGuiHandleCmd.🤖 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/drivers/CI1302.cpp` around lines 244 - 252, The command-byte macros in the CI1302 driver duplicate existing values already defined by the earlier VOICE_* symbols, which creates two parallel sets that can drift out of sync. Remove the redundant VOICE_CMD_* definitions and update voiceControlGuiHandleCmd to reuse the original command constants (such as VOICE_Telemetry, VOICE_SensorStatus, VOICE_ChannelMonitor, VOICE_InternalModule, VOICE_ExternalModule, VOICE_Confirm, VOICE_Cancel, VOICE_ModelMenu, and VOICE_SystemMenu) wherever these command bytes are compared or dispatched.
167-174: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
GetVoiceInputloop is effectively anif.The
whilebody unconditionallyreturn 0after the first read, so it never iterates. If a single read is intended, anifis clearer; if draining is intended, thereturnis misplaced.🤖 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/drivers/CI1302.cpp` around lines 167 - 174, `GetVoiceInput` in `CI1302.cpp` is written as a loop but only ever performs one read because it returns immediately inside the body. Update the logic in `GetVoiceInput` to match the intended behavior: either replace the `while` with an `if` if only one `voiceGetByte` call should be attempted, or move the `return 0` outside the loop if the function is meant to keep reading until `voiceGetByte` stops producing data.radio/src/targets/horus/hal.h (1)
556-557: 📐 Maintainability & Code Quality | 🔵 Trivial | 💤 Low value
VOICE_CTRL_BAUDsits outside theRADIO_V16guard.If
VOICE_CONTROL_SENSORis ever enabled on a non-V16 target,VOICE_CTRL_BAUDis defined but the USART/GPIO/DMA macros aren't, producing confusing build breaks inCI1302.cpprather than a clear "unsupported target" failure. Consider moving it inside theRADIO_V16block (or adding an#else#error``).♻️ Move baud inside the target guard
`#if` defined(VOICE_CONTROL_SENSOR) - `#define` VOICE_CTRL_BAUD 115200 `#if` defined(RADIO_V16) + `#define` VOICE_CTRL_BAUD 115200 `#define` VOICE_CONTROL_USART UART7🤖 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/hal.h` around lines 556 - 557, `VOICE_CTRL_BAUD` is defined outside the `RADIO_V16` target guard, which can let `VOICE_CONTROL_SENSOR` compile partway on unsupported targets and fail later in `CI1302.cpp`. Move the `VOICE_CTRL_BAUD` definition into the same `RADIO_V16` guarded section in `hal.h`, or add an explicit `#else `#error`` branch so non-V16 builds fail immediately with a clear unsupported-target message.
🤖 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/audio.cpp`:
- Around line 206-208: The asset list currently uses the mixed-case name "oKay",
which can break the SIMU path’s exact f_stat lookup on case-sensitive
filesystems even though the radio FAT lookup is case-insensitive. Update the
filename entry in the relevant audio asset list used by the radio audio path to
the conventional lowercase "okay" so it matches the documented okay.wav asset
and resolves consistently.
In `@radio/src/drivers/CI1302.cpp`:
- Around line 471-474: The context check in the CI1302 driver is using a raw
ordinal value, which is brittle and unclear; update the conditional in the
relevant branch of the CI1302 handling logic to compare against
GeneralCustomFunctionsContext instead of 2. Keep the rest of the availability
behavior the same, and use the named context symbol so the intent stays clear if
enum values change.
In `@radio/src/drivers/CI1302.h`:
- Around line 25-38: Remove the per-declaration extern "C" keywords from the
voiceControlPowerOn/voiceControlPowerOff/voiceSerialPutc/voiceGetByte/GetVoiceInput/dbgGetByte/voiceSerialPutstr
declarations in CI1302.h and rely on the surrounding __cplusplus extern "C"
block for C linkage; also add the missing <stdint.h> include so the uint8_t and
uint32_t types used by these declarations are defined. Keep the existing
VoiceGearStatus, VoiceFlapStatus, and VoicePowerStatus extern declarations under
the same C-compat guard.
In `@radio/src/strhelpers.cpp`:
- Around line 805-817: The voice source handling in the strhelpers.cpp switch is
currently range-dependent, so CI1302_voiceIntegrationMixSrcName() is only
reached if the enum values for the voice sources live inside the TX-spare gap.
Verify the MixSources enum layout (especially MIXSRC_VGR, MIXSRC_VFL,
MIXSRC_LAST_GVAR, MIXSRC_TX_GPS, and MIXSRC_FIRST_TIMER) and, if these values
can move, update the switch in strhelpers.cpp so voice-name resolution is done
by explicit symbol checks rather than relying on the reserved range.
In `@radio/src/targets/horus/hal.h`:
- Around line 563-565: The V16 DMA configuration in the HORUS HAL has a stream
conflict: VOICE_CONTROL_DMA_RX_STREAM and EXTMODULE_USART_TX_DMA_STREAM both
resolve to DMA1/LL_DMA_STREAM_3 when VOICE_CONTROL_SENSOR is enabled. Update the
RADIO_V16 branch so one of these users is remapped to a different DMA1 stream or
guarded by mutually exclusive feature conditions, and keep the fix scoped to the
relevant HAL macros such as VOICE_CONTROL_DMA_RX_STREAM and the EXTMODULE USART
TX DMA mapping.
---
Nitpick comments:
In `@radio/src/drivers/CI1302.cpp`:
- Around line 244-252: The command-byte macros in the CI1302 driver duplicate
existing values already defined by the earlier VOICE_* symbols, which creates
two parallel sets that can drift out of sync. Remove the redundant VOICE_CMD_*
definitions and update voiceControlGuiHandleCmd to reuse the original command
constants (such as VOICE_Telemetry, VOICE_SensorStatus, VOICE_ChannelMonitor,
VOICE_InternalModule, VOICE_ExternalModule, VOICE_Confirm, VOICE_Cancel,
VOICE_ModelMenu, and VOICE_SystemMenu) wherever these command bytes are compared
or dispatched.
- Around line 167-174: `GetVoiceInput` in `CI1302.cpp` is written as a loop but
only ever performs one read because it returns immediately inside the body.
Update the logic in `GetVoiceInput` to match the intended behavior: either
replace the `while` with an `if` if only one `voiceGetByte` call should be
attempted, or move the `return 0` outside the loop if the function is meant to
keep reading until `voiceGetByte` stops producing data.
In `@radio/src/targets/horus/hal.h`:
- Around line 556-557: `VOICE_CTRL_BAUD` is defined outside the `RADIO_V16`
target guard, which can let `VOICE_CONTROL_SENSOR` compile partway on
unsupported targets and fail later in `CI1302.cpp`. Move the `VOICE_CTRL_BAUD`
definition into the same `RADIO_V16` guarded section in `hal.h`, or add an
explicit `#else `#error`` branch so non-V16 builds fail immediately with a clear
unsupported-target message.
🪄 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: 2617b34b-a6e2-453f-8342-0cdb4b11ea9b
📒 Files selected for processing (20)
radio/src/audio.cppradio/src/audio.hradio/src/dataconstants.hradio/src/datastructs_model.cppradio/src/drivers/CI1302.cppradio/src/drivers/CI1302.hradio/src/drivers/CI1302_voice_integration.hradio/src/edgetx.cppradio/src/edgetx.hradio/src/functions.cppradio/src/gui/colorlcd/controls/sourcechoice.cppradio/src/gui/gui_common.cppradio/src/hal/switch_driver.cppradio/src/mixer.cppradio/src/storage/yaml/yaml_datastructs_funcs.cppradio/src/strhelpers.cppradio/src/switches.cppradio/src/targets/horus/CMakeLists.txtradio/src/targets/horus/board.cppradio/src/targets/horus/hal.h
helloradiosky
changed the title
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/gui/colorlcd/LvglWrapper.cpp`:
- Around line 185-188: The voice-confirm handling in LvglWrapper::handleKeyboard
should not send LV_EVENT_CLICKED directly to the focused object, because that
bypasses the normal keypad/group Enter behavior. Update the Enter-key path so
voice confirm follows the same flow as a physical Enter press, using the
existing keyboard/indev handling logic in get_focus_obj and the surrounding
keyboard event processing instead of dispatching the click event manually.
🪄 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: 9a541dc2-d00f-40c9-b813-7083d50fbcef
📒 Files selected for processing (8)
radio/src/audio.cppradio/src/drivers/CI1302.cppradio/src/drivers/CI1302.hradio/src/drivers/CI1302_voice_integration.hradio/src/gui/colorlcd/LvglWrapper.cppradio/src/gui/colorlcd/LvglWrapper.hradio/src/strhelpers.cppradio/src/targets/horus/hal.h
🚧 Files skipped from review as they are similar to previous changes (4)
- radio/src/drivers/CI1302.h
- radio/src/drivers/CI1302_voice_integration.h
- radio/src/audio.cpp
- radio/src/drivers/CI1302.cpp
VOICE UART DRIVER main 3.0 voice control add
pfeerick
force-pushed
the
hrs-V16-Voice-control-3.0
branch
from
August 25, 2026 03:21
68a9aa8 to
d4b6866
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
2 participants
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Summary of changes:
Adds CI1302 voice-module support for HelloRadioSky V16 (
PCB=X10,PCBREV=V16).Voice recognition runs on the external CI1302 module over UART. The firmware exposes voice features through virtual switches, mixer sources, and optional menu navigation — with thin integration hooks in shared code guarded by
VOICE_CONTROL_SENSOR.Summary by CodeRabbit