UX-13 P2 review fixes (PR #145, comment 1880)

1. Discard reloads /config/model: after a stale save the page kept the
   new fingerprint with the old values, so Discard + Save overwrote the
   other surface's change without a 409.
2. Removing cards back to one restores the stored inverter shape, so a
   flat site stays flat after add + remove.
3. A choice the page can only keep ("Other time windows") stays offered
   while it is the stored value.
4. A load locks every locked value again.
5. at_zero() needs an accepted 0 W write; before the first one Home says
   "Going to 0 W" (the heartbeat writes every heartbeat_s, so it clears).
6. A field with a unit drops the translation's "(W)" from its label: the
   peak limit read "Peak limit (W)" beside a kW value.
7. A mirrored choice lists the mirrored field's keys, so a search for
   static_profile finds the contract on a fixed-price site.
8. test_settings: a save refused by ui_to_options writes nothing.

Co-Authored-By: Claude Opus 5.5 <noreply@anthropic.com>

goodwe_controller/app/homeview.py

@@ -511,10 +511,12 @@ def build_plan(c, now, shadow, running, held=False, settled=True):
 # Batteries, attention, month peak, control
 # --------------------------------------------------------------------------- #
 def at_zero(inv) -> bool:
-    """Has this inverter's last accepted power request reached 0 W? A stop or
-    an alarm asks for 0 W at once, but the inverter gets it with the next
-    write, so until then the page says "Going to 0 W" (UX-13 P1 test, a)."""
-    return abs(inv.standing_w()) < 1.0
+    """Has this inverter accepted a 0 W power request? A stop or an alarm asks
+    for 0 W at once, but the inverter gets it with the next write, so until
+    then the page says "Going to 0 W" (UX-13 P1 test, a). Before the first
+    accepted write nothing is known to be held; the heartbeat writes every
+    `heartbeat_s`, stopped or not, so this does not stick."""
+    return inv.last_write is not None and abs(inv.last_write) < 1.0
 
 
 def battery_rows(c, shadow, running, held=False):

goodwe_controller/app/static/settings.js

@@ -87,8 +87,10 @@
   }
 
   /* ---------------- controls ---------------- */
-  function choicesOf(f, cur) {
-    const out = (f.choices || []).filter((c) => !c[2] || same(c[0], cur));
+  // A choice the page can only keep stays offered while it is the stored value.
+  function choicesOf(f, card) {
+    const cur = get(f, card), was = valueIn(ORIG, f, card);
+    const out = (f.choices || []).filter((c) => !c[2] || same(c[0], cur) || same(c[0], was));
     if (cur != null && cur !== "" && !out.some((c) => same(c[0], cur))) out.push([cur, String(cur)]);
     return out;
   }
@@ -106,7 +108,7 @@
   function radios(f, card, lid, cards) {
     const cur = get(f, card);
     const g = h("div", { class: cards ? "choices" : "seg", role: "radiogroup", "aria-labelledby": lid });
-    const items = choicesOf(f, cur);
+    const items = choicesOf(f, card);
     const buttons = items.map(([v, label], i) => {
       const on = same(v, cur);
       const hint = cards && f.hints ? f.hints[v] : "";
@@ -132,7 +134,7 @@
   function select(f, card, lid) {
     const cur = get(f, card);
     const s = h("select", { class: "sel", "aria-labelledby": lid });
-    for (const [v, label] of choicesOf(f, cur)) s.append(h("option", { value: JSON.stringify(v), selected: same(v, cur) }, label));
+    for (const [v, label] of choicesOf(f, card)) s.append(h("option", { value: JSON.stringify(v), selected: same(v, cur) }, label));
     for (const [v, label] of f.later || []) s.append(h("option", { value: JSON.stringify(v), disabled: true }, label));
     s.addEventListener("change", () => put(f, card, JSON.parse(s.value), true));
     const preset = f.presets && f.presets[cur];
@@ -324,7 +326,10 @@
         const box = h("div", { class: "inv" }, h("div", { class: "hd" },
           h("span", { class: "grow" }, c.name || W.inverter + " " + (i + 1)), h("span", { class: "key" }, f.key + "[" + i + "]"),
           cards.length > 1 ? h("button", { type: "button", class: "btn link tiny", onclick: () => {
-            V[f.key].splice(i, 1); V[f.count] = V[f.key].length; put(f, null, V[f.key], true);
+            V[f.key].splice(i, 1); V[f.count] = V[f.key].length;
+            // Back to one card, the stored shape returns: a flat inverter stays flat.
+            if (V[f.key].length === 1) for (const k in f.grow) V[k] = clone(ORIG[k]);
+            put(f, null, V[f.key], true);
           } }, W.remove) : null));
         for (const s of f.fields) box.append(fieldRow(s, i, gid));
         wrap.append(box);
@@ -496,6 +501,7 @@
       return;
     }
     M = m; V = clone(m.ui); ORIG = clone(m.ui); FP = m.fingerprint; MSG = []; DEVICE = "";
+    UNLOCKED.clear();
     $("#set-search").hidden = false;
     FILLED = {};
     LIST = null;
@@ -541,9 +547,9 @@
     finally { btn.disabled = false; btn.textContent = W.savebar.save; }
   }
   $("#sb-save").addEventListener("click", save);
-  $("#sb-discard").addEventListener("click", () => {
-    V = clone(ORIG); UNLOCKED.clear(); render(); validate(); toast(W.toast.discarded);
-    if ($("#set-banner .b-bad")) banner();
+  // Discard reloads what is stored: after a stale save ORIG is no longer it.
+  $("#sb-discard").addEventListener("click", async () => {
+    await load(); toast(W.toast.discarded);
   });
   addEventListener("beforeunload", (e) => { if (V && dirtyCount()) { e.preventDefault(); e.returnValue = ""; } });
   document.addEventListener("ux13:tab", (e) => {

goodwe_controller/app/uipage.py

@@ -91,6 +91,10 @@ def _choices(fld, node, T, value_type):
 def _field(fld, schema_ui, translations, lang, T):
     node = _node(schema_ui, fld.key)
     label, text = _words(translations, lang, fld.key, T)
+    if fld.unit:
+        # The page shows the unit beside the control, in its own scale (kW),
+        # so the translation's "(W)" goes; "(optional)" stays.
+        label = re.sub(r"\s*\([^()\s]{1,6}\)", "", label)
     value_type = ("float" if fld.control == L.NUMBER and fld.key.startswith("ui.")
                   else node.get("type") or "")
     out = {"key": fld.key, "control": fld.control, "label": label,
@@ -98,8 +102,9 @@ def _field(fld, schema_ui, translations, lang, T):
            "step": fld.step, "kind": fld.kind,
            "show": [list(c) for c in fld.show], "applies": fld.applies,
            "type": value_type, "optional": not node.get("required", False),
+           # A choice mirrored into a field also shows (and finds) its keys.
            "writes": sorted(k for k, how in L.DERIVED.items()
-                            if how == fld.key)}
+                            if how in (fld.key, fld.mirror))}
     if node.get("lengthMin") is not None:
         out["min"] = node["lengthMin"]
     if node.get("lengthMax") is not None:

goodwe_controller/test_home.py

@@ -123,11 +123,13 @@ MOCK = [.24, .22, .21, .20, .21, .22, .24, .27, .26, .22, .18, .16, .15, .14,
 
 
 def make(opts, *, soc=(62.0, 35.0), batt=(-2500.0, -300.0), grid=10.0,
-         auto=True):
+         auto=True, wrote=0.0):
+    """`wrote` is each inverter's last accepted power request (None: none yet)."""
     c = Controller(dict(opts), _Hass(), _Store(), _Pub(), stop=asyncio.Event())
     c.auto = auto
     for i, inv in enumerate(c.inverters):
         inv.soc, inv.batt = soc[i], batt[i]
+        inv.last_write = wrote
         inv.dev_min, inv.dev_max = -5000.0, 5000.0
     c.grid = grid
     return c
@@ -471,6 +473,11 @@ check("an alarm while the last power request is still 20 W says 'Going to "
 c_sp = make(TWO, auto=False)
 c_sp.inverters[1].last_write = -1500.0
 s_sp = status(c_sp)
+s_new = status(make(TWO, auto=False, wrote=None))
+check("...and before any power request was accepted (just started, or every "
+      "write refused) it says 'Going to 0 W', not 'Held'",
+      [b["now"] for b in s_new["batteries"]]
+      == ["Going to 0 W while control is stopped."] * 2)
 check("...and a stop does the same, per battery",
       [b["now"] for b in s_sp["batteries"]]
       == ["Held at 0 W while control is stopped.",

goodwe_controller/test_settings.py

@@ -103,8 +103,12 @@ check(f"every slider and locked value has its range from the schema "
       f"({len(_ranged)})", all(lo is not None and hi is not None
                                for _k, lo, hi in _ranged)
       and ("peak_cap_w", 500.0, 15000.0) in _ranged)
-check("the peak limit is a kW slider over a value stored in W",
-      field(FIELD, "peak_cap_w")["scale"] == 1000)
+check("the peak limit is a kW slider over a value stored in W, and its label "
+      "does not say W (the unit is shown beside the control)",
+      field(FIELD, "peak_cap_w")["scale"] == 1000
+      and field(FIELD, "peak_cap_w")["label"] == "Peak limit"
+      and not [f["key"] for _g, f in _flat
+               if f["unit"] and re.search(r"\([^()\s]{1,6}\)", f["label"])])
 _choice = [f for _g, f in _flat
            if f["control"] in (L.SEGMENTED, L.SELECT, L.CHOICE)]
 check("every choice control has choices with words",
@@ -139,7 +143,8 @@ check("the inverter cards carry their count, and adding one makes a list",
 check("a page-only field names the option keys it writes, for the grey key "
       "line", _reg["writes"] == ["price_taxes_profile"]
       and field(FIELD, "ui.solar")["writes"]
-      == ["pv_entities", "pv_entity", "pv_invert_entities"])
+      == ["pv_entities", "pv_entity", "pv_invert_entities"]
+      and "static_profile" in field(FIELD, "ui.contract")["writes"])
 check("without a Supervisor schema the page still draws, with no ranges",
       len(uipage.layout([], {})["groups"]) == 6)
 
@@ -183,6 +188,18 @@ check("a locked value shows no input until 'Change' is pressed",
       and "UNLOCKED.has(id)) { open(); return box; }" in JS)
 check("nothing is saved while the add-on's own checks refuse the model",
       "if (MSG.some((m) => m.tier === \"bad\")) { toast(" in JS)
+check("Discard reloads the stored settings (after a stale save the page's "
+      "copy is no longer what is stored)",
+      re.search(r'"#sb-discard"\)\.addEventListener\("click", async \(\) => '
+                r'\{\s*await load\(\);', JS) is not None)
+check("a load locks every locked value again, and a choice the page can only "
+      "keep stays offered while it is the stored value",
+      "UNLOCKED.clear();" in JS.split("async function load")[1].split("render();")[0]
+      and "same(c[0], was)" in JS)
+check("removing cards back to one restores the stored shape (a flat inverter "
+      "is not turned into a list of one)",
+      "if (V[f.key].length === 1) for (const k in f.grow) V[k] = clone(ORIG[k]);"
+      in JS)
 check("the old settings page stays reachable (until P5)",
       'href="config" id="set-classic"' in web.page())
 
@@ -343,6 +360,11 @@ async def _refuse(s, root, sup):
     out["floor"] = await validate(peak_soc_floor=5)
     out["derive"] = await validate(ui__contract="dynamic", price_provider="")
     model = copy.deepcopy(base)
+    model.update({"ui.contract": "dynamic", "price_provider": ""})
+    out["derive_save"] = await _post(s, root + "config/save", {
+        "model": model, "fingerprint": m["fingerprint"]})
+    out["derive_writes"] = len(sup.writes)
+    model = copy.deepcopy(base)
     model["inverters"][1]["setpoint_entity"] = ""
     out["save"] = await _post(s, root + "config/save",
                               {"model": model, "fingerprint": m["fingerprint"]})
@@ -374,6 +396,9 @@ check("a model no blob can say is refused beside its field, in plain words",
       _r["derive"]["messages"] == [{
           "field": "price_provider", "card": None, "tier": "bad",
           "text": W["derive"]["no_source"]["description"]}])
+check("a save no blob can say answers 422 with its sentence and writes nothing",
+      _r["derive_save"][0] == 422 and _r["derive_writes"] == 0
+      and _r["derive_save"][1]["messages"] == _r["derive"]["messages"])
 check("a refused save answers 422 with the same placement and writes nothing",
       _r["save"][0] == 422 and ("inverters.setpoint_entity", 1)
       in _at(_r["save"][1]) and _sup.writes == [])

goodwe_controller/tools/ux13_demo.py

@@ -230,6 +230,8 @@ def controller(state):
     for inv, soc, batt in zip(c.inverters, (62.0, 35.0), (-2500.0, -300.0)):
         inv.soc, inv.batt = soc, (0.0 if state == "shadow" else batt)
         inv.dev_min, inv.dev_max = -5000.0, 5000.0
+        if state != "running":          # 0 W accepted: Home says "Held"
+            inv.last_write = 0.0
     c.grid = -10.0 if state != "shadow" else -2810.0
     c.pv_w = 3900.0
     if state == "alarm":