Skip to content
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

Added a possibility to select which speaker device to use #96

Open
wants to merge 1 commit into
base: main
Choose a base branch
from
Open
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
17 changes: 17 additions & 0 deletions glados.py
Original file line number Diff line number Diff line change
Expand Up @@ -72,6 +72,22 @@ def from_yaml(cls, path: str, key_to_config: Sequence[str] | None = ("Glados",))

return cls(**config)

def configure_io(path: str, key: str = "IO"):
data = None

try:
# First attempt with UTF-8
with open(path, "r", encoding="utf-8") as file:
data = yaml.safe_load(file).get(key, None)
except UnicodeDecodeError:
# Fall back to utf-8-sig if UTF-8 fails (handles BOM)
with open(path, "r", encoding="utf-8-sig") as file:
data = yaml.safe_load(file).get(key, None)

if data is not None:
if "speaker_device" in data.keys():
logger.debug(f"Setting speaker device to {data['speaker_device']}")
sd.default.device = data["speaker_device"]
Comment on lines +75 to +90
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Add error handling and input validation.

The function needs improvements in several areas:

  1. Add error handling for file not found and invalid YAML
  2. Validate the speaker device ID before setting it
  3. Add type hints for parameters

Apply this diff to improve the function:

-def configure_io(path: str, key: str = "IO"):
+def configure_io(path: str, key: str = "IO") -> None:
+    """Configure IO devices from YAML configuration file.
+    
+    Args:
+        path: Path to the YAML configuration file
+        key: Key in the YAML file containing IO configuration (default: "IO")
+        
+    Raises:
+        FileNotFoundError: If the configuration file is not found
+        yaml.YAMLError: If the configuration file is invalid
+        ValueError: If the speaker device ID is invalid
+    """
     data = None
     
     try:
         # First attempt with UTF-8
         with open(path, "r", encoding="utf-8") as file:
             data = yaml.safe_load(file).get(key, None)
     except UnicodeDecodeError:
         # Fall back to utf-8-sig if UTF-8 fails (handles BOM)
         with open(path, "r", encoding="utf-8-sig") as file:
             data = yaml.safe_load(file).get(key, None)
+    except FileNotFoundError:
+        logger.error(f"Configuration file not found: {path}")
+        raise
+    except yaml.YAMLError as e:
+        logger.error(f"Invalid YAML in configuration file: {e}")
+        raise
     
-    if data is not None:
-        if "speaker_device" in data.keys():
-            logger.debug(f"Setting speaker device to {data['speaker_device']}")
-            sd.default.device = data["speaker_device"]
+    if data is not None and "speaker_device" in data:
+        speaker_device = data["speaker_device"]
+        try:
+            # Query available devices to validate the speaker device ID
+            devices = sd.query_devices()
+            if not any(d["index"] == speaker_device for d in devices):
+                raise ValueError(f"Invalid speaker device ID: {speaker_device}")
+            
+            logger.debug(f"Setting speaker device to {speaker_device}")
+            sd.default.device = speaker_device
+        except sd.PortAudioError as e:
+            logger.error(f"Error accessing audio devices: {e}")
+            raise ValueError(f"Error accessing audio devices: {e}") from e
📝 Committable suggestion

‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
def configure_io(path: str, key: str = "IO"):
data = None
try:
# First attempt with UTF-8
with open(path, "r", encoding="utf-8") as file:
data = yaml.safe_load(file).get(key, None)
except UnicodeDecodeError:
# Fall back to utf-8-sig if UTF-8 fails (handles BOM)
with open(path, "r", encoding="utf-8-sig") as file:
data = yaml.safe_load(file).get(key, None)
if data is not None:
if "speaker_device" in data.keys():
logger.debug(f"Setting speaker device to {data['speaker_device']}")
sd.default.device = data["speaker_device"]
def configure_io(path: str, key: str = "IO") -> None:
"""Configure IO devices from YAML configuration file.
Args:
path: Path to the YAML configuration file
key: Key in the YAML file containing IO configuration (default: "IO")
Raises:
FileNotFoundError: If the configuration file is not found
yaml.YAMLError: If the configuration file is invalid
ValueError: If the speaker device ID is invalid
"""
data = None
try:
# First attempt with UTF-8
with open(path, "r", encoding="utf-8") as file:
data = yaml.safe_load(file).get(key, None)
except UnicodeDecodeError:
# Fall back to utf-8-sig if UTF-8 fails (handles BOM)
with open(path, "r", encoding="utf-8-sig") as file:
data = yaml.safe_load(file).get(key, None)
except FileNotFoundError:
logger.error(f"Configuration file not found: {path}")
raise
except yaml.YAMLError as e:
logger.error(f"Invalid YAML in configuration file: {e}")
raise
if data is not None and "speaker_device" in data:
speaker_device = data["speaker_device"]
try:
# Query available devices to validate the speaker device ID
devices = sd.query_devices()
if not any(d["index"] == speaker_device for d in devices):
raise ValueError(f"Invalid speaker device ID: {speaker_device}")
logger.debug(f"Setting speaker device to {speaker_device}")
sd.default.device = speaker_device
except sd.PortAudioError as e:
logger.error(f"Error accessing audio devices: {e}")
raise ValueError(f"Error accessing audio devices: {e}") from e
🧰 Tools
🪛 Ruff (0.8.2)

87-88: Use a single if statement instead of nested if statements

Combine if statements using and

(SIM102)


88-88: Use key in dict instead of key in dict.keys()

Remove .keys()

(SIM118)


class Glados:
def __init__(
Expand Down Expand Up @@ -604,6 +620,7 @@ def _process_chunk(self, line):

def start() -> None:
"""Set up the LLM server and start GlaDOS."""
configure_io("glados_config.yml")
glados_config = GladosConfig.from_yaml("glados_config.yml")
glados = Glados.from_config(glados_config)
glados.start_listen_event_loop()
Expand Down