-
-
Notifications
You must be signed in to change notification settings - Fork 123
New UserMod: Voice Control via DF2301Q #329
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: mdev
Are you sure you want to change the base?
Conversation
📝 WalkthroughWalkthroughAdds a DF2301Q I2C driver header and a DF2301QUsermod that polls a DF2301Q voice module, maps command IDs to WLED actions, persists configuration, and wires the usermod into the build (ID, registration, build flag). Changes
Sequence Diagram(s)sequenceDiagram
participant Sys as "System"
participant UM as "DF2301QUsermod"
participant Driver as "DF2301Q Driver"
participant I2C as "I2C Bus"
participant Mod as "DF2301Q Module"
participant WLED as "WLED Core"
Sys->>UM: setup()
UM->>Driver: construct/init(addr)
UM->>Driver: detect()
Driver->>I2C: probe/ping @ addr
I2C->>Mod: I2C read/write
Mod-->>I2C: response
I2C-->>Driver: ack/result
Driver-->>UM: detection result
loop every pollInterval
UM->>Driver: poll() -> cmdID
Driver->>I2C: readReg(CMD_ID)
I2C->>Mod: read CMD register
Mod-->>I2C: cmdID
I2C-->>Driver: cmdID
Driver-->>UM: new cmdID (if any)
UM->>UM: handleVoiceCommand(cmdID)
UM->>WLED: invoke action (power/brightness/preset/effect/color)
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~45 minutes Poem
🚥 Pre-merge checks | ✅ 2 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (2 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing touches
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.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 5
🤖 Fix all issues with AI agents
In `@usermods/usermod_v2_voice_control/DF2301Q.hpp`:
- Around line 19-85: The task is being pinned to core 1 unconditionally in
DF2301Q::startTask via xTaskCreatePinnedToCore which fails on single‑core ESP32
variants; update the core selection so single‑core builds use core 0 by using
conditional compilation (check CONFIG_FREERTOS_UNICORE) and set
DF2301Q_TASK_CORE to 0 for unicore and 1 otherwise, or compute the core argument
inline in startTask before calling xTaskCreatePinnedToCore; modify the symbol
DF2301Q_TASK_CORE (or the call site in startTask that passes DF2301Q_TASK_CORE)
accordingly so the task is pinned to PRO_CPU (core 0) on single‑core targets and
to core 1 on multicore targets.
In `@usermods/usermod_v2_voice_control/usermod_v2_voice_control.h`:
- Around line 403-410: The local variable that stores the current effect/mode
(currently declared as uint8_t in the block handling cmdPrevEffect and used with
strip.getMainSegment().mode and strip.setMode) must be widened to uint16_t to
avoid truncation for mode IDs >255; update the declaration of mode in this
handler (and any other mode-index locals in this usermod) from uint8_t to
uint16_t, keep palette-related types as uint8_t, and ensure the wrap logic uses
strip.getModeCount() unchanged while still calling
strip.setMode(strip.getMainSegmentId(), ...), and leave
stateUpdated(CALL_MODE_BUTTON) and cmdID/cmdPrevEffect checks as-is.
- Around line 394-400: The branch handling cmdPrevPreset can underflow when
currentPreset is 0; update the conditional in the cmdPrevPreset block (where
cmdPrevPreset, cmdNextPreset, cmdID and applyPreset are used) to prevent calling
applyPreset(currentPreset - 1, CALL_MODE_BUTTON) with currentPreset == 0 —
either skip the action when currentPreset is 0 or compute a safe previous index
(e.g., wrap to last preset by using (currentPreset == 0 ? maxPresetIndex :
currentPreset - 1)) before calling applyPreset and keep the USER_PRINTLN message
unchanged.
- Around line 255-307: The volume range is inconsistent with DF2301Q (should be
1–7): update the dropdown generation in the UI (the SET_F("for(var
i=0;i<=20;i++)addOption(dd,i,i);") call created by addDropdown for 'Volume') to
only emit options 1 through 7, change the default moduleVolume value to a valid
in-range default (replace the current 10 with a value between 1 and 7), and
tighten the bounds in readFromConfig by changing the constrain(moduleVolume, 0,
20) call to constrain(moduleVolume, 1, 7) so moduleVolume is always clamped to
the hardware-supported range. Ensure all references to moduleVolume (including
addToConfig/readFromConfig) rely on that corrected range.
In `@wled00/usermods_list.cpp`:
- Around line 209-212: Remove the redundant self-define inside the conditional
block: in the section guarded by `#ifdef` USERMOD_VOICE_CONTROL, delete the line
that redefines USERMOD_VOICE_CONTROL and leave only the `#include` of
"../usermods/usermod_v2_voice_control/usermod_v2_voice_control.h" (keep the
surrounding `#ifdef/`#endif intact) so that external -DUSERMOD_VOICE_CONTROL=1
values are not clobbered and `#if` USERMOD_VOICE_CONTROL checks continue to work.
🧹 Nitpick comments (1)
usermods/usermod_v2_voice_control/DF2301Q.hpp (1)
151-192: Protect Wire I2C access from concurrent background task execution.The
readReg()andwriteReg()methods at lines 173–191 are called from the background FreeRTOS task (taskFunction), but multiple other usermods also access Wire from the main loop without mutual exclusion. Wrap these methods withesp32SemTake()andesp32SemGive()(or create a dedicated I2C semaphore) to prevent concurrent transactions that corrupt the bus.
…ED by default, but this will do it as usermods are callled one by one.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actionable comments posted: 1
🤖 Fix all issues with AI agents
In `@usermods/usermod_v2_voice_control/usermod_v2_voice_control.h`:
- Around line 96-116: When DF2301Q detection fails in setup(), do not delete the
allocated voiceModule or set it to nullptr; remove the delete voiceModule and
voiceModule = nullptr statements in the else branch so the DF2301Q instance
remains allocated (voiceModule, DF2301Q::detect()) and the existing recovery
logic in loop() can retry initialization; optionally keep the USER_PRINTLN log
but leave the object in place to allow hot‑plug reconnection.
… removed from the else branch. Now when initial detection fails in setup(), the DF2301Q instance remains allocated, allowing the recovery logic in loop() to retry detection every 5 seconds for hot-plug reconnection.
The DF2301Q is a very inexpensive offline voice control board that you can also train for a custom wake-phrase as well as 17 other custom commands - and it understands over 120 generic commands without training.
It takes almost zero resources as everything is done on the board.
Product link: https://www.dfrobot.com/product-2665.html
List of the pre-trained phrases
To keep the data small, I only included a subset of the words it can recognize that are related to lighting or similar things.
Summary by CodeRabbit
New Features
Chores
✏️ Tip: You can customize this high-level summary in your review settings.