UX-13 P4 review fixes (PR #146, comment 1887)

R1: first_start also needs a Store created at this boot (Store.fresh), so
    old never-started sites (both TEST-23 add-ons, one in shadow) stay on Home.
R2: a kettle skip after a fall says on the last step that the reading went down.
R3: the shadow switch goes only on the model the wizard posts, never into V.
R4: Home Assistant not answering is red on the last step.
R5: meter + solar + battery below -300 W flags a reversed battery sensor.
R6: the live checks say what to change and where.

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

goodwe_controller/app/static/settings.js

@@ -636,31 +636,37 @@
       if (k.sig !== sig) { k.before = null; k.ok = false; k.sig = sig; }
       const w = await readMeter(src);
       if (w == null) WZ.kettle = Object.assign({}, k, { sig, say: "unread" });
-      else if (k.before == null || k.ok) WZ.kettle = { sig, before: w };
+      else if (k.before == null || k.ok) WZ.kettle = { sig, before: w, wrong: !k.ok && !!k.wrong };
       else {
         const o = kettleOutcome(src, k.before, w);
-        WZ.kettle = { sig, before: o.say === "none" ? k.before : null, say: o.say, d: o.d, ok: o.sets != null };
+        // `wrong`: the reading went the way no option can fix. A skip after
+        // that keeps saying so on the last step (review R2).
+        const wrong = o.say === "none" ? !!k.wrong : o.sets == null;
+        WZ.kettle = { sig, before: o.say === "none" ? k.before : null, say: o.say, d: o.d, ok: o.sets != null, wrong };
         if (o.sets) { Object.assign(V, o.sets); queueValidate(); }
       }
       drawStep();
     });
     const row = h("div", { class: "row" }, read);
     if (!k.ok) row.append(h("button", { type: "button", class: "btn link", onclick: () => {
-      WZ.kettle = { sig: kettleSig(src), say: "skipped", ok: true, skipped: true }; drawStep();
+      WZ.kettle = { sig: kettleSig(src), say: "skipped", ok: true, skipped: true, wrong: !!k.wrong }; drawStep();
     } }, WS.kettle.skip));
     box.append(row);
     return box;
   }
 
   // Step 6: the shadow switch, the checks, and Save and restart.
-  function applyShadow() {
+  // The shadow switch goes only on the model the wizard posts, never into V,
+  // which the Installer view saves too (review R3).
+  function shadowValue() {
     const s = M.setup.shadow;
-    V[s.field] = WZ.shadowOn ? s.on : (ORIG[s.field] !== s.on ? ORIG[s.field] : s.off);
+    return WZ.shadowOn ? s.on : (ORIG[s.field] !== s.on ? ORIG[s.field] : s.off);
   }
+  function posted() { return Object.assign({}, V, { [M.setup.shadow.field]: shadowValue() }); }
   function shadowRow() {
     const lid = "wzs";
     const b = h("button", { type: "button", class: "switch", role: "switch", "aria-checked": String(WZ.shadowOn), "aria-labelledby": lid });
-    b.addEventListener("click", () => { WZ.shadowOn = !WZ.shadowOn; applyShadow(); queueValidate(); drawStep(); });
+    b.addEventListener("click", () => { WZ.shadowOn = !WZ.shadowOn; drawStep(); });
     return h("div", { class: "f inline" },
       h("div", {}, h("div", { class: "lbl", id: lid }, WS.shadow.name), h("div", { class: "help" }, WS.shadow.description)),
       h("div", { class: "ctl" }, b));
@@ -669,7 +675,7 @@
     const list = $("#wz-checks");
     if (list) list.replaceChildren(line("info", WS.check.running.description));
     clearTimeout(vtimer);
-    const b = await validate({ setup: true });
+    const b = await validate({ setup: true, model: posted() });
     WZ.checked = b;
     if (!list || !list.isConnected) return b;
     if (!b) { list.replaceChildren(line("bad", WS.check.failed.description)); return b; }
@@ -682,7 +688,7 @@
     if (b.device_note) add("info", b.device_note);
     const src = kettleSource();
     if (!src) add("info", WS.kettle.self.description);
-    else if (WZ.kettle && WZ.kettle.skipped) add("warn", WS.kettle.skipped.description);
+    else if (WZ.kettle && WZ.kettle.skipped) add("warn", (WZ.kettle.wrong ? WS.kettle.skipped_wrong : WS.kettle.skipped).description);
     else if (kettleOk()) add("ok", WS.kettle.tested.description);
     const rank = { bad: 0, warn: 1, ok: 2, info: 3 };
     items.sort((a, z) => rank[a[0]] - rank[z[0]]);
@@ -708,11 +714,11 @@
     const b = await runChecks();
     if (!b) { toast(WS.check.failed.description); return; }
     if (WZ.blocked) { toast(WS.check.blocked.description); return; }
-    const shadow = V[M.setup.shadow.field] === M.setup.shadow.on;
+    const shadow = shadowValue() === M.setup.shadow.on;
     ask(shadow ? WS.dialog : WS.dialog_live, async () => {
       wzBanner();
       let res;
-      try { res = await postSave({ setup: "done" }); } catch (e) { toast(W.toast.failed); return; }
+      try { res = await postSave({ setup: "done", model: posted() }); } catch (e) { toast(W.toast.failed); return; }
       const { r, b: body } = res;
       if (r.status === 409 && body.kind === "stale") { wzBanner("bad", W.stale.name, staleText(body)); return; }
       if (r.status === 422) {
@@ -758,7 +764,6 @@
     ROWS.length = 0;
     const body = [];
     if (n === 1 && !M.setup_needed) body.push(h("div", { class: "msg info" }, WS.done_before.description));
-    if (n === S.final) applyShadow();
     for (const f of allFields()) if (f.wizard === n && f.key !== S.shadow.field) body.push(fieldRow(f, null, "wz"));
     if (n === 1) body.push(kettleBox());
     if (n === 5) body.push(h("p", { class: "small muted" }, WS.modes_later.description));

goodwe_controller/app/store.py

@@ -585,6 +585,9 @@ class Store:
     def __init__(self, path: str = "/data/state.json"):
         self.path = path
         self.data = dict(DEFAULTS)
+        # UX-13 P4: True when this boot found no state file, i.e. a new /data.
+        # Only the setup wizard's first-run rule reads it (uipage.setup_needed).
+        self.fresh = False
         self.load()
 
     def load(self) -> None:
@@ -598,6 +601,7 @@ class Store:
                           self.data.get("phase"), self.data.get("last_completed"),
                           self.data.get("sign_tripped"))
         except FileNotFoundError:
+            self.fresh = True
             _LOG.info("no saved state, starting fresh")
         except (json.JSONDecodeError, OSError) as err:
             # A corrupt state file must not stop the controller: losing the

goodwe_controller/app/ui_text/en.json

@@ -191,6 +191,7 @@
         "wrong_dsmr": {"description": "The reading went down by {d}, so check that import and export are not swapped."},
         "self": {"description": "The add-on reads this meter itself, so Home shows its direction after the restart."},
         "skipped": {"description": "The meter direction is not tested, so check it on Home after the restart."},
+        "skipped_wrong": {"description": "The kettle test saw the meter reading go down, so check its direction in step 1 before you start control."},
         "tested": {"description": "The meter direction was tested with a kettle."},
         "needed": {"description": "Do the kettle test first, or skip it."}
       },

goodwe_controller/app/uipage.py

@@ -291,19 +291,19 @@ SETUP_DONE = "setup_done"
 STARTED = "auto"
 
 
-def setup_needed(stored: dict, store_data, started) -> str | None:
+def setup_needed(stored: dict, store_data, started, fresh=False) -> str | None:
     """Why the page opens the wizard by itself, or None.
 
     "no_power_request": the stored options name no inverter to control, so
-    the add-on cannot start on them. "first_start": this /data has never been
-    through the wizard and control was never started or stopped, which is a
-    new install: config.yaml ships `auto_start` off, so it comes up stopped.
-    A site that runs, or ever ran, control is never sent to the wizard.
+    the add-on cannot start on them. "first_start": a new install. This boot
+    found no Store file (`fresh`), the wizard never saved and control was never
+    started or stopped. `fresh` is what tells a new install from an old site
+    that was never started, e.g. one left in shadow for weeks (TEST-23).
     `store_data` is the Store's dict (None: unknown, so no), `started` is
     whether control is started now."""
     if not power_request_entities(stored or {}):
         return "no_power_request"
-    if store_data is None:
+    if store_data is None or not fresh:
         return None
     if store_data.get(SETUP_DONE) or STARTED in store_data or started:
         return None

goodwe_controller/app/validate.py

@@ -1782,6 +1782,9 @@ def _entity_notes(notes, opts: dict, entities: dict, bad=()) -> None:
 # --------------------------------------------------------------------------- #
 # Below this a battery or solar reading counts as idle for a sign check.
 IDLE_W = 50.0
+# Meter + solar + battery is the house's use, never below zero. Below minus
+# this, read seconds apart, a direction is reversed (a battery giving power).
+BALANCE_W = 300.0
 # The keys that pick the meter. A meter the add-on reads itself (HomeWizard,
 # MQTT) is only known live when the running options read the same one.
 _METER_KEYS = ("meter_source", "p1_host", "meter_mqtt_topic")
@@ -1848,33 +1851,50 @@ def setup_checks(opts: dict, *, entities: dict | None, running=None,
     """
     notes: list[Note] = []
     if entities is None:
-        _advise(notes, "warn", (), "Home Assistant is not answering, so "
-                "no entity was checked yet.")
+        # Red: only the list can tell a missing power request entity (SAFETY-77).
+        _advise(notes, "bad", ("setpoint_entity",), "Home Assistant is not "
+                "answering, so press Check again once it is back.")
     else:
         _entity_notes(notes, opts, entities, bad=("setpoint_entity",))
-    _meter_check(notes, opts, entities, running, meter_now_w)
-    for who, entry in _cards(opts):
-        _card_checks(notes, who, entry, entities)
+    meter = _meter_check(notes, opts, entities, running, meter_now_w)
+    batts = [_card_checks(notes, who, entry, entities)
+             for who, entry in _cards(opts)]
+    solar = []
     if entities is not None:
         flipped = pv.inverted(opts)
         for entity in pv.entities(opts):
             w, _unit = _reading(entities, entity, convert=True)
             if entity not in entities or w is None:
+                solar.append(None)
                 continue
             if w < -IDLE_W and entity not in flipped:
+                solar.append(None)
                 _advise(notes, "warn", ("pv_invert_entities",),
-                        f"{entity} reads below zero now, so its solar "
-                        "production sign looks reversed.")
+                        f"{entity} reads below zero now, so turn on its "
+                        "“Reads negative” switch in the Solar step.")
             else:
                 w = -w if entity in flipped else w
+                solar.append(w)
                 _advise(notes, "ok", ("pv_entity",),
                         f"{entity} reads {_signed(w)} of solar production "
                         "now.")
+    # A battery sensor that reads the wrong way while giving power makes the
+    # house use less than nothing. Only with every term known: a missing
+    # solar reading could be the difference.
+    known = [meter] + batts + solar
+    if solar and batts and all(w is not None for w in known) \
+            and sum(known) < -BALANCE_W:
+        _advise(notes, "warn", ("batt_invert",),
+                f"The readings add up to {_signed(sum(known))} of house "
+                "use, so check each "
+                "battery's “Battery power sign is reversed” switch in step 2.")
     _price_check(notes, opts, running, prices_known)
     return notes
 
 
-def _meter_check(notes, opts, entities, running, meter_now_w) -> None:
+def _meter_check(notes, opts, entities, running, meter_now_w):
+    """The meter line. Returns the grid reading in W (positive: taken from
+    the grid) as the add-on will read it, or None when it is not known."""
     source = str(opts.get("meter_source") or "off").strip()
     if source == "off":
         parts = [(opts.get("meter_entity"), 1.0)]
@@ -1888,28 +1908,29 @@ def _meter_check(notes, opts, entities, running, meter_now_w) -> None:
             running.get(k) == opts.get(k) for k in _METER_KEYS)
         if same and meter_now_w is not None:
             _meter_line(notes, float(meter_now_w))
-        else:
-            _advise(notes, "info", ("meter_source",), "The add-on reads "
-                    "this meter itself, so Home shows its reading after "
-                    "the restart.")
-        return
+            return float(meter_now_w)
+        _advise(notes, "info", ("meter_source",), "The add-on reads "
+                "this meter itself, so Home shows its reading after "
+                "the restart.")
+        return None
     if entities is None:
-        return
+        return None
     total = 0.0
     for entity, sign in parts:
         entity = str(entity or "").strip()
         if not entity or entity not in entities:
-            return              # a refusal or a missing-entity note says so
+            return None         # a refusal or a missing-entity note says so
         w, text = _reading(entities, entity)
         if w is None:
             _advise(notes, "warn", ("meter_source",),
-                    f"{entity} reads {text} now, so the meter has no "
-                    "reading yet.")
-            return
+                    f"{entity} reads {text} now, so check it in Home "
+                    "Assistant before you start.")
+            return None
         total += sign * w
     if source == "off" and opts.get("meter_invert"):
         total = -total
     _meter_line(notes, total)
+    return total
 
 
 def _meter_line(notes, w) -> None:
@@ -1922,9 +1943,11 @@ def _meter_line(notes, w) -> None:
     _advise(notes, "ok", ("meter_source",), text)
 
 
-def _card_checks(notes, who, entry, entities) -> None:
+def _card_checks(notes, who, entry, entities):
+    """The checks for one inverter's card. Returns its battery power in W,
+    positive while giving power, or None when it is not known."""
     if entities is None:
-        return
+        return None
     setpoint = str(entry.get("setpoint_entity") or "").strip()
     if setpoint and setpoint in entities:
         _advise(notes, "ok", ("setpoint_entity",),
@@ -1933,7 +1956,8 @@ def _card_checks(notes, who, entry, entities) -> None:
     soc = str(entry.get("soc_entity") or "").strip()
     if not soc:
         _advise(notes, "warn", ("soc_entity",), f"There is no battery "
-                f"level entity{who}, so the add-on holds 0 W.")
+                f"level entity{who}, so pick one in step 2 or the add-on "
+                "holds 0 W.")
     elif soc in entities:
         level, _unit = _reading(entities, soc)
         if level is None or not 0.0 <= level <= 100.0:
@@ -1948,7 +1972,7 @@ def _card_checks(notes, who, entry, entities) -> None:
         if w is None:
             _advise(notes, "warn", ("batt_entity",), f"The battery "
                     f"power{who} reads nothing now, so check {batt}.")
-            return
+            return None
         w = -w if entry.get("batt_invert") else w
         if abs(w) < IDLE_W:
             _advise(notes, "info", ("batt_entity",), f"The battery{who} is "
@@ -1958,6 +1982,8 @@ def _card_checks(notes, who, entry, entities) -> None:
             _advise(notes, "info", ("batt_invert",),
                     f"The battery{who} reads {messages.watts(w)} {doing} "
                     "now, so check the inverter shows the same.")
+        return w
+    return None
 
 
 def _price_check(notes, opts, running, prices_known) -> None:

goodwe_controller/app/web.py

@@ -199,7 +199,9 @@ def build_app(controller, selfcfg=None) -> web.Application:
             # UX-13 P4: the setup wizard, and why it opens by itself.
             "setup": uipage.setup_spec(),
             "setup_needed": uipage.setup_needed(
-                options, store_data(), bool(getattr(controller, "auto", False))),
+                options, store_data(), bool(getattr(controller, "auto", False)),
+                fresh=getattr(getattr(controller, "store", None), "fresh",
+                              False) is True),
         })
         return web.json_response(model)