Skip to content
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
31 changes: 28 additions & 3 deletions src/piper_control/piper_connect.py
Original file line number Diff line number Diff line change
Expand Up @@ -6,6 +6,7 @@
from higher-level Python code.
"""

import shutil
import subprocess
import time

Expand Down Expand Up @@ -99,13 +100,37 @@ def get_can_adapter_serial(can_port: str) -> str | None:


def _check_dependencies() -> None:
# Detect package manager
def _pacman_query(pkg: str) -> list[str]:
return ["pacman", "-Q", pkg]

def _dpkg_query(pkg: str) -> list[str]:
return ["dpkg", "-s", pkg]

if shutil.which("pacman"):
query_cmd = _pacman_query
install_cmd = "sudo pacman -S"
elif shutil.which("dpkg"):
query_cmd = _dpkg_query
install_cmd = "sudo apt install"
else:
raise RuntimeError(
"Unsupported package manager. "
"Please ensure ethtool and can-utils are installed manually."
)

for pkg in ["ethtool", "can-utils"]:
try:
subprocess.run(["dpkg", "-s", pkg], check=True, stdout=subprocess.DEVNULL)
subprocess.run(
query_cmd(pkg),
check=True,
stdout=subprocess.DEVNULL,
stderr=subprocess.DEVNULL,
)
except subprocess.CalledProcessError as exc:
raise RuntimeError(
f"Missing dependency: {pkg}. Please install with `sudo apt install "
f"{pkg}`."
f"Missing dependency: {pkg}. \
Please install with `{install_cmd} {pkg}`."
) from exc
Comment on lines +124 to 134

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Unhandled FileNotFoundError from subprocess.run

subprocess.run() raises FileNotFoundError (a subclass of OSError, not CalledProcessError) when the executable itself cannot be found — e.g. if pacman or dpkg disappears between the shutil.which() call and the subprocess.run() call, or if PATH is altered by the time the process is spawned. This exception is not caught by the current except subprocess.CalledProcessError handler and will propagate as an unhandled error with a confusing message unrelated to the missing dependency.

Fix in Claude Code Fix in Codex

Comment on lines 131 to 134

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 The backslash line-continuation inside the f-string does not strip leading whitespace from the next physical line — it simply removes the newline character, leaving all the indentation spaces intact. The resulting error message will read something like "Missing dependency: ethtool. Please install with …" with a large gap of spaces between the period and "Please".

Suggested change
raise RuntimeError(
f"Missing dependency: {pkg}. Please install with `sudo apt install "
f"{pkg}`."
f"Missing dependency: {pkg}. \
Please install with `{install_cmd} {pkg}`."
) from exc
raise RuntimeError(
f"Missing dependency: {pkg}. "
f"Please install with `{install_cmd} {pkg}`."
) from exc

Fix in Claude Code Fix in Codex



Expand Down