Skip to content

Compose EV chargers across components via a registry - #4928

Open
mgazza wants to merge 3 commits into
mainfrom
feat/charger-registry
Open

Compose EV chargers across components via a registry#4928
mgazza wants to merge 3 commits into
mainfrom
feat/charger-registry

Conversation

@mgazza

@mgazza mgazza commented Sep 4, 2026

Copy link
Copy Markdown
Collaborator

The problem

Predbat has no charger concept — only a flat, index-keyed list of "cars" (car_n, num_cars) where each index is really a car-charger pair. Five integrations autoconfigure into the same shared args, and they collide in two distinct ways:

1. List replacement. Gateway, GivEnergy Cloud, Ohme and myenergi each assign their own discovered list to car_charging_planned / _energy / _power. The last component to run wins; every earlier component's chargers vanish from the plan. _register_ev_car() in gateway.py admits it in its own comment: "we overwrite the first charger only; multi-charger support needs work".

2. num_cars is a max, not a sum. Even the raise-only discipline in gecloud.py — whose comment correctly worries about "another component [that] may already have registered cars of its own" — cannot compose: one GivEnergy charger plus one Ohme charger yields num_cars = 1, and both claim slot 0.

The change

apps/predbat/charger_registry.py. Components stop assigning car_charging_* directly and call register_chargers(source, entries) — an atomic, idempotent per-source replace that covers add, update and removal. The registry keys chargers on (source, device_id), allocates a slot each, and materialises the flat args once.

It also exposes slot_for(source, device_id), and that is why the control loops are in this PR too. Every one of them previously assumed its own charger was car 0, or that its Nth charger was car N — myenergi.controlled_zappis()'s docstring stated the invariant outright ("Zappi N is the same car as auto-config's Nth"). Reallocating slots without fixing those loops would start or stop a physical charger from another car's plan, so discovery and control are migrated together for all four components.

Behaviour changes to review carefully

  • num_cars is no longer raise-only. It is derived from registered chargers and floored at any value an external source claims, via a new set_external_num_cars() that Octopus and Kraken call for their IOG car counts. It can decrease when a charger source empties. docs/car-charging.md updated accordingly.
  • Charger N is no longer car N. Slots are allocated across all components by (source, device_id) sort, legacy slots first. Adding a source renumbers later chargers, and per-car apps.yaml settings are slot-addressed, so they follow the position rather than the car. Pinned by test_adding_an_earlier_sorting_charger_shifts_later_slots.
  • Hand-written car_charging_* config now composes with autodiscovery instead of being overwritten — pre-registered as legacy slots, preserved verbatim (interior holes included), with discovered chargers appended after. Installs on the stock template are unchanged: an unresolved re: entry is treated as unconfigured.
  • set_arg_auto's apps.yaml-override note ([Solis Cloud] batteryHealthSoh of 0 from the API silently zeroes soc_max and causes unbounded grid charging #4494 / fix(solis): guard battery_scaling against a documented 0% SOH API response #4500) no longer fires for charger keys, because config is now merged rather than discarded — the note would mislead.
  • gateway_evc_control without gateway_evc_automatic now warns once rather than silently doing nothing. The initialize() docstring already declared control requires the automatic flag; nothing enforced or reported it.
  • EV commands now carry charge_point_id. Older gateway firmware falls back to firstConnected(), so it degrades to the previous behaviour rather than misrouting.

Known limitations (documented, not fixed here)

  • The gateway's 6-character entity slug is not collision-proof (its own docstring says so). Two charge points with matching slug tails now get two car slots sharing one entity namespace.
  • alphaess.py still writes car_charging_energy / _power directly and is not migrated — last-writer-wins with the registry on those two keys, as before this PR.
  • Component restart/disable leaves stale registrations, as it left stale args before.
  • Legacy duplicate detection matches a myenergi/gecloud serial inside a legacy entity id with alphanumeric boundaries; it cannot catch a third-party integration whose entity naming shares no serial.

Testing

  • 57 registry tests, registered in unit_test.py's TEST_REGISTRY so they run under --quick.
  • Regressions added to the ohme, myenergi, ge_cloud, gateway, kraken and octopus suites — including the composition case (two components → two distinct slots, neither erasing the other) that no test covered before.
  • A compatibility test asserting the registry is a no-op on legacy-only config, and one exercising the real auto_config() regex-resolution sequence before pre-registration.
  • unit_test.py --quick green: charger_registry, ohme, myenergi, ge_cloud, gateway, kraken, octopus_misc, web_if all PASSED.

Predbat has no charger concept - only a flat, index-keyed list of "cars"
(car_n, num_cars) where each index is really a car-charger pair. Five
integrations autoconfigure into the same shared args, and they collide.

Two distinct defects:

1. List replacement. Gateway, GivEnergy Cloud, Ohme and myenergi each
   assign their OWN discovered list to car_charging_planned/_energy/
   _power. The last component to run wins; every earlier component's
   chargers vanish from the plan.

2. num_cars is a max, not a sum. Even the raise-only discipline in
   gecloud cannot compose: one GivEnergy charger plus one Ohme charger
   yields num_cars=1, and both claim slot 0.

This adds apps/predbat/charger_registry.py. Components stop assigning
car_charging_* directly and call register_chargers(source, entries) - an
atomic, idempotent per-source replace. The registry keys chargers on
(source, device_id), allocates a slot each, and materialises the flat
args once.

It also exposes slot_for(source, device_id), and that is why the control
loops are here too. Every one of them previously assumed its own charger
was car 0, or that its Nth charger was car N (myenergi's
controlled_zappis docstring stated the invariant outright). Allocating
slots across components without fixing those loops would start or stop a
physical charger from another car's plan, so discovery and control are
migrated together for all four components.

Slot identity is carried through to the plan. A charger reassigned to a
different slot could otherwise consume the previous occupant's published
plan, so the registry's allocation version travels from configuration
fetch through to plan publication and every control loop waits for a
plan built from the current mapping. Each car's published window list is
also kept separate; it was shared across loop iterations, so later cars
inherited earlier cars' windows.

Behaviour changes worth calling out in review:

- num_cars is no longer raise-only. It is derived from registered
  chargers and floored at any value an external source claims, via a new
  set_external_num_cars() that Octopus and Kraken use for their IOG car
  counts. It can decrease when a charger source empties.
- Charger N is no longer car N. Slots are allocated across all
  components by (source, device_id) sort, legacy slots first. Adding a
  source renumbers later chargers, and per-car apps.yaml settings are
  slot-addressed, so they follow the position.
- Hand-written car_charging_* config now composes with autodiscovery
  instead of being overwritten: it is pre-registered as legacy slots,
  preserved verbatim (interior holes included), and discovered chargers
  append after it. Installs on the stock template are unchanged - an
  unresolved "re:" entry is treated as unconfigured.
- Measurements are composed rather than clobbered. Ohme keeps its energy
  and power contribution when another component registers first or
  discovery repeats, and legacy and discovered measurements of the same
  charger are counted once, matched on exact entity or a bounded
  manufacturer serial.
- set_arg_auto's apps.yaml-override note no longer fires for charger
  keys, because config is merged rather than discarded.
- gateway_evc_control without gateway_evc_automatic now warns once
  rather than silently doing nothing.
- EV commands carry charge_point_id. Older gateway firmware falls back
  to firstConnected(), so it degrades to the previous behaviour.

Known limitations, documented rather than fixed here: the gateway's
6-character entity slug is not collision-proof, so two charge points
with matching slug tails share an entity namespace; alphaess still
writes car_charging_energy/_power directly and is not migrated;
component restart leaves stale registrations, as it left stale args
before.

Tests: 62 registry regressions registered in unit_test.py's
TEST_REGISTRY so they run under --quick, plus regressions in the ohme,
myenergi, ge_cloud, gateway, kraken and octopus suites covering each
control loop waiting after reassignment and rejecting a plan published
for an older allocation.
@mgazza
mgazza force-pushed the feat/charger-registry branch from 0b7720e to 569f325 Compare September 5, 2026 00:24
@mgazza

mgazza commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Updated: the review fixes that were in #4934 are now folded into this PR (569f3255), and the branch is rebased onto current main. #4934 is superseded and can be closed. The fold adds slot identity carried through to plan publication (a charger reassigned to a new slot could otherwise act on the previous occupant's plan), per-car window list isolation, and composed rather than clobbered measurements.

Overlap worth deciding before either lands: #4880 addresses car_charging_power double-counting when two cars share one charger, by adding a num_chargers config key. This PR reaches the same symptom from the other side — car_charging_power is composed by the registry, and duplicate chargers are matched on exact entity or a bounded manufacturer serial, so a repeated charger is counted once without new configuration.

They aren't incompatible, but merging both as-is would leave two mechanisms for one problem, and num_chargers would become a second source of truth alongside the registry's allocation. Happy to rework this PR on top of #4880 if you'd prefer that direction, or to drop the dedupe here if num_chargers is the shape you want — your call on which is the primary.

@springfall2008

Copy link
Copy Markdown
Owner

Thanks for this — the core design holds up well. Identity on (source, device_id), deterministic sort-based allocation, per-source atomic replace, explicit external claims for Octopus/Kraken, and the generation/confirm handshake are all the right shape, and the control-loop migration looks complete (I checked _ev_charging_active and _configured_ev_chargers in gateway, and Ohme's enable_control already requires ohme_automatic so its new slot is None early-return can't strand a controllable charger).

Your test selection is green on the branch for me too. The findings below are things the tests don't cover; I reproduced each by driving the registry directly.


1. Hand-written car_charging_soc / car_charging_now are silently deleted 🔴

This is the one that needs fixing before merge.

The all-or-omit rule at charger_registry.py:444-451 treats any component slot with no value as an unrepresentable gap and nulls the whole key. But myenergi supplies neither soc nor now, Ohme supplies no now, and gateway supplies no now when gateway_evc_control is on. Before this PR none of those components ever wrote those keys, so a user's apps.yaml value survived untouched.

User with car_charging_soc: [sensor.my_car_soc] and car_charging_now: [binary_sensor.my_car_charging], plus a Zappi discovered:

Warn: ChargerRegistry: not setting car_charging_soc - no value for [('myenergi', '10000001')]
Warn: ChargerRegistry: not setting car_charging_now - no value for [('myenergi', '10000001')]

  car_charging_planned = ['binary_sensor.my_car_plugged', 'sensor.predbat_myenergi_zappi_10000001_plug_status']
  car_charging_soc     = <ABSENT>    # was ['sensor.my_car_soc']
  car_charging_now     = <ABSENT>    # was ['binary_sensor.my_car_charging']

It also fails to compose between two discovering components. Adding an Ohme to a gateway install drops car_charging_now for the gateway car, which did supply it:

gateway only -> car_charging_now = ['binary_sensor.gw_session_active']
+ ohme      -> car_charging_now = <ABSENT>

Since gateway is the only source that ever populates now, that key gets dropped in essentially every mixed install.

The consequence isn't cosmetic. With car_charging_soc gone, fetch.py:1383 reads get_arg("car_charging_soc", 0.0, index=car_n) and gets 0.0, so Predbat treats a possibly-full car as empty and plans a full charge into it. With car_charging_now gone, fetch.py:2400 falls back to "no" and the charging-now path stops firing. The only signal to the user is a Warn: line, and docs/car-charging.md doesn't mention it — which matters because it directly contradicts the PR's own headline claim that hand-written config "now composes with autodiscovery instead of being overwritten". For these two keys it goes from preserved to deleted.

I agree padding isn't available to you — the validation at predbat.py:1646 does reject a non-string list element, so your reasoning there is right. But deleting is worse than not writing. The minimal fix I'd suggest: when missing is non-empty, leave the key alone rather than set_arg(arg, None) if it currently holds only legacy values. That reproduces exactly the pre-PR outcome (a short list, plus an out-of-range warn for the extra cars) without the registry inventing anything or destroying anything.

2. The gecloud half of the legacy dedup never fires 🟡

_names_a_discovered_charger (charger_registry.py:263) does a case-sensitive re.search for the raw device_id. GivEnergy Cloud serials are uppercase — EVC123456, WE1913G005 in your own fixtures — and async_automatic_config_evc lowercases the serial when it builds the entity name, while HA entity ids are always lowercase. So the serial can never match:

gecloud  "EVC123456" vs legacy sensor.givenergy_evc123456_status        -> num_cars = 2   # not deduped
myenergi "10000001"  vs legacy sensor.myenergi_zappi_10000001_plug_status -> num_cars = 1   # deduped

myenergi serials are numeric so the tests pass and the bug is invisible. re.IGNORECASE, or casefolding both sides, should do it. Worth a fixture with an uppercase serial to pin it.

3. All four control loops now share a single point of failure 🟠

charger_plan_ready() gates gateway, ohme, myenergi and gecloud, and the only thing that ever satisfies it is confirm_plan() at the tail of publish_car_plan() (output.py:159), called from the middle of fetch_sensor_data() (fetch.py:1243). The ordering is correct — fetch_config_options then fetch_sensor_data — so the happy path is fine. But any exception in the rate-scanning work before line 1243 now stalls every EV charger, where previously each control loop was independent.

I don't think the gate is wrong; it's the property you wanted. What I'd like is for it to be diagnosable — every blocked path is currently a bare return, so "my charger stopped responding" leaves nothing in the log to explain it. A one-shot log line on the wait would be enough.

4. Registry lock held across blocking HA I/O 🟡

materialise() takes self._lock at charger_registry.py:390 and still holds it at line 466 where it calls expose_config() for car_charging_rate. That reaches ha.set_state → a synchronous HTTP POST (ha.py:1076). replace_source() runs on component threads including the gateway's MQTT callback, while the main loop blocks on the same lock via snapshot_generation() in fetch_config_options (fetch.py:2836).

Only gateway sets max_rate_kw, so only gateway installs hit it, and the I/O itself isn't new — but the lock around it is. Collecting the (slot, rate) pairs under the lock and calling expose_config after releasing would avoid it.


One thing worth calling out

Moving plan = [] inside the per-car loop in publish_car_plan fixes a real multi-car bug on main — the list was accumulating across cars, so car N's published planned attribute contained every earlier car's windows too. That's exactly what the new control loops read, so it matters more after this PR than before. Worth a line in the description, and ideally a regression test of its own, since it's a fix that existing multi-car users benefit from independently of the registry.


Happy to merge once #1 is addressed; #2 is a small fix and #3/#4 I'm content to see as follow-ups if you'd rather keep this PR focused.

… dedupe, unlocked config exposure

Addresses maintainer review on the charger registry PR.

1. Hand-written car_charging_soc / car_charging_now were being deleted.

   The all-or-omit rule wrote set_arg(arg, None) - which removes the key -
   whenever any component slot had no value for a slot-aligned field. But
   myenergi supplies neither soc nor now, ohme supplies no now, and gateway
   supplies no now under gateway_evc_control, so the gap is permanent on
   those installs. Before the registry existed none of those components
   ever wrote these keys, so a user's apps.yaml value simply survived
   discovery; after it, discovering a Zappi silently removed their
   car_charging_soc. fetch.py:1383 then read the SoC as 0.0 and Predbat
   planned a full charge into a possibly-full car, and with
   car_charging_now gone fetch.py:2400 fell back to "no". It also dropped
   the gateway car's own car_charging_now - gateway being the only source
   that populates it - as soon as an ohme registered alongside, which is
   essentially every mixed install.

   The key is now left exactly as it stands: not written, and never
   cleared. That reproduces the pre-registry outcome - a list that is short
   for the new car count, plus fetch.py's own out-of-range warning for the
   cars past its end - without the registry inventing or destroying
   anything. This holds equally when the standing value is one the registry
   itself wrote for an earlier composition: a stale value is recoverable, a
   deleted one is not. The Warn line is kept but now says the key is being
   left as it is, and is latched per field so a permanent gap costs one
   line rather than one per rediscovery.

   docs/car-charging.md documents the rule and the log line, which it did
   not before.

2. The gecloud half of the legacy dedup never fired.

   _names_a_discovered_charger matched the raw device_id case-sensitively.
   GivEnergy Cloud serials are upper case (EVC123456, WE1913G005) while HA
   entity ids never are, and async_automatic_config_evc lower-cases the
   serial when it builds an entity name - so a gecloud serial could never
   match and the duplicated charger stayed as its own car. myenergi serials
   are numeric, which is why the tests passed and the bug was invisible.
   The search is now case insensitive; the word-boundary rule is unchanged
   and still keeps WE1913G005 from claiming a WE1913G0055 charger.

3. A blocked control loop said nothing.

   charger_plan_ready() gates all four control loops and every wait on it
   is a bare return, so "my charger stopped responding" left nothing in the
   log to explain it. The wait now logs one line naming the component,
   latched the same way as the gateway's _ev_no_slot_warned: cleared when a
   plan for the current allocation arrives, so a later recurrence is
   reported afresh and a plan that never comes costs one line, not one per
   cycle.

4. The registry lock was held across blocking HA I/O.

   materialise() held the lock through expose_config() for
   car_charging_rate, which reaches ha.set_state and a synchronous HTTP
   POST. replace_source() runs on component threads - the gateway's
   straight from an MQTT callback - while the main loop waits on the same
   lock in snapshot_generation(), so every gateway registration blocked the
   main loop for as long as HA took to answer. The (item, value) pairs are
   now composed under the lock and written after it is released.

5. Moving plan = [] inside the per-car loop in publish_car_plan fixes a
   real multi-car bug that predates this branch: the window list was built
   once outside the loop, so car 1's published "planned" attribute carried
   car 0's windows as well as its own and car 2's carried both. It matters
   more now because the new control loops read exactly that attribute to
   decide whether to charge, so a leaked window starts a charger outside
   its own plan. It now has a regression test of its own, independent of
   the registry.

Tests: eight new regressions - hand-written soc/now surviving a myenergi
registration, a second source not deleting the first's now, the key being
written again once the gap closes, the gap log firing once per gap, an
uppercase gecloud serial deduping (and still respecting the word
boundary), the control-loop wait line firing once and clearing, and the
per-car plan isolation. Seven of the eight fail against the previous
commit. test_car_charging_now_omitted_when_controlling now asserts the key
is left alone rather than nulled, which is what the pre-registry gateway
path did.

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

mgazza commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

Thanks — that's a sharp review, and #1 was a real regression. All four are fixed in 8bc3d17b, plus the regression test you suggested.

1. Hand-written car_charging_soc / car_charging_now deleted. You were right that deleting is worse than not writing, and right that padding isn't available (validation rejects a non-string list element). A component-caused gap now leaves the key alone entirely instead of nulling it, reproducing the pre-PR outcome. It also no longer deletes a value the registry itself wrote when a later composition develops a gap — a stale value is recoverable, a deleted one isn't. The warning is reworded to say the key is being left as-is. Tests cover your two reproductions: hand-written soc/now surviving a Zappi, and gateway's car_charging_now surviving an Ohme registering alongside it.

This one also contradicted the PR's own headline claim, so thank you for catching it — the description overstated what composition preserved.

2. gecloud dedupe never fired. Correct: uppercase GivEnergy serials against lowercase entity ids. Now matched case-insensitively, with an uppercase-serial fixture to pin it. The boundary-collision guard stays green — the numeric myenergi serials were exactly why the tests didn't show this.

3. Shared gate, undiagnosable. Each control loop now emits a one-shot line while it waits for a plan built for the current allocation, cleared when the plan arrives, so a stalled charger says why rather than returning silently. No per-cycle spam.

4. Lock held across HA I/O. The (slot, rate) pairs are collected under the lock and expose_config is called after releasing it, so the gateway MQTT callback can't hold the main loop behind a synchronous POST.

5. plan = []. Agreed it stands on its own — there's now a standalone regression asserting each car's published planned contains only its own windows, independent of the registry, and I've noted it in the description as a fix existing multi-car users get regardless.

Verification: 261 targeted tests, 297 gateway, and --quick green on charger_registry, ohme, myenergi, ge_cloud, gateway, kraken, kraken_auth, octopus_misc and web_if. Two suites fail identically on unmodified main and are untouched here — data_age_metrics (data_age_days 8 vs 0) and control_conflicts_metrics (control_conflicts_24h 4 vs 0); I verified both against a clean origin/main checkout rather than assuming. Worth a look separately — they look like the same shape of environmental issue.

The #4880 question from my earlier comment is still open whenever you want to decide it.

Copilot AI left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🟡 Changes recommended

Unresolved critical charger-slot and Gateway routing issues could control or plan against the wrong vehicle.

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

Pull request overview

Introduces a central EV charger registry so multiple integrations can compose charger configuration and control distinct car slots.

Changes:

  • Adds deterministic charger registration, legacy configuration merging, and external car-count floors.
  • Migrates Gateway, GivEnergy Cloud, myenergi, and Ohme discovery/control.
  • Adds regression tests and documentation.

Review comments:

  • Critical (2 votes) — charger_registry.py:500: Slot-aligned lists can retain stale positions when sorting introduces a component gap, potentially assigning telemetry such as SoC to the wrong car. Preserve only unchanged slots or rebuild/invalidate affected lists.
  • Critical (1 vote) — gateway.py:549: Older firmware may ignore charge_point_id, causing multiple per-device commands to target the first charger. Detect addressing support and restrict multi-charger control when unsupported.
  • Moderate (1 vote) — gateway.py:1421: Empty telemetry does not clear prior Gateway registrations, and removal is not detected by _needs_reconfigure(). Detect charger-set changes and register an authoritative empty set.
  • Moderate (1 vote) — ohme.py:490: A captured legacy aggregate prevents Ohme energy and power from being registered, omitting its charging data. Distinguish legacy registry aggregates from runtime direct writers.
File summaries
File Description
docs/components.md Documents shared charger slots.
docs/car-charging.md Explains allocation, composition, and legacy behavior.
docs/apps-yaml.md Updates charger configuration guidance.
apps/predbat/utils.py Excludes the registry from debug serialization.
apps/predbat/unit_test.py Registers the charger registry tests.
apps/predbat/tests/test_ohme.py Tests Ohme registration and slot-aware control.
apps/predbat/tests/test_octopus_misc.py Tests Octopus car-count claims.
apps/predbat/tests/test_myenergi.py Tests slot-aware Zappi control.
apps/predbat/tests/test_kraken.py Tests Kraken claim lifecycle.
apps/predbat/tests/test_ge_cloud.py Tests GivEnergy registry integration.
apps/predbat/tests/test_gateway.py Tests Gateway multi-charger behavior.
apps/predbat/tests/test_charger_registry.py Adds registry and composition coverage.
apps/predbat/predbat.py Initializes and seeds the registry.
apps/predbat/output.py Publishes isolated per-car plans.
apps/predbat/ohme.py Registers and controls Ohme by allocated slot.
apps/predbat/octopus.py Manages Octopus car-count claims.
apps/predbat/myenergi.py Registers and controls Zappis by allocated slot.
apps/predbat/mock_base.py Adds registry support to test mocks.
apps/predbat/kraken.py Manages Kraken car-count claims.
apps/predbat/gecloud.py Registers GivEnergy chargers centrally.
apps/predbat/gateway.py Adds multi-charger registration and routing.
apps/predbat/fetch.py Snapshots charger-map generation.
apps/predbat/component_base.py Adds shared registry helpers.
apps/predbat/charger_registry.py Implements charger composition and slot allocation.
.cspell/custom-dictionary-workspace.txt Adds test vocabulary.
Review details
  • Files reviewed: 25/25 changed files
  • Comments generated: 4
  • Review effort level: Balanced

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

Comment thread apps/predbat/charger_registry.py Outdated
Comment on lines +500 to +504
missing = [entry.key() for entry, value in zip(entries, values) if not value and entry.source != LEGACY_SOURCE]
if missing:
# A component-caused gap cannot be expressed: None fails validation and a
# placeholder would be read as a real entity. So the key is left exactly as it
# stands - not written, and above all not cleared.
Comment thread apps/predbat/gateway.py
Comment on lines +549 to +553
await self._send_ev_command({"action": "SetChargingProfile", "current_a": int(current_a), "charge_point_id": cp_id})
await self._send_ev_command({"action": "RemoteStartTransaction", "id_tag": "predbat", "charge_point_id": cp_id})
else:
self.log(f"Info: GatewayMQTT: EVC stop charging for '{cp_id}' (car {slot})")
await self._send_ev_command({"action": "RemoteStopTransaction", "charge_point_id": cp_id})
Comment thread apps/predbat/gateway.py Outdated
Comment on lines 1421 to 1423
chargers = list(status.ev_chargers)
if not chargers:
return
Comment thread apps/predbat/ohme.py Outdated
Comment on lines +490 to +493
existing = self.get_arg("car_charging_energy", default=None, indirect=False)
if isinstance(existing, list) and len(existing) == 1:
existing = existing[0]
# Ohme already owns the energy figure when the entity configured is its own, whether that
# was set here on a previous run or written into apps.yaml by hand (#4715 review)
if (not existing) or (isinstance(existing, str) and existing.startswith("re:")) or existing == ENERGY_TODAY_ENTITY:
self.set_arg_auto("car_charging_energy", ENERGY_TODAY_ENTITY)
# Live charge power, for the web power flow diagram and the predbat.car_charging_power
# sensor. Deliberately tied to the same decision as the energy sensor above: the two
# have to describe the same charger, so when another charger owns the energy figure
# Ohme's own power reading is left out rather than reported beside it.
self.set_arg_auto("car_charging_power", POWER_WATTS_ENTITY)
else:
owns_energy = self.base.charger_registry.owns_aggregate("energy") or (not existing) or (isinstance(existing, str) and existing.startswith("re:")) or existing == ENERGY_TODAY_ENTITY
…way command keys, release removed chargers, compose ohme with legacy aggregates
@mgazza

mgazza commented Sep 5, 2026

Copy link
Copy Markdown
Collaborator Author

All four Copilot findings addressed in d5477395. Three were real; the first was a regression I introduced in the previous round, so thank you for catching it.

Positionally stale slot lists. @springfall2008's prescription was to leave the key alone when it holds only legacy values; I broadened that to retain registry-owned lists too, and the broadening was unsafe. Ohme registering alone writes car_charging_soc = [ohme_soc] at slot 0; a Zappi then sorts ahead of it (myenergi < ohme) and supplies no SoC, so the retained list left the Ohme's SoC sitting in the Zappi car's slot — and planning reads it.

Fixed by trimming a composed list at the first gap rather than keeping or clearing it wholesale. A prefix is always positionally valid, and because legacy slots sort first they are always inside the kept head — so the trim drops exactly what the registry invented and never the user's own config. Both of your earlier reproductions stay green, which a blanket clear would have broken.

Both gateway command keys. Rather than feature-detecting and disabling multi-charger control, the commands now carry target alongside charge_point_id. ev_command.h reads target first and falls back, so this routes correctly on both current firmware and the older builds that read only target — no degraded mode, and no version negotiation to maintain.

Removed chargers are now released. _needs_reconfigure() only ever computed additions, so a departing charge point kept its slot and its num_cars contribution forever. It now detects a changed id set and re-registers, including the empty case, debounced over consecutive frames so a transient telemetry dropout doesn't release a live charger.

Ohme composes with a legacy aggregate. It was backing off whenever any car_charging_energy existed, which pre-dated the registry — the back-off existed to avoid clobbering a third-party sensor, but the registry concatenates, so there is nothing to clobber. It now distinguishes a registry-captured legacy aggregate (composes, both sensors summed) from a runtime value owned by another direct writer such as alphaess (still backs off). That also supersedes the multi-charger unwrap limitation noted in the description.

--quick green on charger_registry, ohme, myenergi, ge_cloud, gateway, kraken, octopus_misc and web_if; 263 targeted and 303 gateway tests pass. The two metrics suites that fail on unmodified main are unchanged and still untouched here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants