From b5c5431e858e33dfab5b36bce0117c9756926bdc Mon Sep 17 00:00:00 2001 From: Chuck Date: Mon, 23 Feb 2026 17:36:04 -0500 Subject: [PATCH] fix(dev-preview): address code review issues MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit - Use get_logger() from src.logging_config instead of logging.getLogger() in visual_display_manager.py to match project logging conventions - Eliminate duplicate public/private weather draw methods — public draw_sun/ draw_cloud/draw_rain/draw_snow now delegate to the private _draw_* variants so plugins get consistent pixel output in tests vs production - Default install_deps=False in dev_server.py and render_plugin.py — dev scripts don't need to run pip install; developers are expected to have plugin deps installed in their venv already - Guard plugins_dir fixture against PermissionError during directory iteration - Fix PluginInstallManager.updateAll() to fall back to window.installedPlugins when PluginStateManager.installedPlugins is empty (plugins_manager.js populates window.installedPlugins independently of PluginStateManager) - Remove 5 debug console.log statements from plugins_manager.js button setup and initialization code Co-Authored-By: Claude Sonnet 4.6 --- scripts/dev_server.py | 2 +- scripts/render_plugin.py | 2 +- .../testing/visual_display_manager.py | 44 +++---------------- test/plugins/conftest.py | 17 ++++--- .../static/v3/js/plugins/install_manager.js | 13 ++++-- web_interface/static/v3/plugins_manager.js | 22 ++-------- 6 files changed, 32 insertions(+), 68 deletions(-) diff --git a/scripts/dev_server.py b/scripts/dev_server.py index 9c90bc9c..33df0a11 100644 --- a/scripts/dev_server.py +++ b/scripts/dev_server.py @@ -213,7 +213,7 @@ def api_render(): display_manager=display_manager, cache_manager=cache_manager, plugin_manager=plugin_manager, - install_deps=True, + install_deps=False, ) except Exception as e: return jsonify({'error': f'Failed to load plugin: {e}'}), 500 diff --git a/scripts/render_plugin.py b/scripts/render_plugin.py index f36a50f8..eedc6352 100644 --- a/scripts/render_plugin.py +++ b/scripts/render_plugin.py @@ -147,7 +147,7 @@ def main(): display_manager=display_manager, cache_manager=cache_manager, plugin_manager=plugin_manager, - install_deps=True, + install_deps=False, ) except Exception as e: print(f"Error loading plugin: {e}") diff --git a/src/plugin_system/testing/visual_display_manager.py b/src/plugin_system/testing/visual_display_manager.py index 2c066e93..c77cae7a 100644 --- a/src/plugin_system/testing/visual_display_manager.py +++ b/src/plugin_system/testing/visual_display_manager.py @@ -12,7 +12,6 @@ MagicMock (which tracks nothing visual), this class creates a real PIL Image canvas and draws text using the actual project fonts. """ -import logging import math import os import time @@ -21,7 +20,9 @@ from typing import Any, Dict, List, Optional, Tuple from PIL import Image, ImageDraw, ImageFont -logger = logging.getLogger(__name__) +from src.logging_config import get_logger + +logger = get_logger(__name__) class _MatrixProxy: @@ -322,50 +323,19 @@ class VisualTestDisplayManager: def draw_sun(self, x: int, y: int, size: int = 16): """Draw a sun icon using yellow circles and lines.""" - center = (x + size // 2, y + size // 2) - radius = size // 3 - self.draw.ellipse( - [center[0] - radius, center[1] - radius, - center[0] + radius, center[1] + radius], - fill=(255, 255, 0), - ) - ray_length = size // 4 - for angle in range(0, 360, 45): - rad = math.radians(angle) - start_x = center[0] + (radius * math.cos(rad)) - start_y = center[1] + (radius * math.sin(rad)) - end_x = center[0] + ((radius + ray_length) * math.cos(rad)) - end_y = center[1] + ((radius + ray_length) * math.sin(rad)) - self.draw.line([start_x, start_y, end_x, end_y], fill=(255, 255, 0), width=2) + self._draw_sun(x, y, size) def draw_cloud(self, x: int, y: int, size: int = 16, color=(200, 200, 200)): """Draw a cloud icon.""" - self.draw.ellipse([x + size // 4, y + size // 3, x + size // 4 + size // 2, y + size // 3 + size // 2], fill=color) - self.draw.ellipse([x + size // 2, y + size // 3, x + size // 2 + size // 2, y + size // 3 + size // 2], fill=color) - self.draw.ellipse([x + size // 3, y + size // 6, x + size // 3 + size // 2, y + size // 6 + size // 2], fill=color) + self._draw_cloud(x, y, size) def draw_rain(self, x: int, y: int, size: int = 16): """Draw rain icon with cloud and droplets.""" - self.draw_cloud(x, y, size) - drop_color = (0, 0, 255) - drop_size = size // 6 - for i in range(3): - drop_x = x + size // 4 + (i * size // 3) - drop_y = y + size // 2 - self.draw.line([drop_x, drop_y, drop_x, drop_y + drop_size], fill=drop_color, width=2) + self._draw_rain(x, y, size) def draw_snow(self, x: int, y: int, size: int = 16): """Draw snow icon with cloud and snowflakes.""" - self.draw_cloud(x, y, size) - snow_color = (200, 200, 255) - for i in range(3): - center_x = x + size // 4 + (i * size // 3) - center_y = y + size // 2 + size // 4 - for angle in range(0, 360, 60): - rad = math.radians(angle) - end_x = center_x + (size // 8 * math.cos(rad)) - end_y = center_y + (size // 8 * math.sin(rad)) - self.draw.line([center_x, center_y, end_x, end_y], fill=snow_color, width=1) + self._draw_snow(x, y, size) def _draw_sun(self, x: int, y: int, size: int) -> None: """Draw a sun icon with rays (internal weather icon version).""" diff --git a/test/plugins/conftest.py b/test/plugins/conftest.py index bde0ccb5..5e5a5b2f 100644 --- a/test/plugins/conftest.py +++ b/test/plugins/conftest.py @@ -30,12 +30,17 @@ def plugins_dir(): plugin_repos_path = project_root / 'plugin-repos' # Prefer plugins/ if it has actual plugin directories - if plugins_path.exists() and any( - p for p in plugins_path.iterdir() - if p.is_dir() and not p.name.startswith('.') - ): - return plugins_path - elif plugin_repos_path.exists(): + if plugins_path.exists(): + try: + has_plugins = any( + p for p in plugins_path.iterdir() + if p.is_dir() and not p.name.startswith('.') + ) + if has_plugins: + return plugins_path + except PermissionError: + pass + if plugin_repos_path.exists(): return plugin_repos_path return plugins_path diff --git a/web_interface/static/v3/js/plugins/install_manager.js b/web_interface/static/v3/js/plugins/install_manager.js index c6cb5805..1a13d05d 100644 --- a/web_interface/static/v3/js/plugins/install_manager.js +++ b/web_interface/static/v3/js/plugins/install_manager.js @@ -85,11 +85,16 @@ const PluginInstallManager = { * @returns {Promise} Update results */ async updateAll(onProgress) { - if (!window.PluginStateManager || !window.PluginStateManager.installedPlugins) { - throw new Error('Installed plugins not loaded'); - } + // Prefer PluginStateManager if populated, fall back to window.installedPlugins + // (plugins_manager.js populates window.installedPlugins independently) + const stateManagerPlugins = window.PluginStateManager && window.PluginStateManager.installedPlugins; + const plugins = (stateManagerPlugins && stateManagerPlugins.length > 0) + ? stateManagerPlugins + : (window.installedPlugins || []); - const plugins = window.PluginStateManager.installedPlugins; + if (!plugins.length) { + return []; + } const results = []; for (let i = 0; i < plugins.length; i++) { diff --git a/web_interface/static/v3/plugins_manager.js b/web_interface/static/v3/plugins_manager.js index e592076c..3d693fcd 100644 --- a/web_interface/static/v3/plugins_manager.js +++ b/web_interface/static/v3/plugins_manager.js @@ -998,26 +998,17 @@ window.initPluginsPage = function() { const onDemandForm = document.getElementById('on-demand-form'); const onDemandModal = document.getElementById('on-demand-modal'); - console.log('[initPluginsPage] Setting up button listeners:', { - refreshBtn: !!refreshBtn, - updateAllBtn: !!updateAllBtn, - restartBtn: !!restartBtn - }); - if (refreshBtn) { refreshBtn.replaceWith(refreshBtn.cloneNode(true)); document.getElementById('refresh-plugins-btn').addEventListener('click', refreshPlugins); - console.log('[initPluginsPage] Attached refreshPlugins listener'); } if (updateAllBtn) { updateAllBtn.replaceWith(updateAllBtn.cloneNode(true)); document.getElementById('update-all-plugins-btn').addEventListener('click', runUpdateAllPlugins); - console.log('[initPluginsPage] Attached runUpdateAllPlugins listener'); } if (restartBtn) { restartBtn.replaceWith(restartBtn.cloneNode(true)); document.getElementById('restart-display-btn').addEventListener('click', restartDisplay); - console.log('[initPluginsPage] Attached restartDisplay listener'); } if (searchBtn) { searchBtn.replaceWith(searchBtn.cloneNode(true)); @@ -1064,28 +1055,22 @@ window.initPluginsPage = function() { // Consolidated initialization function function initializePluginPageWhenReady() { - console.log('Checking for plugin elements...'); return window.initPluginsPage(); } // Single initialization entry point (function() { - console.log('Plugin manager script loaded, setting up initialization...'); - let initTimer = null; - + function attemptInit() { // Clear any pending timer if (initTimer) { clearTimeout(initTimer); initTimer = null; } - + // Try immediate initialization - if (initializePluginPageWhenReady()) { - console.log('Initialized immediately'); - return; - } + initializePluginPageWhenReady(); } // Strategy 1: Immediate check (for direct page loads) @@ -1683,7 +1668,6 @@ function startOnDemandStatusPolling() { window.loadOnDemandStatus = loadOnDemandStatus; function runUpdateAllPlugins() { - console.log('[runUpdateAllPlugins] Button clicked, checking for updates...'); const button = document.getElementById('update-all-plugins-btn'); if (!button) {