fix(dev-preview): address code review issues

- 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 <noreply@anthropic.com>
This commit is contained in:
Chuck
2026-02-23 17:36:04 -05:00
parent 2bff30038e
commit b5c5431e85
6 changed files with 32 additions and 68 deletions

View File

@@ -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

View File

@@ -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}")

View File

@@ -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)."""

View File

@@ -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

View File

@@ -85,11 +85,16 @@ const PluginInstallManager = {
* @returns {Promise<Array>} 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++) {

View File

@@ -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) {