fix:add hotwords kwarg #154
Conversation
handle mic hickups, docstrs say they may return None but we throw an exception when that happens
WalkthroughThe changes in this pull request enhance the Changes
Possibly related PRs
Suggested reviewers
Poem
Thank you for using CodeRabbit. We offer it for free to the OSS community and would appreciate your support in helping us grow. If you find it useful, would you consider giving us a shout-out on your favorite social media? 🪧 TipsChatThere are 3 ways to chat with CodeRabbit:
Note: Be mindful of the bot's finite context window. It's strongly recommended to break down tasks such as reading entire modules into smaller chunks. For a focused discussion, use review comments to chat about specific files and their changes, instead of using the PR comments. CodeRabbit Commands (Invoked using PR comments)
Other keywords and placeholders
CodeRabbit Configuration File (
|
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 3
🧹 Outside diff range and nitpick comments (3)
ovos_dinkum_listener/voice_loop/hotwords.py (2)
100-102: LGTM! Consider adding parameter documentation.The new parameters
reload_allowedandautoloadenhance control over hotword engine lifecycle, which could help prevent microphone interruptions. The implementation looks correct.Consider adding docstring documentation for the new parameters:
def __init__(self, bus=FakeBus(), expected_duration=3, sample_rate=16000, sample_width=2, reload_allowed=True, autoload=False): """ @param reload_allowed: If False, prevents reloading of already loaded hotword engines @param autoload: If True, automatically loads hotword engines during initialization """Also applies to: 110-111
Line range hint
279-293: Consider enhancing microphone data handling.While the changes help prevent unnecessary reloads that could cause mic interruptions, the
updatemethod could be enhanced to handle None chunks from the microphone.Consider adding explicit None handling:
def update(self, chunk: bytes): """ Update appropriate engines based on self.state @param chunk: bytes of audio to feed to hotword engines """ + if chunk is None: + LOG.debug("Received None chunk from microphone, skipping update") + return self.audio_buffer.append(chunk) if self.state == HotwordState.LISTEN: engines = self.listen_words.values()🧰 Tools
🪛 Ruff
99-99: Do not perform function call
FakeBusin argument defaults; instead, perform the call within the function, or read the default from a module-level singleton variable(B008)
ovos_dinkum_listener/voice_loop/voice_loop.py (1)
210-212: Consider keeping the warning with DEBUG level.The change from assertion to conditional check improves robustness by handling
Nonechunks gracefully. However, completely removing the warning message reduces observability of microphone issues.if chunk is None: - #LOG.warning("No audio from microphone") + LOG.debug("No audio from microphone") continue
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (3)
- ovos_dinkum_listener/service.py (7 hunks)
- ovos_dinkum_listener/voice_loop/hotwords.py (1 hunks)
- ovos_dinkum_listener/voice_loop/voice_loop.py (1 hunks)
🔇 Additional comments (7)
ovos_dinkum_listener/voice_loop/voice_loop.py (1)
210-212: LGTM! The changes improve robustness.The replacement of the assertion with a conditional check successfully addresses the microphone interruption issue while maintaining the integrity of the voice loop state machine.
ovos_dinkum_listener/service.py (6)
157-158: Addition ofmicandbusparameters enhances configurabilityThe inclusion of optional
micandbusparameters in the constructor allows for more flexible instantiation ofOVOSDinkumVoiceService, facilitating easier testing and integration with custom microphone and bus instances.
163-163: Addition ofhotwordsparameter improves hotword managementIntroducing the optional
hotwordsparameter enables custom hotword configurations through theHotwordContainerinstance, enhancing control over the hotword engine's lifecycle.
200-201: Initializehotwordsandvadwith provided instances or defaultsThe code correctly initializes
self.hotwordsandself.vadusing the provided instances or defaults, ensuring modularity and allowing for custom implementations if needed.
202-203: Wrap non-streaming STT instances withFakeStreamingSTTWrapping non-streaming STT instances with
FakeStreamingSTTensures compatibility by providing a uniform streaming interface within the voice loop.
207-207: Verify the logic ofdisable_hotword_reloadSetting
self.disable_hotword_reloadtohotwords is not Nonemeans that hotword reloading is disabled when ahotwordsinstance is provided. Please confirm that this behavior aligns with the intended functionality.
1140-1140: Conditional hotword reload respectsdisable_hotword_reloadflagThe conditional check ensures that hotwords are reloaded only when
disable_hotword_reloadisFalseand the hotword configuration has changed, preventing unnecessary reloads.
handle mic hickups, docstrs say they may return None but we throw an exception when that happens
Summary by CodeRabbit
New Features
Bug Fixes
Documentation