UX-02: the validator's refusals, next to the field, before the save
checks / unit (pull_request) Successful in 6m17s
checks / unit (pull_request) Successful in 6m17s
The Configuration tab can only render types and ranges. `validate.py`'s most
useful refusals are CROSS-FIELD - two inverters naming one `setpoint_entity`, a
`maintenance_charge_w` above `max_w`, a flat `setpoint_entity` left set beside a
declared roster - and the Supervisor's UI has no way to express a rule spanning
two keys. Today the operator meets those as exit 78 in a log, after saving and
restarting. This puts them on the screen beside the field they name, before
anything is written.
WHAT IT ADDS
app/configui.py the field model, built from `GET /addons/self/info`'s own
`schema` + `translations`. No option key, no label, no
rule of its own. UX-01's `Group · ` label prefix becomes a
real section heading, in whichever language resolved.
app/selfconfig.py the Supervisor conversation and the write gate.
app/config_page.py the page. Its longest function decides which HTML input a
type deserves; it contains no rule.
app/web.py five routes. Still the only file a reviewer has to read to
answer "does anything here reach the inverter".
THE FOUR THINGS THAT COULD HURT SOMEBODY, AND HOW EACH IS HELD
1. A blob the SHIPPED validator refuses is never written - not warned about,
not written. An invalid blob plus a restart is exit 78, and the panel is
gone with the add-on that serves it.
2. A blob that moved underneath is refused, not clobbered. Read hands out a
sha256 of the canonical blob, save hands it back, and the stored blob is
re-read IMMEDIATELY BEFORE the write. This is SAFETY-28's discipline
applied to configuration - "a blob reaches the Supervisor only if it was
derived from the blob stored right now" - with the honest weakening stated
in the decision record: a second writer is DETECTED, not made
inexpressible.
3. Every message is the validator's own, byte for byte. test_selfconfig.py
part 6 inserts ONE new rule into a COPY of validate.py, hashes every file
under app/ to show validate.py is the only one that differs, and serves the
real /config/validate route out of that copy in a subprocess. The new
sentence arrives, filed under the key it names.
4. Nothing on the surface commands an inverter or clears a latch. Saving does
not restart: applying costs a control interruption - the wind-down commands
0 W on every inverter - so that is a separate, confirmed button that says
so first.
MEASURED, NOT CITED (tools/supervisor_selfapi_probe.py, run against
Supervisor 2026.08.0's own code; it contacts nothing)
- the three self-paths are permitted, and `/addons/self/info` is admitted by
the `default` role too while `options` and `restart` rest on the bypass
ALONE. The probe's first draft claimed the role admitted none of the three
and went red.
- `/addons/self/options/validate` is NOT reachable from inside an add-on, so
the only validator this panel can call is ours - which is the one with the
cross-field rules.
- a POST that omits a key does not preserve it: the key reverts to
config.yaml's default. Hence whole-blob read-modify-write.
- the Configuration tab CAN be removed, and doing so drops 0-of-71 options
through and takes the add-on down. The card said it could not be; it stays
by decision, which is a better reason.
DOES NOT TOUCH config.yaml, main.py or any control path. UX-01 is a dependency,
not something this supersedes: its translations are where every label,
description and section heading on this page comes from, and the Configuration
tab is still the recovery surface. DECISIONS/UX-02-two-config-surfaces.md
records which surface is authoritative (neither: the stored blob is) and what
the second surface costs.
Also corrects a stale "70 options" in three prose spots to the 71 the probe
counts - config.yaml grew a key in SAFETY-42 and the sentences did not follow.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
---
⚠️ REBASED ONTO release/1.0 @ 3368231, AND HISTORY DELIBERATELY FLATTENED.
The original branch carried `653874d` - an ORPHANED first attempt at PKG-07
whose commit message justified the version bump with a claim that was FALSE
(that `slew_w: 0` used to mean "no limit"; the schema was `int(50,5000)` from
the repo's first commit, so 0 was never legal). That commit's CONTENT was
superseded when release/1.0 was merged in, and the merge resolved correctly -
one 0.8.0 section, no fabricated text. But the retracted claim would have
entered release/1.0's history as a commit message with no retraction attached
to it, where PKG-07's own merged commit is the thing that retracts it.
So the branch is replayed as a single commit on top of the corrected PKG-07
rather than merged with the bad one trailing behind. The tree is byte-identical
to the reviewed branch tip; only the ancestry changed.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Claude-Session: https://claude.ai/code/session_01BwRLw7eKYj2byVdwB6kbjU
This commit is contained in:
co-authored by
Claude Opus 5
parent
3368231048
commit
fdaf3fb54f
@@ -8,12 +8,33 @@ perfectly on a developer's laptop.
|
||||
Design bias: this page must be readable at the end of a long day, on a phone, in
|
||||
a cellar, by someone who did not write it. Numbers alone are not enough - it
|
||||
says what is wrong in words.
|
||||
|
||||
⚠️ SINCE UX-02 THERE ARE TWO PAGES HERE, AND ONLY ONE OF THEM IS THIS FILE'S.
|
||||
`/` is the status and control surface and is unchanged. `/config` is the
|
||||
configuration surface: its markup lives in app/config_page.py, its field model
|
||||
in app/configui.py and its Supervisor conversation in app/selfconfig.py. What
|
||||
stays HERE is the routing, so that the question a reviewer has to answer - does
|
||||
anything the panel exposes reach the inverter? - is answered by reading one
|
||||
short file.
|
||||
|
||||
⚠️ THE ANSWER TO THAT QUESTION, STATED SO IT CAN BE CHECKED: the configuration
|
||||
routes below read `GET /addons/self/info`, run the SHIPPED validator, write
|
||||
`POST /addons/self/options` and, on an explicitly confirmed request, `POST
|
||||
/addons/self/restart`. NONE of them writes a setpoint, clears a latch, changes a
|
||||
mode or reaches `controller.handle_action()`. The set of things an operator can
|
||||
do to the inverter through this panel is exactly what `/action` offered before
|
||||
this card - test_selfconfig.py part 7 asserts that by listing the routes and the
|
||||
actions and comparing them with the shipped set.
|
||||
"""
|
||||
|
||||
import logging
|
||||
|
||||
from aiohttp import web
|
||||
|
||||
from . import selfconfig
|
||||
from .config_page import CONFIG_PAGE
|
||||
from .configui import changed_keys, field_model, problems_by_field
|
||||
|
||||
_LOG = logging.getLogger("goodwe.web")
|
||||
|
||||
PAGE = """<!doctype html>
|
||||
@@ -49,6 +70,7 @@ PAGE = """<!doctype html>
|
||||
.muted{color:var(--muted);font-size:13px}
|
||||
</style></head><body>
|
||||
<h1>GoodWe RS485 Controller</h1>
|
||||
<p class="muted"><a href="config">Configuration →</a></p>
|
||||
<div id="banner" class="banner b-warn">loading…</div>
|
||||
|
||||
<div class="card">
|
||||
@@ -95,7 +117,33 @@ refresh(); setInterval(refresh, 3000);
|
||||
"""
|
||||
|
||||
|
||||
def build_app(controller) -> web.Application:
|
||||
# ⚠️ WHAT THE PANEL SAYS WHEN IT COULD NOT READ A DEVICE RANGE. Two of
|
||||
# validate.py's rules compare a configured ceiling against the setpoint
|
||||
# device's OWN reported range, and that range is only knowable by asking Home
|
||||
# Assistant. When ESPHome is asleep or the entity does not exist yet the
|
||||
# startup gate SKIPS those two rules rather than guessing - so the panel skips
|
||||
# them too, and says so, because a green page that is green for a reason the
|
||||
# operator cannot see is the exact failure this add-on keeps refusing to ship.
|
||||
DEVICE_NOTE = (
|
||||
"%s did not report a range, so the two checks that compare a configured "
|
||||
"ceiling against what the device itself accepts were skipped - here and at "
|
||||
"startup, identically. Everything else was checked. If the device answers "
|
||||
"later and disagrees, the add-on refuses to start then, with the same "
|
||||
"message you would have seen here.")
|
||||
|
||||
|
||||
def build_app(controller, selfcfg=None) -> web.Application:
|
||||
"""The ingress app. `selfcfg` is injectable so a check can drive it.
|
||||
|
||||
⚠️ IT IS BUILT FROM THE CONTROLLER'S OWN SESSION AND TOKEN, not from a
|
||||
second `ClientSession`. `amain()` owns one session for the process and every
|
||||
Home Assistant call already goes through it; a second one here would be a
|
||||
second connection pool whose failures nothing reports.
|
||||
"""
|
||||
if selfcfg is None:
|
||||
selfcfg = selfconfig.SelfConfig(controller.hass.session,
|
||||
controller.hass.token)
|
||||
|
||||
async def index(_request):
|
||||
return web.Response(text=PAGE, content_type="text/html")
|
||||
|
||||
@@ -113,17 +161,152 @@ def build_app(controller) -> web.Application:
|
||||
raise web.HTTPBadRequest(text=f"unknown action {what!r}")
|
||||
return web.json_response({"ok": True, "result": result})
|
||||
|
||||
# --- UX-02: the configuration surface ---------------------------------
|
||||
async def config_index(_request):
|
||||
return web.Response(text=CONFIG_PAGE, content_type="text/html")
|
||||
|
||||
async def _proposed(request):
|
||||
try:
|
||||
body = await request.json()
|
||||
except ValueError:
|
||||
raise web.HTTPBadRequest(text="expected JSON")
|
||||
options = body.get("options")
|
||||
if not isinstance(options, dict):
|
||||
raise web.HTTPBadRequest(text="expected an options object")
|
||||
return body, options
|
||||
|
||||
async def config_model(request):
|
||||
"""Everything the page needs to draw itself, from ONE Supervisor call.
|
||||
|
||||
⚠️ `running_differs` IS NOT COSMETIC. `controller.o` is the blob this
|
||||
PROCESS started with (`/data/options.json`); `info["options"]` is what
|
||||
the Supervisor has STORED and would apply at the next start. They differ
|
||||
exactly when somebody has saved and not restarted - here, or in the
|
||||
Configuration tab. Printing the difference is the only way an operator
|
||||
finds out that the page they are reading is not describing the running
|
||||
system, and it is also how an edit made on the other surface becomes
|
||||
visible on this one.
|
||||
"""
|
||||
try:
|
||||
info = await selfcfg.info()
|
||||
except selfconfig.SupervisorUnavailable as err:
|
||||
_LOG.error("cannot read the add-on's own options: %s", err)
|
||||
return web.json_response({"error": str(err)}, status=502)
|
||||
options = info.get("options") or {}
|
||||
lang = str(request.query.get("lang", "en") or "en")[:8]
|
||||
model = field_model(info.get("schema") or [],
|
||||
info.get("translations") or {}, lang)
|
||||
model.update({
|
||||
"options": options,
|
||||
"fingerprint": selfconfig.fingerprint(options),
|
||||
"running": controller.o,
|
||||
"running_differs": changed_keys(controller.o, options),
|
||||
"lang": lang,
|
||||
})
|
||||
return web.json_response(model)
|
||||
|
||||
async def config_validate(request):
|
||||
"""The SHIPPED validator, run against a proposed blob. Writes nothing.
|
||||
|
||||
⚠️ IT READS HOME ASSISTANT AND WRITES NOTHING ANYWHERE. The two
|
||||
device-range rules need `hass.limits()`, which is a GET of an entity's
|
||||
state. The page debounces, so a burst of typing is one call after the
|
||||
typing stops rather than one per keystroke.
|
||||
|
||||
⚠️ THE STRINGS ARE PASSED THROUGH UNTOUCHED. Not summarised, not
|
||||
renumbered, not mapped to a severity. validate.py's sentences are the
|
||||
product this card is delivering; anything this layer did to them would
|
||||
be a second opinion about a rule it does not own.
|
||||
"""
|
||||
_body, options = await _proposed(request)
|
||||
problems, unread = await selfconfig.problems_and_ranges(
|
||||
options, controller.hass)
|
||||
by_key, unanchored = problems_by_field(problems, _keys_of(options))
|
||||
payload = {"problems": problems, "by_key": by_key,
|
||||
"unanchored": unanchored}
|
||||
if unread:
|
||||
payload["device_note"] = DEVICE_NOTE % ", ".join(sorted(set(unread)))
|
||||
return web.json_response(payload)
|
||||
|
||||
def _keys_of(options):
|
||||
# ⚠️ THE ANCHOR LIST IS THE KEYS OF THE BLOB UNDER TEST, and that is
|
||||
# sound only because the page posts the WHOLE blob it read - which is
|
||||
# `App.options`, the merge of config.yaml's defaults with the stored
|
||||
# ones, so every option key is in it. Nothing here types a key name;
|
||||
# see configui.problems_by_field() for what is done with them.
|
||||
return sorted(options)
|
||||
|
||||
async def config_save(request):
|
||||
"""Validate, compare-and-swap, write. Or refuse and write NOTHING."""
|
||||
body, options = await _proposed(request)
|
||||
expect = str(body.get("fingerprint") or "")
|
||||
try:
|
||||
result = await selfconfig.save(selfcfg, options, expect,
|
||||
hass=controller.hass)
|
||||
except selfconfig.SaveRefused as refused:
|
||||
if refused.kind == "stale":
|
||||
stored = refused.stored or {}
|
||||
return web.json_response(
|
||||
{"kind": "stale", "changed": refused.changed,
|
||||
"stored": stored,
|
||||
# The page needs the CURRENT token to be able to save
|
||||
# again after the operator has read the difference. That
|
||||
# is not a force door: it takes a second, deliberate press
|
||||
# made in front of what changed.
|
||||
"fingerprint": selfconfig.fingerprint(stored)},
|
||||
status=409)
|
||||
by_key, unanchored = problems_by_field(refused.problems,
|
||||
_keys_of(options))
|
||||
return web.json_response(
|
||||
{"kind": "invalid", "problems": refused.problems,
|
||||
"by_key": by_key, "unanchored": unanchored}, status=422)
|
||||
except selfconfig.SupervisorUnavailable as err:
|
||||
_LOG.error("saving options failed: %s", err)
|
||||
return web.json_response({"error": str(err)}, status=502)
|
||||
controller.log_event("configuration saved from the panel - "
|
||||
"not applied until the add-on restarts")
|
||||
return web.json_response({"ok": True, **result})
|
||||
|
||||
async def config_restart(request):
|
||||
"""Restart the add-on, at the operator's explicit request.
|
||||
|
||||
⚠️ THIS IS THE ONE ROUTE ON THIS SURFACE THAT COSTS ANYTHING. The
|
||||
wind-down commands 0 W on every inverter (`Controller.shutdown()`), so
|
||||
the request must carry the intent token; a reload, a retry or a stray
|
||||
POST cannot spend a control interruption. It restarts THIS ADD-ON and
|
||||
nothing else: no setpoint is written from here, and a latched failsafe
|
||||
is restored from the Store on the way back up rather than cleared.
|
||||
"""
|
||||
try:
|
||||
body = await request.json()
|
||||
except ValueError:
|
||||
raise web.HTTPBadRequest(text="expected JSON")
|
||||
try:
|
||||
await selfcfg.restart(str(body.get("confirm") or ""))
|
||||
except selfconfig.NotConfirmed as err:
|
||||
return web.json_response({"error": str(err)}, status=400)
|
||||
except selfconfig.SupervisorUnavailable as err:
|
||||
_LOG.error("restart request failed: %s", err)
|
||||
return web.json_response({"error": str(err)}, status=502)
|
||||
controller.log_event("restart requested from the configuration panel")
|
||||
return web.json_response({"ok": True})
|
||||
|
||||
app = web.Application()
|
||||
app.add_routes([
|
||||
web.get("/", index),
|
||||
web.get("/status", status),
|
||||
web.post("/action", action),
|
||||
web.get("/config", config_index),
|
||||
web.get("/config/model", config_model),
|
||||
web.post("/config/validate", config_validate),
|
||||
web.post("/config/save", config_save),
|
||||
web.post("/config/restart", config_restart),
|
||||
])
|
||||
return app
|
||||
|
||||
|
||||
async def start(controller, port: int = 8099):
|
||||
runner = web.AppRunner(build_app(controller))
|
||||
async def start(controller, port: int = 8099, selfcfg=None):
|
||||
runner = web.AppRunner(build_app(controller, selfcfg))
|
||||
await runner.setup()
|
||||
site = web.TCPSite(runner, "0.0.0.0", port)
|
||||
await site.start()
|
||||
|
||||
Reference in New Issue
Block a user