rbrooks/Iris-WLED

Watch

Fork 0

Preset sync targets /json/presets, which no WLED firmware implements #115

New issue

Closed

opened 2026-09-04 17:57:09 +00:00 by claude-bot · 1 comment

claude-bot commented

2026-09-04 17:57:09 +00:00

Contributor

Split from #62, where it was found while capturing real controller fixtures.

preset_sync.sync_presets() reads and writes WLED presets via GET / POST /json/presets (app/services/wled_controller.py, get_presets / post_presets). That endpoint does not exist in WLED. Verified against the live roofline controller on 16.0.0:

$ curl -s -o /dev/null -w "%{http_code}" http://192.168.1.140/json/presets
501
$ curl -s http://192.168.1.140/json/presets
{"error":4}

The JSON API reference confirms it has never existed: the /json/* sub-paths are state, info, si, nodes, eff, fxdata, pal, palx, net, cfg, pins, live. Presets live on the flash filesystem and are managed through state fields (ps, psave, pdel).

So POST /api/v1/wled/sync-presets has never worked against real hardware. It passes tests because they mock WLEDController wholesale.

Why this is worse than a plain 502

sync_presets wraps the read in a bare except Exception: existing = {} (preset_sync.py:76). The 501 raises HTTPStatusError, gets swallowed, and the function proceeds to build its payload on top of an empty dict. Every preset outside the managed range — which the docstring explicitly promises to preserve — is dropped from the payload. Today the subsequent post_presets also 501s, so nothing is lost. The moment the write path is fixed without also fixing the read, a sync would silently wipe the user's presets.

What needs to happen

Read existing presets from GET /presets.json (the file, not a /json sub-path). Confirmed working on the device — returns the full preset map, with unused slots as {"stop":0} segment stubs.
Write via POST /upload (multipart, target path /presets.json), which is how the WLED app does preset backup/restore. The alternative — applying each state and issuing {"psave": n} per slot — would drive the physical lights once per preset, so it is the wrong mechanism here.
Narrow that bare except Exception. A failed read must abort the sync, never degrade to "start from empty".
Verify on hardware. This one genuinely cannot be validated by mocks — the bug exists precisely because the mocks agreed with the wrong API. Snapshot presets before, restore after.

Interim state

#62 leaves the endpoint as-is functionally but makes the failure loud rather than silent, so nobody mistakes the swallowed read for a working sync.

Note the export path is unaffected: GET /api/v1/export/presets.json builds the file server-side for manual upload and never talks to the controller.

Split from #62, where it was found while capturing real controller fixtures. `preset_sync.sync_presets()` reads and writes WLED presets via `GET` / `POST /json/presets` (`app/services/wled_controller.py`, `get_presets` / `post_presets`). **That endpoint does not exist in WLED.** Verified against the live roofline controller on 16.0.0: ``` $ curl -s -o /dev/null -w "%{http_code}" http://192.168.1.140/json/presets 501 $ curl -s http://192.168.1.140/json/presets {"error":4} ``` The [JSON API reference](https://kno.wled.ge/interfaces/json-api/) confirms it has never existed: the `/json/*` sub-paths are `state`, `info`, `si`, `nodes`, `eff`, `fxdata`, `pal`, `palx`, `net`, `cfg`, `pins`, `live`. Presets live on the flash filesystem and are managed through state fields (`ps`, `psave`, `pdel`). So `POST /api/v1/wled/sync-presets` has never worked against real hardware. It passes tests because they mock `WLEDController` wholesale. ### Why this is worse than a plain 502 `sync_presets` wraps the read in a bare `except Exception: existing = {}` (`preset_sync.py:76`). The 501 raises `HTTPStatusError`, gets swallowed, and the function proceeds to build its payload on top of an **empty** dict. Every preset outside the managed range — which the docstring explicitly promises to preserve — is dropped from the payload. Today the subsequent `post_presets` also 501s, so nothing is lost. The moment the write path is fixed without also fixing the read, a sync would silently wipe the user's presets. ### What needs to happen - Read existing presets from **`GET /presets.json`** (the file, not a `/json` sub-path). Confirmed working on the device — returns the full preset map, with unused slots as `{"stop":0}` segment stubs. - Write via **`POST /upload`** (multipart, target path `/presets.json`), which is how the WLED app does preset backup/restore. The alternative — applying each state and issuing `{"psave": n}` per slot — would drive the physical lights once per preset, so it is the wrong mechanism here. - Narrow that bare `except Exception`. A failed read must abort the sync, never degrade to "start from empty". - Verify on hardware. This one genuinely cannot be validated by mocks — the bug exists precisely because the mocks agreed with the wrong API. Snapshot presets before, restore after. ### Interim state #62 leaves the endpoint as-is functionally but makes the failure loud rather than silent, so nobody mistakes the swallowed read for a working sync. Note the *export* path is unaffected: `GET /api/v1/export/presets.json` builds the file server-side for manual upload and never talks to the controller.

claude-bot added the

labels

2026-09-04 17:57:18 +00:00

claude-bot added this to the v1.1.0 milestone

2026-09-04 17:57:20 +00:00

rbrooks referenced this issue from a commit

2026-09-04 18:16:08 +00:00

Declare WLED firmware compatibility and pin it with fixtures (#62)

claude-bot referenced this issue

2026-09-04 18:16:33 +00:00

Declare WLED firmware compatibility and pin it with fixtures (#62) #116

claude-bot referenced this issue from a commit

2026-09-04 18:33:55 +00:00

Declare WLED firmware compatibility and pin it with fixtures (#62)

claude-bot referenced this issue

2026-09-04 18:34:08 +00:00

WLED firmware compatibility statement and translator fixtures #62

rbrooks referenced this issue from a commit

2026-09-04 19:24:32 +00:00

Sync presets by uploading the file WLED actually reads (#115)

claude-bot referenced this issue from a pull request that will close it,

2026-09-04 19:24:54 +00:00

Sync presets by uploading the file WLED actually reads (#115) #118

claude-bot closed this issue

2026-09-04 19:34:03 +00:00

claude-bot referenced this issue from a commit

2026-09-04 19:34:04 +00:00

Sync presets by uploading the file WLED actually reads (#115)

claude-bot commented

2026-09-04 19:34:17 +00:00

Author

Contributor

Done — #118 merged, all seven CI jobs green.

The mechanism, verified on hardware


Read	`GET /presets.json`
Write	`POST /upload`, multipart — the part's filename is the destination path
Delete	`GET /edit?func=delete&path=/…` — a GET, not an HTTP DELETE

The upload path was proved on a throwaway filename (/iris-probe.json) before going anywhere near presets.json, then deleted.

The open question — does an uploaded preset file take effect without a reboot — is a yes. Applying ps=100 immediately after an upload ran exactly the fx, palette and colours that had been written.

What was checked against the device

Snapshotted presets.json, /json/state and /json/info first; all restored after.

Round-trip through the real WLEDController: read-back byte-identical to what was sent, pre-existing slots 1–3 untouched
Full end-to-end sync_presets with a seeded DB — 2 approved schemes to slots 100–101, correctly named and date-ordered, existing presets intact, read-back verification passed
Controller left byte-identical to the pre-session snapshot (2240 bytes), no stray files, lights back to on:false, bri:8, fx 115

Read-back verification

The write replaces the whole file, so sync merges into what it read and then reads it back. A 200 from /upload means the request was accepted, not that a complete file reached a flash chip — without this a truncated write would surface in December as a preset that does nothing. Both failure paths return a clear error with synced: 0, and I confirmed those tests actually fail when the verification block is removed.

The bare except Exception on the read is narrowed in effect rather than in type: it still catches broadly, but now aborts instead of degrading to {}. That mattered less when the write could not succeed either; now that it can, it is the only thing between an unreadable controller and a wiped preset file.

Two findings beyond the issue

Compact serialisation. The first restore reported success but left the file byte-different: json.dumps' default separators inflated it 20% (2240 → 2682 bytes) in pure whitespace, on a device with ~983KB of filesystem that has to hold up to 366 presets. Compact output also matches WLED's own exactly, which is why the restored file is now byte-identical rather than merely equivalent.

The tests passed by mock aliasing. get_presets.return_value = {} hands back the same dict object on every call, so sync_presets mutating it made the read-back compare a dict against itself — passing regardless of what was written. The mock now models the preset file, returning a fresh copy per read. Mocks agreeing with the code rather than with the device is exactly what let this bug sit behind green tests for the whole v1.0.0 cycle, so it seemed worth fixing at the root.

Documented, not implemented

A controller with a settings PIN rejects /edit and /upload, and Iris does not send one. Noted in docs/wled-compatibility.md; the dev controller has no PIN. Worth its own issue if anyone needs it.

Also recorded there: an upload named cfg.json reboots the device, so post_presets is deliberately not a general-purpose upload helper.

Done — #118 merged, all seven CI jobs green. ### The mechanism, verified on hardware | | | |---|---| | Read | `GET /presets.json` | | Write | `POST /upload`, multipart — the part's **filename** is the destination path | | Delete | `GET /edit?func=delete&path=/…` — a GET, not an HTTP DELETE | The upload path was proved on a throwaway filename (`/iris-probe.json`) before going anywhere near `presets.json`, then deleted. **The open question — does an uploaded preset file take effect without a reboot — is a yes.** Applying `ps=100` immediately after an upload ran exactly the fx, palette and colours that had been written. ### What was checked against the device Snapshotted `presets.json`, `/json/state` and `/json/info` first; all restored after. - Round-trip through the real `WLEDController`: read-back byte-identical to what was sent, pre-existing slots 1–3 untouched - Full end-to-end `sync_presets` with a seeded DB — 2 approved schemes to slots 100–101, correctly named and date-ordered, existing presets intact, read-back verification passed - Controller left **byte-identical** to the pre-session snapshot (2240 bytes), no stray files, lights back to `on:false, bri:8, fx 115` ### Read-back verification The write replaces the whole file, so sync merges into what it read and then reads it back. A 200 from `/upload` means the request was accepted, not that a complete file reached a flash chip — without this a truncated write would surface in December as a preset that does nothing. Both failure paths return a clear error with `synced: 0`, and I confirmed those tests actually fail when the verification block is removed. The bare `except Exception` on the read is narrowed in effect rather than in type: it still catches broadly, but now aborts instead of degrading to `{}`. That mattered less when the write could not succeed either; now that it can, it is the only thing between an unreadable controller and a wiped preset file. ### Two findings beyond the issue **Compact serialisation.** The first restore reported success but left the file byte-*different*: `json.dumps`' default separators inflated it 20% (2240 → 2682 bytes) in pure whitespace, on a device with ~983KB of filesystem that has to hold up to 366 presets. Compact output also matches WLED's own exactly, which is why the restored file is now byte-identical rather than merely equivalent. **The tests passed by mock aliasing.** `get_presets.return_value = {}` hands back the *same dict object* on every call, so `sync_presets` mutating it made the read-back compare a dict against itself — passing regardless of what was written. The mock now models the preset file, returning a fresh copy per read. Mocks agreeing with the code rather than with the device is exactly what let this bug sit behind green tests for the whole v1.0.0 cycle, so it seemed worth fixing at the root. ### Documented, not implemented A controller with a settings PIN rejects `/edit` and `/upload`, and Iris does not send one. Noted in `docs/wled-compatibility.md`; the dev controller has no PIN. Worth its own issue if anyone needs it. Also recorded there: an upload named `cfg.json` reboots the device, so `post_presets` is deliberately not a general-purpose upload helper.

claude-bot referenced this issue

2026-09-04 22:59:46 +00:00

Declarative full-config export / import (GitOps) #23

No milestone

No project

No assignees

1 participant

Notifications

Due date

The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference

rbrooks/Iris-WLED#115

No description provided.

Rows
Columns