Phase 1 — DeviceConfig hierarchy (device_config.py): - 17 @dataclass(frozen=True) subclasses (WLEDConfig, AdalightConfig, …) sharing BaseDeviceConfig; DeviceConfig = Union[all 17] - Device.to_config() in device_store.py: single flat→typed dispatch point Phase 2+3 — Typed provider signatures + call-site migration: - ProviderDeps(device_store) frozen dataclass in led_client.py - LEDDeviceProvider.create_client(config, *, deps) abstract signature - create_led_client(config, *, deps) factory dispatches via config.device_type - All 17 providers narrowed to their specific config type; drop kwargs.get() - GroupLEDClient.connect() uses device.to_config() + create_led_client() - wled_target_processor: replaced 21-field DeviceInfo unpacking with to_config() + dataclasses.replace(config, use_ddp=…) for DDP override - device_test_mode: build typed config via to_config() + ProviderDeps - Deleted DeviceInfo dataclass, _get_device_info(), _DEVICE_FIELD_DEFAULTS - TargetContext: replaced get_device_info callback with is_test_mode_active Phase 4 — Test migration: - 47-case test suite in tests/core/devices/test_device_config.py (100% coverage) - test_group_device.py TestGroupLEDClient migrated to GroupConfig + ProviderDeps - Removed legacy keyword-arg init path from GroupLEDClient
14 KiB
Refactor Plan: Per-Provider Typed Device Configs
Status: Planned, not started.
Target branch: refactor/device-typed-configs
Intended executor: Sonnet agent (one phase per invocation; human review between phases).
Goal
Replace the flat DeviceInfo dataclass (and the **kwargs-based LEDDeviceProvider.create_client(url, **kwargs) contract) with a discriminated union of per-provider config dataclasses. Each provider owns its config type and reads typed fields instead of guessing kwargs.
Motivation
Current pain points:
- server/src/ledgrab/core/processing/wled_target_processor.py unpacks ~21 fields by hand into
create_led_client(**kwargs). - Every provider's
create_clientstarts withkwargs.get("x", default)— no type safety, no IDE hints, no way to know at a glance which fields a provider actually uses. - Adding a new per-device-type field requires threading it through
Device→DeviceInfo→_DEVICE_FIELD_DEFAULTS→ call-site unpacking → kwargs bag → provider. - Fields leak across device types (a WLED device carries
ble_govee_key=""at runtime for no reason).
Scope guardrails
- Storage schema (SQLite) unchanged. Columns stay, dead-for-this-type fields stay, no destructive migration.
- Frontend HTML/TS unchanged in phases 1-4. It already branches on
device_typewith show/hide logic. Frontend changes are deferred to Phase 5. - API schemas are last. Phase 5 converts
DeviceCreate/DeviceUpdate/DeviceResponseto a Pydantic v2 discriminated union. This is the only breaking external change and can be deferred indefinitely if needed.
Phase 1 — Config hierarchy (foundation, non-breaking)
Create
File: server/src/ledgrab/core/devices/device_config.py
Pattern:
from dataclasses import dataclass
from typing import List, Literal, Optional, Union
@dataclass(frozen=True)
class BaseDeviceConfig:
device_id: str
device_url: str
led_count: int
software_brightness: int = 255
test_mode_active: bool = False
auto_shutdown: bool = False
rgbw: bool = False
@dataclass(frozen=True)
class WLEDConfig(BaseDeviceConfig):
device_type: Literal["wled"] = "wled"
use_ddp: bool = False
# ... one @dataclass(frozen=True) per provider
Config field inventory
Base: device_id, device_url, led_count, software_brightness, test_mode_active, auto_shutdown, rgbw.
| Config | Extra fields beyond Base |
|---|---|
| WLEDConfig | use_ddp: bool = False |
| AdalightConfig | baud_rate: Optional[int] = None |
| AmbiLEDConfig | baud_rate: Optional[int] = None |
| DMXConfig | dmx_protocol, dmx_start_universe, dmx_start_channel |
| ESPNowConfig | baud_rate, espnow_peer_mac, espnow_channel |
| HueConfig | hue_username, hue_client_key, hue_entertainment_group_id |
| SPIConfig | spi_speed_hz, spi_led_type |
| ChromaConfig | chroma_device_type |
| GameSenseConfig | gamesense_device_type |
| BLEConfig | ble_family, ble_govee_key |
| GroupConfig | group_mode, group_device_ids (no device_store here — see Phase 2) |
| OpenRGBConfig | zone_mode |
| MockConfig | send_latency_ms: int = 0 |
| DemoConfig | send_latency_ms: int = 0 |
| MQTTConfig | (none) |
| WSConfig | (none) |
| USBHIDConfig | (none — hid_usage_page is parsed from the URL, not config) |
DeviceConfig = Union[
WLEDConfig, AdalightConfig, AmbiLEDConfig, DMXConfig, ESPNowConfig,
HueConfig, SPIConfig, ChromaConfig, GameSenseConfig, BLEConfig,
GroupConfig, MQTTConfig, WSConfig, USBHIDConfig, OpenRGBConfig,
MockConfig, DemoConfig,
]
Add
Device.to_config() -> DeviceConfig in server/src/ledgrab/storage/device_store.py (around lines 14-97 where Device lives).
- Dispatches on
self.device_type. - Constructs the right subclass, pulling only relevant columns.
- Ignores columns that don't apply to the type.
- This is the only place that knows the flat→typed mapping.
Do NOT touch in Phase 1
- Provider signatures (still
create_client(self, url, **kwargs)). create_led_clientfactory.- Any call site.
DeviceInfoitself.
Acceptance
- New unit test
server/tests/core/devices/test_device_config.py:- For each provider, build a
Devicewith thatdevice_type, callto_config(), assert right subclass and right fields. - Edge case: extra/irrelevant Device fields must not leak into the wrong config type.
- For each provider, build a
cd server && ruff check src/ tests/ --fix— green.cd server && py -3.13 -m pytest tests/ --no-cov -q— green (existing tests untouched, new test passes).cd server && npx tsc --noEmit— green (no TS impact this phase, just a sanity check).
Phase 2 + Phase 3 — Provider API migration + call-site migration (single PR)
These must land in one commit because the provider signature change would otherwise break the 3 call sites immediately.
Change the abstract base
server/src/ledgrab/core/devices/led_client.py:
class LEDDeviceProvider(ABC):
@abstractmethod
def create_client(self, config: DeviceConfig, *, deps: ProviderDeps) -> LEDClient: ...
ProviderDeps is a tiny new dataclass:
@dataclass(frozen=True)
class ProviderDeps:
device_store: "DeviceStore"
# Add future cross-cutting runtime deps here (http_client, etc.)
create_led_client:
def create_led_client(config: DeviceConfig, *, deps: ProviderDeps) -> LEDClient:
return get_provider(config.device_type).create_client(config, deps=deps)
Update every provider (17 files)
- Narrow signature per provider: e.g.
WLEDDeviceProvider.create_client(self, config: WLEDConfig, *, deps: ProviderDeps). - Drop all
kwargs.get("x")lookups — read typed fields directly. - Providers that don't need
depsjust ignore it. - GroupDeviceProvider is the only current consumer of
deps: readsdeps.device_store.
Call sites (3)
- server/src/ledgrab/core/processing/wled_target_processor.py lines ~120-148 — the 21-field unpacking. Replace with:
config = device.to_config() self._led_client = create_led_client(config, deps=self._provider_deps)self._provider_depsis plumbed in fromProcessorManagerwhen the target processor is constructed. - server/src/ledgrab/core/processing/device_test_mode.py lines 72-78 — minimal test-mode client. Build a synthetic config via a helper
_minimal_config_for_test_mode(device)(keeps justdevice_id,device_url,led_count,baud_rate) and pass it. - server/src/ledgrab/core/devices/group_client.py lines 47-70 — child client construction inside the group. Same pattern:
child_config = child_device.to_config(); passdepsthrough.
Delete
DeviceInfodataclass in server/src/ledgrab/core/processing/target_processor.py lines 71-109.ProcessorManager._get_device_info()and_DEVICE_FIELD_DEFAULTSin server/src/ledgrab/core/processing/processor_manager.py lines 230-275 —Device.to_config()subsumes this. Verify no other callers viaast-index usages "_get_device_info".
Acceptance
ast-index search "device_info\."— no hits in non-test code.ast-index search "DeviceInfo"— no hits outside archival comments.cd server && py -3.13 -m pytest tests/ --no-cov -q— all tests pass.- Manual smoke: start server, create a WLED device, start processing, verify LEDs update (or mock output shows frames).
cd server && ruff check src/ tests/ --fix— green.
Phase 4 — Test migration
Update these files:
server/tests/storage/test_device_store.py— addto_config()cases per device type.server/tests/api/routes/test_devices_routes.py— should be mostly untouched (API schemas still flat until Phase 5).server/tests/e2e/test_device_flow.py— update internal assertions only if they touchDeviceInfodirectly.server/tests/test_group_device.py— construct child clients withGroupConfig.- Any fixture helper that builds a fake
DeviceInfo— migrate to the right*Configsubclass.
Acceptance
cd server && py -3.13 -m pytest tests/ --no-cov -q— all green.- Coverage of
device_config.pyandDevice.to_config()≥ 90%.
Phase 5 — API discriminated union (OPTIONAL, separate PR)
Do not start until Phases 1-4 are merged and stable. Flag this to the human before beginning. This is the only phase with an externally breaking change.
Backend
server/src/ledgrab/api/schemas/devices.py — replace flat DeviceCreate/DeviceUpdate with Pydantic v2 tagged unions:
class WLEDDeviceCreate(BaseModel):
device_type: Literal["wled"]
name: str
url: str
led_count: int
use_ddp: bool = False
# ... base fields only
DeviceCreate = Annotated[
Union[WLEDDeviceCreate, AdalightDeviceCreate, ...],
Field(discriminator="device_type"),
]
Add model_config = ConfigDict(extra="ignore") on each union member for one release cycle so existing clients (frontend, HAOS integration, curl scripts) that send extra fields don't 422 immediately. Add a deprecation note and tighten to extra="forbid" in a follow-up.
Frontend
- server/src/ledgrab/static/js/features/devices.ts and related — when building the POST/PATCH body, scope the payload to the selected
device_typeusing the show/hide knowledge already indevice-discovery.ts. - No plain
<select>elements — any new pickers use IconSelect or EntitySelect (see root CLAUDE.md UI rules).
Tests
- Update
test_devices_routes.pyto assert discriminated union rejection of mismatched shapes. - Add round-trip tests: create device of each type via API → fetch → compare fields.
Acceptance
cd server && py -3.13 -m pytest tests/ --no-cov -q— green.cd server && npx tsc --noEmit && npm run build— green.- Manual smoke for at least 3 device types (WLED, DMX, Hue) — create, edit, delete via UI.
- HAOS integration still works against the server (spot-check; not automated).
Conventions the implementing agent must follow
- Project task tracker is
TODO.md— check the "Refactor: Per-Provider Device Configs" section, tick boxes as phases land. Do not use theTodoWritetool. - Auto-restart after Python changes. See contexts/server-operations.md.
- No commits without explicit user approval. Present each phase's diff for review first.
- Pre-commit gate every phase:
cd server && ruff check src/ tests/ --fixcd server && py -3.13 -m pytest tests/ --no-cov -q- Phase 5 additionally:
cd server && npx tsc --noEmit && npm run build
- No plain
<select>— Phase 5 uses IconSelect / EntitySelect. - Android parity: if you add any new runtime dep to
server/pyproject.toml, updateandroid/app/build.gradle.ktsper the root CLAUDE.md "Android Dependency Sync" section. This refactor should not need any new deps. - Data migration policy: storage schema is unchanged, so no JSON-file migration is needed. But if you rename any serialized field during
to_dict/from_dict, add migration logic per the root CLAUDE.md "Data Migration Policy" section. - Use
ast-indexfor code search (ast-index search,ast-index usages,ast-index callers,ast-index class). Fall back to Grep only for regex/string-literal/comment searches. - Never run
cdin Bash. Use absolute paths or the project-relativecd server && <cmd>idiom (one-shot, same invocation).
Known risks
- Frozen dataclass + inheritance + defaults — Python's
@dataclass(frozen=True)with inheritance requires every subclass field to have a default if any parent field does. Base has defaulted fields. Verify in Phase 1. If it breaks, usekw_only=True(Python 3.10+). use_ddporigin — currently inferred fromself._protocol == "ddp"at the call site, not from Device storage. Options: add a column (schema change, more work), or keep inference logic insideDevice.to_config()(recommended — no schema change). Prefer the latter.- Test-mode minimal client (device_test_mode.py lines 72-78) may not have all
BaseDeviceConfigfields available. Build a synthetic config via a named helper; do not leak the hack intoDevice.to_config(). - Group
device_storeimport cycle —GroupConfigmust not holddevice_store(would pull storage into the config module).ProviderDepsis the deliberate cut. - BLE optional import —
BLEDeviceProvideris conditionally registered (see led_client.py lines 321-330). EnsureBLEConfigstill imports cleanly even whenbleakis absent — putBLEConfigindevice_config.py(not inble_provider.py) so it's always importable.
Deliverables per phase
- Branch:
refactor/device-typed-configs. - One commit per phase, conventional-commit messages:
refactor(devices): phase 1 — add DeviceConfig hierarchyrefactor(devices): phases 2+3 — typed provider signatures + call-site migrationrefactor(devices): phase 4 — test migration to typed configsrefactor(devices): phase 5 — API discriminated union(separate PR)
- Phase-by-phase diffs presented for user review before each commit.
- Final PR body linking all phases, with manual test plan per device type touched.