fix(gateway): pin EV charger entity ids to the charge point id (#4489)

* fix(gateway): pin EV entity ids to the charge point id

_ev_suffix() returned a bare "ev" when a single charger was visible and
"ev_<id>" only when several were, so the suffix depended on how many chargers
happened to be in one status message. Entity ids therefore moved under the user:
a site with one charger produced sensor.predbat_gateway_ev_power, but the moment
a second charger appeared — or the same charger was briefly seen twice across a
reconnect — everything silently renamed to sensor.predbat_gateway_ev_<id>_power,
orphaning the old entities and every dashboard and automation bound to them.

Seen in production: one hub accumulated both a predbat_gateway_ev_536212_* set
and a predbat_gateway_ev_* set for the same physical charger.

Always identify the charger by the last 6 chars of its charge point id, falling
back to "ev" only when the charger has not reported one yet (no BootNotification).

Renames entities on single-charger sites from sensor.predbat_gateway_ev_* to
sensor.predbat_gateway_ev_<id>_*. These entities only exist on PredBat Gateway
hubs running an OCPP charger; at the time of writing that is two hubs (one of
them a test rig), so no migration is warranted — but any dashboard bound to the
unsuffixed names needs repointing, and the old entities are left orphaned.

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

* fix(gateway): harden EV entity naming and suppress stale offline telemetry

Follow-up to the entity-id pinning, from an adversarial review of that change.

- Slugify the charge point id before truncating. An OCPP chargePointSerialNumber
  is an arbitrary vendor string and may contain spaces, dots, slashes, dashes or
  non-ASCII, none of which are legal in a Home Assistant entity id, and nothing
  downstream sanitised it. `CP/AB-123` previously produced
  sensor.predbat_gateway_ev_ab-123_power, which HA rejects — while auto-config
  still pointed car_charging_* at that nonexistent namespace.

- Suppress live-session fields while a charger is disconnected. Only the derived
  `connected` sensor was gated on ev.connected; session_active, power, session
  energy and the SoC fallback were published straight from the payload. Since
  car_charging_now is wired to session_active and PredBat plans whenever it is
  true, a disconnected charger carrying a stale session_active would schedule a
  charging slot for a charger that is not there. Gateway firmware now zeroes
  these, but PredBat and firmware version independently, so gate here too.

- Prefer a connected charger in _register_ev_car() instead of blindly taking
  chargers[0]. The gateway now reports known-but-offline chargers rather than
  omitting them, so slot 0 can be a stale entry; a site with an old disconnected
  charger listed first would have bound all car_charging_* args to it and ignored
  the live one. Falls back to the first entry so a charger that is merely offline
  right now still registers.

- Update docs/car-charging.md, which still listed all ten entities under the old
  unsuffixed names.

Known limitation, documented in the docstring: a 6-char tail is not
collision-proof — two chargers whose slugified ids share a tail map to one
namespace. Identity stability was the priority; anything collision-proof either
depends on which other chargers are present (the churn this fixes) or is an
opaque digest.

Tests: 269 pass, including slugification cases and assertions that a
disconnected charger's session_active/power/session_energy are forced idle.

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

* Update car-charging.md for clarity on EV charger entities

Clarified entity naming and registration details for GivEnergy Gateway OCPP EV charger in documentation.

* [pre-commit.ci lite] apply automatic fixes

---------

Co-authored-by: Claude Opus 5 <noreply@anthropic.com>
Co-authored-by: Trefor Southwell <48591903+springfall2008@users.noreply.github.com>
Co-authored-by: pre-commit-ci-lite[bot] <117423508+pre-commit-ci-lite[bot]@users.noreply.github.com>

Author: 4 people

.cspell/custom-dictionary-workspace.txt

@@ -442,6 +442,10 @@ sigencloud
 sigenergy
 sigenstor
 Slee
+slugified
+slugifies
+slugify
+slugifying
 snakeviz
 socb
 socketloop

apps/predbat/gateway.py

@@ -946,20 +946,46 @@ def _inject_inverter_entities(self, inv, suffix):
     def _ev_suffix(ev, multi):
         """Build the entity suffix for an EV charger.
 
-        Single charger (the v1 case) uses a stable "ev" suffix; with more than one
-        charger present, disambiguate by the last 6 chars of the OCPP charge point id.
+        Always identifies the charger by the last 6 chars of its OCPP charge point
+        id, so a given charger keeps the same entity ids for its whole life.
+
+        This used to return a bare "ev" whenever only one charger was present, and
+        only disambiguate when several were. That made the suffix a function of how
+        many chargers happened to be visible in a single status message, so entity
+        ids moved under the user: a site with one charger produced
+        ``sensor.predbat_gateway_ev_power``, but the moment a second charger
+        appeared — or the same charger was briefly seen twice across a reconnect —
+        everything silently renamed to ``sensor.predbat_gateway_ev_<id>_power``,
+        orphaning the old entities and every dashboard and automation bound to
+        them. Pinning to the charge point id removes that whole class of churn.
+
+        Falls back to "ev" only when the charger reports no id at all (a charger
+        that has not completed its BootNotification yet).
+
+        The id is slugified first: an OCPP chargePointSerialNumber is an arbitrary
+        vendor string and may contain spaces, dots, slashes, dashes or non-ASCII,
+        none of which are legal in a Home Assistant entity id. Anything outside
+        [a-z0-9_] is dropped, then the last 6 characters are taken — slugifying
+        before truncating keeps the result stable and legal.
+
+        Note: 6 characters is not collision-proof. Two chargers whose slugified ids
+        share a tail (or differ only by case) map to one entity namespace and the
+        later one wins. Identity stability was the priority here — anything
+        collision-proof either depends on the other chargers present (which is the
+        churn this fixes) or is an opaque digest.
 
         Args:
             ev: An EvCharger protobuf message.
-            multi: True when more than one charger is present in the status.
+            multi: Unused; retained for call-site compatibility. The suffix no
+                longer depends on how many chargers are present.
 
         Returns:
-            str: The entity suffix (e.g. "ev" or "ev_b749").
+            str: The entity suffix (e.g. "ev_b749", or "ev" when the id is unknown
+                or slugifies to nothing).
         """
-        if not multi:
-            return "ev"
         charge_point_id = ev.charge_point_id or ""
-        return f"ev_{charge_point_id[-6:].lower()}" if charge_point_id else "ev"
+        slug = "".join(c for c in charge_point_id.lower() if c.isascii() and (c.isalnum() or c == "_"))
+        return f"ev_{slug[-6:]}" if slug else "ev"
 
     @staticmethod
     def _ev_charge_rate_kw(ev):
@@ -994,11 +1020,21 @@ def _inject_ev_entities(self, status):
             self.dashboard_item(f"binary_sensor.{pfx}_online", ev.connected, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_online", {}), app="gateway")
             ev_car_connected = ev.connected and ev.status in {"Preparing", "Charging", "SuspendedEV", "SuspendedEVSE", "Finishing"}
             self.dashboard_item(f"binary_sensor.{pfx}_connected", ev_car_connected, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_connected", {}), app="gateway")
-            self.dashboard_item(f"binary_sensor.{pfx}_session_active", ev.session_active, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_session_active", {}), app="gateway")
+            # Live-session fields are only meaningful while the charger is connected.
+            # The gateway now reports known-but-offline chargers instead of omitting
+            # them, and older firmware can leave session_active/power/energy at their
+            # last values. car_charging_now is wired to session_active and PredBat
+            # plans whenever it is true, so an offline charger with a stale
+            # session_active would create a charging slot for a charger that is not
+            # there. Force them to their idle values rather than trusting the payload.
+            session_active = ev.connected and ev.session_active
+            power_w = ev.power_w if ev.connected else 0
+            session_energy_wh = ev.session_energy_wh if ev.connected else 0
+            self.dashboard_item(f"binary_sensor.{pfx}_session_active", session_active, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_session_active", {}), app="gateway")
             if ev.status:
                 self.dashboard_item(f"sensor.{pfx}_status", ev.status, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_status", {}), app="gateway")
-            self.dashboard_item(f"sensor.{pfx}_power", ev.power_w, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_power", {}), app="gateway")
-            self.dashboard_item(f"sensor.{pfx}_session_energy", round(ev.session_energy_wh / 1000.0, 2), attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_session_energy", {}), app="gateway")
+            self.dashboard_item(f"sensor.{pfx}_power", power_w, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_power", {}), app="gateway")
+            self.dashboard_item(f"sensor.{pfx}_session_energy", round(session_energy_wh / 1000.0, 2), attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_session_energy", {}), app="gateway")
             if ev.current_limit_a:
                 self.dashboard_item(f"sensor.{pfx}_current_limit", ev.current_limit_a, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_current_limit", {}), app="gateway")
             if ev.soc_percent:
@@ -1011,7 +1047,9 @@ def _inject_ev_entities(self, status):
                     battery_size_kwh = float(battery_size_kwh)
                 except (ValueError, TypeError):
                     battery_size_kwh = 100.0
-                ev_soc = round(min((ev.session_energy_wh / 1000.0) / battery_size_kwh * 100.0, 100.0), 1)
+                # session_energy_wh is forced to 0 above while disconnected, so an
+                # offline charger estimates 0% rather than replaying the last session.
+                ev_soc = round(min((session_energy_wh / 1000.0) / battery_size_kwh * 100.0, 100.0), 1)
             self.dashboard_item(f"sensor.{pfx}_soc", ev_soc, attributes=GATEWAY_ATTRIBUTE_TABLE.get("ev_soc", {}), app="gateway")
             if ev.max_current_a:
                 self._ev_max_current[ev.charge_point_id or ""] = ev.max_current_a
@@ -1305,12 +1343,17 @@ def _register_ev_car(self, status):
         if not self.gateway_evc_automatic:
             return
 
+        # Prefer a connected charger. Gateway firmware now reports known-but-offline
+        # chargers with connected=false (previously it omitted them entirely), so
+        # slot 0 can be a stale entry — registering that as the car would wire
+        # car_charging_* to a charger that is not there. Fall back to the first
+        # entry so a charger that is merely offline right now still registers.
         chargers = list(status.ev_chargers)
         if not chargers:
             return
 
         # For now we overwrite the first charger only; multi-charger support needs work
-        ev = chargers[0]
+        ev = next((c for c in chargers if c.connected), chargers[0])
         pfx = f"{self.prefix}_gateway_{self._ev_suffix(ev, multi=False)}"
         self.set_arg("num_cars", 1)
         # Entity-reference args (resolved by get_arg indirect lookup at fetch time).

apps/predbat/tests/test_gateway.py

@@ -3945,7 +3945,7 @@ def test_ev_entities_published(self):
         gw = self._make_gateway()
         gw._inject_ev_entities(self._status_with_ev())
 
-        base = "predbat_gateway_ev"
+        base = "predbat_gateway_ev_3xb749"
         assert gw._dashboard_calls[f"binary_sensor.{base}_online"][0] is True
         assert gw._dashboard_calls[f"binary_sensor.{base}_connected"][0] is True  # status="Charging" → car connected
         assert gw._dashboard_calls[f"binary_sensor.{base}_session_active"][0] is True
@@ -3961,13 +3961,78 @@ def test_ev_entities_published(self):
         # Derived charge-rate capability in kW: 32 A × 240 V / 1000
         assert approx_equal(gw._dashboard_calls[f"sensor.{base}_charge_rate"][0], 7.68)
 
+    def test_ev_suffix_is_stable_regardless_of_charger_count(self):
+        """A charger keeps the same entity ids whether or not others are present.
+
+        Regression: the suffix used to be a bare "ev" when a single charger was
+        visible and "ev_<id>" only when several were, so entity ids silently
+        renamed the moment a second charger appeared (or the same charger was
+        briefly seen twice across a reconnect), orphaning dashboards bound to them.
+        """
+        from gateway import GatewayMQTT
+
+        ev = self._status_with_ev().ev_chargers[0]
+        assert GatewayMQTT._ev_suffix(ev, False) == "ev_3xb749"
+        assert GatewayMQTT._ev_suffix(ev, True) == "ev_3xb749"
+
+    def test_ev_suffix_falls_back_when_id_unknown(self):
+        """A charger that has not sent its BootNotification yet still gets a suffix."""
+        from gateway import GatewayMQTT
+
+        ev = self._status_with_ev(charge_point_id="").ev_chargers[0]
+        assert GatewayMQTT._ev_suffix(ev, False) == "ev"
+
+    def test_disconnected_charger_reports_offline(self):
+        """A charger the gateway reports as disconnected surfaces as fully idle.
+
+        The gateway now sends known-but-disconnected chargers with connected=false
+        instead of omitting them, so these entities must go False rather than
+        freezing at their last values (which is how a 2-day EV outage went unseen).
+
+        Critically this asserts the LIVE-SESSION fields too, not just online/connected:
+        car_charging_now is wired to session_active and PredBat plans a charging slot
+        whenever it is true, so a disconnected charger carrying a stale
+        session_active would schedule charging for a charger that is not there.
+        The fixture deliberately supplies session_active=True, power_w=7200 and
+        session_energy_wh=12400 alongside connected=False — the inconsistent payload
+        older firmware can emit.
+        """
+        gw = self._make_gateway(battery_size=100)
+        gw._inject_ev_entities(self._status_with_ev(connected=False, status="Unavailable"))
+
+        base = "predbat_gateway_ev_3xb749"
+        assert gw._dashboard_calls[f"binary_sensor.{base}_online"][0] is False
+        assert gw._dashboard_calls[f"binary_sensor.{base}_connected"][0] is False
+        # Stale live-session values must be suppressed, not republished
+        assert gw._dashboard_calls[f"binary_sensor.{base}_session_active"][0] is False
+        assert gw._dashboard_calls[f"sensor.{base}_power"][0] == 0
+        assert gw._dashboard_calls[f"sensor.{base}_session_energy"][0] == 0
+
+    def test_suffix_slugifies_unsafe_charge_point_ids(self):
+        """Charge point ids are vendor strings; entity ids must stay legal.
+
+        Spaces, dots, slashes, dashes and non-ASCII are not valid in a Home Assistant
+        entity id, and nothing downstream sanitises the value.
+        """
+        from gateway import GatewayMQTT
+
+        for cp_id, expected in [
+            ("CP/AB-123", "ev_pab123"),  # punctuation dropped, last 6 kept
+            ("cp 42.7", "ev_cp427"),  # -> "cp427", shorter than 6
+            ("ABC", "ev_abc"),  # shorter than 6 -> whole id
+            ("charge_point_Ω", "ev_point_"),  # non-ASCII dropped, underscore kept
+            ("///", "ev"),  # slugifies to nothing -> fallback
+        ]:
+            ev = self._status_with_ev(charge_point_id=cp_id).ev_chargers[0]
+            assert GatewayMQTT._ev_suffix(ev, False) == expected, cp_id
+
     def test_ev_entity_attributes_from_table(self):
         """Published EV entities carry their GATEWAY_ATTRIBUTE_TABLE attributes."""
         from gateway import GATEWAY_ATTRIBUTE_TABLE
 
         gw = self._make_gateway()
         gw._inject_ev_entities(self._status_with_ev())
-        _, attrs = gw._dashboard_calls["sensor.predbat_gateway_ev_power"]
+        _, attrs = gw._dashboard_calls["sensor.predbat_gateway_ev_3xb749_power"]
         assert attrs == GATEWAY_ATTRIBUTE_TABLE["ev_power"]
 
     def test_not_reported_fields_skipped(self):
@@ -3977,7 +4042,7 @@ def test_not_reported_fields_skipped(self):
         status = self._status_with_ev(soc_percent=0, voltage_v=0, max_current_a=0, current_limit_a=0, eco_mode="", status="")
         gw._inject_ev_entities(status)
 
-        base = "predbat_gateway_ev"
+        base = "predbat_gateway_ev_3xb749"
         # soc is now always published — falls back to session_energy / battery_size * 100
         assert approx_equal(gw._dashboard_calls[f"sensor.{base}_soc"][0], 12.4)
         assert f"sensor.{base}_voltage" not in gw._dashboard_calls
@@ -3997,14 +4062,14 @@ def test_soc_fallback_uses_battery_size(self):
         gw = self._make_gateway(battery_size=50)
         # session_energy_wh=12400 → 12.4 kWh; battery_size=50 → 12.4/50*100 = 24.8%
         gw._inject_ev_entities(self._status_with_ev(soc_percent=0))
-        assert approx_equal(gw._dashboard_calls["sensor.predbat_gateway_ev_soc"][0], 24.8)
+        assert approx_equal(gw._dashboard_calls["sensor.predbat_gateway_ev_3xb749_soc"][0], 24.8)
 
     def test_charge_rate_uses_230v_when_voltage_missing(self):
         """With max current but no voltage, charge rate assumes 230 V."""
         gw = self._make_gateway()
         gw._inject_ev_entities(self._status_with_ev(max_current_a=16, voltage_v=0))
         # 16 A × 230 V / 1000
-        assert approx_equal(gw._dashboard_calls["sensor.predbat_gateway_ev_charge_rate"][0], 3.68)
+        assert approx_equal(gw._dashboard_calls["sensor.predbat_gateway_ev_3xb749_charge_rate"][0], 3.68)
 
     def test_no_chargers_publishes_nothing(self):
         """A status with no EV chargers publishes no EV entities."""
@@ -4067,20 +4132,52 @@ def _status_with_ev(self, charge_point_id="CP1", max_current_a=32, voltage_v=240
         ev.voltage_v = voltage_v
         return status
 
+    def test_registers_connected_charger_not_a_stale_slot(self):
+        """A disconnected charger in slot 0 must not be registered over a connected one.
+
+        Gateway firmware now reports known-but-offline chargers with connected=false
+        (it used to omit them), so the first entry is no longer guaranteed to be live.
+        """
+        status = pb.GatewayStatus()
+        stale = status.ev_chargers.add()
+        stale.connected = False
+        stale.charge_point_id = "CP0000"
+        live = status.ev_chargers.add()
+        live.connected = True
+        live.charge_point_id = "CP0001"
+
+        gw = self._make_gateway(ev_enable=True, num_cars=0)
+        gw._register_ev_car(status)
+
+        assert gw._args["car_charging_planned"] == ["binary_sensor.predbat_gateway_ev_cp0001_connected"]
+
+    def test_registers_offline_charger_when_none_connected(self):
+        """A charger that is merely offline right now still registers, so it reappears."""
+        status = pb.GatewayStatus()
+        offline = status.ev_chargers.add()
+        offline.connected = False
+        offline.charge_point_id = "CP0002"
+
+        gw = self._make_gateway(ev_enable=True, num_cars=0)
+        gw._register_ev_car(status)
+
+        assert gw._args["num_cars"] == 1
+        assert gw._args["car_charging_planned"] == ["binary_sensor.predbat_gateway_ev_cp0002_connected"]
+
     def test_registers_car_when_none_configured(self):
         """With the flag on and no existing cars, the charger is registered as car 1."""
         gw = self._make_gateway(ev_enable=True, num_cars=0)
         gw._register_ev_car(self._status_with_ev())
 
         assert gw._args["num_cars"] == 1
-        assert gw._args["car_charging_planned"] == ["binary_sensor.predbat_gateway_ev_connected"]
-        assert gw._args["car_charging_now"] == ["binary_sensor.predbat_gateway_ev_session_active"]
-        assert gw._args["car_charging_soc"] == ["sensor.predbat_gateway_ev_soc"]
+        assert gw._args["car_charging_planned"] == ["binary_sensor.predbat_gateway_ev_cp1_connected"]
+        assert gw._args["car_charging_now"] == ["binary_sensor.predbat_gateway_ev_cp1_session_active"]
+        assert gw._args["car_charging_soc"] == ["sensor.predbat_gateway_ev_cp1_soc"]
         # car_charging_rate is a UI config item — set via expose_config, not set_arg
         assert "car_charging_rate" not in gw._args
         gw.base.expose_config.assert_called_once_with("car_charging_rate", 7.68)  # 32A * 240V / 1000
         # Session energy sensor for subtracting EV load from history
-        assert gw._args["car_charging_energy"] == "sensor.predbat_gateway_ev_session_energy"
+        assert gw._args["car_charging_energy"] == "sensor.predbat_gateway_ev_cp1_session_energy"
         # Battery size and target limit are left to the existing car_charging_* settings
         assert "car_charging_battery_size" not in gw._args
         assert "car_charging_limit" not in gw._args
@@ -4102,7 +4199,7 @@ def test_car_charging_now_set_when_not_controlling(self):
         """car_charging_now is wired to session_active when gateway_evc_control is False."""
         gw = self._make_gateway(ev_enable=True, num_cars=0, evc_control=False)
         gw._register_ev_car(self._status_with_ev())
-        assert gw._args["car_charging_now"] == ["binary_sensor.predbat_gateway_ev_session_active"]
+        assert gw._args["car_charging_now"] == ["binary_sensor.predbat_gateway_ev_cp1_session_active"]
 
     def test_car_charging_now_omitted_when_controlling(self):
         """car_charging_now is not set when gateway_evc_control is True to prevent feedback loop."""