Find the controller and read its segments, instead of typing both (#50) #129

Merged
claude-bot merged 1 commit from feat/50-discovery into main 2026-09-05 00:41:00 +00:00
Contributor

Closes #50.

Two independent halves of the same onboarding friction: today someone sets Iris up by reading an IP off their router and segment indexes off the WLED UI, then typing both in again.

Discovery

Browses for WLED's _wled._tcp mDNS advertisement. The shape here comes from what a real 16.0.0 device actually broadcasts, not from the docs:

wled-450664._wled._tcp.local. -> 192.168.1.140:80, TXT {mac: 78421c450664}

So the instance name is the useful label, the TXT record carries only the MAC, and the friendly device name has to come from /json/info afterwards.

The Docker caveat is the interesting case. mDNS is multicast to 224.0.0.251:5353 and does not cross a bridge network, so on a default docker compose install discovery finds nothing however healthy the controller is. Returning an empty list there would be the wrong diagnosis — it reads as "no controllers on this network" and sends people to look at their lights. An empty scan now says which case applies; docs/setup.md covers network_mode: host and the Avahi reflector; and typing the address in stays fully supported, since everything else Iris does over HTTP routes fine from a bridge.

Advertised addresses are checked before they are followed. mDNS is unauthenticated: anything on the network segment can claim to be a WLED controller and point at any address, and discovery then fetches /json/info from what it is told. An advert naming a public host would turn an admin's "scan my LAN" click into an outbound request of the advertiser's choosing. Private, loopback and link-local only.

Segment import

Reads /json/state and proposes one Iris segment per WLED segment, defaulting each to themed.

It proposes; it does not apply. Segment identity is what every scheme is written against, so a silent overwrite could rename or drop segments out from under approved schemes — and invalidate_for_segment_change would then correctly mark a season's work stale. A confirm step is cheap; that is not.

Two behaviours that took thought:

  • Matching is by wled_index, not name. The index addresses hardware; the name is a label the user may well have improved on. Matching by name would report a segment renamed to "Roofline" as one addition plus one deletion, and invite the user to accept losing their own naming.
  • A segment Iris knows that the controller no longer reports is kept and listed, never removed. It may simply have been reconfigured, and deleting it would take its schemes' segment rows with it.

Bounds are shown, not stored. start/stop are the most useful thing for telling two unnamed segments apart ("LEDs 0–35" vs "LEDs 36–71"), but Iris addresses segments by index, and adding a column to display a string would be storing a copy of something the controller owns and can change without telling us. So they go in the preview, not the database.

The server also returns the exact body for PUT /segments. That endpoint replaces the set, so a client sending only the additions would delete everything it omitted — assembling it client-side is a trap worth removing.

The open question in the issue

Note: WLED_HOST is env-only today; decide whether discovery writes a runtime setting that overrides it

It already does. wled_host has been a runtime setting seeded from env since v1, and the database value wins once set. So discovery fills the existing field and the existing Save controller applies it — no new mechanism, same confirmation, same audit entry. Documented rather than reinvented.

Verification

Both halves were verified against the live controller before the tests were written, not after:

  • Discovery found 192.168.1.140, enriched it correctly (WLED, firmware 16.0.0, 36 LEDs, 1 segment).
  • Import proposed Segment 0 from its unnamed segment; re-importing against an Iris that had renamed it to "Roofline" reported no changes and preserved the user's name, with an extra Iris-only segment carried through untouched.

The /json/state fixture in the tests is captured verbatim from that device. That distinction has cost this project once already: in #62 the segment count read leds.segs, a field no firmware has ever sent, and the test passed because the fixture had invented the same field the code was reading.

53 new backend tests. Backend ruff/mypy clean, 722 tests passing. Frontend in node:22: tsc -b clean, eslint clean, 81 vitest passing, npm run build succeeds.

zeroconf is a new runtime dependency — pure Python, no native build, and only imported when someone clicks Discover, so an install that types its address in never binds a multicast socket.

🤖 Generated with Claude Code

Closes #50. Two independent halves of the same onboarding friction: today someone sets Iris up by reading an IP off their router and segment indexes off the WLED UI, then typing both in again. ## Discovery Browses for WLED's `_wled._tcp` mDNS advertisement. The shape here comes from what a real 16.0.0 device **actually broadcasts**, not from the docs: ``` wled-450664._wled._tcp.local. -> 192.168.1.140:80, TXT {mac: 78421c450664} ``` So the instance name is the useful label, the TXT record carries only the MAC, and the friendly device name has to come from `/json/info` afterwards. **The Docker caveat is the interesting case.** mDNS is multicast to 224.0.0.251:5353 and does not cross a bridge network, so on a default `docker compose` install discovery finds nothing however healthy the controller is. Returning an empty list there would be the *wrong diagnosis* — it reads as "no controllers on this network" and sends people to look at their lights. An empty scan now says which case applies; `docs/setup.md` covers `network_mode: host` and the Avahi reflector; and typing the address in stays fully supported, since everything else Iris does over HTTP routes fine from a bridge. **Advertised addresses are checked before they are followed.** mDNS is unauthenticated: anything on the network segment can claim to be a WLED controller and point at any address, and discovery then fetches `/json/info` from what it is told. An advert naming a public host would turn an admin's "scan my LAN" click into an outbound request of the advertiser's choosing. Private, loopback and link-local only. ## Segment import Reads `/json/state` and proposes one Iris segment per WLED segment, defaulting each to `themed`. **It proposes; it does not apply.** Segment identity is what every scheme is written against, so a silent overwrite could rename or drop segments out from under approved schemes — and `invalidate_for_segment_change` would then correctly mark a season's work stale. A confirm step is cheap; that is not. Two behaviours that took thought: - **Matching is by `wled_index`, not name.** The index addresses hardware; the name is a label the user may well have improved on. Matching by name would report a segment renamed to "Roofline" as one addition plus one deletion, and invite the user to accept losing their own naming. - **A segment Iris knows that the controller no longer reports is kept and listed, never removed.** It may simply have been reconfigured, and deleting it would take its schemes' segment rows with it. **Bounds are shown, not stored.** `start`/`stop` are the most useful thing for telling two unnamed segments apart ("LEDs 0–35" vs "LEDs 36–71"), but Iris addresses segments by index, and adding a column to display a string would be storing a copy of something the controller owns and can change without telling us. So they go in the preview, not the database. The server also returns the exact body for `PUT /segments`. That endpoint *replaces* the set, so a client sending only the additions would delete everything it omitted — assembling it client-side is a trap worth removing. ## The open question in the issue > Note: `WLED_HOST` is env-only today; decide whether discovery writes a runtime setting that overrides it **It already does.** `wled_host` has been a runtime setting seeded from env since v1, and the database value wins once set. So discovery fills the existing field and the existing **Save controller** applies it — no new mechanism, same confirmation, same audit entry. Documented rather than reinvented. ## Verification **Both halves were verified against the live controller before the tests were written**, not after: - Discovery found `192.168.1.140`, enriched it correctly (`WLED`, firmware 16.0.0, 36 LEDs, 1 segment). - Import proposed `Segment 0` from its unnamed segment; re-importing against an Iris that had renamed it to "Roofline" reported no changes and preserved the user's name, with an extra Iris-only segment carried through untouched. The `/json/state` fixture in the tests is captured verbatim from that device. That distinction has cost this project once already: in #62 the segment count read `leds.segs`, a field no firmware has ever sent, and the test passed because the fixture had invented the same field the code was reading. 53 new backend tests. Backend `ruff`/`mypy` clean, **722 tests passing**. Frontend in `node:22`: `tsc -b` clean, `eslint` clean, 81 vitest passing, `npm run build` succeeds. `zeroconf` is a new runtime dependency — pure Python, no native build, and only imported when someone clicks Discover, so an install that types its address in never binds a multicast socket. 🤖 Generated with [Claude Code](https://claude.com/claude-code)
Find the controller and read its segments, instead of typing both (#50)
All checks were successful
CI / Dockerfile lint (pull_request) Successful in 10s
CI / Alembic migration check (pull_request) Successful in 1m5s
CI / Frontend lint, test & build (pull_request) Successful in 1m10s
CI / Pre-commit hooks (pull_request) Successful in 1m12s
CI / Python lint & type-check (pull_request) Successful in 1m32s
CI / Python tests (pull_request) Successful in 6m5s
CI / Docker build, health smoke & E2E (pull_request) Successful in 2m3s
3a48495e3c
Two independent halves of the same onboarding friction: today someone
sets Iris up by reading an IP off their router and segment indexes off
the WLED UI, then typing both in again.

Discovery browses for WLED's _wled._tcp mDNS advert. The shape here comes
from what a real 16.0.0 device actually broadcasts, not from the docs --
wled-450664._wled._tcp.local. at 192.168.1.140:80 with TXT {mac:
78421c450664} -- so the instance name is the label, the TXT carries only
the MAC, and the friendly name has to come from /json/info afterwards.

mDNS is multicast and does not cross a Docker bridge network, so on a
default compose install discovery finds nothing no matter how healthy the
controller is. That is the interesting case, and returning an empty list
would be the wrong diagnosis -- it reads as "no controllers on this
network" and sends people to look at their lights. An empty scan now says
which case applies, docs/setup.md covers network_mode: host and the Avahi
reflector, and typing the address in stays fully supported: everything
else Iris does over HTTP routes fine from a bridge.

Advertised addresses are checked before being followed. mDNS is
unauthenticated, anything on the segment can claim to be WLED and point
anywhere, and discovery then fetches /json/info from what it is told --
so an advert naming a public host would turn an admin's scan into an
outbound request of the advertiser's choosing. Private, loopback and
link-local only.

Segment import reads /json/state and proposes one Iris segment per WLED
segment, defaulting to themed. It proposes; it does not apply. Segment
identity is what every scheme is written against, so a silent overwrite
could rename or drop segments out from under approved schemes, and
invalidate_for_segment_change would then correctly stale a season's work.
A confirm step is cheap; that is not.

Two behaviours that took thought:

Matching is by wled_index, not name. The index addresses hardware; the
name is a label the user may well have improved on. Matching by name
would report a segment renamed to "Roofline" as one addition plus one
deletion and invite them to accept losing it.

A segment Iris knows that the controller no longer reports is kept and
listed, never removed. It may simply have been reconfigured, and deleting
it would take its schemes' segment rows with it.

Bounds are shown, not stored. start/stop are the most useful thing for
telling two unnamed segments apart, but Iris addresses segments by index
and adding a column to display a string would be storing a copy of
something the controller owns and can change without telling us.

The issue asked whether discovery should write a runtime setting that
overrides WLED_HOST. It already does: wled_host has been a runtime
setting seeded from env since v1, the database value wins once set, so
discovery fills the existing field and the existing Save applies it. No
new mechanism, and the same audit trail.

Both halves verified against the live controller before the tests were
written: discovery found it and enriched it correctly, and import
proposed "Segment 0" from its unnamed segment, then reported no changes
on re-import against an Iris that had renamed it.

Co-Authored-By: Claude Opus 5 (1M context) <noreply@anthropic.com>
claude-bot deleted branch feat/50-discovery 2026-09-05 00:41:01 +00:00
Sign in to join this conversation.
No description provided.