-
Notifications
You must be signed in to change notification settings - Fork 21
Optimized command.py #11
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: main
Are you sure you want to change the base?
Changes from all commits
a328145
e671c67
7850726
b31fde2
7d35f8a
adee71c
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -1,98 +1,170 @@ | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import eel | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import threading | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import pyttsx3 | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import speech_recognition as sr | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.config import (SPEECH_LANGUAGE, SPEECH_PAUSE_THRESHOLD, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SPEECH_PHRASE_TIMEOUT, SPEECH_TIMEOUT, TTS_ENGINE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TTS_RATE, TTS_VOICE_ID) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def speak(text): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| text = str(text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine = pyttsx3.init(TTS_ENGINE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| voices = engine.getProperty("voices") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| # print(voices) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine.setProperty("voice", voices[TTS_VOICE_ID].id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eel.DisplayMessage(text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine.say(text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine.runAndWait() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| engine.setProperty("rate", TTS_RATE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eel.receiverText(text) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| import eel | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from typing import Optional | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.feature import openCommand, findContact, whatsApp, PlayYoutube, chatBot | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| from backend.config import ( | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| TTS_VOICE_ID, TTS_RATE, TTS_VOLUME, TTS_ENGINE, | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| SPEECH_LANGUAGE, SPEECH_TIMEOUT, SPEECH_PHRASE_TIMEOUT, SPEECH_PAUSE_THRESHOLD | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| ) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _engine: Optional[pyttsx3.Engine] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _engine_lock = threading.Lock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _recognizer: Optional[sr.Recognizer] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _rec_lock = threading.Lock() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _microphone: Optional[sr.Microphone] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _calibrated = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _cached_energy_threshold: Optional[float] = None | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _CALL_KEYS = ("send message", "video call", "call") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _YT_KEY = "on youtube" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _OPEN_KEY = "open" | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _get_engine() -> pyttsx3.Engine: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| global _engine | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with _engine_lock: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if _engine is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eng = pyttsx3.init(TTS_ENGINE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| voices = eng.getProperty("voices") | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if 0 <= TTS_VOICE_ID < len(voices): | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eng.setProperty("voice", voices[TTS_VOICE_ID].id) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
Comment on lines
+35
to
+36
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eng.setProperty("rate", TTS_RATE) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| eng.setProperty("volume", TTS_VOLUME) | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _engine = eng | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _engine | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| def _get_recognizer() -> sr.Recognizer: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| global _recognizer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| with _rec_lock: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| if _recognizer is None: | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| r = sr.Recognizer() | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| r.pause_threshold = SPEECH_PAUSE_THRESHOLD | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| r.dynamic_energy_threshold = False | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| _recognizer = r | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _recognizer | ||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
|
||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||||
| return _recognizer | |
| return _recognizer |
Copilot
AI
Oct 21, 2025
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.
The microphone singleton initialization is not protected by a lock, which could lead to a race condition if multiple threads call _get_microphone() simultaneously. Add a lock similar to _engine_lock and _rec_lock to ensure thread-safe initialization.
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.
@copilot open a new pull request to apply changes based on this feedback
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.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Jan 2, 2026
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.
Trailing whitespace after the microphone initialization. This should be removed for code cleanliness.
| _microphone = sr.Microphone() | |
| _microphone = sr.Microphone() |
Copilot
AI
Jan 1, 2026
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.
The microphone singleton lacks thread-safety protection. Unlike the engine and recognizer which use locks, the microphone is created without synchronization. If multiple threads call _get_microphone() simultaneously, race conditions could lead to multiple microphone instances being created, defeating the purpose of the singleton pattern and potentially causing resource issues.
Copilot
AI
Jan 2, 2026
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.
The microphone singleton is not thread-safe. The check for _microphone is None and the assignment happen outside any lock, which can lead to race conditions where multiple threads create multiple instances. This undermines the singleton pattern and the optimization goals of this PR. Add thread-safety using a lock similar to the engine and recognizer implementations.
vannu07 marked this conversation as resolved.
Show resolved
Hide resolved
Copilot
AI
Oct 21, 2025
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.
The calibration logic has a race condition where multiple threads could enter the if not _calibrated: block simultaneously before _calibrated is set to True. This should use a lock or implement double-checked locking similar to the singleton patterns used elsewhere.
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.
@copilot open a new pull request to apply changes based on this feedback
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.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Jan 1, 2026
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.
The calibration logic has thread-safety issues. The global variables _calibrated and _cached_energy_threshold are accessed and modified without lock protection. If multiple threads call takecommand() simultaneously, they could all see _calibrated as False and perform calibration concurrently, or one thread might set _calibrated = True while another is still in the middle of calibration, leading to an incomplete cached threshold being used.
Copilot
AI
Jan 2, 2026
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.
Holding the engine lock during the entire TTS operation (eng.say() and eng.runAndWait()) blocks all other threads from using TTS, even though the engine might support concurrent access for reads. This could create a bottleneck where multiple speak() calls from different threads are serialized. Consider if a finer-grained locking strategy would be appropriate, or document that TTS operations are intentionally serialized.
| with _engine_lock: | |
| eng.say(s) | |
| eng.runAndWait() | |
| eng.say(s) | |
| eng.runAndWait() |
Copilot
AI
Jan 1, 2026
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.
The TTS engine usage has a potential thread-safety issue. While the engine is retrieved with proper locking, the actual eng.say() and eng.runAndWait() calls lock the entire operation. However, pyttsx3 engines are generally not thread-safe for concurrent speech operations. If multiple threads call speak() simultaneously, they will queue up at the lock, but the engine's internal state might not handle rapid sequential calls correctly. Consider whether the engine needs to be recreated per call or if a queue-based approach would be more robust.
| """TTS + Eel UI; reuses engine; thread-safe.""" | |
| s = str(text) | |
| eel.DisplayMessage(s) | |
| eel.receiverText(s) | |
| try: | |
| eng = _get_engine() | |
| with _engine_lock: | |
| eng.say(s) | |
| eng.runAndWait() | |
| except Exception as e: | |
| print(f"TTS error: {e}") | |
| """TTS + Eel UI; use a fresh TTS engine per call to avoid cross-thread state issues.""" | |
| s = str(text) | |
| eel.DisplayMessage(s) | |
| eel.receiverText(s) | |
| eng: Optional[pyttsx3.Engine] = None | |
| try: | |
| eng = pyttsx3.init(TTS_ENGINE) | |
| voices = eng.getProperty("voices") | |
| if 0 <= TTS_VOICE_ID < len(voices): | |
| eng.setProperty("voice", voices[TTS_VOICE_ID].id) | |
| eng.setProperty("rate", TTS_RATE) | |
| eng.setProperty("volume", TTS_VOLUME) | |
| eng.say(s) | |
| eng.runAndWait() | |
| except Exception as e: | |
| print(f"TTS error: {e}") | |
| finally: | |
| if eng is not None: | |
| try: | |
| eng.stop() | |
| except Exception: | |
| pass |
Copilot
AI
Jan 1, 2026
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.
The error handling swallows specific exception information. Using a bare except Exception as e without logging the exception type or traceback makes debugging difficult. Consider using print(f"Listen error: {type(e).__name__}: {e}") or proper logging to preserve more diagnostic information.
Copilot
AI
Jan 2, 2026
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.
The function returns None when query is empty after stripping (line 113: "return normalized or None"). However, the check at line 151 is "if not query", which would be True for both None and empty string. This means the empty string case won't trigger the "No command was given" message, creating inconsistent behavior. The original code always returned query.lower() from takecommand, never None on empty strings.
| return normalized or None | |
| return normalized |
Copilot
AI
Jan 1, 2026
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.
In the message send flow, if takecommand() returns None (due to an error or timeout), the code correctly handles it by speaking "I didn't catch the message." However, this creates a dead-end in the conversation flow. The user would have to restart the entire process. Consider whether this should return to allow the user to retry, or if the function should loop to give the user another chance to provide the message.
| speak("What message to send?") | |
| msg = takecommand() | |
| if msg: | |
| whatsApp(Phone, msg, "message", name) | |
| else: | |
| speak("I didn't catch the message.") | |
| max_retries = 3 | |
| for attempt in range(max_retries): | |
| speak("What message to send?") | |
| msg = takecommand() | |
| if msg: | |
| whatsApp(Phone, msg, "message", name) | |
| break | |
| else: | |
| speak("I didn't catch the message.") | |
| if attempt < max_retries - 1: | |
| speak("Please say the message again.") | |
| else: | |
| speak("I couldn't get the message. Let's try again later.") |
Copilot
AI
Jan 2, 2026
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.
When takecommand is called recursively for "send message" (line 130), if the user's message is not recognized (msg is None), the code speaks "I didn't catch the message" but still returns from the handler without completing the WhatsApp message flow. The original code would have passed the None/failed query to whatsApp(). Verify that this change in behavior is intentional and that the whatsApp function can handle this properly.
| speak("I didn't catch the message.") | |
| speak("I didn't catch the message.") | |
| # Maintain original behavior: still pass the (possibly None) msg to whatsApp | |
| whatsApp(Phone, msg, "message", name) |
Copilot
AI
Jan 1, 2026
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.
The variable name Phone uses PascalCase which is unconventional for Python variable names. According to PEP 8, variable names should use snake_case (e.g., phone). While this preserves the original code's naming, it would be better to use consistent naming conventions throughout the refactoring.
| Phone, name = findContact(query) | |
| if Phone != 0: | |
| if "send message" in query: | |
| speak("What message to send?") | |
| msg = takecommand() | |
| if msg: | |
| whatsApp(Phone, msg, "message", name) | |
| else: | |
| speak("I didn't catch the message.") | |
| elif "call" in query and "video call" not in query: | |
| whatsApp(Phone, query, "call", name) | |
| else: | |
| whatsApp(Phone, query, "video call", name) | |
| phone, name = findContact(query) | |
| if phone != 0: | |
| if "send message" in query: | |
| speak("What message to send?") | |
| msg = takecommand() | |
| if msg: | |
| whatsApp(phone, msg, "message", name) | |
| else: | |
| speak("I didn't catch the message.") | |
| elif "call" in query and "video call" not in query: | |
| whatsApp(phone, query, "call", name) | |
| else: | |
| whatsApp(phone, query, "video call", name) |
Copilot
AI
Jan 1, 2026
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.
The logic for determining call type has a subtle issue. The condition "call" in query and "video call" not in query (line 138) could fail for queries like "call video call" or similar edge cases where both substrings appear. While unlikely in practice, the order of checks means "call" would match first if "send message" isn't present. Consider restructuring to check for "video call" first (most specific), then "send message", then plain "call" to avoid ambiguity.
Copilot
AI
Jan 2, 2026
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.
When a voice command is not recognized (takecommand returns None), the function now speaks "No command was given" which is different from the original behavior that silently returned. This could be annoying to users if recognition failures are frequent (e.g., due to background noise). Consider whether this change in user experience is intentional.
Copilot
AI
Oct 21, 2025
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.
The logic duplicates the speak('No command was given.') call that appears on line 170. Consider removing this redundant error handling since the same condition is handled in the else block below.
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.
@copilot open a new pull request to apply changes based on this feedback
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.
@copilot open a new pull request to apply changes based on this feedback
Copilot
AI
Jan 2, 2026
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.
The text input path doesn't call speak(query) like the voice input path does at line 111. This creates inconsistent behavior where voice commands are echoed back via TTS but text commands are not. If this inconsistency is intentional, it should be documented; otherwise, consider whether text input should also trigger TTS echo.
| eel.senderText(q) | |
| eel.senderText(q) | |
| speak(q) |
Copilot
AI
Jan 1, 2026
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.
The condition if query: on line 166 has a logical issue. After the changes:
- If
message is Noneandtakecommand()returns None or empty string (after strip), we return early at line 155-157 - If
message is Noneand query is truthy, we reach line 166 with a valid query - If
message is not None, query is set tostr(message).strip().lower()on line 163, which could be an empty string
This means when a text message is passed but it's empty/whitespace, we'll reach line 166 with an empty query string. The check correctly handles this, but the else clause speaking "No command was given." creates inconsistency: voice commands get this message AND return early, while empty text messages get this message AND continue to the finally block. Consider making behavior consistent.
Copilot
AI
Jan 2, 2026
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.
The condition at line 162 checks "if query" after the try block starts, but query is always truthy at this point. In the voice input path, line 151 returns early if not query, and in the text input path, query is assigned from a message that was provided. This check is redundant and the else block at line 165 is unreachable, making this condition unnecessary.
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.
Import of 'time' is not used.