mirror of
https://github.com/ChuckBuilds/LEDMatrix.git
synced 2026-10-04 06:15:09 +00:00
fix: settings the display ignored, a memory leak, and the plugin card handler (#605)
* fix(errors): stop affected_plugins growing without bound Each repeat of an error pattern appended every plugin in the time window to the pattern's list again, so a plugin failing in a loop grew the display process's memory without limit: 3,000 errors from three plugins reached 2.5 million entries. Keep the list unique. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(fonts): load a BDF font at its native size instead of PIL's default FreeType rejects any size but a BDF strike's own, and FontManager answered that with ImageFont.load_default() -- a different typeface -- so 5x7.bdf requested at 8 or 10px rendered as PIL's default font. Retry at the native strike, as element_style already does. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): plugin toggle failures no longer claim "operation in progress" Every exception in POST /plugins/toggle was mapped to PLUGIN_OPERATION_CONFLICT, so any failure told the user "A plugin operation is already in progress". Report the failure as what it is, and record the plugin id in the operation history for form posts too (it read a `data` variable that only the JSON path set). Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(web): route plugin card clicks through handlePluginAction The document-level delegation checked `typeof handlePluginAction`, which is scoped inside the plugin-manager IIFE and so never visible to it. Every card click took a copied fallback that stopped propagation (the grid's own listener never ran), confirmed an uninstall twice, and sent Starlark app uninstalls to POST /plugins/uninstall instead of DELETE /starlark/apps/<id>. Expose the handler on window and delegate to it. Also run every test/js/unit suite under pytest: they need only node, but CI ran one of the eight. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix(display): apply Rotation durations, WiFi messages and Vegas settings Three settings the web UI saves never reached the display: - Rotation & Durations: display.display_durations was never read. Every plugin inherits get_display_duration() and the plugin was asked first. A saved value now wins. The page shows unsaved screens blank with the plugin's own duration as a placeholder, and saving a blank removes the override, so one save no longer pins every screen. - WiFi status overlay: the controller looked for wifi_status.json one directory above the repo. Both sides now use wifi_manager.get_wifi_status_path(). The message is written by rename so the display never reads it half-written, and the resumed plugin redraws the whole panel afterwards. - Vegas: nothing called coordinator.update_config(), so saved Vegas settings never reached a running scroll. They are now queued when display.vegas_scroll changes, and applied while Vegas is stopped too, so a disable then re-enable works. The follower's scroll-speed default (75) now matches VegasModeConfig's (50). Also throttles Vegas's per-frame live-priority scan to 4Hz. It cost 139us per frame on a Pi 4 with two scoreboards (1.7% of a 125fps frame) and grows with each plugin. Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com> * fix: keep affected_plugins order when serialized; guard non-Element targets ErrorPattern.to_dict() ran the now-ordered list through set(), so get_error_summary() listed plugins in an unstable order. The document-level card-action listener called event.target.closest() without checking the target is an Element. 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
@@ -985,6 +985,10 @@ def save_main_config():
|
||||
mode_key = field[len('duration__'):]
|
||||
if not mode_key:
|
||||
continue
|
||||
if raw_value is None or str(raw_value).strip() == '':
|
||||
# Blank means "use the plugin's own duration".
|
||||
current_config['display']['display_durations'].pop(mode_key, None)
|
||||
continue
|
||||
try:
|
||||
int_value = int(raw_value)
|
||||
except (ValueError, TypeError, OverflowError):
|
||||
|
||||
@@ -409,6 +409,8 @@ def manage_plugin_limits(plugin_id):
|
||||
@api_v3.route('/plugins/toggle', methods=['POST'])
|
||||
def toggle_plugin():
|
||||
"""Toggle plugin enabled/disabled"""
|
||||
plugin_id = None
|
||||
enabled = None
|
||||
try:
|
||||
if not api_v3.plugin_manager or not api_v3.config_manager:
|
||||
return jsonify({'status': 'error', 'message': 'Plugin or config manager not initialized'}), 500
|
||||
@@ -506,21 +508,21 @@ def toggle_plugin():
|
||||
message=f"Plugin {plugin_id} {'enabled' if enabled else 'disabled'} successfully"
|
||||
)
|
||||
except Exception as e:
|
||||
from src.web_interface.errors import WebInterfaceError
|
||||
error = WebInterfaceError.from_exception(e, ErrorCode.PLUGIN_OPERATION_CONFLICT)
|
||||
# Not PLUGIN_OPERATION_CONFLICT: that told the user "an operation is
|
||||
# already in progress" whatever actually went wrong.
|
||||
logger.error('Error toggling plugin %s', plugin_id, exc_info=True)
|
||||
if api_v3.operation_history:
|
||||
toggle_type = "enable" if ('data' in locals() and data.get('enabled')) else "disable"
|
||||
api_v3.operation_history.record_operation(
|
||||
toggle_type,
|
||||
plugin_id=data.get('plugin_id') if 'data' in locals() else None,
|
||||
"enable" if enabled else "disable",
|
||||
plugin_id=plugin_id,
|
||||
status="failed",
|
||||
error=str(e)
|
||||
)
|
||||
action = 'enable' if enabled else 'disable' if enabled is not None else 'toggle'
|
||||
return error_response(
|
||||
error.error_code,
|
||||
error.message,
|
||||
details=error.details,
|
||||
context=error.context,
|
||||
ErrorCode.UNKNOWN_ERROR,
|
||||
f"Failed to {action} plugin {plugin_id or ''}".rstrip(),
|
||||
details=describe_exception(e),
|
||||
status_code=500
|
||||
)
|
||||
@api_v3.route('/plugins/operation/<operation_id>', methods=['GET'])
|
||||
|
||||
@@ -552,14 +552,39 @@ def _load_display_partial():
|
||||
logger.error("Error loading partial", exc_info=True)
|
||||
return "Error loading partial", 500
|
||||
|
||||
def _plugin_default_duration(plugin_id, plugin_config):
|
||||
"""Seconds a plugin shows each screen when the Rotation page sets none.
|
||||
|
||||
Mirrors BasePlugin.get_display_duration's config fallback: the plugin's
|
||||
display_duration, else its schema default, else 15.
|
||||
"""
|
||||
def _valid(v):
|
||||
return isinstance(v, (int, float)) and not isinstance(v, bool) and v > 0
|
||||
|
||||
value = plugin_config.get('display_duration')
|
||||
if not _valid(value):
|
||||
schema = None
|
||||
schema_mgr = getattr(pages_v3, 'schema_manager', None)
|
||||
if schema_mgr is not None:
|
||||
try:
|
||||
schema = schema_mgr.load_schema(plugin_id)
|
||||
except Exception:
|
||||
logger.debug("durations: no schema for %s", plugin_id, exc_info=True)
|
||||
value = plugin_config_defaults(schema if isinstance(schema, dict) else None).get(
|
||||
'display_duration')
|
||||
if not _valid(value):
|
||||
value = 15
|
||||
return int(value) if float(value).is_integer() else value
|
||||
|
||||
|
||||
def _load_durations_partial():
|
||||
"""Load rotation & durations partial.
|
||||
|
||||
Builds one duration entry per display mode of every enabled plugin
|
||||
(falling back to the display controller's 30s default), overlaid with any
|
||||
values saved in display.display_durations. Historically the template only
|
||||
looped over saved keys, and nothing ever populated them, so the page
|
||||
rendered empty.
|
||||
Builds one duration entry per display mode of every enabled plugin. A mode
|
||||
with a value saved in display.display_durations shows it; the rest are
|
||||
blank, with the plugin's own duration as the placeholder, because a saved
|
||||
value overrides the plugin (see DisplayController._get_display_duration).
|
||||
Pre-filling every mode would pin them all on the first save.
|
||||
"""
|
||||
try:
|
||||
if pages_v3.config_manager:
|
||||
@@ -578,10 +603,12 @@ def _load_durations_partial():
|
||||
continue
|
||||
modes = pages_v3.plugin_manager.get_plugin_display_modes(pid) or [pid]
|
||||
covered_keys.update(modes)
|
||||
default = _plugin_default_duration(pid, main_config.get(pid, {}) or {})
|
||||
duration_groups.append({
|
||||
'plugin_id': pid,
|
||||
'plugin_name': info.get('name') or pid,
|
||||
'modes': [{'key': m, 'value': saved.get(m, 30)} for m in modes],
|
||||
'modes': [{'key': m, 'value': saved.get(m, ''), 'default': default}
|
||||
for m in modes],
|
||||
})
|
||||
# Saved keys not owned by any enabled plugin (disabled or
|
||||
# uninstalled plugins) stay visible rather than vanishing.
|
||||
|
||||
@@ -401,124 +401,25 @@ window.__pendingInstalledPlugins = window.__pendingInstalledPlugins || null;
|
||||
window.__pendingStorePlugins = window.__pendingStorePlugins || null;
|
||||
window.__pluginDomReady = window.__pluginDomReady || false;
|
||||
|
||||
// Set up global event delegation for plugin actions (works even before plugins are loaded)
|
||||
// Document-level delegation for plugin card actions, so a card works even if
|
||||
// it was rendered before the grid's own listener was attached. It hands the
|
||||
// event to handlePluginAction, which the plugin-manager IIFE below exposes on
|
||||
// window. (It used to test `typeof handlePluginAction`, which is IIFE-scoped
|
||||
// and so never visible here: every click took a copied fallback instead, which
|
||||
// asked to confirm an uninstall twice and sent Starlark app uninstalls to the
|
||||
// plugin endpoint.)
|
||||
(function setupGlobalEventDelegation() {
|
||||
// Use document-level delegation so it works for dynamically added content
|
||||
const handleGlobalPluginAction = function(event) {
|
||||
// Only handle if it's a plugin action
|
||||
const button = event.target.closest('button[data-action][data-plugin-id]') ||
|
||||
event.target.closest('input[data-action][data-plugin-id]');
|
||||
if (!button) return;
|
||||
|
||||
const action = button.getAttribute('data-action');
|
||||
const pluginId = button.getAttribute('data-plugin-id');
|
||||
|
||||
// For toggle and configure, ensure functions are available
|
||||
if (action === 'toggle' || action === 'configure') {
|
||||
const funcName = action === 'toggle' ? 'togglePlugin' : 'configurePlugin';
|
||||
if (!window[funcName] || typeof window[funcName] !== 'function') {
|
||||
// Prevent default and stop propagation immediately to avoid double handling
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
|
||||
console.warn(`[GLOBAL DELEGATION] ${funcName} not available yet, waiting...`);
|
||||
|
||||
// Capture state synchronously from plugin data (source of truth)
|
||||
let targetChecked = false;
|
||||
if (action === 'toggle') {
|
||||
const plugin = (window.installedPlugins || []).find(p => p.id === pluginId);
|
||||
|
||||
let currentEnabled;
|
||||
if (plugin) {
|
||||
currentEnabled = Boolean(plugin.enabled);
|
||||
} else if (button.type === 'checkbox') {
|
||||
currentEnabled = button.checked;
|
||||
} else {
|
||||
currentEnabled = false;
|
||||
}
|
||||
|
||||
targetChecked = !currentEnabled; // Toggle to opposite state
|
||||
}
|
||||
|
||||
// Wait for function to be available
|
||||
let attempts = 0;
|
||||
const maxAttempts = 20; // 1 second total
|
||||
const checkInterval = setInterval(() => {
|
||||
attempts++;
|
||||
if (window[funcName] && typeof window[funcName] === 'function') {
|
||||
clearInterval(checkInterval);
|
||||
// Call the function directly
|
||||
if (action === 'toggle') {
|
||||
window.togglePlugin(pluginId, targetChecked);
|
||||
} else {
|
||||
window.configurePlugin(pluginId);
|
||||
}
|
||||
} else if (attempts >= maxAttempts) {
|
||||
clearInterval(checkInterval);
|
||||
console.error(`[GLOBAL DELEGATION] ${funcName} not available after ${maxAttempts} attempts`);
|
||||
if (typeof showNotification === 'function') {
|
||||
showNotification(`${funcName} not loaded. Please refresh the page.`, 'error');
|
||||
}
|
||||
}
|
||||
}, 50);
|
||||
return; // Don't proceed with normal handling
|
||||
}
|
||||
}
|
||||
|
||||
// Prevent default and stop propagation to avoid double handling
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
|
||||
// If handlePluginAction exists, use it; otherwise handle directly
|
||||
if (typeof handlePluginAction === 'function') {
|
||||
handlePluginAction(event);
|
||||
} else {
|
||||
// Fallback: handle directly if functions are available
|
||||
if (action === 'toggle' && window.togglePlugin) {
|
||||
// Get the current enabled state from plugin data (source of truth)
|
||||
const plugin = (window.installedPlugins || []).find(p => p.id === pluginId);
|
||||
|
||||
let currentEnabled;
|
||||
if (plugin) {
|
||||
currentEnabled = Boolean(plugin.enabled);
|
||||
} else if (button.type === 'checkbox') {
|
||||
currentEnabled = button.checked;
|
||||
} else {
|
||||
currentEnabled = false;
|
||||
}
|
||||
|
||||
// Toggle the state - we want the opposite of current state
|
||||
const isChecked = !currentEnabled;
|
||||
|
||||
// Prevent default behavior to avoid double-toggling and change event
|
||||
// (Already done at start of function, but safe to repeat)
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
|
||||
debugLog('[DEBUG toggle fallback] Plugin:', pluginId, 'Current enabled (from data):', currentEnabled, 'New state:', isChecked);
|
||||
|
||||
window.togglePlugin(pluginId, isChecked);
|
||||
} else if (action === 'configure' && window.configurePlugin) {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
window.configurePlugin(pluginId);
|
||||
} else if (action === 'update' && window.updatePlugin) {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
debugLog('[DEBUG update fallback] Updating plugin:', pluginId);
|
||||
window.updatePlugin(pluginId);
|
||||
} else if (action === 'uninstall' && window.uninstallPlugin) {
|
||||
event.preventDefault();
|
||||
event.stopPropagation();
|
||||
debugLog('[DEBUG uninstall fallback] Uninstalling plugin:', pluginId);
|
||||
if (confirm(`Are you sure you want to uninstall ${pluginId}?`)) {
|
||||
window.uninstallPlugin(pluginId);
|
||||
}
|
||||
}
|
||||
}
|
||||
const target = event.target;
|
||||
if (!target || typeof target.closest !== 'function') return;
|
||||
const el = target.closest('button[data-action][data-plugin-id]') ||
|
||||
target.closest('input[data-action][data-plugin-id]');
|
||||
if (!el || typeof window.handlePluginAction !== 'function') return;
|
||||
window.handlePluginAction(event);
|
||||
};
|
||||
|
||||
// Set up delegation on document (capture phase for better reliability)
|
||||
// Capture phase, so this runs before the grid's own listener;
|
||||
// handlePluginAction stops propagation, so an action is handled once.
|
||||
document.addEventListener('click', handleGlobalPluginAction, true);
|
||||
document.addEventListener('change', handleGlobalPluginAction, true);
|
||||
debugLog('[PLUGINS SCRIPT] Global event delegation set up');
|
||||
@@ -1846,6 +1747,7 @@ function handlePluginAction(event) {
|
||||
break;
|
||||
}
|
||||
}
|
||||
window.handlePluginAction = handlePluginAction;
|
||||
|
||||
function findInstalledPlugin(pluginId) {
|
||||
const plugins = window.installedPlugins || installedPlugins || [];
|
||||
|
||||
@@ -36,7 +36,7 @@
|
||||
<div class="bg-gray-50 rounded-lg p-4 space-y-5">
|
||||
<div>
|
||||
<h3 class="text-md font-medium text-gray-900 mb-1">Screen Durations</h3>
|
||||
<p class="text-sm text-gray-600">How long each screen stays on before rotating to the next one, in seconds (5–600, default 30).</p>
|
||||
<p class="text-sm text-gray-600">How long each screen stays on before rotating to the next one, in seconds (5–600). Leave a screen blank to use its plugin's own duration, shown greyed out.</p>
|
||||
</div>
|
||||
{% for group in duration_groups %}
|
||||
<div>
|
||||
@@ -45,12 +45,13 @@
|
||||
{% for mode in group.modes %}
|
||||
<div class="form-group" id="setting-durations-{{ mode.key }}" data-setting-key="display.display_durations.{{ mode.key }}">
|
||||
<label for="duration__{{ mode.key }}" class="block text-sm font-medium text-gray-700">
|
||||
{{ mode.key | replace('_', ' ') | title }}{{ ui.help_tip('How long the ' ~ (mode.key | replace('_', ' ')) ~ ' screen stays on before rotating to the next one, in seconds.\nRange: 5–600. Currently ' ~ mode.value ~ 's.', mode.key | replace('_', ' ') | title) }}
|
||||
{{ mode.key | replace('_', ' ') | title }}{{ ui.help_tip('How long the ' ~ (mode.key | replace('_', ' ')) ~ ' screen stays on before rotating to the next one, in seconds.\nRange: 5–600.' ~ (' Blank uses the plugin\'s own ' ~ mode.default ~ 's.' if mode.default else ''), mode.key | replace('_', ' ') | title) }}
|
||||
</label>
|
||||
<input type="number"
|
||||
id="duration__{{ mode.key }}"
|
||||
name="duration__{{ mode.key }}"
|
||||
value="{{ mode.value }}"
|
||||
{% if mode.default %}placeholder="{{ mode.default }}"{% endif %}
|
||||
min="5"
|
||||
max="600"
|
||||
class="form-control">
|
||||
|
||||
Reference in New Issue
Block a user