Distrust stale current readings; measure runtime from flow

.claude/commands/commit_pr.md

@@ -4,8 +4,21 @@ description: Run pre-commit hooks to a clean state, commit, push, open a PR, mer
 
 Do everything the `commit` command does, then take the change all the way through a merged PR and back to a clean `master`.
 
+Every step below operates on **this repo only** (`nuttytree/ESPHome-Devices`, default branch `master`). Never push to, or open a PR against, `esphome/esphome` or any other upstream — the sibling `dev_esphome/` and `esphome.io/` working directories are reference checkouts of other people's projects, not targets for this workflow.
+
 1. Follow the `commit` command's steps 1–5 in full: branch check/creation (`f/`/`b/`/`chore/` naming, never commit on `master`), `python -m pre_commit run --all-files` to a clean state, the commit itself (with the `Co-Authored-By: Claude <noreply@anthropic.com>` trailer), and the push (`git push` / `git push -u origin <branch>`). Never skip hooks, never work around a real hook failure.
-2. Open the PR using the `pr-workflow` skill so it follows this repo's established PR conventions (title, body, labels, etc.) rather than reinventing them here.
+2. Open the PR **against this repo only** — `nuttytree/ESPHome-Devices`, base branch `master`:
+
+   ```
+   gh pr create --base master --title "<title>" --body-file -
+   ```
+
+   > **Do not use the `pr-workflow` skill here, and never target `esphome/esphome`.** That skill lives in the sibling `dev_esphome/` checkout (upstream ESPHome core, wired in only for IntelliSense — see CLAUDE.md) and describes how to contribute *to upstream*: it says to base on `upstream/dev`, fill in ESPHome's `.github/PULL_REQUEST_TEMPLATE.md`, prefix the title with `[component]`, and run `gh pr create --repo esphome/esphome --base dev`. All of that is wrong for this repo. It surfaces as an available skill purely because `dev_esphome/` is an additional working directory. Ignore it; the conventions below are this repo's.
+
+   This repo has no PR template and uses no labels. Match the style of recent merged PRs (`gh pr list --state merged --limit 5`, then `gh pr view <n> --json body`):
+   - **Title**: sentence-case description of the change, no bracket/component prefix (e.g. "Fix pump anomaly detection: startup window, latching, drift, variance").
+   - **Body**: a prose statement of the problem and why it mattered, then a `Changes:` bullet list of what was actually done. Reference related PRs/issues with `#<n>` where relevant, and call out anything that silently changes behaviour on upgrade (e.g. persisted state being discarded).
+   - Note how the change was verified (`esphome config <device>.yaml`, or an `esphome compile` if the C++ changed).
 3. Merge the PR: this repo only allows squash merges (`allow_squash_merge: true`; `allow_merge_commit`/`allow_rebase_merge` are both `false`), so merge with `gh pr merge <number> --squash`. `master` has no required status checks configured, so the merge should go through immediately — if `gh` reports it can't merge (conflicts, an unexpected required check, etc.), stop and report rather than forcing it. Don't pass `--delete-branch`: the repo already has `delete_branch_on_merge` enabled, so GitHub deletes the remote branch itself on merge.
 4. Switch back to `master`: `git checkout master`.
 5. Sync `master`: `git pull` (should fast-forward cleanly since nothing else is committed directly to `master`).

CLAUDE.md

@@ -53,7 +53,11 @@ Installed via `pipx install esphome`, which creates its own isolated virtualenv
 
 **2. Build to a local drive, not the UNC share.** This repo lives on `\\nas\docker\esphome\config`, and `packages/device_base.yaml` sets `build_path: ./build/${device_id}`, which resolves onto that share. `cmd.exe` cannot use a UNC path as a working directory, so ninja's *link* step runs from `C:\Windows` and fails with `ld.exe: cannot open output file ...: Permission denied`. The compile steps succeed first, which makes this read as a linker bug rather than a path problem. Override `build_path` to a local path (e.g. `C:/eh/bld/<name>`) when compiling from Windows. Source files can stay on the share — gcc handles UNC paths fine; only the working directory is the problem.
 
-To compile-check a component without disturbing the container's build caches, copy the device YAML to a throwaway `device_id` with a local `build_path`, compile that copy, then delete both the scratch YAML and its build directory. A full IDF build takes roughly 10–15 minutes.
+**3. Don't run any other `esphome` command while a compile is in progress.** Even `esphome config` rewrites state under `.esphome/`, and because that directory is shared with the build, cmake sees a changed input and re-runs configure mid-build. The initial configure succeeds off the resolved dependency lock, but a *re-run* re-resolves component manifests and dies on `.esphome/pio_components/idf/*/esphome/noise-c/idf_component.yml`, which contains `override_path: /config/.esphome/...` — a container path, written by the Docker ESPHome where this directory is mounted at `/config`. Windows resolves it against the UNC root as `\\nas\docker\config\...`, which doesn't exist, and the build dies with `The "override_path" field ... does not point to a directory`. That cache is gitignored and regenerable, so leave it alone rather than "fixing" it — the container needs those paths. Just validate before you build, then stay off the config directory until it finishes.
+
+Similarly, don't kill a compile during its managed-component download: a partial `lvgl/lvgl` left without its `.component_hash` fails every later build with `File .component_hash or CHECKSUMS.json ... does not exist or cannot be parsed`. Delete the build directory and start over.
+
+To compile-check a component without disturbing the container's build caches, copy the device YAML to a throwaway `device_id` with a local `build_path`, compile that copy, then delete both the scratch YAML and its build directory. A full IDF build takes roughly 10–15 minutes. Note that `esphome config` validates schema and lambdas' *syntax* but does not compile lambda C++ — only a real compile catches errors inside a `lambda:` block.
 
 ## Git workflow
 

components/pool_controller/README.md

@@ -79,17 +79,19 @@ Accepts all standard [ESPHome Switch options](https://esphome.io/components/swit
     * **end_time** (Required, time): Window end on the hour; use `'24:00'` for midnight end-of-day.
     * **minutes_per_hour** (Required, int, 1–60): How many minutes out of each 60 the pump should run within this window.
     * **days_of_week** (Optional): Days this runtime applies to (e.g. `MON-FRI`, `SAT,SUN`). Defaults to every day.
-* **current_sensor** (Optional, id): ID of a sensor reporting current draw in amps. Required when `on_anomaly` is configured. When set, turning the switch on always publishes an on state and energizes the output, but the pump's runtime is only accumulated while current confirms the motor is actually running.
+* **current_sensor** (Optional, id): ID of a sensor reporting current draw in amps. Required when `on_anomaly` is configured. Turning the switch on always publishes an on state and energizes the output; current then confirms whether the motor is actually turning, which drives the anomaly startup window, the no-current sensor, and the flow-loss watchdog. It also accumulates runtime, but only when there is no `flow_sensor` — see [Runtime Accounting](#runtime-accounting).
 * **current_on_threshold** (Optional, float, default: `0.5`): Current in amps above which the pump is considered running (used whenever `current_sensor` is configured).
-* **no_current_binary_sensor** (Optional): Problem binary sensor, automatically added whenever `current_sensor` is configured. Turns on whenever the switch is commanded on but no current is detected (e.g. a manual override switch has the pump physically off). Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> No Current`.
+* **no_current_binary_sensor** (Optional): Problem binary sensor, automatically added whenever `current_sensor` is configured. Turns on whenever the switch is commanded on but no current is detected (e.g. a manual override switch has the pump physically off). Suppressed while the current reading is stale — see [Stale Current Readings](#stale-current-readings). Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> No Current`.
+* **current_timeout** (Optional, Time, default: `30s`): How long `current_sensor` may go without publishing before its last value stops being treated as evidence. See [Stale Current Readings](#stale-current-readings).
+* **current_stale_binary_sensor** (Optional): Problem binary sensor, automatically added whenever `current_sensor` is configured. Turns on when the current sensor has published nothing for `current_timeout`, meaning its readings are frozen rather than merely low. Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> Current Sensor Stale`.
 * **anomaly_detection_switch** (Optional): A Switch entity, created whenever `current_sensor` is configured, that enables or disables anomaly detection at runtime. Supports all standard [ESPHome Switch options](https://esphome.io/components/switch/index.html); restores to on by default and defaults to `<pump name> Anomaly Detection`.
 * **anomaly_status_binary_sensor** (Optional): Problem binary sensor, created whenever `current_sensor` is configured, that turns on when a current-draw anomaly is currently active. Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> Anomaly Status`.
 * **anomaly_baseline_reset_button** (Optional): A Button entity, created whenever `current_sensor` is configured, that discards the learned current baseline (steady state, drift, variance, startup inrush) and clears any latched anomaly. The press is never refused and can be made at any time; relearning does not start immediately but at the pump's next turn-on, so the new baseline is always built from a complete run — see [Baseline Reset](#baseline-reset). Supports all standard [ESPHome Button options](https://esphome.io/components/button/index.html); defaults to `<pump name> Reset Anomaly Baseline`.
 * **anomaly_reason_text_sensor** (Optional): Text sensor, created whenever `current_sensor` is configured, publishing the reason string of the most recent anomaly — lets a single `anomaly_status_binary_sensor` on-state be disambiguated in the UI/history. Supports all standard [ESPHome Text Sensor options](https://esphome.io/components/text_sensor/index.html); defaults to `<pump name> Anomaly Reason`.
 * **anomaly_threshold_pct** (Optional, int 1–100, default: `10`): Percentage deviation from baseline required to trigger an anomaly event.
 * **learning_samples** (Optional, int 10–10000, default: `200`): Number of steady-state current samples to collect before the baseline is locked.
 * **on_anomaly** (Optional, automation): Automation triggered when an anomaly is detected. The trigger variable `x` is a string describing the anomaly: `CURRENT_HIGH`, `CURRENT_LOW`, `NO_STARTUP_SPIKE`, `BASELINE_DRIFT` (either direction — rising suggests bearing wear, falling suggests prime/flow loss), or `VARIANCE_SPIKE` (current spread has grown well beyond its learned baseline, often the earliest sign of trouble).
-* **flow_sensor** (Optional, id): ID of a binary sensor that detects water flow. If no `current_sensor` is configured, the pump's runtime is only accumulated while flow is actually detected. If a `current_sensor` is configured, runtime is controlled by current instead (see above), and the pump is additionally shut down if current confirms the motor is running but flow is lost for longer than `flow_timeout`. Without a `current_sensor`, no-flow alone never shuts the pump down — a manual override switch physically holding the pump off would otherwise look identical to a real no-flow fault. When both sensors are configured, flow also gates anomaly statistics — see [Current-based Anomaly Detection](#current-based-anomaly-detection).
+* **flow_sensor** (Optional, id): ID of a binary sensor that detects water flow. Whenever it is configured it owns runtime accounting, current sensor or not — see [Runtime Accounting](#runtime-accounting). If a `current_sensor` is also configured, the pump is additionally shut down when current confirms the motor is running but flow is lost for longer than `flow_timeout`. Without a `current_sensor`, no-flow alone never shuts the pump down — a manual override switch physically holding the pump off would otherwise look identical to a real no-flow fault. Flow also gates anomaly statistics — see [Current-based Anomaly Detection](#current-based-anomaly-detection).
 * **flow_timeout** (Optional, Time, default: `2s`): How long flow must be absent (while a current sensor confirms the motor is running) before the pump is shut down.
 * **flow_loss_binary_sensor** (Optional): Problem binary sensor, automatically added whenever both `current_sensor` and `flow_sensor` are configured. Latches on when the pump is shut down due to lost flow, and only clears once the pump is on, current confirms the motor is running, and flow is detected again. Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> Flow Loss`.
 * **unexpected_flow_binary_sensor** (Optional): Problem binary sensor, automatically added whenever `flow_sensor` is configured. Turns on when flow is detected while the pump is commanded off for longer than `flow_timeout` (e.g. a stuck valve, a neighboring pump pushing water through this branch, or a manual override running the pump), and clears as soon as flow stops or the pump turns on. Supports all standard [ESPHome Binary Sensor options](https://esphome.io/components/binary_sensor/index.html); defaults to `<pump name> Unexpected Flow`.
@@ -149,5 +151,27 @@ Capture is then *armed rather than started*. Nothing is sampled until the pump n
 
 Relearning then follows the normal path: the first run seeds the startup reference, `NO_STARTUP_SPIKE` checks resume once three starts have accumulated, and the steady-state baseline locks after `learning_samples` valid samples.
 
+### Runtime Accounting
+Two different questions get asked about a running pump, and they want different answers:
+
+* **How long did the pump do useful work?** Runtime, which the fractional-runtime scheduler spends against `minutes_per_hour`. Moving water is what a pool schedule is actually buying, so this is measured from `flow_sensor` whenever one is configured, falling back to `current_sensor`, and finally to the output command. Flow is also the more robust source: a current sensor that stalls while reading `0 A` would otherwise stop the clock on a pump that is running perfectly well.
+* **When was the motor energised?** The anchor for the anomaly startup window, the auxiliary `sequence_delay`, and the heater's warm-up gate. This is measured from `current_sensor` whenever one is configured, because flow lags the motor by a second or more — easily long enough to miss the inrush peak the startup check exists to measure.
+
+The two are tracked independently. Between the output being commanded on and the first sensor confirming the motor, the turn-on timestamp reads as "no run time yet" rather than as an elapsed time, so nothing that gates on it fires early.
+
+### Stale Current Readings
+An ESPHome sensor keeps serving its last value indefinitely when its source stops responding — there is no outward difference between a live reading and a frozen one. A meter that has gone silent while reporting `0 A` is byte-identical to a pump that genuinely isn't drawing current, and everything downstream believes it: the no-current sensor reports a fault that isn't happening, runtime stops accumulating for a pump that is actually running, and anomaly detection measures a flat line.
+
+Each pump therefore timestamps every publish from its `current_sensor`. If nothing arrives for `current_timeout`, the reading is declared stale and:
+
+* `current_stale_binary_sensor` turns on, so the real fault — a dead sensor — is visible and distinguishable in history from a pump fault.
+* `motor_running` is no longer recomputed. The last known state is held rather than inferred from a frozen number, so runtime tracking neither invents a stop nor invents a start.
+* `no_current_binary_sensor` is suppressed. "Commanded on but drawing nothing" is a claim about the pump, and a sensor that stopped publishing supports no such claim.
+* Anomaly detection stops sampling, since every sample would be the same stale number.
+
+When readings resume, the first value is a step change away from whatever was frozen — potentially a very large one. Feeding that straight into the detector reads as an enormous deviation and sets the variance trip chattering while the estimate re-converges, so the out-of-band debounce streak and the live spread estimate are both cleared on recovery. A run whose startup window contained a gap also has its inrush peak discarded (logged as a warning) rather than averaged into the startup reference, since the window was only partly observed.
+
+Note that the flow-loss watchdog still acts on the held `motor_running` state. That is deliberate: dry-running is the expensive failure, so a stale current reading should not disarm the protection that stops it.
+
 ### Flow Sensor Protection
 If a `flow_sensor` is configured and reports no flow for longer than `flow_timeout` while the pump output is commanded on, the pump is shut down to protect against dry-running or blockage.

components/pool_controller/__init__.py

@@ -43,7 +43,7 @@
         cv.Optional(
             CONF_STATE_SAVE_INTERVAL, default="5min"
         ): cv.positive_time_period_milliseconds,
-        cv.Optional(CONF_MAX_RESUME_AGE, default="15min"): cv.All(
+        cv.Optional(CONF_MAX_RESUME_AGE, default="10min"): cv.All(
             cv.positive_time_period_seconds,
             cv.Range(min=cv.TimePeriod(seconds=1)),
         ),

components/pool_controller/auxiliary_pump_switch.cpp

@@ -35,9 +35,14 @@ void AuxiliaryPumpSwitch::write_state(bool state) {
     return;
   }
 
+  if (state)
+    this->arm_new_run_();
   this->output_->set_state(state);
   this->publish_state(state);
   if (!this->has_current_sensor() && !this->has_flow_sensor()) {
+    // No sensors at all — the command is the only evidence available for either job.
+    if (state)
+      this->turned_on_ms_ = millis_64();
     this->track_runtime(state);
   } else if (!state) {
     // Output commanded off — stop runtime counting immediately regardless of sensor confirmation.

components/pool_controller/const.py

@@ -24,6 +24,8 @@
 
 CONF_CURRENT_ON_THRESHOLD = "current_on_threshold"
 CONF_NO_CURRENT_BINARY_SENSOR = "no_current_binary_sensor"
+CONF_CURRENT_TIMEOUT = "current_timeout"
+CONF_CURRENT_STALE_BINARY_SENSOR = "current_stale_binary_sensor"
 
 CONF_FLOW_SENSOR = "flow_sensor"
 CONF_FLOW_TIMEOUT = "flow_timeout"

components/pool_controller/primary_pump_switch.cpp

@@ -52,9 +52,14 @@ void PrimaryPumpSwitch::write_state(bool state) {
     }
   }
 
+  if (state)
+    this->arm_new_run_();
   this->output_->set_state(state);
   this->publish_state(state);
   if (!this->has_current_sensor() && !this->has_flow_sensor()) {
+    // No sensors at all — the command is the only evidence available for either job.
+    if (state)
+      this->turned_on_ms_ = millis_64();
     this->track_runtime(state);
   } else if (!state) {
     // Output commanded off — stop runtime counting immediately regardless of sensor confirmation.