mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-05 14:55:08 +00:00
refactor: delete dead Python code in the core (and stop storing Wi-Fi passwords) (#608)
* refactor(plugins): remove the no-op PluginHealthMonitor Its monitor loop did nothing (`if callbacks: pass`), register_health_check had no callers and api_v3.health_monitor was never read by any route. The live health data comes from PluginHealthTracker, which is untouched. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(store): drop the never-set uninstall tombstones Nothing in production called mark_recently_uninstalled, so the reconciler's was_recently_uninstalled check was always False. The persistent uninstall registry is what actually stops resurrection; the reconciler test now exercises that gate instead. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(common): delete unused config/display/game helpers, utils and error_handler Nothing in core, the web UI, scripts or the plugin monorepo imports config_helper, display_helper, game_helper, utils or error_handler; only their own tests did. The error_handler re-exports leave src.common's __all__; APIHelper, TextHelper, ScrollHelper, LogoHelper and the adaptive layout exports are unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(config): drop ConfigService's unused versioning and save API ConfigVersion, get_version/get_version_history/get_version_config, rollback, save_config, reload, get_plugin_config and the backward-compat load_config/get_config_path/get_secrets_path had no callers. The display controller only uses get_config, subscribe, unsubscribe and shutdown, plus the file watcher. Change detection now compares against the current checksum instead of the last history entry. The subscriber tests asserted `callback.called or True`; they now reload the way the watcher does and assert the notification. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): drop unread plugin state history and callbacks plugin_state.PluginStateManager kept a bounded per-plugin transition history that only get_state_history (tests only) read; get_state_info reports a separate lifetime count, which stays. set_error_info and record_display had no callers, and set_state_with_error's `error` argument only fed the history. The web-side state_manager.PluginStateManager loses subscribe_to_state_changes, _notify_callbacks, set_plugin_error and get_state_version, none of which had callers; with no subscribers the old-state copy in update_plugin_state went with them. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): remove unused PluginManager methods and attribute guards update_all_plugins was only called by a test (the display loop uses run_scheduled_updates); get_plugin_health_metrics, get_plugin_resource_metrics and get_plugin_state had no callers; and plugin_modules was written but never read. plugin_directories is now initialised in __init__, so the hasattr() guards around it go. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(plugins): remove unused executor, loader, store and package helpers - PluginExecutor.execute_safe: no callers. - PluginLoader._parse_semver: only its own tests; compatibility.parse_semver is the live copy and test_compatibility.py already covers it. - PluginStoreManager.get_installed_plugin_info: no callers. - PluginResourceMonitor._local: never read. - src.plugin_system.get_store_manager and __api_version__: no importers in core, scripts or the plugin monorepo. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(wifi): stop storing Wi-Fi passwords in wifi_config.json WiFiManager appended every joined network's SSID and password, in plaintext, to saved_networks in config/wifi_config.json, and nothing (web UI, backup restore, scripts) ever read them back: NetworkManager keeps its own credentials. The writes are gone, and loading the config now drops any saved_networks key and rewrites the file, so passwords already on disk are scrubbed. Also removes _check_dnsmasq_conflict (never called) and _detect_trixie, whose result only reached one log line, along with the NM_CONNECTIONS_PATHS constant only it used. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(display): remove unreachable and unused DisplayController code - _follower_rebuild_scroll_image: never called. - mode_duration (never read) and last_mode_change (write-only). - The `chosen_cap <= 0` branch: chosen_cap is either the minimum of caps already filtered to > 0 or DEFAULT_DYNAMIC_DURATION_CAP (180). - The `max_duration < min_duration` branch directly after `max_duration = max(min_duration, max_duration)`. - The circuit-breaker branch's `display_result = False` and `manager_to_display = None`: the first is overwritten a few lines later, the second is already None there. - The bool-to-bool conversion of execute_display's result, which is always a bool. - The `loaded_plugins` lookup in _update_modules: PluginManager has no such attribute, so it always fell through to `plugins`. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(vegas): remove unused config update, boundary finder and refresh VegasModeConfig.update had no callers outside its own tests (the coordinator rebuilds the config with from_config on a change); geometry.find_item_boundary and StreamManager._refresh_plugin_content had no callers at all. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * refactor(run): drop the debug block that pretended to import the plugin system In debug mode run.py put src/plugin_system itself on sys.path and printed "Plugin system import successful" without importing anything. Nothing imports plugin_system modules by bare name, so the path entry did nothing either. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: delete tests that test nothing - test/plugins/test_{basketball_scoreboard,calendar,clock_simple, odds_ticker,soccer_scoreboard,text_display}.py skip everywhere the named plugins are not installed, including CI (LEDMATRIX_PLUGINS_DIR holds only the fixture plugin); test_plugin_matrix.py already covers every discovered plugin. Their PluginTestBase and the fixtures only it used (plugins_dir, mock_display_manager, mock_cache_manager, mock_plugin_manager, base_plugin_config in test/plugins/conftest.py) go with them. - test_plugin_system.py: test_discover_plugins (body was `pass`) and test_dependency_check (a comment), plus the test_plugin_manager fixture only the former requested. - test_display_manager.py: test_draw_image asserted that an image it had just assigned was not None. - test_display_controller.py: the rotation and schedule-override tests re-implemented the run-loop arithmetic inline and asserted on their own result without calling the controller. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * test: expect one plugin_last_update success stamp after update_all_plugins EveryStampRecordsACompletion required at least two success-path stamps; the second was update_all_plugins, removed as test-only. The worker and synchronous paths share the remaining stamp in _execute_update_now, and the check that every stamp calls _note_update_completed is unchanged. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> --------- Co-authored-by: Claude Opus 5.5 <noreply@anthropic.com>
This commit is contained in:
Chuck
+8
-105
@@ -81,12 +81,6 @@ DEFAULT_AP_CHANNEL = 7
|
||||
# LED status message file (for display_controller integration)
|
||||
LED_STATUS_FILE = None # Will be set dynamically
|
||||
|
||||
# NetworkManager connection file locations (Trixie uses /run, Bookworm uses /etc)
|
||||
NM_CONNECTIONS_PATHS = [
|
||||
Path("/etc/NetworkManager/system-connections"),
|
||||
Path("/run/NetworkManager/system-connections"), # Trixie with Netplan
|
||||
]
|
||||
|
||||
|
||||
@dataclass
|
||||
class WiFiNetwork:
|
||||
@@ -140,9 +134,6 @@ class WiFiManager:
|
||||
# Discover WiFi interface (don't hardcode wlan0)
|
||||
self._wifi_interface = self._discover_wifi_interface()
|
||||
|
||||
# Detect if we're running on Trixie (Netplan-based NetworkManager)
|
||||
self._is_trixie = self._detect_trixie()
|
||||
|
||||
# Initialize disconnected check counter for grace period
|
||||
# This prevents AP mode from enabling on transient network hiccups
|
||||
self._disconnected_checks = 0
|
||||
@@ -155,7 +146,7 @@ class WiFiManager:
|
||||
|
||||
logger.info(f"WiFi Manager initialized - nmcli: {self.has_nmcli}, iwlist: {self.has_iwlist}, "
|
||||
f"hostapd: {self.has_hostapd}, dnsmasq: {self.has_dnsmasq}, "
|
||||
f"interface: {self._wifi_interface}, trixie: {self._is_trixie}")
|
||||
f"interface: {self._wifi_interface}")
|
||||
|
||||
# Once per process: remove a stale force-AP flag left by a prior crash.
|
||||
# Guard with a class-level flag so the nmcli AP-state check only runs
|
||||
@@ -301,44 +292,6 @@ class WiFiManager:
|
||||
logger.warning("Could not discover WiFi interface, defaulting to wlan0")
|
||||
return "wlan0"
|
||||
|
||||
def _detect_trixie(self) -> bool:
|
||||
"""
|
||||
Detect if running on Raspberry Pi OS Trixie (Debian 13).
|
||||
|
||||
Trixie uses Netplan with NetworkManager, which changes behavior:
|
||||
- Connection files are stored in /run/NetworkManager/system-connections
|
||||
- nmcli hotspot requires different handling
|
||||
- PMF (Protected Management Frames) may need to be disabled
|
||||
"""
|
||||
try:
|
||||
# Check for Netplan (primary indicator of Trixie)
|
||||
netplan_path = Path("/etc/netplan")
|
||||
if netplan_path.exists() and any(netplan_path.glob("*.yaml")):
|
||||
logger.debug("Detected Trixie: Netplan configuration found")
|
||||
return True
|
||||
|
||||
# Check Debian version
|
||||
os_release = Path("/etc/os-release")
|
||||
if os_release.exists():
|
||||
content = os_release.read_text()
|
||||
if 'VERSION_CODENAME=trixie' in content or 'VERSION_ID="13"' in content:
|
||||
logger.debug("Detected Trixie: os-release indicates Debian 13")
|
||||
return True
|
||||
|
||||
# Check if NM connections are in /run (Trixie behavior)
|
||||
# NM_CONNECTIONS_PATHS[0] = /etc/..., NM_CONNECTIONS_PATHS[1] = /run/...
|
||||
etc_nm_path = NM_CONNECTIONS_PATHS[0] # Bookworm location
|
||||
run_nm_path = NM_CONNECTIONS_PATHS[1] # Trixie location
|
||||
if run_nm_path.exists() and any(run_nm_path.glob("*.nmconnection")):
|
||||
if not etc_nm_path.exists() or not any(etc_nm_path.glob("*.nmconnection")):
|
||||
logger.debug("Detected Trixie: NM connections in /run only")
|
||||
return True
|
||||
|
||||
except (OSError, PermissionError) as e:
|
||||
logger.debug(f"Could not detect Trixie: {e}")
|
||||
|
||||
return False
|
||||
|
||||
def _load_config(self):
|
||||
"""Load WiFi configuration from file"""
|
||||
if self.config_path.exists():
|
||||
@@ -353,8 +306,7 @@ class WiFiManager:
|
||||
self.config = {
|
||||
"ap_ssid": DEFAULT_AP_SSID,
|
||||
"ap_channel": DEFAULT_AP_CHANNEL,
|
||||
"auto_enable_ap_mode": True, # Default: auto-enable when no network (safe due to grace period)
|
||||
"saved_networks": []
|
||||
"auto_enable_ap_mode": True # Default: auto-enable when no network (safe due to grace period)
|
||||
}
|
||||
self._save_config()
|
||||
|
||||
@@ -362,6 +314,12 @@ class WiFiManager:
|
||||
if "auto_enable_ap_mode" not in self.config:
|
||||
self.config["auto_enable_ap_mode"] = True # Default: auto-enable when no network (safe due to grace period)
|
||||
self._save_config()
|
||||
|
||||
# Older versions stored every joined network's password here in
|
||||
# plaintext and never read it back; scrub it from existing files.
|
||||
if "saved_networks" in self.config:
|
||||
del self.config["saved_networks"]
|
||||
self._save_config()
|
||||
|
||||
def _save_config(self):
|
||||
"""Save WiFi configuration to file"""
|
||||
@@ -1621,9 +1579,6 @@ class WiFiManager:
|
||||
break
|
||||
|
||||
if connected:
|
||||
# Save network to config
|
||||
self._save_network(ssid, password)
|
||||
|
||||
ip = status.ip_address or "Unknown"
|
||||
self._show_led_message(f"Connected! {ip}", duration=5)
|
||||
logger.info(f"Successfully connected to {ssid} with IP {ip}")
|
||||
@@ -1635,7 +1590,6 @@ class WiFiManager:
|
||||
|
||||
# No existing connection or activation failed, create new connection
|
||||
logger.info(f"Creating new connection for {ssid}...")
|
||||
self._save_network(ssid, password)
|
||||
|
||||
# Connect using nmcli
|
||||
if password:
|
||||
@@ -1765,8 +1719,6 @@ class WiFiManager:
|
||||
def _connect_wpa_supplicant(self, ssid: str, password: str) -> Tuple[bool, str]:
|
||||
"""Connect using wpa_supplicant (fallback)"""
|
||||
try:
|
||||
self._save_network(ssid, password)
|
||||
|
||||
# This would require modifying /etc/wpa_supplicant/wpa_supplicant.conf
|
||||
# For now, return not implemented
|
||||
return False, "wpa_supplicant connection not yet implemented. Please use NetworkManager (nmcli)."
|
||||
@@ -1854,23 +1806,6 @@ class WiFiManager:
|
||||
logger.error(f"Error disconnecting from WiFi: {e}")
|
||||
return False, str(e)
|
||||
|
||||
def _save_network(self, ssid: str, password: str):
|
||||
"""Save network credentials to config"""
|
||||
# Remove existing entry for this SSID
|
||||
self.config["saved_networks"] = [
|
||||
n for n in self.config["saved_networks"]
|
||||
if n.get("ssid") != ssid
|
||||
]
|
||||
|
||||
# Add new entry
|
||||
self.config["saved_networks"].append({
|
||||
"ssid": ssid,
|
||||
"password": password,
|
||||
"saved_at": time.time()
|
||||
})
|
||||
|
||||
self._save_config()
|
||||
|
||||
def _ensure_wifi_radio_enabled(self, max_retries: int = 3) -> bool:
|
||||
"""
|
||||
Ensure WiFi radio is enabled (not soft-blocked) with retry logic and verification.
|
||||
@@ -2592,38 +2527,6 @@ ignore_broadcast_ssid=0
|
||||
logger.error(f"Error creating hostapd config: {e}")
|
||||
raise
|
||||
|
||||
def _check_dnsmasq_conflict(self) -> Tuple[bool, str]:
|
||||
"""
|
||||
Check if dnsmasq is already in use for other purposes (e.g., Pi-hole).
|
||||
|
||||
Returns:
|
||||
Tuple of (conflict_detected, description)
|
||||
"""
|
||||
try:
|
||||
# Check if dnsmasq service is active
|
||||
result = subprocess.run(
|
||||
["systemctl", "is-active", "dnsmasq"],
|
||||
capture_output=True,
|
||||
text=True,
|
||||
timeout=5
|
||||
)
|
||||
if result.stdout.strip() == "active":
|
||||
# Check if it's configured for something other than our AP
|
||||
if DNSMASQ_CONFIG_PATH.exists():
|
||||
try:
|
||||
content = DNSMASQ_CONFIG_PATH.read_text()
|
||||
# Check for Pi-hole or other common dnsmasq uses
|
||||
if 'pihole' in content.lower() or 'pi-hole' in content.lower():
|
||||
return True, "Pi-hole detected - dnsmasq is in use"
|
||||
if 'server=' in content and self._wifi_interface not in content:
|
||||
return True, "dnsmasq appears to be configured for DNS forwarding"
|
||||
except (OSError, PermissionError):
|
||||
pass
|
||||
|
||||
return False, ""
|
||||
except (subprocess.TimeoutExpired, subprocess.SubprocessError):
|
||||
return False, ""
|
||||
|
||||
def _create_dnsmasq_config(self):
|
||||
"""
|
||||
Create dnsmasq drop-in configuration for captive portal DNS redirection.
|
||||
|
||||
Reference in New Issue
Block a user