fix(octopus): choose on purpose between conflicting catalogue entries for a car (#5438)

apps/predbat/fetch.py

@@ -1562,7 +1562,8 @@ def fetch_sensor_data_cars(self, save=True):
                     if car_n < len(self.car_charging_manual_soc) and not self.car_charging_manual_soc[car_n]:
                         self.car_charging_soc[car_n] = (self.get_arg("car_charging_soc", 0.0, index=car_n) * self.car_charging_battery_size[car_n]) / 100.0
                 if rate:
-                    # Take the max as Octopus over reports
+                    # Octopus's figure only ever raises the configured rate, never lowers it. Where Octopus's catalogue
+                    # gives the charger more than one power, the component passes on the lowest (catalogue_value(), #5438)
                     self.log("Car {} rate from Octopus is {}kW and configured rate {}".format(car_n, rate, self.car_charging_rate[car_n]))
                     self.car_charging_rate[car_n] = max(rate, self.car_charging_rate[car_n])
 

apps/predbat/octopus.py

@@ -14,6 +14,7 @@
 """
 
 import asyncio
+import math
 import requests
 import re
 from datetime import datetime, timedelta, timezone
@@ -56,6 +57,18 @@
 # The EV/charge-point catalogue is static reference data, so it is refreshed daily
 CATALOGUE_FRESH_MINUTES = 24 * 60
 CATALOGUE_STALE_MINUTES = 25 * 60
+# Logged by log_catalogue_ambiguity() when the catalogue gives a make and model more than one value (issue #5438).
+# A charge point's power only ever raises input_number.{prefix}_car_charging_rate (fetch.py), so that is what decides
+# the rate; a vehicle's battery size from Octopus replaces car_charging_battery_size, so there is no setting to change.
+# {prefix} is the entity prefix in use (apps.yaml prefix, "predbat" by default).
+CATALOGUE_AMBIGUOUS_CHARGER = (
+    "Octopus's catalogue lists charge point {device} at more than one power ({values} {unit}) and Predbat can't tell which yours is,"
+    " so it uses the lowest, {value} {unit}, unless input_number.{prefix}_car_charging_rate (_1, _2 for further cars) is higher. Set that to your charger's rate"
+)
+CATALOGUE_AMBIGUOUS_VEHICLE = (
+    "Octopus's catalogue lists vehicle {device} with more than one battery size ({values} {unit}) and Predbat can't tell which yours is,"
+    " so it plans with the largest, {value} {unit}. A battery size from Octopus is used in place of car_charging_battery_size, so that setting can't change it"
+)
 OCTOPUS_SLOT_MAX_DEFAULT = 48  # 24 hours with 30-minute slots
 OCTOPUS_SLOT_MAX_CAPPED = 12  # 6 hours with 30-minute slots
 IO_RATE_TOLERANCE = 0.01  # A dispatch lowers a rate by pence; anything closer is rate_min_base's dp2 rounding against the unrounded tariff feed (#5392)
@@ -90,6 +103,49 @@ def is_active(now_utc, activeFrom, activeTo):
     return True
 
 
+def catalogue_value(entries, make, model, field, pick):
+    """
+    Look up one make and model's value in an Octopus catalogue list (chargePointVariants or electricVehicles).
+
+    The catalogue's names aren't unique: the same make and model can be listed more than once with
+    different values - myenergi's "zappi (all models)" is there at both 7.4 and 22 kW - and the name is
+    all a device record gives to match on (issue #5438). Taking whichever match came last planned a
+    7.4 kW zappi at 22 kW. So when the matches disagree, pick chooses between them, deliberately rather
+    than by catalogue order: min for a charge point's power (the most every variant can do), max for a
+    vehicle's battery size (so the car's remaining charge is over- rather than under-estimated).
+
+    GraphQL returns these figures as strings ("22.000"), so they are converted to float here. Entries
+    whose value is missing, not a number, not finite, or not above zero are ignored.
+
+    Args:
+        entries (list): The catalogue list, each entry {"make": ..., "models": [{"model": ..., field: ...}]}
+        make (str): The device's make
+        model (str): The device's model
+        field (str): The value to read from each matching model, e.g. "powerInKw" or "batterySize"
+        pick (callable): Chooses one value from the list when the matches disagree, e.g. min or max
+
+    Returns:
+        tuple: (value, values) - value is the matches' value as a float (pick's choice when they
+        disagree), or None if nothing usable matched; values is the sorted list of the distinct values
+        the matches gave, so more than one means the catalogue was ambiguous.
+    """
+    values = set()
+    for entry in entries or []:
+        if entry.get("make", None) != make:
+            continue
+        for info in entry.get("models", None) or []:
+            if info.get("model", None) != model:
+                continue
+            try:
+                value = float(info.get(field, None))
+            except (TypeError, ValueError):
+                continue
+            if math.isfinite(value) and value > 0:
+                values.add(value)
+    values = sorted(values)
+    return (pick(values) if values else None), values
+
+
 def parse_date(dt_str):
     """Convert a date string to a date object."""
     try:
@@ -628,6 +684,9 @@ def initialize(self, key, account_id, automatic):
         self.saving_sessions = {}
         self.saving_sessions_to_join = []
         self.intelligent_devices = {}
+        # (message, make, model) the catalogue gave conflicting values for and that have been logged, so the
+        # warning is given once rather than on every device poll (see catalogue_value())
+        self.catalogue_ambiguity_logged = set()
         # Signature of the dispatches last published, so a poll that changes them can request a replan
         self.intelligent_dispatch_signature = None
         # Active device IDs automatic_config() last wired the car slots to - None until it has run.
@@ -2424,6 +2483,31 @@ async def async_graphql_query(self, query, request_context, returns_data=True, i
 
         return None
 
+    def log_catalogue_ambiguity(self, message, make, model, values, value, unit):
+        """
+        Warn, once per make and model, when the catalogue lists a device more than once with different values.
+
+        The log line is the only sign a user gets that catalogue_value() had to choose, so it names the
+        values the catalogue gave and the one Predbat is using.
+
+        Args:
+            message (str): What was looked up and what the user can do about it, formatted with the
+                device ({device}), the values ({values}), the one in use ({value}), the unit ({unit}) and
+                the entity prefix in use ({prefix})
+            make (str): The device's make
+            model (str): The device's model
+            values (list): The distinct values the catalogue gave, from catalogue_value()
+            value (float): The value catalogue_value() chose
+            unit (str): The values' unit
+        """
+        if len(values) < 2:
+            return
+        key = (message, make, model)
+        if key in self.catalogue_ambiguity_logged:
+            return
+        self.catalogue_ambiguity_logged.add(key)
+        self.log("Warn: OctopusAPI: " + message.format(device="{} '{}'".format(make, model), values=", ".join("{:g}".format(v) for v in values), value="{:g}".format(value), unit=unit, prefix=self.prefix))
+
     async def async_get_vehicle_catalogue(self):
         """
         Get the global EV / charge-point catalogue, cached across polls and instances.
@@ -2526,19 +2610,11 @@ async def async_get_intelligent_devices(self, account_id, device_id):
                                         device_setting_result[setting_key] = cached_device[setting_key]
 
                         if isCharger:
-                            for charger in chargePointVariants:
-                                if charger.get("make", None) == make:
-                                    models = charger.get("models", [])
-                                    for charger_info in models:
-                                        if charger_info.get("model", None) == model:
-                                            chargePointPowerInKw = charger_info.get("powerInKw", None)
+                            chargePointPowerInKw, values = catalogue_value(chargePointVariants, make, model, "powerInKw", min)
+                            self.log_catalogue_ambiguity(CATALOGUE_AMBIGUOUS_CHARGER, make, model, values, chargePointPowerInKw, "kW")
                         else:
-                            for vehicle in electricVehicles:
-                                if vehicle.get("make", None) == make:
-                                    models = vehicle.get("models", [])
-                                    for vehicle_info in models:
-                                        if vehicle_info.get("model", None) == model:
-                                            vehicleBatterySizeInKwh = vehicle_info.get("batterySize", None)
+                            vehicleBatterySizeInKwh, values = catalogue_value(electricVehicles, make, model, "batterySize", max)
+                            self.log_catalogue_ambiguity(CATALOGUE_AMBIGUOUS_VEHICLE, make, model, values, vehicleBatterySizeInKwh, "kWh")
 
                         intelligent_device = {
                             "deviceType": deviceType,

apps/predbat/tests/test_octopus_catalogue_cache.py

@@ -142,7 +142,8 @@ async def test_octopus_catalogue_cache(my_predbat):
         print("\n*** Test 3: battery size resolved from cached catalogue ***")
         device = (result or {}).get("device-1", {})
         battery = device.get("vehicle_battery_size_in_kwh")
-        if battery != "57.5":
+        # The catalogue's "57.5" string comes back as a number (catalogue_value(), issue #5438)
+        if battery != 57.5:
             print("ERROR: Expected battery size 57.5 from the cached catalogue, got {}".format(battery))
             failed = True
         else: