myenergi: add myenergi_automatic_zappi to gate the Zappi half of automatic config

[springfall2008]

This is an automated draft PR generated from issue #4996 — a maintainer should review it before merging.

Fixes #4996

Summary

MyEnergiComponent.automatic_config() wired two unrelated capabilities behind one flag, so an Eddi owner who charges their car with a different make of charger had no way to keep iboost_energy_today without also having their Zappi registered as a car. This adds myenergi_automatic_zappi, which gates only the Zappi half — car_charging_energy, car_charging_planned and car_charging_power. The Eddi branch is untouched and has no equivalent flag because it registers nothing.

Shape follows the existing ge_cloud_automatic_evc precedent (gecloud.py:435, components.py:162, config.py:2567, and the "Separate from ge_cloud_automatic because it registers a car" note in docs/components.md), including the warn-and-disable convention: enable_control() now refuses Zappi charge control when automatic_zappi is off and logs which setting is missing, exactly as it already does for myenergi_automatic.

It defaults to true, unlike ge_cloud_automatic_evc which defaults false. GECloud's EVC wiring was opt-in from the start; myenergi already wires Zappis under myenergi_automatic, so defaulting the new flag off would silently take that away from existing users on upgrade. The issue flagged this as a maintainer call — easy to flip if you would rather the two components match.

automatic	automatic_zappi	Result
false	(any)	No automatic wiring at all — unchanged
true	true (default)	Zappi and Eddi wired — unchanged
true	false	Eddi wired; no Zappi car inputs — the new case

Changed: myenergi.py (new initialize() parameter, gated Zappi branch, extended control check), components.py and config.py (new key and boolean validation), plus docs/components.md and docs/apps-yaml.md.

Deliberately a per-account boolean and not per-device selection — it cannot express "use Zappi A but not Zappi B". A serial allowlist would be the natural shape for that, but the issue argues against introducing two mechanisms without a demonstrated need.

Testing

• cd coverage && ./run_pre_commit — exit 0, all hooks Passed, quick suite green (4 slow tests skipped).
• tools/triage_test.sh myenergi — exit 0, whole myenergi suite passed.

Four new tests plus one extended case in apps/predbat/tests/test_myenergi.py:

• test_automatic_config_zappi_disabled_still_wires_the_eddi — the decisive case: a mixed Zappi+Eddi account with automatic_zappi: false still wires iboost_energy_today, contributes no Zappi car inputs, and leaves another charger's hand-written car_charging_energy untouched.
• test_automatic_config_zappi_disabled_on_a_zappi_only_site — a Zappi-only account wires nothing rather than a partial set.
• test_automatic_config_zappi_half_defaults_on — pins the true default on both the component attribute and the COMPONENT_LIST entry, so the behaviour-preserving choice cannot be changed silently.
• test_boost_still_works_with_the_zappi_half_disabled — the flag gates car wiring only; the manual Eddi boost path is unaffected.
• test_control_gating_refuses_with_a_reason — extended with automatic_zappi: false, asserting control stays disabled and the log names myenergi_automatic_zappi.

The existing test_component_registration asserts exact set equality between COMPONENT_LIST["myenergi"]["args"] and initialize()'s parameters, so the new argument is covered on both sides by a test that was already there.

Notes

Relationship to #4928 (charger registry, open) is as the issue describes: they compose rather than conflict. With the registry in place the implementation becomes "do not submit these Zappis to the registry" rather than "do not wire these entities", and the user-facing meaning is unchanged. This lands against main without creating a competing allocator.

Not covered here: the --no-automatic CLI harness flag in run_myenergi_cli() has no --no-automatic-zappi counterpart. The CLI passes automatic explicitly and picks up the new parameter's True default, so its behaviour is unchanged; adding a second flag looked like scope creep, but say the word if you want it for hand-testing.

[springfall2008]

Automated comment from the triage bot.

Review of PR #4997 (myenergi_automatic_zappi split)

Multi-angle review (10 finder angles + per-candidate verification + a final sweep, all checked against the actual diff). No blocking correctness bug found in the core wiring — registration, defaults, coercion, event routing and the Eddi branch were all verified clean. Findings ranked most-severe first; line numbers are post-PR positions.

1. Flag-off refusal never releases a held Zappi — apps/predbat/myenergi.py:918

The new enable_control() gate disables Zappi control without releasing a held Zappi. release_zappis() has exactly one caller (control_tick, myenergi.py:979), which is only reached under if self.control_active: in run() — and enable_control() returns before setting control_active. There is no release on component stop either (ComponentBase.api_stop only sets a flag), and a fresh instance starts with control_modes/control_saved_modes empty.

Failure scenario: user runs myenergi_zappi_control: true, Predbat holds the Zappi in 'Stopped' (outside a window) or 'Fast' (inside one). User sets myenergi_automatic_zappi: false; the component restarts (can_restart: True), the new warning logs, and the Zappi is never released. A Zappi left in 'Stopped' strands the car unable to charge until the user intervenes manually — exactly the walk-away outcome control_tick's release logic exists to avoid. The same hole exists for myenergi_automatic/myenergi_enable_controls, but this PR adds a new, more likely route into it (the flag is the natural thing to flip when a user wants auto-config to stop claiming their charger), and the updated docs ("Predbat logs which one and leaves the Zappi alone") now cover the stranding case. A release attempt (best-effort, logged) in the refusal path — or before returning when a Zappi was previously controlled — would close it for all four gates.

2. Flag-off skips Zappi wiring with zero log output — apps/predbat/myenergi.py:859

With the flag off, the skipped Zappi half of automatic_config() logs nothing: the three Info: myenergi: setting car_charging_* ... lines sit inside the if zappi_energy_entities: blocks, which stay empty, and the enable_control() warning only fires when myenergi_zappi_control is set.

Failure scenario: a Zappi owner sets the flag off. Zappi session energy silently stops being subtracted from house load (charging gets learnt as base load, corrupting the load model) and car detection falls back to the car_charging_threshold heuristic — with no Info line and no warning, so the intended change is indistinguishable from a silent regression. A single Info: myenergi: automatic_zappi off, skipping Zappi auto-wiring line would fix it.

3. Stale switch.predbat_myenergi_zappi_control entity lingers 'on' — apps/predbat/myenergi.py:1196

publish_data() gates the switch on control_active, which prevents creating it but nothing removes it: dashboard_item/set_state are upsert-only (output.py:3757, ha.py:1067) and no entity-removal path exists in the codebase. After a flag-off restart the entity lingers frozen at 'on' (control_enabled defaults on and is restored from storage), contradicting the publish_data comment that gating avoids "a switch reading on for a feature that cannot run". Pre-existing pattern for the other disable paths; the new flag inherits it.

4. Explicit-null apps.yaml value defeats the default True — apps/predbat/userinterface.py:287

myenergi_automatic_zappi: with an empty value (YAML null) bypasses the default: get_arg's self.args.get(arg, default) only applies the default when the key is absent, the bool-coercion branch requires isinstance(value, str), and validate_is_boolean(None) passes silently (predbat.py:1445-1456). The component stores None; both gates (... and self.automatic_zappi / not self.automatic_zappi) treat it as off. Pre-existing mechanism (myenergi_automatic behaves identically) — but this PR advertises a default-True boolean, so a scaffolded-but-empty key silently disables Zappi wiring and control with no warning and no arg_errors entry.

5. Quoted '1' in apps.yaml silently disables the flag — apps/predbat/userinterface.py:319

myenergi_automatic_zappi: '1' (or 'y') passes validate_is_boolean (anything bool()-able is accepted), but get_arg's boolean coercion only recognises ['on','true','yes','enabled','enable','connected'], so the component stores False — the feature silently turns off despite the user asking to turn it on. Pre-existing generic get_arg behaviour; unquoted 1 parses as int and stays truthy, so the trap is specifically quoted strings.

6. Guard order changes the logged reason for monitor-only sites — apps/predbat/myenergi.py:907

The new automatic_zappi guard sits before the enable_controls guard, so when both are off the reason logged changes: a monitor-only user (myenergi_enable_controls: false) who also sets the flag false is now told "needs myenergi_automatic_zappi" instead of "ignored while myenergi_enable_controls is off" — pointing them at a fix (turn the flag on) that still leaves control disabled.

7. Docs: switch-entity claim is now conditional — docs/apps-yaml.md:2055

The updated myenergi_zappi_control bullet still asserts "A switch.predbat_myenergi_zappi_control entity appears when this is set", but publish_data() only publishes it when control_active is True, which the new (and pre-existing) prerequisites suppress. Users setting the prerequisites off get no switch and only a log warning explains why.

8. Duplicate boost test — apps/predbat/tests/test_myenergi.py:1683

test_boost_still_works_with_the_zappi_half_disabled is near-verbatim test_boost_eddi_skips_mode_check (tree line ~1771: same Eddi device, same switch_event_handler call, same assertion) with only _make_component(automatic_zappi=False) differing — and the boost path never reads the flag. The name claims the "zappi half" is disabled but no Zappi boost / ZAPPI_BOOSTABLE_MODES refusal path is ever run with the flag off. Suggest deleting it or renaming it to what it actually tests.

9. Substring assertion is ambiguous — apps/predbat/tests/test_myenergi.py:1181

test_control_gating_refuses_with_a_reason asserts with substring containment, and 'myenergi_automatic' is a substring of the new 'myenergi_automatic_zappi' message — so if the first two gates were later merged/reordered, the ({'automatic': False}, 'myenergi_automatic') case would still pass via the wrong message. Asserting on 'needs myenergi_automatic to' (or a word-boundary check) makes it one-way strong.

Consolidation candidates (quality, non-blocking)

• Fourth copy of the warn-and-return gate (myenergi.py:918): the two consecutive myenergi guards are byte-identical except the config-key name, and the pattern already exists at gecloud.py:1469 and ohme.py:281 with wording already drifted. A loop over ('automatic', 'automatic_zappi') with the message derived from the failing key would collapse them; a ComponentBase helper would cover all components.
• Inline gate in the shared device loop (myenergi.py:859): and self.automatic_zappi gates collection by placement, not structure — the flag's meaning holds only because downstream wiring blocks are transitively empty. gecloud's split-function precedent (async_automatic_config_evc) is structural; a future Zappi-derived wiring landing outside the gated branch would silently bypass the flag.
• enable_control() docstring (myenergi.py:902) still documents a single prerequisite while the function now has a four-way gate.
• CLI harness gap (myenergi.py:1270): run_myenergi_cli has no --no-automatic-zappi equivalent, so the flag-off path isn't exercisable there (acknowledged in the PR body; the four unit tests cover it in CI).
• Rationale stated three times (myenergi.py:775): initialize() comment + automatic_config() docstring paragraph + docs; the gecloud precedent explains the same pattern in 3 lines.

Refuted during verification (for transparency)

• Docs-staleness claims for the zappi_control prerequisite sentences — both docs/apps-yaml.md and docs/components.md are updated in the PR.
• "Control can run without auto-config wiring" coupling — verified airtight today: controlled_zappis() is reachable only via control_charge/release_zappis, both gated on control_active, which only enable_control() sets.
• test_automatic_config_zappi_disabled_on_a_zappi_only_site subsumption and the standalone default-value test — judgement calls, left to the author.

---

Method: 10 finder angles (5 correctness, 5 quality) on the PR diff → dedup → per-candidate verification against the tree → final sweep phase. Findings 1, 4 and 5 are new trigger paths into pre-existing mechanisms rather than newly introduced bug classes, kept because the PR adds the routes and (for #1) newly documents the behaviour.

[springfall2008]

Automated comment from the triage bot.

Late-arriving sweep finding: auto-wired car_charging_* args survive a component-only restart with the flag off

Follow-up to my earlier review. One additional verified finding from the review's final sweep phase (verified directly against the tree after the review was posted):

Auto-wired args persist across Components.restart() — apps/predbat/myenergi.py:857

When a user flips myenergi_automatic_zappi to false and restarts the component only (web UI restart button, or the component auto-restarting after a health failure), the previously auto-wired values survive:

• set_arg_auto("car_charging_energy", ...) (myenergi.py:857) delegates to self.base.set_arg — the write lands in the shared base.args dict.
• Components.restart(only) (components.py:864-879) stops and re-creates only the component instance; base.args is never reset from apps.yaml.
• The new instance re-runs automatic_config() (_auto_configured=False, myenergi.py:1075), but with the flag off it wires only iboost_energy_today and never unsets the car_charging_energy/car_charging_planned/car_charging_power values the previous instance wrote. MyEnergiAPI has no stop() cleanup.

Failure scenario: an Eddi owner whose Zappi was registered as a car flips the flag and restarts the component; the Zappi keeps being subtracted from house load as car charging for the rest of the session — silently defeating the flag's purpose. Only a full Predbat restart (fresh process, args rebuilt from apps.yaml) clears it.

Suggested fix: in the flag-off path of automatic_config(), unset/blank the three auto-wired args (or clear them in stop()), so toggling the flag takes effect across a component-only restart.

Two earlier review angles reasoned "no stale wiring survives a restart" — that held only for a full process restart; the component-restart path is the gap. Finding #2 of the posted review (silent skip) compounds this: with the flag off and no log line, the user can't tell whether the flag took effect, was mis-typed, or the component failed its first poll.

[springfall2008]

This seems wrong as the new arg has not been added to the components argument list.

And the ask was for automatic=both on or individual flags for zappi/eddie.

[springfall2008]

Automated reply from the triage bot.

Both points checked against the branch; one correction and one change, pushed as 42f1780.

1. The arg is registered in both lists. myenergi_automatic_zappi is declared in COMPONENT_LIST["myenergi"]["args"] (apps/predbat/components.py:272) and in APPS_SCHEMA (apps/predbat/config.py:2682), and MyEnergiAPI.initialize() takes it (apps/predbat/myenergi.py:764). The PR's registration test asserts this in both directions: every COMPONENT_LIST arg must name an APPS_SCHEMA key, and COMPONENT_LIST args must exactly match initialize() parameters (apps/predbat/tests/test_myenergi.py:1537-1549) — it passes on the branch. These are the same two registration sites as ge_cloud_automatic_evc (components.py:163, config.py:2567), which the PR follows as precedent. If you meant a third list, name it and I'll add the entry.

2. Individual flags for zappi/eddi — implemented. myenergi_automatic_eddi (default true) now gates the Eddi half alongside myenergi_automatic_zappi, so automatic stays both-on by default and either device kind can be excluded on its own: myenergi_automatic_zappi: false wires the Eddi but not the Zappis, myenergi_automatic_eddi: false wires the Zappis but not the Eddi. myenergi_automatic: false still disables both. Tests cover the default, the Eddi-flag-off wiring (manual iboost_energy_today preserved, Zappi inputs wired), the Eddi-only site case, and the args/params alignment.

One note on your reply: if "This seems wrong" was about the earlier sweep finding (auto-wired car_charging_* args surviving a component-only restart with the flag off), that finding does not hinge on arg registration — set_arg_auto writes land in the shared base.args dict (component_base.py:97-101) and Components.restart() re-creates only the component instance without resetting base.args from apps.yaml (components.py:864-880), so a flag flip followed by a component-only restart leaves the old wiring in place until the next full Predbat restart. That mechanism is unchanged by this commit.

[Copilot]

🟡 Changes recommended

The implementation introduces an additional new flag (myenergi_automatic_eddi) that conflicts with the PR’s stated scope and should be either removed or explicitly reflected in the PR description/title/docs for clear, reviewable intent.

Once you've addressed the issues Copilot identified, you can request another Copilot review.

Pull request overview

This PR adjusts the myenergi component’s auto-wiring so Zappi EV-charger wiring can be disabled independently, preventing Zappis from competing with a user’s actual car charger while still allowing Eddi iboost_energy_today wiring.

Changes:

• Add a new myenergi_automatic_zappi configuration flag and use it to gate the Zappi portion of MyEnergiAPI.automatic_config() and Zappi charge-control prerequisites.
• Extend configuration plumbing (components.py args + config.py schema) and documentation to describe the new behaviour.
• Add/extend myenergi unit tests covering defaulting, mixed-device accounts, and control gating.

File summaries

File	Description
docs/components.md	Documents the new automatic gating behaviour and control prerequisites.
docs/apps-yaml.md	Adds myenergi_automatic_zappi (and related) configuration documentation.
apps/predbat/tests/test_myenergi.py	Adds test coverage for the new gating and control behaviour.
apps/predbat/myenergi.py	Implements the new gating in initialize(), automatic_config(), and enable_control().
apps/predbat/config.py	Adds schema validation for the new config keys.
apps/predbat/components.py	Wires the new args/config keys into the component registry.

Review details

• Files reviewed: 6/6 changed files
• Comments generated: 2
• Review effort level: Lite

---

💡 Add a code-review agent skill or configure MCP servers for context-aware, tailored reviews. Learn more in the docs.