From 153972fcd53365b2aa53b0acb072f35d554c7b7f Mon Sep 17 00:00:00 2001 From: "alexei.dolgolyov" Date: Fri, 13 Mar 2026 21:36:26 +0300 Subject: [PATCH] Fix device provider kwargs, camera crash guard, target API, and graph color picker - Refactor all device providers to use explicit kwargs.get() instead of fragile pop-then-passthrough (fixes WLED target start failing with unexpected dmx_protocol kwarg) - Add process-wide camera index registry to prevent concurrent opens of the same physical camera which crashes the DSHOW backend on Windows - Fix OutputTargetResponse validation error when brightness_value_source_id is None (coerce to empty string in response and from_dict) - Replace native in graph editor with the custom color picker popover used throughout the app, positioned via getScreenCTM() inside an absolute overlay on .graph-container Co-Authored-By: Claude Opus 4.6 --- .../api/routes/output_targets.py | 2 +- .../core/capture_engines/camera_engine.py | 78 ++++++++++++++++--- .../core/devices/adalight_provider.py | 12 ++- .../core/devices/ambiled_provider.py | 12 ++- .../core/devices/mock_provider.py | 7 +- .../core/devices/mqtt_provider.py | 5 +- .../core/devices/openrgb_provider.py | 10 +-- .../core/devices/wled_provider.py | 10 +-- .../core/devices/ws_provider.py | 5 +- .../static/js/core/graph-nodes.js | 62 ++++++++------- .../storage/wled_output_target.py | 2 +- 11 files changed, 135 insertions(+), 70 deletions(-) diff --git a/server/src/wled_controller/api/routes/output_targets.py b/server/src/wled_controller/api/routes/output_targets.py index 058fcdf..d83360e 100644 --- a/server/src/wled_controller/api/routes/output_targets.py +++ b/server/src/wled_controller/api/routes/output_targets.py @@ -98,7 +98,7 @@ def _target_to_response(target) -> OutputTargetResponse: target_type=target.target_type, device_id=target.device_id, color_strip_source_id=target.color_strip_source_id, - brightness_value_source_id=target.brightness_value_source_id, + brightness_value_source_id=target.brightness_value_source_id or "", fps=target.fps, keepalive_interval=target.keepalive_interval, state_check_interval=target.state_check_interval, diff --git a/server/src/wled_controller/core/capture_engines/camera_engine.py b/server/src/wled_controller/core/capture_engines/camera_engine.py index 301d316..90a9efa 100644 --- a/server/src/wled_controller/core/capture_engines/camera_engine.py +++ b/server/src/wled_controller/core/capture_engines/camera_engine.py @@ -10,8 +10,9 @@ Prerequisites (optional dependency): import platform import sys +import threading import time -from typing import Any, Dict, List, Optional +from typing import Any, Dict, List, Optional, Set import numpy as np @@ -26,6 +27,13 @@ from wled_controller.utils import get_logger logger = get_logger(__name__) _MAX_CAMERA_INDEX = 10 # probe indices 0..9 + +# Process-wide registry of cv2 camera indices currently held open. +# Prevents _enumerate_cameras from probing an in-use camera (which can +# crash the DSHOW backend on Windows) and prevents two CameraCaptureStreams +# from opening the same physical camera concurrently. +_active_cv2_indices: Set[int] = set() +_camera_lock = threading.Lock() _CV2_BACKENDS = { "auto": None, "dshow": 700, # cv2.CAP_DSHOW @@ -103,7 +111,29 @@ def _enumerate_cameras(backend_name: str = "auto") -> List[Dict[str, Any]]: cameras: List[Dict[str, Any]] = [] sequential_idx = 0 + with _camera_lock: + active = set(_active_cv2_indices) + for i in range(max_probe): + if i in active: + # Camera already held open — use cached metadata if available, + # otherwise add a placeholder so display_index mapping stays stable. + if _camera_cache is not None: + prev = [c for c in _camera_cache if c["cv2_index"] == i] + if prev: + cameras.append(prev[0]) + sequential_idx += 1 + continue + cameras.append({ + "cv2_index": i, + "name": friendly_names.get(sequential_idx, f"Camera {sequential_idx}"), + "width": 0, + "height": 0, + "fps": 30.0, + }) + sequential_idx += 1 + continue + if backend_id is not None: cap = cv2.VideoCapture(i, backend_id) else: @@ -149,6 +179,7 @@ class CameraCaptureStream(CaptureStream): def __init__(self, display_index: int, config: Dict[str, Any]): super().__init__(display_index, config) self._cap = None + self._cv2_index: Optional[int] = None def initialize(self) -> None: if self._initialized: @@ -173,18 +204,34 @@ class CameraCaptureStream(CaptureStream): camera = cameras[self.display_index] cv2_index = camera["cv2_index"] - # Open the camera - backend_id = _cv2_backend_id(backend_name) - if backend_id is not None: - self._cap = cv2.VideoCapture(cv2_index, backend_id) - else: - self._cap = cv2.VideoCapture(cv2_index) + # Prevent concurrent opens of the same physical camera (crashes DSHOW) + with _camera_lock: + if cv2_index in _active_cv2_indices: + raise RuntimeError( + f"Camera {self.display_index} (cv2 index {cv2_index}) " + f"is already in use by another stream" + ) + _active_cv2_indices.add(cv2_index) - if not self._cap.isOpened(): - raise RuntimeError( - f"Failed to open camera {self.display_index} " - f"(cv2 index {cv2_index})" - ) + try: + # Open the camera + backend_id = _cv2_backend_id(backend_name) + if backend_id is not None: + self._cap = cv2.VideoCapture(cv2_index, backend_id) + else: + self._cap = cv2.VideoCapture(cv2_index) + + if not self._cap.isOpened(): + raise RuntimeError( + f"Failed to open camera {self.display_index} " + f"(cv2 index {cv2_index})" + ) + except Exception: + with _camera_lock: + _active_cv2_indices.discard(cv2_index) + raise + + self._cv2_index = cv2_index # Apply optional resolution override res_w = self.config.get("resolution_width", 0) @@ -198,6 +245,9 @@ class CameraCaptureStream(CaptureStream): if not ret or frame is None: self._cap.release() self._cap = None + with _camera_lock: + _active_cv2_indices.discard(cv2_index) + self._cv2_index = None raise RuntimeError( f"Camera {self.display_index} opened but test read failed" ) @@ -234,6 +284,10 @@ class CameraCaptureStream(CaptureStream): if self._cap is not None: self._cap.release() self._cap = None + if self._cv2_index is not None: + with _camera_lock: + _active_cv2_indices.discard(self._cv2_index) + self._cv2_index = None self._initialized = False logger.info(f"Camera capture stream cleaned up (display={self.display_index})") diff --git a/server/src/wled_controller/core/devices/adalight_provider.py b/server/src/wled_controller/core/devices/adalight_provider.py index cbd7822..b64f0d3 100644 --- a/server/src/wled_controller/core/devices/adalight_provider.py +++ b/server/src/wled_controller/core/devices/adalight_provider.py @@ -13,10 +13,8 @@ class AdalightDeviceProvider(SerialDeviceProvider): def create_client(self, url: str, **kwargs) -> LEDClient: from wled_controller.core.devices.adalight_client import AdalightClient - - led_count = kwargs.pop("led_count", 0) - baud_rate = kwargs.pop("baud_rate", None) - kwargs.pop("use_ddp", None) # Not applicable for serial - kwargs.pop("send_latency_ms", None) - kwargs.pop("rgbw", None) - return AdalightClient(url, led_count=led_count, baud_rate=baud_rate) + return AdalightClient( + url, + led_count=kwargs.get("led_count", 0), + baud_rate=kwargs.get("baud_rate"), + ) diff --git a/server/src/wled_controller/core/devices/ambiled_provider.py b/server/src/wled_controller/core/devices/ambiled_provider.py index 0b09c09..011a1a6 100644 --- a/server/src/wled_controller/core/devices/ambiled_provider.py +++ b/server/src/wled_controller/core/devices/ambiled_provider.py @@ -13,10 +13,8 @@ class AmbiLEDDeviceProvider(SerialDeviceProvider): def create_client(self, url: str, **kwargs) -> LEDClient: from wled_controller.core.devices.ambiled_client import AmbiLEDClient - - led_count = kwargs.pop("led_count", 0) - baud_rate = kwargs.pop("baud_rate", None) - kwargs.pop("use_ddp", None) - kwargs.pop("send_latency_ms", None) - kwargs.pop("rgbw", None) - return AmbiLEDClient(url, led_count=led_count, baud_rate=baud_rate) + return AmbiLEDClient( + url, + led_count=kwargs.get("led_count", 0), + baud_rate=kwargs.get("baud_rate"), + ) diff --git a/server/src/wled_controller/core/devices/mock_provider.py b/server/src/wled_controller/core/devices/mock_provider.py index 6faf10f..5d6d337 100644 --- a/server/src/wled_controller/core/devices/mock_provider.py +++ b/server/src/wled_controller/core/devices/mock_provider.py @@ -24,8 +24,11 @@ class MockDeviceProvider(LEDDeviceProvider): return {"manual_led_count", "power_control", "brightness_control"} def create_client(self, url: str, **kwargs) -> LEDClient: - kwargs.pop("use_ddp", None) - return MockClient(url, **kwargs) + return MockClient( + url, + led_count=kwargs.get("led_count", 0), + send_latency_ms=kwargs.get("send_latency_ms", 0), + ) async def check_health(self, url: str, http_client, prev_health=None) -> DeviceHealth: return DeviceHealth(online=True, latency_ms=0.0, last_checked=datetime.now(timezone.utc)) diff --git a/server/src/wled_controller/core/devices/mqtt_provider.py b/server/src/wled_controller/core/devices/mqtt_provider.py index 46c4013..413d3d0 100644 --- a/server/src/wled_controller/core/devices/mqtt_provider.py +++ b/server/src/wled_controller/core/devices/mqtt_provider.py @@ -31,7 +31,10 @@ class MQTTDeviceProvider(LEDDeviceProvider): return {"manual_led_count"} def create_client(self, url: str, **kwargs) -> LEDClient: - return MQTTLEDClient(url, **kwargs) + return MQTTLEDClient( + url, + led_count=kwargs.get("led_count", 0), + ) async def check_health( self, url: str, http_client, prev_health=None, diff --git a/server/src/wled_controller/core/devices/openrgb_provider.py b/server/src/wled_controller/core/devices/openrgb_provider.py index ece2d28..5aed60b 100644 --- a/server/src/wled_controller/core/devices/openrgb_provider.py +++ b/server/src/wled_controller/core/devices/openrgb_provider.py @@ -30,12 +30,10 @@ class OpenRGBDeviceProvider(LEDDeviceProvider): return {"health_check", "auto_restore", "static_color"} def create_client(self, url: str, **kwargs) -> LEDClient: - zone_mode = kwargs.pop("zone_mode", "combined") - kwargs.pop("led_count", None) - kwargs.pop("baud_rate", None) - kwargs.pop("send_latency_ms", None) - kwargs.pop("rgbw", None) - return OpenRGBLEDClient(url, zone_mode=zone_mode, **kwargs) + return OpenRGBLEDClient( + url, + zone_mode=kwargs.get("zone_mode", "combined"), + ) async def check_health(self, url: str, http_client, prev_health=None) -> DeviceHealth: return await OpenRGBLEDClient.check_health(url, http_client, prev_health) diff --git a/server/src/wled_controller/core/devices/wled_provider.py b/server/src/wled_controller/core/devices/wled_provider.py index f37f225..125c976 100644 --- a/server/src/wled_controller/core/devices/wled_provider.py +++ b/server/src/wled_controller/core/devices/wled_provider.py @@ -53,12 +53,10 @@ class WLEDDeviceProvider(LEDDeviceProvider): def create_client(self, url: str, **kwargs) -> LEDClient: from wled_controller.core.devices.wled_client import WLEDClient - kwargs.pop("led_count", None) - kwargs.pop("baud_rate", None) - kwargs.pop("send_latency_ms", None) - kwargs.pop("rgbw", None) - kwargs.pop("zone_mode", None) - return WLEDClient(url, **kwargs) + return WLEDClient( + url, + use_ddp=kwargs.get("use_ddp", False), + ) async def check_health(self, url: str, http_client, prev_health=None) -> DeviceHealth: from wled_controller.core.devices.wled_client import WLEDClient diff --git a/server/src/wled_controller/core/devices/ws_provider.py b/server/src/wled_controller/core/devices/ws_provider.py index 2369fe0..51fde3b 100644 --- a/server/src/wled_controller/core/devices/ws_provider.py +++ b/server/src/wled_controller/core/devices/ws_provider.py @@ -27,7 +27,10 @@ class WSDeviceProvider(LEDDeviceProvider): return {"manual_led_count"} def create_client(self, url: str, **kwargs) -> LEDClient: - return WSLEDClient(url, **kwargs) + return WSLEDClient( + url, + led_count=kwargs.get("led_count", 0), + ) async def check_health( self, url: str, http_client, prev_health=None, diff --git a/server/src/wled_controller/static/js/core/graph-nodes.js b/server/src/wled_controller/static/js/core/graph-nodes.js index b8cb3b7..aabb6f2 100644 --- a/server/src/wled_controller/static/js/core/graph-nodes.js +++ b/server/src/wled_controller/static/js/core/graph-nodes.js @@ -4,6 +4,7 @@ import { ENTITY_COLORS, NODE_WIDTH, NODE_HEIGHT, computePorts } from './graph-layout.js'; import { EDGE_COLORS } from './graph-edges.js'; +import { createColorPicker, registerColorPicker, closeAllColorPickers } from './color-picker.js'; import * as P from './icon-paths.js'; const SVG_NS = 'http://www.w3.org/2000/svg'; @@ -143,34 +144,43 @@ function renderNode(node, callbacks) { barHit.style.cursor = 'pointer'; barHit.addEventListener('click', (e) => { e.stopPropagation(); - // Create temporary color input positioned near the click - const input = document.createElement('input'); - input.type = 'color'; - input.value = color; - input.style.position = 'fixed'; - input.style.left = e.clientX + 'px'; - input.style.top = e.clientY + 'px'; - input.style.width = '0'; - input.style.height = '0'; - input.style.padding = '0'; - input.style.border = 'none'; - input.style.opacity = '0'; - input.style.pointerEvents = 'none'; - document.body.appendChild(input); - input.addEventListener('input', () => { - const c = input.value; - bar.setAttribute('fill', c); - barCover.setAttribute('fill', c); - _saveNodeColor(id, c); + const svg = barHit.ownerSVGElement; + const container = svg?.closest('.graph-container'); + if (!svg || !container) return; + + // Remove any previous graph color picker overlay + container.querySelector('.graph-cp-overlay')?.remove(); + closeAllColorPickers(); + + // Compute position relative to container + const ctm = barHit.getScreenCTM(); + const cr = container.getBoundingClientRect(); + const px = (ctm ? ctm.e : e.clientX) - cr.left; + const py = (ctm ? ctm.f : e.clientY) - cr.top; + + // Create an HTML overlay with the custom color picker + const pickerId = `graph-node-${id}`; + const overlay = document.createElement('div'); + overlay.className = 'graph-cp-overlay'; + overlay.style.cssText = `position:absolute; left:${px}px; top:${py}px; z-index:100;`; + overlay.innerHTML = createColorPicker({ + id: pickerId, + currentColor: color, + anchor: 'left', }); - input.addEventListener('change', () => { - input.remove(); + container.appendChild(overlay); + + // Register callback to update the bar color + registerColorPicker(pickerId, (hex) => { + color = hex; + bar.setAttribute('fill', hex); + barCover.setAttribute('fill', hex); + _saveNodeColor(id, hex); + overlay.remove(); }); - // Fallback remove if user cancels - input.addEventListener('blur', () => { - setTimeout(() => input.remove(), 200); - }); - input.click(); + + // Open the popover immediately + window._cpToggle(pickerId); }); g.appendChild(barHit); diff --git a/server/src/wled_controller/storage/wled_output_target.py b/server/src/wled_controller/storage/wled_output_target.py index 77fcea0..5268f50 100644 --- a/server/src/wled_controller/storage/wled_output_target.py +++ b/server/src/wled_controller/storage/wled_output_target.py @@ -114,7 +114,7 @@ class WledOutputTarget(OutputTarget): target_type="led", device_id=data.get("device_id", ""), color_strip_source_id=data.get("color_strip_source_id", ""), - brightness_value_source_id=data.get("brightness_value_source_id", ""), + brightness_value_source_id=data.get("brightness_value_source_id") or "", fps=data.get("fps", 30), keepalive_interval=data.get("keepalive_interval", 1.0), state_check_interval=data.get("state_check_interval", DEFAULT_STATE_CHECK_INTERVAL),