-
-
Notifications
You must be signed in to change notification settings - Fork 0
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
feat!:move to simple listener #1
Conversation
be even more lightweight
WalkthroughThe changes in this pull request primarily focus on updating the Changes
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 (
|
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: 4
🧹 Outside diff range and nitpick comments (11)
requirements.txt (1)
1-9
: Consider adding version constraints for critical dependencies.To ensure stability and compatibility, consider adding version constraints for critical dependencies, particularly:
ovos-simple-listener
ovos-audio
hivemind_bus_client
(currently constrained, good!)Example:
hivemind_bus_client>=0.0.4a10 -ovos-audio -ovos-simple-listener +ovos-audio>=0.0.1 +ovos-simple-listener>=0.0.1README.md (5)
3-5
: Improve clarity of project description.The description could be clearer about the architectural tradeoffs.
-OpenVoiceOS Relay, connect to [HiveMind](https://github.com/JarbasHiveMind/HiveMind-listener) +OpenVoiceOS Relay that connects to [HiveMind](https://github.com/JarbasHiveMind/HiveMind-listener) -A lightweight version of [voice-satellite](https://github.com/JarbasHiveMind/HiveMind-voice-sat), but STT and TTS are sent to HiveMind instead of handled on device +A lightweight version of [voice-satellite](https://github.com/JarbasHiveMind/HiveMind-voice-sat) that offloads STT and TTS processing to the HiveMind server instead of handling it on the device🧰 Tools
🪛 LanguageTool
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... sent to HiveMind instead of handled on device ## Server requirements >⚠️ `hivemind...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
9-11
: Clarify server-side requirements.The warning about server requirements needs better formatting and clarity.
-> ⚠️ `hivemind-listener` is required server side, the default `hivemind-core` does not provide STT and TTS capabilities. - -> Alternatively run `hivemind-core` together with `ovos-audio` and `ovos-dinkum-listener` +> ⚠️ **Server-side Requirements:** +> - `hivemind-listener` is required (the default `hivemind-core` does not provide STT and TTS capabilities) +> - Alternatively, run `hivemind-core` together with `ovos-audio` and `ovos-dinkum-listener`🧰 Tools
🪛 LanguageTool
[formatting] ~11-~11: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...ot provide STT and TTS capabilities. > Alternatively runhivemind-core
together with `ovos...(CONJUNCTIVE_LINKING_ADVERB_COMMA_PREMIUM)
🪛 Markdownlint
10-10: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
31-31
: Fix command formatting.The pip install command should follow markdown best practices.
-$ pip install HiveMind-voice-relay +pip install HiveMind-voice-relay🧰 Tools
🪛 Markdownlint
31-31: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
53-63
: Enhance plugin documentation.The plugin section needs better organization and clarity about requirements.
-Supported plugin types: -- Microphone (required) -- VAD (required) -- WakeWord (required) -- G2P mouth movements (optional, None by default) -- Dialog Transformers (optional, None by default) -- TTS Transformers (optional, None by default) -- PHAL (optional, None by default) +### Supported Plugin Types + +#### Required Plugins +- Microphone +- VAD (Voice Activity Detection) +- WakeWord + +#### Optional Plugins +- G2P mouth movements (disabled by default) +- Dialog Transformers (disabled by default) +- TTS Transformers (disabled by default) +- PHAL (disabled by default)
24-24
: Add performance comparison section.Since the PR's objective is to be "even more lightweight", consider adding concrete performance metrics.
Would you like me to help draft a performance comparison section that includes:
- Memory usage comparison
- CPU utilization metrics
- Startup time differences
between this lightweight version and the full voice satellite?hivemind_voice_relay/__main__.py (4)
12-19
: Consider adding type hints to the connect function parameters.While the CLI parameters are well-documented, adding type hints would improve code maintainability and IDE support.
-def connect(host, key, password, port, selfsigned, siteid): +def connect(host: str, key: str, password: str, port: int, selfsigned: bool, siteid: str) -> None:
Line range hint
34-37
: Enhance error message for invalid host protocol.The error message could be more specific about the required format and provide an example.
- LOG.error("Invalid host, please specify a protocol") - LOG.error(f"ws://{host} or wss://{host}") + LOG.error(f"Invalid host protocol: {host}") + LOG.error("Host must start with 'ws://' or 'wss://'") + LOG.error("Example: ws://localhost or wss://example.com")
Line range hint
61-62
: Enhance the PHAL unavailability message.The current message could be more informative about why PHAL might be needed and how to install it.
- print("PHAL is not available") + LOG.info("OVOS-PHAL is not available. Install it with 'pip install ovos-PHAL' if you need peripheral hardware abstraction layer support.")
Line range hint
19-71
: Excellent architectural simplification!The removal of the bus daemon and standardization on FakeBus has successfully made the system more lightweight while maintaining robust functionality. The code is now more maintainable and has fewer moving parts.
hivemind_voice_relay/service.py (1)
Line range hint
137-144
: Handle potential NoneType 'message' in 'execute_tts' methodThe
message
parameter inexecute_tts
defaults toNone
, but the code callsmessage.forward(...)
without checking ifmessage
isNone
. This can raise anAttributeError
ifmessage
is not provided. Consider adding a check or providing a default message to ensure robustness.Apply this diff to fix the issue:
def execute_tts(self, utterance, ident, listen=False, message: Message = None): """Mute mic and start speaking the utterance using selected tts backend. Args: utterance: The sentence to be spoken ident: Ident tying the utterance to the source query listen: True if a user response is expected """ LOG.info("Speak: " + utterance) + if message is None: + message = Message("") # request synth in HM master side self.bus.emit(message.forward('speak:b64_audio', {"utterance": utterance, "listen": listen}))
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (5)
- README.md (2 hunks)
- hivemind_voice_relay/init.py (0 hunks)
- hivemind_voice_relay/main.py (2 hunks)
- hivemind_voice_relay/service.py (4 hunks)
- requirements.txt (1 hunks)
💤 Files with no reviewable changes (1)
- hivemind_voice_relay/init.py
🧰 Additional context used
🪛 LanguageTool
README.md
[uncategorized] ~5-~5: You might be missing the article “the” here.
Context: ... sent to HiveMind instead of handled on device ## Server requirements >⚠️ `hivemind...(AI_EN_LECTOR_MISSING_DETERMINER_THE)
[formatting] ~11-~11: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...ot provide STT and TTS capabilities. > Alternatively runhivemind-core
together with `ovos...(CONJUNCTIVE_LINKING_ADVERB_COMMA_PREMIUM)
[misspelling] ~13-~13: This word is normally spelled with a hyphen.
Context: ...penVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins This repo is bu...(EN_COMPOUNDS_FULL_FEATURED)
🪛 Markdownlint
README.md
31-31: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
10-10: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
🔇 Additional comments (4)
requirements.txt (1)
1-9
: LGTM! Changes align well with the lightweight objective.The switch to
ovos-simple-listener
and removal of heavier dependencies likeovos-dinkum-listener
,ovos-ww-plugin-vosk
, andhivemind-ggwave
successfully makes the project more lightweight while maintaining core functionality.README.md (1)
13-24
: Verify feature comparison accuracy.The feature comparison between regular voice satellite and this lightweight version needs verification.
✅ Verification successful
Feature comparison in README.md is accurate
Based on the code analysis, the README.md accurately lists the missing features in this lightweight implementation. The code shows:
- The implementation uses
ovos-simple-listener
as stated (requirements.txt)- While it includes some plugin dependencies, they are basic ones (microphone, VAD) and the code confirms the limited functionality:
- No audio transformer plugins support in the code
- No recording/sleep mode implementations
- Basic wake word support without multiple wake word capability
- STT/TTS functionality is delegated to the server via b64 encoding/decoding
The comparison between the full-featured
ovos-dinkum-listener
and this lightweight version is correctly represented in the documentation.🏁 Scripts executed
The following scripts were executed for the analysis:
Script:
#!/bin/bash # Description: Verify the listed features against both repositories # Check ovos-dinkum-listener features echo "Checking ovos-dinkum-listener features..." rg -l "class.*Plugin|def.*plugin" $(fd -t f . $(fd -t d "ovos-dinkum-listener" -d 1)) # Check ovos-simple-listener features echo "Checking ovos-simple-listener features..." rg -l "class.*Plugin|def.*plugin" $(fd -t f . $(fd -t d "ovos-simple-listener" -d 1))Length of output: 727
Script:
#!/bin/bash # Let's try a different approach by checking the dependencies and imports # Check dependencies in setup.py and requirements.txt echo "Checking dependencies..." cat setup.py requirements.txt # Check service implementation for feature support echo -e "\nChecking service implementation..." cat hivemind_voice_relay/service.py # Check imports and main functionality echo -e "\nChecking main functionality..." cat hivemind_voice_relay/__main__.pyLength of output: 12152
🧰 Tools
🪛 LanguageTool
[misspelling] ~13-~13: This word is normally spelled with a hyphen.
Context: ...penVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins This repo is bu...(EN_COMPOUNDS_FULL_FEATURED)
hivemind_voice_relay/__main__.py (2)
4-4
: LGTM! Clean import structure.The imports are well-organized and align with the simplified architecture.
Also applies to: 6-8
40-47
: Verify compatibility with version 1.0.0.The user agent version bump to 1.0.0 indicates a breaking change. Please ensure all dependent systems are prepared for this update.
be even more lightweight
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: 0
🧹 Outside diff range and nitpick comments (2)
hivemind_voice_relay/service.py (2)
Line range hint
94-126
: Consider enhancing error handling and configuration.The STT implementation could be improved in two areas:
- The 20-second timeout is hardcoded
- There's no retry mechanism for failed transcriptions
Consider applying these improvements:
class HiveMindSTT(STT): def __init__(self, bus: HiveMessageBusClient, config=None): super().__init__(config) self.bus = bus self._response = threading.Event() self._transcripts: List[Tuple[str, float]] = [] + self.timeout = config.get('timeout', 20) if config else 20 + self.max_retries = config.get('max_retries', 3) if config else 3 self.bus.on_mycroft("recognizer_loop:b64_transcribe.response", self.handle_transcripts) def execute(self, audio: AudioData, language: Optional[str] = None) -> str: - wav = audio.get_wav_data() - b64audio = base64.b64encode(wav).decode("utf-8") - m = dig_for_message() or Message("") - m = m.forward("recognizer_loop:b64_transcribe", - {"audio": b64audio, "lang": self.lang}) - self._response.clear() - self._transcripts = [] - self.bus.emit(m) - self._response.wait(20) - if self._response.is_set(): - if not self._transcripts: - LOG.error("Empty STT") - return "" - return self._transcripts[0][0] - else: - LOG.error("Timeout waiting for STT transcriptions") - return "" + retries = 0 + while retries < self.max_retries: + wav = audio.get_wav_data() + b64audio = base64.b64encode(wav).decode("utf-8") + m = dig_for_message() or Message("") + m = m.forward("recognizer_loop:b64_transcribe", + {"audio": b64audio, "lang": self.lang}) + self._response.clear() + self._transcripts = [] + self.bus.emit(m) + if self._response.wait(self.timeout): + if self._transcripts: + return self._transcripts[0][0] + LOG.warning(f"Empty STT response (attempt {retries + 1}/{self.max_retries})") + else: + LOG.warning(f"STT timeout (attempt {retries + 1}/{self.max_retries})") + retries += 1 + LOG.error("STT failed after all retries") + return ""
Line range hint
128-171
: Implement cleanup for temporary audio files.The class stores TTS audio files in
/tmp/
but doesn't implement cleanup. This could lead to disk space issues over time.Consider implementing a cleanup mechanism:
class HMPlayback(PlaybackService): + def __init__(self, bus: HiveMessageBusClient, ready_hook=on_ready, error_hook=on_error, + stopping_hook=on_stopping, alive_hook=on_alive, + started_hook=on_started, watchdog=lambda: None): + self._temp_files = set() super().__init__(ready_hook, error_hook, stopping_hook, alive_hook, started_hook, watchdog=watchdog, bus=bus, validate_source=False, disable_fallback=True) self.bus.on("speak:b64_audio.response", self.handle_tts_b64_response) self.start() + def cleanup_temp_files(self): + """Remove temporary audio files that are no longer needed.""" + for file_path in list(self._temp_files): + try: + os.remove(file_path) + self._temp_files.remove(file_path) + except OSError as e: + LOG.warning(f"Failed to remove temporary file {file_path}: {e}") def handle_tts_b64_response(self, message: Message): LOG.debug("Received TTS audio") b64data = message.data["audio"] listen = message.data.get("listen", False) utt = message.data["utterance"] tts_id = message.data.get("tts_id", "b64TTS") audio_file = f"/tmp/{hash_sentence(utt)}.wav" + self._temp_files.add(audio_file) with open(audio_file, "wb") as f: f.write(base64.b64decode(b64data)) # queue audio for playback TTS.queue.put( (audio_file, None, listen, tts_id, message) ) + # Schedule cleanup after playback + threading.Timer(60.0, self.cleanup_temp_files).start()
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (1)
- hivemind_voice_relay/service.py (4 hunks)
🔇 Additional comments (3)
hivemind_voice_relay/service.py (3)
Line range hint
49-68
: LGTM! Well-structured lifecycle logging.The callback functions provide clear and appropriate logging for different service states.
70-93
: LGTM! Well-implemented callback system with proper bus communication.The callbacks are properly implemented with appropriate event emissions and error handling. The utterance list issue from previous reviews has been addressed.
173-193
: LGTM! Clean implementation with proper configuration handling.The class properly initializes all required components and the wakeword is correctly configured through the Configuration system.
be even more lightweight
be even more lightweight
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
🧹 Outside diff range and nitpick comments (3)
README.md (3)
13-26
: Enhance feature comparison clarityThe feature comparison section effectively lists the limitations but could be more structured for better readability.
Consider organizing the missing features into categories (e.g., Core Features, Advanced Features) using a table format similar to the plugin support table below. This would make it easier for users to assess the trade-offs of using this lightweight version.
🧰 Tools
🪛 LanguageTool
[misspelling] ~13-~13: This word is normally spelled with a hyphen.
Context: ...penVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins This repo is bu...(EN_COMPOUNDS_FULL_FEATURED)
33-33
: Fix command formattingRemove the
$
prefix from the pip install command to align with markdown best practices.-$ pip install HiveMind-voice-relay +pip install HiveMind-voice-relay🧰 Tools
🪛 Markdownlint
33-33: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
13-13
: Fix hyphenation in compound adjectiveAdd a hyphen to maintain consistent technical writing style.
-The regular voice satellite is built on top of [ovos-dinkum-listener](https://github.com/OpenVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins +The regular voice satellite is built on top of [ovos-dinkum-listener](https://github.com/OpenVoiceOS/ovos-dinkum-listener) and is full-featured supporting all plugins🧰 Tools
🪛 LanguageTool
[misspelling] ~13-~13: This word is normally spelled with a hyphen.
Context: ...penVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins This repo is bu...(EN_COMPOUNDS_FULL_FEATURED)
📜 Review details
Configuration used: CodeRabbit UI
Review profile: CHILL
📒 Files selected for processing (2)
- README.md (2 hunks)
- requirements.txt (1 hunks)
🚧 Files skipped from review as they are similar to previous changes (1)
- requirements.txt
🧰 Additional context used
🪛 LanguageTool
README.md
[formatting] ~11-~11: Consider adding a comma after ‘Alternatively’ for more clarity.
Context: ...ot provide STT and TTS capabilities. > Alternatively runhivemind-core
together with `ovos...(CONJUNCTIVE_LINKING_ADVERB_COMMA_PREMIUM)
[misspelling] ~13-~13: This word is normally spelled with a hyphen.
Context: ...penVoiceOS/ovos-dinkum-listener) and is full featured supporting all plugins This repo is bu...(EN_COMPOUNDS_FULL_FEATURED)
🪛 Markdownlint
README.md
33-33: null
Dollar signs used before commands without showing output(MD014, commands-show-output)
10-10: null
Blank line inside blockquote(MD028, no-blanks-blockquote)
🔇 Additional comments (2)
README.md (2)
3-5
: LGTM: Clear architectural change descriptionThe updated introduction effectively communicates the key architectural change of offloading STT and TTS processing to HiveMind, which aligns with the PR's goal of being more lightweight.
58-71
: LGTM: Well-structured plugin support tableThe table provides a comprehensive overview of plugin support with clear required/optional indicators and documentation links. This helps users understand the system's capabilities and requirements.
be even more lightweight
Summary by CodeRabbit
Release Notes
Documentation
hivemind-listener
.pip install HiveMind-voice-relay
.New Features
HiveMindVoiceRelay
class for improved voice relay functionality.Bug Fixes
Chores
requirements.txt
to remove outdated packages and includeovos-simple-listener
.