diff --git a/.github/workflows/python-tests.yaml b/.github/workflows/python-tests.yaml index 97b700f..8ae7dd6 100644 --- a/.github/workflows/python-tests.yaml +++ b/.github/workflows/python-tests.yaml @@ -23,5 +23,20 @@ jobs: print(next(req for req in requirements if req.startswith("mg-ismart-india-client"))) PY )" + - name: Check for undefined names + # A missing import is invisible to py_compile and to unit tests that + # never reach the line — 1.2.7-beta8 shipped a NameError in the + # coordinator's poll path this way. pyflakes catches the whole class + # in under a second. Only undefined names fail the build; unused + # imports and similar are reported but tolerated. + run: | + python -m pip install pyflakes + python -m pyflakes custom_components/mg_saic > pyflakes.txt || true + cat pyflakes.txt + if grep -q "undefined name" pyflakes.txt; then + echo "::error::pyflakes found undefined name(s) — see above" + exit 1 + fi + - name: Run unit tests run: python -m unittest discover -s tests -p "test_*.py" diff --git a/README.md b/README.md index a83dacf..95b63c0 100644 --- a/README.md +++ b/README.md @@ -134,7 +134,7 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - Tyre Pressure Rear Left - Tyre Pressure Rear Right #### Electric / Hybrid -- State of Charge (SOC) +- State of Charge (SOC) *(BEV/PHEV; also HEV on self-charging hybrids with no charge port, e.g. MG3 Hybrid+ — see [Vehicle Profiles](#vehicle-profiles))* - Electric Range - Instant Power *(kW draw/regen while driving; negative = traction, positive = regen/charge)* - Fuel Level *(PHEV/HEV/ICE only)* @@ -150,14 +150,17 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - Charging Current - Charging Current Limit - Charging Power -- Estimated Range After Charging +- Estimated Range After Charging *(the range the car expects to reach when the current charge completes)* - Target SOC *(read-only mirror of the Target SOC slider — shown only on models where the iSmart app supports it)* - Charging Duration - Remaining Charging Time -- Added Electric Range +- Added Electric Range *(live during a charge session only, and not reported at all by most cars — see [Trip & efficiency statistics](#trip--efficiency-statistics))* - Power Usage Since Last Charge - Mileage Since Last Charge - Efficiency Since Last Charge *(BEV/PHEV; km/kWh, derived from the two sensors above — see [Trip & efficiency statistics](#trip--efficiency-statistics))* +- Efficiency Since Charge (SOC) *(BEV/PHEV; km/kWh, an SOC/odometer-only alternative independent of the counters above — see [Trip & efficiency statistics](#trip--efficiency-statistics))* +- Last Charge Range Added *(BEV/PHEV; electric range the last completed charge put back — shown in your Home Assistant unit system, so miles if that's what you use)* +- Last Charge Energy *(BEV/PHEV; kWh put **into** the battery by the last completed charge — see [Trip & efficiency statistics](#trip--efficiency-statistics))* - Last Trip Distance *(distance driven on the last completed drive)* - Last Trip Efficiency *(BEV/PHEV; switchable km/kWh · mi/kWh · kWh/100km, full breakdown in attributes)* - Last Trip Fuel Economy *(ICE/HEV/PHEV; L/100km, with the full breakdown in its attributes)* @@ -171,9 +174,26 @@ The integration derives per-trip and per-charge efficiency from data it already **Efficiency Since Last Charge** *(BEV/PHEV)* comes straight from the car's own `Mileage Since Last Charge` and `Power Usage Since Last Charge` figures, so it's available immediately and needs no trip tracking. +**Efficiency Since Charge (SOC)** *(BEV/PHEV)* is an alternative to the sensor above, computed entirely from the odometer and battery percentage — it never touches the `Mileage Since Last Charge` / `Power Usage Since Last Charge` fields at all. It exists because those fields are unreliable on some cars (they can reset spuriously without an actual charge — see below) and permanently unpopulated (`Unknown`) on others; this sensor works either way, and lets you compare the two where both are available. Its "since charge" point is whenever the car's battery percentage was last seen to rise while parked, which may not always be a full charge to 100%. + +**Last Charge Energy** *(BEV/PHEV)* reports how much energy the last completed charge put **into** the battery. The API has no field for this — it reports charging power live, and `Power Usage Since Last Charge` (energy taken back *out* afterwards), but there is no "starting power" to subtract from `lastChargeEndingPower` — so the session is measured across its start and end. Two independent figures are produced, and both appear in the attributes: + +- `energy_added_kWh_soc` — the rise in battery percentage × the usable capacity. This is the headline value, because it works on any car that reports SOC and has a known capacity (see [Battery capacity override](#battery-capacity-override) if yours is wrong). +- `energy_added_kWh_counter` — the change in the car's own pack-energy figure (`lastChargeEndingPower` minus `Power Usage Since Last Charge`). Independent of the capacity figure, but it relies on the car refreshing `lastChargeEndingPower` promptly when the charge ends, so it's omitted when it doesn't look plausible. + +Also in the attributes: `range_added_km` (with `range_start_km` / `range_end_km`), `soc_start_pct`, `soc_end_pct`, `soc_added_pct`, `duration_s`, `average_power_kW`, `method` (which figure was used), and the session's start/end timestamps. A `mg_saic_charge_completed` event fires when a charge finishes, carrying the same data, so you can log or notify on it. + +The same figure is also published as its own **Last Charge Range Added** sensor. Prefer that one for dashboards: sensor states are converted to your Home Assistant unit system (so miles on an imperial setup), whereas attribute values never are — the `*_km` attributes below are always kilometres regardless of your settings. + +`range_added_km` is the electric range the charge added, measured across the session. Note this is *not* the same as the **Added Electric Range** sensor, which exposes the API's own `chrgngAddedElecRng` — a live counter that runs during a session and resets when it ends, and which on the cars observed so far stays at 0 throughout. The range delta here is derived from the electric range reading at each boundary instead. + +Note the energy figure is measured **at the battery**, so it will read lower than a wall meter or smart charger, which also pay for charger and cable losses. A charge that delivers less than 0.5% is ignored (that's the small percentage rebound the pack reports after a drive, not a charge), and a session left open more than 48 hours is abandoned rather than reported. A charging-data dropout is never mistaken for the end of a charge — on some cars the charging endpoint goes quiet the moment a session completes. + **Last Trip** sensors are populated when a drive ends (the car powers off). Distance and electric energy come from the car's own cumulative counters (`Mileage Since Last Charge` / `Power Usage Since Last Charge`), diffed between one trip and the next — so they match the car's own measurements and don't depend on exactly when the trip was detected. (For non-charging models, distance falls back to the odometer.) A charge between trips is handled automatically (the counters reset). A trip is one power-on to power-off, so a journey with a stop in the middle counts as two trips. -On some cars, the since-charge counters occasionally reset on their own without an actual charge. If that happens mid-trip, the trip falls back to the odometer for distance and to the battery-percentage change for energy, and carries a `counter_reset_detected` attribute so it's visible when this happened. +Because the counters aren't always trustworthy (see below), `Last Trip Distance` and `Last Trip Efficiency` also expose the counter-only and odometer/SOC-only figures **independently**, as attributes, alongside the primary (counter-preferred) value — so you can compare them directly for any trip: `distance_km_counter` / `distance_mi_counter` and `distance_km_odometer` / `distance_mi_odometer` on Last Trip Distance; `energy_kWh_counter` / `efficiency_km_per_kWh_counter` / `efficiency_mi_per_kWh_counter` / `consumption_kWh_per_100km_counter` / `consumption_kWh_per_100mi_counter` and the equivalent `_soc` set on Last Trip Efficiency. The counter figures are shown raw/unfiltered, even on a trip where the primary figure discarded them (see `counter_reset_detected` below) — seeing what the counter actually reported is itself useful. + +On some cars, the since-charge counters occasionally reset on their own without an actual charge. If that happens mid-trip, the primary trip figure falls back to the odometer for distance and to the battery-percentage change for energy, and carries a `counter_reset_detected` attribute so it's visible when this happened. If a drive is never seen live — the car wasn't polled while it was powered (a short trip that fell between polls, or a missed vehicle-start message) — the trip is reconstructed afterwards from the odometer movement once the car is next seen parked. These reconstructed trips carry `retrospective: true` and `timing: approximate` attributes, because the exact start/end times aren't known and several short hops in the same gap may be merged into one. If a trip ever gets stuck "open" (its power-off was missed), it's force-closed automatically so it doesn't block new trips. @@ -518,7 +538,9 @@ Some MG models share one series code across several battery sizes (the MG4, for The **Usable battery capacity override (kWh)** option (under **Configure**) lets you set your car's usable capacity yourself. When set, it takes priority over both our built-in per-model value and the API-reported value, and it becomes the figure used everywhere capacity matters: the **Total Battery Capacity** sensor and the electric energy/efficiency calculations (including Last Trip figures on models that fall back to a battery-percentage estimate). Enter the **usable** capacity for your variant; leave it blank to go back to the automatic value. Saving the option takes effect immediately — no restart or reload needed. -The Total Battery Capacity sensor carries a `capacity_source` attribute (`user_override`, `profile`, or `api`) so you can see — and template off — exactly where the displayed figure came from. +The Total Battery Capacity sensor carries a `capacity_source` attribute (`user_override`, `profile`, or `api`) so you can see — and template off — exactly where the displayed figure came from. The same resolved figure feeds every energy calculation derived from capacity, so the displayed pack size and the sensors derived from it can't disagree. + +Where a car reports a capacity that can't be trusted, none is used: the `totalBatteryCapacity=725` placeholder (→ 72.5 kWh) is rejected outright, as is anything outside 5–200 kWh. On such a car with no profile figure and no override, Total Battery Capacity reads blank and `capacity_source` is absent, rather than showing a number the car invented and deriving charge and efficiency figures from it. Setting a [battery capacity override](#battery-capacity-override) is the fix if you know your real capacity. ## 📋 Entity States Reference @@ -617,7 +639,9 @@ The integration includes built-in profiles for specific MG/SAIC models that corr | `EC32` | MG Cyberster | 2-door BEV roadster; no rear doors/windows; unreliable live electric range field (falls back to estimated range) | | `IS31P` | MG S9 PHEV (2025) | Climate status/fan speed mappings confirmed by physical testing | | `AS33P` | MG HS PHEV (Super Hybrid 2025/2026) | Battery capacity 24.7 kWh; Target SOC and Charging Current Limit not supported by iSmart; electric range uses live SOC-tracking field; energy values corrected for ~3x API over-reporting | -| `S12L` | IM6 (IM by MG Motor) | Battery capacity 100 kWh — corrects the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh) for the Platinum/Performance pack (#53). ⚠️ Confirmed on the 100 kWh Platinum; if the 75 kWh LFP Premium reports the same series, this will need splitting — Premium owners, please open an issue with debug logs | +| `S12L` | IM6 (IM by MG Motor) | Battery capacity 96.5 kWh usable (100 kWh nominal NMC) — replaces the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh) (#53). In the UK/EU the IM6 is sold on the 100 kWh pack only, so this covers every variant; a 75 kWh LFP Premium exists in some other markets and would need 73.5 kWh and a split if it reports the same series | +| `P12L` | IM5 (IM by MG Motor) | Mode-select climate scheme mirroring the MGS6 (status code 2 = cool, confirmed, #326) — fixes the car showing as "Fan only" while genuinely cooling. Fan-only/heat/defrost/max-cool values are still unconfirmed best-effort, pending a debug log with the AC confirmed on. Battery capacity set to **96.5 kWh usable** for the confirmed Long Range/Performance pack (100 kWh nominal NCM, #326), replacing the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh). ⚠️ The IM5 **Standard Range** (75 kWh LFP, 73.5 kWh usable) reports the same series code and will read too high — set a [battery capacity override](#battery-capacity-override) to 73.5 and please comment on #326 so the variants can be split | +| `ZP22 EU` | MG3 Hybrid+ | Self-charging full hybrid (1.83 kWh HV battery, no charge port); reports as vehicle type HEV. State of Charge is now populated from `basicVehicleStatus.extendedData1`, since this vehicle type has no charging-endpoint data to read (#318) | Models not listed above use safe default values and should work normally. If you notice incorrect sensor readings for your model, please open an issue with your vehicle's debug logs. diff --git a/RELEASE_NOTES_1.2.7-beta5.md b/RELEASE_NOTES_1.2.7-beta5.md new file mode 100644 index 0000000..35488c1 --- /dev/null +++ b/RELEASE_NOTES_1.2.7-beta5.md @@ -0,0 +1,38 @@ +# 1.2.7-beta5 + +Two fixes that affect every car, and one new sensor. All three came out of [discussion #262](https://github.com/townsmcp/mg-saic-ha/discussions/262) — thanks to @HarryFlatter for the detail. + +## Fixed: Efficiency Since Charge (SOC) never showed a value + +This sensor has been stuck on `Unknown` since it was introduced — on **all** models, not just PHEVs, and regardless of how much you'd driven since charging. + +Two separate faults were stacked on top of each other. The sensor was handing the wrong object to its odometer lookup, and that lookup's backup route was searching a part of the API response that doesn't contain an odometer at all. With no odometer reading, the sensor had nothing to calculate a distance from, so it never produced a figure. + +Both are fixed. The underlying charge-detection was working correctly all along, so the sensor starts reporting as soon as you've driven after a charge. + +## Fixed: Power Usage Since Last Charge still ~3× too high on MG HS PHEV + +The correction for this was added in 1.2.6, but it was applied in a code path this particular sensor never takes — so in practice nothing changed and the sensor kept showing the inflated figure. It's now applied wherever the value is read. + +On an HS PHEV, a reading like 20.20 kWh should have been around 6.9 kWh. + +Two related sensors — `Efficiency Since Last Charge` and the `Last Trip` energy figures — were already correcting this properly and were never affected. If you have an HS PHEV and your energy figures looked inconsistent with each other, this is why. + +## New: Last Charge Energy sensor *(BEV/PHEV)* + +Tells you how much energy the last completed charge put **into** the battery. Useful when you've charged somewhere that isn't home and want to know what you actually took — the car reports charging power while it's happening, and how much you've used *since* charging, but nothing about the charge itself. + +The API has no field for this, so the integration measures it across the charging session. Two independent figures are calculated and both appear in the sensor's attributes: + +- **From battery percentage** — the rise in charge level against your car's usable capacity. This is the headline value, as it works on any car that reports a battery percentage. If your `Total Battery Capacity` looks wrong, set a [battery capacity override](https://github.com/townsmcp/mg-saic-ha#battery-capacity-override) and this will follow it. +- **From the car's own energy figures** — shown alongside for comparison, and omitted when it doesn't look trustworthy. + +Also in the attributes: start and end battery percentage, percentage added, how long the charge took, average power, and the session's timestamps. A `mg_saic_charge_completed` event fires at the end of each charge carrying the same data, so you can notify or log on it. + +**Worth knowing:** this is energy measured at the battery, so it will read lower than your wall meter, Zappi or similar — those also pay for charger and cable losses. Expect a gap of roughly 5–10%. + +Some deliberate limits: charges that add less than 0.5% are ignored (that's the small rebound the pack reports after a drive rather than a real charge), and a session left open more than 48 hours is dropped rather than reported as a nonsense number. Crucially, a charging-data dropout is never mistaken for the end of a charge — on some cars the charging endpoint goes quiet the instant a session finishes, which would otherwise log a phantom charge every time. + +## Upgrading + +No action needed. The Last Charge Energy sensor appears after a restart and populates once it has seen a complete charge from start to finish — so it will read `Unknown` until your next charge finishes. diff --git a/custom_components/mg_saic/backends/__init__.py b/custom_components/mg_saic/backends/__init__.py index 0f07003..b8d9f2e 100644 --- a/custom_components/mg_saic/backends/__init__.py +++ b/custom_components/mg_saic/backends/__init__.py @@ -55,6 +55,7 @@ class Feature(str, Enum): # Data retrieval STATUS = "status" # get_vehicle_status STATE_OF_CHARGE = "state_of_charge" # SOC from status or charging data + STATE_OF_CHARGE_NON_BEV = "state_of_charge_non_bev" # PHEV/HEV SOC: basicVehicleStatus.extendedData1 is battery SOC, not fuel CHARGING_DATA = "charging_data" # get_charging_info (incl. SOC) ALARM_MESSAGES = "alarm_messages" # get_alarm_messages / set_alarm_switches / message poller @@ -87,14 +88,21 @@ class Feature(str, Enum): # Features implemented AND confirmed on a real vehicle by the India TAP # client (John Lazarus, mg-ismart-india-ha). MG India reports BEV state of -# charge in the ordinary vehicle-status payload, but separate charging data -# and charging controls are deliberately absent. +# charge in the ordinary vehicle-status payload, and charging telemetry in a +# separate frame; charging controls are deliberately absent. # ALARM_MESSAGES is absent because the TAP protocol has no message-list # endpoint — the account message poller must not run for India accounts. +# STATE_OF_CHARGE_NON_BEV is absent because India repurposes +# basicVehicleStatus.extendedData1 to carry fuel level: on a BEV that field is +# the SOC, but on a PHEV/HEV it is litres of petrol, so a non-BEV must not read +# SOC from it even though CHARGING_DATA is now supported. INDIA_FEATURES: frozenset[Feature] = frozenset( { Feature.STATUS, Feature.STATE_OF_CHARGE, + # Charging telemetry decoded from the 63-byte TAP frame: voltage, + # current, power and charge state (get_charging_info in india.py). + Feature.CHARGING_DATA, Feature.LOCK, Feature.TAILGATE, Feature.WINDOWS, diff --git a/custom_components/mg_saic/backends/india.py b/custom_components/mg_saic/backends/india.py index 9895647..7b5cf30 100644 --- a/custom_components/mg_saic/backends/india.py +++ b/custom_components/mg_saic/backends/india.py @@ -11,7 +11,7 @@ from aiohttp import ClientSession from mg_ismart_india_client import MgIndiaApiError, MgIndiaClient, hash_control_pin -from ..const import LOGGER +from ..const import CHARGING_CURRENT_FACTOR, CHARGING_VOLTAGE_FACTOR, LOGGER from . import INDIA_FEATURES @@ -72,6 +72,38 @@ def _tyre_pressure(status, attribute: str) -> float | None: return round(psi * _BAR_PER_PSI / _EU_TYRE_BAR_PER_UNIT, 2) +# The global SAIC protocol encodes charging current as an offset from a 1000 A +# zero point (decoded as 1000 - raw * CHARGING_CURRENT_FACTOR), so real amps are +# re-encoded against the same zero point below. +_CHARGING_CURRENT_ZERO_A = 1000 + +# bmsChrgSts codes the shared charging sensors decode; names match the mapping in +# sensor.py. India reports charging as two booleans, so only these three are used. +_BMS_CHRG_STS_UNPLUGGED = 0 +_BMS_CHRG_STS_CHARGING = 3 +_BMS_CHRG_STS_PLUGGED_IN = 7 # connected but not charging: paused or full + +# The Charging Duration sensor reads rvsChargeStatus.chargingDuration with +# DATA_100_DECIMAL_CORRECTION and labels the result minutes, i.e. it expects +# hundredths of a minute. The India client reports elapsed session time in +# seconds, so convert instead of passing the seconds straight through. +_SECONDS_PER_MINUTE = 60 +_CHARGING_DURATION_UNITS_PER_MINUTE = 100 + + +def _charging_duration_units(seconds: int | None) -> int | None: + """Convert elapsed seconds into the hundredths-of-a-minute the sensor decodes. + + :param seconds: :attr:`~mg_ismart_india_client.models.ChargeStatus.charge_time_elapsed_s`. + :returns: the value for ``rvsChargeStatus.chargingDuration``, or ``None``. + """ + if seconds is None: + return None + return round( + seconds / _SECONDS_PER_MINUTE * _CHARGING_DURATION_UNITS_PER_MINUTE + ) + + def _micro_degrees(value: float | None) -> int | None: if value is None: return None @@ -168,6 +200,8 @@ def __init__(self, username, password, vin=None, pin_hash=None, country_code=Non self.region_name = "India" self._session: ClientSession | None = None self._client: MgIndiaClient | None = None + self._charge_status_by_vin = {} + self._electric_vins = set() self._seat_levels = {"front_left": 0, "front_right": 0} async def _ensure_client(self) -> MgIndiaClient: @@ -201,13 +235,21 @@ async def close(self): async def get_vehicle_info(self): client = await self._ensure_client() vehicles = await client.vehicles() + self._electric_vins = { + vehicle.vin for vehicle in vehicles if _looks_electric(vehicle) + } if self.vin is None and vehicles: self._set_vin(vehicles[0].vin) return [self._map_vehicle(vehicle) for vehicle in vehicles] async def get_vehicle_status(self, vin: str | None = None): self._set_vin(vin) - return self._map_status(await (await self._ensure_client()).status()) + self._charge_status_by_vin.pop(self.vin, None) + status = await (await self._ensure_client()).status( + include_charge=self.vin in self._electric_vins + ) + self._charge_status_by_vin[self.vin] = status.charge + return self._map_status(status) def _map_vehicle(self, vehicle): model_name = ( @@ -361,6 +403,58 @@ async def stop_ac(self, vin): self._set_vin(vin) await (await self._ensure_client()).control_climate(False) + async def get_charging_info(self, vin): + """Map the India EV charging status onto the ``chrgMgmtData`` / + ``rvsChargeStatus`` shapes the shared charging sensors read. + + Voltage, current, SOC and range come from the declared-unit fields of + :class:`~mg_ismart_india_client.models.ChargeStatus` (volts, amps, + percent, km) and are re-encoded onto the global SAIC raw scales the + shared sensors decode (:data:`~..const.CHARGING_VOLTAGE_FACTOR` / + :data:`~..const.CHARGING_CURRENT_FACTOR` / tenths), rather than assuming + the India protocol's raw field values happen to share the global + protocol's raw scale. ``rvsChargeStatus`` is likewise built field by + field, so every value the sensors read has a named source and a stated + scale assumption. + + The preceding status refresh requests both frames from the shared TAP + stream and stores the charging frame here. This method only maps that + result; it must not start a second poll. + + :param vin: VIN to report charging status for. + :returns: a namespace carrying ``chrgMgmtData`` and ``rvsChargeStatus``, + or ``None`` when the status poll did not receive a charging frame. + """ + self._set_vin(vin) + charge = self._charge_status_by_vin.pop(self.vin, None) + if charge is None: + return None + if charge.is_charging: + bms_chrg_sts = _BMS_CHRG_STS_CHARGING + elif charge.is_plugged_in: + bms_chrg_sts = _BMS_CHRG_STS_PLUGGED_IN + else: + bms_chrg_sts = _BMS_CHRG_STS_UNPLUGGED + chrg_mgmt = _ns( + bmsPackVol=round(charge.charging_voltage / CHARGING_VOLTAGE_FACTOR), + bmsPackCrnt=round( + (_CHARGING_CURRENT_ZERO_A - charge.charging_current) + / CHARGING_CURRENT_FACTOR + ), + bmsPackSOCDsp=_tenths(charge.soc), + bmsChrgSts=bms_chrg_sts, + ) + rvs = _ns( + # Range and odometer come back in real units and re-encode to tenths. + fuelRangeElec=_tenths(charge.range_km), + mileage=_tenths(charge.odometer_km), + chargingGunState=charge.is_plugged_in, + chargingDuration=_charging_duration_units(charge.charge_time_elapsed_s), + totalBatteryCapacity=_tenths(charge.total_battery_capacity_kwh), + mileageSinceLastCharge=_tenths(charge.distance_since_last_charge_km), + ) + return _ns(chrgMgmtData=chrg_mgmt, rvsChargeStatus=rvs) + async def control_heated_seat(self, vin, seat, level): self._set_vin(vin) if seat not in self._seat_levels: diff --git a/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index 3676b58..9e147e1 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -157,6 +157,87 @@ # electrive.com, and Carwow (77 kWh gross / 74.3 kWh usable, same across # single-motor Long Range and Dual Motor variants). VEHICLE_PROFILES = { + "P12L": { # MG IM5 (IM Motors, "IM presented by MG Motor") — BEV liftback saloon. See #326. + # Reported by tabannis (#326): iSmart app showed AC on, 20°C, Auto (owner + # confirmed by sitting in the car — genuinely cooling). Debug logs showed + # 'P12L' as unprofiled, falling to DEFAULT_VEHICLE_PROFILE (fan_speed + # scheme). A follow-up screenshot of the HA Climate Ctrl card confirmed the + # failure: Mode showed "Fan only" (plus an irrelevant Fan mode "Medium" + # slider) while the car was genuinely cooling. This is the exact #277 + # (MGS5) signature: DEFAULT maps remoteClimateStatus=2 (the car's real + # "cooling" status) to climate_status_fan_only={2}, not cool. + # + # The IM5/IM6 are a separate platform from the classic MG ICE-derived + # range, architecturally contemporary with the MGS6/MGS5 (MIS3E/MZS3E). + # tabannis confirmed (#326 comments) the iSmart app shows Temperature + + # AC on/off with NO fan-speed control, matching mode_select rather than + # fan_speed — so this profile mirrors MIS3E's mode_select mapping: + # climate_mode_cool / climate_status_cool = 2 — CONFIRMED (screenshot: + # genuine cooling while unprofiled DEFAULT reported the status-2 + # -derived "Fan only" state) + # fan_only / heat / defrost / max_cool values — STILL UNCONFIRMED, + # inherited from MIS3E as best-effort. The app's "Low"/"High" + # temperature buttons (tabannis, #326) are one-tap max-cool/ + # max-heat presets, not a separate remote fan-only mode — so + # these four codes still await a debug log captured while the + # AC is confirmed on, which tabannis is sending separately. + # AC-gates-heat quirk (tabannis, #326: "to get heat or cool, the AC + # has to be on. No AC, no heat") mirrors the MGS6 (James, same + # thread) — supporting evidence the MIS3E-inherited values are + # directionally right, though not a wire confirmation. + # + # Temperature range/offset: 16-28°C / offset 2 — CONFIRMED by tabannis + # (#326: "16 - 28°C plus a Low and a High"), matches what was already + # set from DEFAULT. + # + # Battery capacity: 96.5 kWh USABLE (100 kWh nominal). tabannis + # confirmed (#326) his car is + # the Long Range variant, i.e. the 100 kWh NCM/800V pack — resolving + # the ambiguity noted below. The log shows the same bogus + # totalBatteryCapacity=725 placeholder seen on EC32/AS33P/S12L + # (bmsPackSOCDsp=479 x 725 = realtimePower=347 exactly, confirming + # it's pure SOC arithmetic on the placeholder, not a measured value), + # and bmsPackVol=3032 (~758V, 800V-class) independently corroborates + # the 100 kWh Long Range/Performance pack (same cross-check pattern as + # S12L/#53). The IM5 also ships a 75 kWh Standard Range (LFP, 400V) + # variant that 'P12L' cannot yet distinguish from Long + # Range/Performance by series code alone (same unresolved ambiguity as + # S12L/IM6 Premium vs Platinum, #53) — if a Standard Range owner's car + # is later found to also report 'P12L', this entry will need + # splitting by a better discriminator (e.g. the ~400V vs ~758V + # bmsPackVol split used here). No charging_capacity_correction is + # applied — same as S12L, this is a display-only capacity override, + # not an energy-scaling correction. + "min_temp": 16, + "max_temp": 28, + "temp_offset": 2, + "battery_capacity_kwh": 96.5, # usable; 100 nominal + "fuel_tank_litres": None, # BEV — no fuel (mirrors DEFAULT) + "climate_control_scheme": "mode_select", + "climate_mode_cool": 2, # CONFIRMED (#326 screenshot + logs) + "climate_mode_fan_only": 1, # unconfirmed on IM5 (no app control seen) + "climate_mode_heat": 4, # unconfirmed on IM5 (no app control seen) + "climate_mode_max_cool": 3, # unconfirmed on IM5 ("High" button is a temp preset, not a distinct wire status) + "climate_mode_defrost": 5, # unconfirmed on IM5 (no app control seen) + "climate_status_cool": {2, 3}, + "climate_status_fan_only": {1}, + "climate_status_heat": {4}, + "climate_status_defrost": {5}, + # Unused under mode_select (no FAN_MODE feature is exposed — see + # climate.py), but kept for consistency with MIS3E/MZS3E and as a + # safe fallback should the scheme ever need revisiting. + "fan_speed_low": 1, + "fan_speed_medium": 2, + "fan_speed_high": 3, + "temp_idx_inverted": False, + "supports_target_soc": True, + "supports_charging_current_limit": True, + "reliable_fuel_range_elec": True, + "charging_capacity_correction": None, + "model_year_override": None, + "has_rear_doors": True, + "has_rear_windows": True, + }, "ZP22": { # MG3 Hybrid (HEV) — see #258 "min_temp": 16, "max_temp": 30, @@ -592,7 +673,17 @@ "min_temp": 16, "max_temp": 28, "temp_offset": 2, - "battery_capacity_kwh": 100.0, + # 96.5 kWh USABLE (100 kWh nominal NMC). Profiles store usable + # capacity, not marketed pack size — see AS33P (23.2 usable / 24.7 + # nominal). + # + # The 75 kWh LFP Premium noted in the Australian spec sheet above is + # not offered in the UK/EU: the IM6 ships there on the 100 kWh NMC + # pack only, in Long Range and Performance. So for these markets + # S12L is unambiguous and the trim caveat below does not apply. It is + # kept because a 75 kWh Premium may exist in other markets; if one + # ever reports 'S12L' it needs 73.5 usable and its own split. + "battery_capacity_kwh": 96.5, "fuel_tank_litres": None, # BEV — no fuel (mirrors DEFAULT) "climate_status_cool": {3}, "climate_status_fan_only": {2}, @@ -988,6 +1079,11 @@ def parse_capacity_override(raw): # frequent refresh cadence as AC/DC charging sessions. CHARGING_STATUS_CODES = {1, 3, 10, 12, 13} +# Statuses that count as energy going INTO the battery, for the Last Charge +# Energy session tracking (#262). Deliberately excludes 13 (V2X_DISCHARGING): +# that is energy flowing the other way, so it must never open a charge session. +CHARGE_SESSION_STATUS_CODES = {1, 3, 10, 12} + # Charging Current Limit options CHARGING_CURRENT_OPTIONS = ["0A (Ignore)", "6A", "8A", "16A", "Max"] diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index df67ede..a13edb5 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -11,8 +11,14 @@ from .api import SAICMGAPIClient, CommandsLimitReachedException from .backends import Feature from .backends import backend_supports as _backend_supports -from .logic import select_update_interval -from .trip_stats import TripStatsManager, TripSnapshot +from .logic import ( + apply_energy_correction, + electric_range_km, + odometer_km, + resolve_battery_capacity, + select_update_interval, +) +from .trip_stats import TripStatsManager, TripSnapshot, ChargeSnapshot # After the car turns off, fire extra refreshes at these intervals (seconds) # to catch plug-in as quickly as possible. The coordinator is still on its @@ -28,6 +34,7 @@ MILEAGE_UINT16_SATURATION, AFTER_ACTION_UPDATE_INTERVAL_DELAY, CHARGING_STATUS_CODES, + CHARGE_SESSION_STATUS_CODES, CONF_ABRP_API_KEY, CONF_ABRP_USER_TOKEN, DEFAULT_AC_LONG_INTERVAL, @@ -1366,23 +1373,26 @@ def _extract_since_charge(self, charging_data): so apply the profile's charging_capacity_correction to the ENERGY only — distance is never inflated — giving real kWh for trip energy/efficiency.""" km, kwh = self._extract_since_charge_raw(charging_data) - if kwh is not None and self.charging_capacity_correction is not None: - kwh = kwh * self.charging_capacity_correction + kwh = apply_energy_correction( + "powerUsageSinceLastCharge", kwh, self.charging_capacity_correction + ) return km, kwh @staticmethod def _extract_odometer_km(basic_status, charging_data): - """Odometer in km, or None. Rejects 0/-128 and the uint16 saturation.""" - for source, factor in ((basic_status, DATA_DECIMAL_CORRECTION),): - raw = getattr(source, "mileage", None) if source is not None else None - if raw is not None and raw > 0 and raw != MILEAGE_UINT16_SATURATION: - return raw * factor - # Fall back to the wider odometer field in charging data. - chrg = getattr(charging_data, "chrgMgmtData", None) if charging_data else None - raw = getattr(chrg, "mileage", None) if chrg is not None else None - if raw is not None and raw > 0: - return raw * DATA_DECIMAL_CORRECTION - return None + """Odometer in km, or None. Rejects 0/-128 and the uint16 saturation. + + Delegates to logic.odometer_km so the source preference and the + fallback order are unit-testable without Home Assistant — see #262, + where the charging-data fallback read a field that doesn't exist and + nothing caught it because nothing could test it directly. + """ + return odometer_km( + basic_status, + charging_data, + factor=DATA_DECIMAL_CORRECTION, + saturation=MILEAGE_UINT16_SATURATION, + ) @staticmethod def _extract_soc_pct(basic_status, charging_data): @@ -1429,7 +1439,7 @@ def _update_trip_state(self, power_mode, basic_status, charging_data): snap = self._trip_snapshot(basic_status, charging_data) trip_kwargs = dict( - capacity_kwh=self.known_battery_capacity_kwh, + capacity_kwh=self.effective_battery_capacity_kwh, tank_litres=self.known_fuel_tank_litres, is_electric=self.vehicle_type in ("BEV", "PHEV"), is_combustion=self.vehicle_type in ("ICE", "HEV", "PHEV"), @@ -1452,6 +1462,12 @@ def _update_trip_state(self, power_mode, basic_status, charging_data): # Parked (or unknown) — a reading is needed to close or reconstruct. if snap is None: return + # Track the SOC-based "since reset" baseline only while parked, so a + # mid-drive regen SOC uptick can never be mistaken for a charge (#301: + # this sensor is deliberately independent of the since-charge counter + # fields, which are unreliable on some cars and absent on others). + if self.trip_stats.note_soc_reset_baseline(snap.soc_pct, snap.odometer_km, snap.ts): + self._schedule_trip_save() if open_snap is not None: trip = self.trip_stats.close(snap, **trip_kwargs) LOGGER.debug("Trip closed for VIN %s: %s", self.vin, trip) @@ -1468,6 +1484,105 @@ def _update_trip_state(self, power_mode, basic_status, charging_data): elif was_seeded: self._schedule_trip_save() + @property + def battery_capacity_resolution(self): + """(capacity_kwh, source) using override > profile > API, or (None, None). + + Every capacity consumer reads this, so the Total Battery Capacity + sensor and the energy maths derived from it can no longer disagree + about what the pack holds — which they did: the sensor honoured the + API tier while the derived figures did not, leaving unprofiled cars + with a populated capacity next to three blank sensors (#262, #302). + """ + return resolve_battery_capacity( + self.battery_capacity_override, + getattr(self, "_profile_battery_capacity_kwh", None), + self._api_battery_capacity_raw(), + factor=DATA_DECIMAL_CORRECTION, + ) + + @property + def effective_battery_capacity_kwh(self): + """Usable capacity in kWh from any tier, or None if nothing is usable.""" + return self.battery_capacity_resolution[0] + + def _api_battery_capacity_raw(self): + """The car's own totalBatteryCapacity, raw and uncorrected, or None.""" + charging_data = (self.data or {}).get("charging") + rcs = getattr(charging_data, "rvsChargeStatus", None) if charging_data else None + raw = getattr(rcs, "totalBatteryCapacity", None) if rcs is not None else None + return raw if raw is not None and raw > 0 else None + + def _extract_pack_energy_kwh(self, charging_data): + """Energy currently held in the pack (kWh), per the car's own figures. + + ``lastChargeEndingPower`` is what the pack held when the last charge + finished; ``powerUsageSinceLastCharge`` is what has been taken out + since. The difference is therefore the energy in the pack right now, + and it holds at both charge boundaries — at the end of a charge the + since-charge counter is ~0, so it collapses to lastChargeEndingPower. + + Both fields are inflated ~3× on some models, so both get the profile's + charging_capacity_correction (#262). Returns None if either is missing. + """ + rcs = getattr(charging_data, "rvsChargeStatus", None) if charging_data else None + if rcs is None: + return None + raw = getattr(rcs, "lastChargeEndingPower", None) + if raw is None or raw < 0: + return None + ending = apply_energy_correction( + "lastChargeEndingPower", + raw * DATA_DECIMAL_CORRECTION, + self.charging_capacity_correction, + ) + _, used = self._extract_since_charge(charging_data) + return ending - (used or 0.0) + + def _charge_snapshot(self, basic_status, charging_data): + """Build a ChargeSnapshot for the charge-session tracker, or None.""" + return ChargeSnapshot( + ts=datetime.now(timezone.utc).isoformat(), + soc_pct=self._extract_soc_pct(basic_status, charging_data), + pack_energy_kwh=self._extract_pack_energy_kwh(charging_data), + odometer_km=self._extract_odometer_km(basic_status, charging_data), + range_km=electric_range_km( + basic_status, charging_data, factor=DATA_DECIMAL_CORRECTION + ), + ) + + def _update_charge_state(self, basic_status, charging_data): + """Open/close a charging session so Last Charge Energy can report how + much went IN — the API has no such field (#262, @HarryFlatter). + + Only ever evaluated when the charging endpoint actually answered. A + failed charging fetch drops charging_data to None and would look + identical to the charge ending, and on some cars that endpoint goes + quiet the instant a session completes — so a dropout must not be + allowed to close (or open) a session. + """ + if self.trip_stats is None: + return + chrg_mgmt_data = ( + getattr(charging_data, "chrgMgmtData", None) if charging_data else None + ) + if chrg_mgmt_data is None: + return + status = getattr(chrg_mgmt_data, "bmsChrgSts", None) + if status is None: + return + charge, changed = self.trip_stats.note_charge_state( + status in CHARGE_SESSION_STATUS_CODES, + self._charge_snapshot(basic_status, charging_data), + capacity_kwh=self.effective_battery_capacity_kwh, + now_iso=datetime.now(timezone.utc).isoformat(), + ) + if charge is not None: + LOGGER.debug("Charge session completed for VIN %s: %s", self.vin, charge) + self.trip_stats.fire_charge_event(charge) + if changed: + self._schedule_trip_save() + def _schedule_trip_save(self): """Persist trip state in the background (best-effort).""" try: @@ -1535,6 +1650,14 @@ def _update_state(self, data): getattr(chrg_mgmt_data, "bmsChrgSts", None) in CHARGING_STATUS_CODES ) + # Charge-session tracking for Last Charge Energy (#262). Runs before + # the transition handling below because it needs the raw charging data + # to distinguish a real charge-stop from the endpoint dropping out. + self._update_charge_state( + getattr(status_data, "basicVehicleStatus", None) if status_data else None, + charging_data, + ) + # A charging -> not-charging transition (charge complete, or the # charging endpoint dropping out) is registered as activity so the # grace-period poll re-checks soon, instead of the interval jumping diff --git a/custom_components/mg_saic/logic.py b/custom_components/mg_saic/logic.py index 2f304f8..2f75c0c 100644 --- a/custom_components/mg_saic/logic.py +++ b/custom_components/mg_saic/logic.py @@ -92,3 +92,127 @@ def select_update_interval( raise TypeError("default_update_interval must be a timedelta") return default_update_interval + + +# Energy fields that some models (e.g. MG HS PHEV / AS33P) report inflated by +# ~3× — the same quirk that makes totalBatteryCapacity read 72.5 kWh on a +# 24.7 kWh pack. The profile's charging_capacity_correction is applied to each +# of these wherever they are read (#262, #310). +ENERGY_CORRECTION_FIELDS = frozenset( + {"lastChargeEndingPower", "powerUsageSinceLastCharge"} +) + + +def apply_energy_correction(field, value, correction): + """Scale an inflated energy field by the per-model correction factor. + + Returns ``value`` unchanged for fields that aren't inflated, for models + with no correction configured, or for a missing value. Distance fields are + never corrected — only the energy fields above. + + Lives here rather than on the sensor because it has to be applied from + several call sites (both numeric branches of the charging sensor, and the + coordinator's charge-session maths). Keeping one implementation is what + stops a repeat of #310, where the correction was added in a branch the + field never reached and so silently did nothing. + """ + if value is None or correction is None: + return value + if field not in ENERGY_CORRECTION_FIELDS: + return value + return value * correction + + +def odometer_km(basic_status, charging_data, *, factor, saturation): + """Odometer in km from a poll's data, or None. + + Prefers ``basicVehicleStatus.mileage``, then falls back to the odometer + carried in the charging data. The fallback reads ``rvsChargeStatus``, + which is where ``mileage`` actually lives — ``chrgMgmtData`` has no such + field, so looking there (as this once did) meant the fallback could never + fire, and any caller relying on it got None (#262). + + Rejects 0, negatives and the uint16 saturation sentinel. + """ + raw = getattr(basic_status, "mileage", None) if basic_status is not None else None + if raw is not None and raw > 0 and raw != saturation: + return raw * factor + if charging_data is not None: + source = getattr(charging_data, "rvsChargeStatus", None) + raw = getattr(source, "mileage", None) if source is not None else None + if raw is not None and raw > 0 and raw != saturation: + return raw * factor + return None + + +# The API reports -128 for fuelRangeElec on several models when the value +# isn't live (typically while parked) rather than omitting the field. +ELECTRIC_RANGE_SENTINEL = -128 + + +def electric_range_km(basic_status, charging_data, *, factor): + """Remaining electric range in km, or None. + + Prefers the charging block's figure and falls back to basicVehicleStatus, + matching the Electric Range sensor. Rejects negatives and the -128 + sentinel; 0 is allowed through, since a flat pack really does have no + range left. + """ + rcs = getattr(charging_data, "rvsChargeStatus", None) if charging_data else None + for source in (rcs, basic_status): + if source is None: + continue + raw = getattr(source, "fuelRangeElec", None) + if raw is not None and raw >= 0 and raw != ELECTRIC_RANGE_SENTINEL: + return round(raw * factor, 1) + return None +# The API's totalBatteryCapacity is unreliable on several MG series, which is +# why VEHICLE_PROFILES carries known-good figures. 725 (-> 72.5 kWh with the +# x0.1 decimal correction) is a documented placeholder rather than a real pack +# size, seen identically on EC32/AS33P/S12L and others. A car that reports it +# is far more likely to be emitting the placeholder than to genuinely hold +# 72.5 kWh — and a car that genuinely does gets its figure from its profile. +BATTERY_CAPACITY_PLACEHOLDER_RAW = 725 + +# Sanity bounds for an API-reported capacity, in kWh. Wide on purpose: this +# only has to reject nonsense (0, negatives, absurd magnitudes), not second +# guess a plausible pack. +MIN_PLAUSIBLE_BATTERY_KWH = 5.0 +MAX_PLAUSIBLE_BATTERY_KWH = 200.0 + + +def resolve_battery_capacity( + override_kwh, + profile_kwh, + api_raw, + *, + factor, +): + """Resolve the usable battery capacity and say where it came from. + + Precedence is the one the integration has always documented: + user override > our per-model profile > the API's own figure. Returns + ``(capacity_kwh, source)`` where source is ``"user_override"``, + ``"profile"``, ``"api"``, or ``None`` when nothing usable is available. + + Resolving this in one place matters: the Total Battery Capacity sensor + honoured all three tiers, but ``known_battery_capacity_kwh`` — which the + charge-session and SOC-efficiency maths read — only ever saw the first + two. So an unprofiled car showed a populated capacity sensor next to three + blank sensors derived from it (#262, #302). + + The API tier is guarded: the placeholder is rejected, as are values + outside a wide plausibility band. A rejected API value yields ``None``, + which is honest — better a blank capacity than energy figures confidently + derived from a number the car made up. + """ + if override_kwh is not None: + return override_kwh, "user_override" + if profile_kwh is not None: + return profile_kwh, "profile" + if api_raw is None or api_raw == BATTERY_CAPACITY_PLACEHOLDER_RAW: + return None, None + capacity = round(api_raw * factor, 2) + if not MIN_PLAUSIBLE_BATTERY_KWH <= capacity <= MAX_PLAUSIBLE_BATTERY_KWH: + return None, None + return capacity, "api" diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 0e288e6..c9bf1ec 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -13,7 +13,7 @@ "requests", "pycryptodome", "mg-saic-client==0.9.4", - "mg-ismart-india-client==0.1.5" + "mg-ismart-india-client==0.1.7" ], - "version": "1.2.6" + "version": "1.2.7" } diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index ec6f237..eea7dfb 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -36,8 +36,9 @@ CHARGING_VOLTAGE_FACTOR, DATA_100_DECIMAL_CORRECTION, ) +from .logic import apply_energy_correction from .utils import create_device_info -from .trip_stats import compute_since_charge_efficiency +from .trip_stats import compute_since_charge_efficiency, compute_soc_since_reset_efficiency async def async_setup_entry(hass, entry, async_add_entities): @@ -341,12 +342,25 @@ async def async_setup_entry(hass, entry, async_add_entities): ) # SOC may come from ordinary status or the charging endpoint. + # + # HEV is included here (issue #318): the MG3 Hybrid+ is a + # self-charging hybrid with no charge port, so it has no + # meaningful charging-endpoint data, but basicVehicleStatus. + # extendedData1 was confirmed to independently track its HV + # battery SoC (values fell from 78 -> 73 across polls while + # driving). Gating HEV and PHEV on STATE_OF_CHARGE_NON_BEV rather + # than adding it unconditionally keeps this off the India backend, + # where extendedData1 is repurposed to carry fuel_level rather than + # battery SoC. India advertises CHARGING_DATA (its charging frame + # carries a real bmsPackSOCDsp), but the SOC sensor falls back to + # extendedData1 whenever that frame is missing, so the gate has to + # be the narrower feature and not CHARGING_DATA. if ( - vehicle_type in ["BEV", "PHEV"] + vehicle_type in ["BEV", "PHEV", "HEV"] and coordinator.backend_supports(Feature.STATE_OF_CHARGE) and ( vehicle_type == "BEV" - or coordinator.backend_supports(Feature.CHARGING_DATA) + or coordinator.backend_supports(Feature.STATE_OF_CHARGE_NON_BEV) ) ): sensors.append( @@ -495,7 +509,13 @@ async def async_setup_entry(hass, entry, async_add_entities): coordinator, entry, "Estimated Range After Charging", - "bmsEstdElecRng", + # Was bmsEstdElecRng, which does not track a projected + # range: on an MGS6 at 57% SOC with 285 km showing and + # an 80% target it reported 761 km — nearly double the + # ~400 km projection, and beyond what the car does on a + # full charge. imcuChrgngEstdElecRng read 410 against a + # 398 km projection on the same car (#262). + "imcuChrgngEstdElecRng", SensorDeviceClass.DISTANCE, UnitOfLength.KILOMETERS, "mdi:map-marker-distance", @@ -656,6 +676,7 @@ async def async_setup_entry(hass, entry, async_add_entities): UnitOfLength.KILOMETERS, "mdi:map-marker-distance", "measurement", + with_attributes=True, ) ) if vehicle_type in ["BEV", "PHEV"]: @@ -676,6 +697,16 @@ async def async_setup_entry(hass, entry, async_add_entities): sensors.append( SAICMGEfficiencySinceChargeSensor(coordinator, entry) ) + # How much energy the last charge put IN (#262) — measured + # across the session, since the API only reports energy taken + # back out afterwards. + sensors.append(SAICMGLastChargeEnergySensor(coordinator, entry)) + sensors.append(SAICMGLastChargeRangeSensor(coordinator, entry)) + # SOC/odometer-based alternative — independent of the + # since-charge counter fields, so available on every BEV/PHEV + # regardless of whether those fields are reliable or populated + # at all on this car (#301). + sensors.append(SAICMGEfficiencySinceResetSensor(coordinator, entry)) if vehicle_type in ["ICE", "HEV", "PHEV"]: sensors.append( SAICMGLastTripSensor( @@ -1839,8 +1870,15 @@ def native_value(self): soc = getattr(charging_data, self._field_charging, None) if soc is not None: # -128 is the SAIC sentinel for "no valid data" — reject it - # before applying the decimal factor (which would produce -12.8) - if soc == -128: + # before applying the decimal factor (which would produce -12.8). + # 0 is also treated as suspect here: on this raw, unscaled + # bmsPackSOCDsp field a genuine near-empty pack still reports + # a small positive value in practice, so an exact 0 has only + # been observed as a stale/unpopulated reading rather than a + # real 0% SoC. Falling back to basicVehicleStatus.extendedData1 + # below is safe even for a truly near-empty battery, since that + # field independently tracks SoC as a truncated whole percent. + if soc in (-128, 0): soc = None else: soc = soc * DATA_DECIMAL_CORRECTION_SOC @@ -2203,6 +2241,40 @@ class SAICMGChargingSensor(CoordinatorEntity, SensorEntity): # V2X_DISCHARGING (13) is deliberately absent — it has live current/voltage data. _INACTIVE_CHARGING_STATUSES = frozenset({0, 5}) + # Fields whose companion "V" field says whether the value is live. A + # capture taken mid-charge with current flowing showed V=0 on + # chrgngRmnngTime while it reported a healthy 300 minutes, so 0 is the + # valid state and anything else means don't trust the number (#262). + _VALIDITY_GATED_FIELDS = { + "imcuChrgngEstdElecRng": "imcuChrgngEstdElecRngV", + } + + def _is_invalidated(self, data_source): + """True when the car flags this field's value as not live.""" + flag_field = self._VALIDITY_GATED_FIELDS.get(self._field) + if flag_field is None or data_source is None: + return False + flag = getattr(data_source, flag_field, None) + return flag is not None and flag != 0 + def _apply_energy_correction(self, value): + """Scale an inflated energy field by the profile's correction factor. + + Applied from BOTH numeric branches below. It previously lived only + inside the _NOT_CHARGING_ZERO_FIELDS branch, which + powerUsageSinceLastCharge never enters — so that sensor kept showing + the raw ~3× figure on affected models (#262, @HarryFlatter: 20.20 kWh + reported against a 24.7 kWh pack after ~39 km). + + The field list and the maths live in logic.apply_energy_correction, so + the coordinator's charge-session figures and this sensor can't drift + apart, and the rule is unit-testable without Home Assistant. + """ + return apply_energy_correction( + self._field, + value, + getattr(self.coordinator, "charging_capacity_correction", None), + ) + def __init__( self, coordinator, @@ -2272,24 +2344,30 @@ def extra_state_attributes(self): """ if self._field != "totalBatteryCapacity": return None - if self.coordinator.battery_capacity_override is not None: - source = "user_override" - elif self.coordinator.known_battery_capacity_kwh is not None: - source = "profile" - else: - source = "api" - return {"capacity_source": source} + # Reported, not inferred. This used to guess "profile" from + # known_battery_capacity_kwh being set, which would mislabel an + # API-derived value the moment that attribute gained an API tier — + # the exact confusion this attribute was added to prevent (#301). + source = self.coordinator.battery_capacity_resolution[1] + return {"capacity_source": source} if source else None @property def native_value(self): """Return the state of the sensor.""" # Total Battery Capacity: prefer coordinator's known-good value when set. if self._field == "totalBatteryCapacity": - known_capacity = getattr( - self.coordinator, "known_battery_capacity_kwh", None - ) - if known_capacity is not None: - return known_capacity + # Single resolution point (override > profile > API, placeholder + # rejected), shared with the energy maths so the two can't + # disagree about the pack size. + capacity = self.coordinator.effective_battery_capacity_kwh + if capacity is not None: + self._last_valid_value = capacity + return capacity + # A poll that carried no charging data can't resolve the API tier; + # hold the last good figure rather than blinking to Unknown. A + # capacity rejected on its merits never became a last valid value, + # so this can't resurrect the placeholder. + return self._last_valid_value try: charging_data = getattr( @@ -2299,6 +2377,10 @@ def native_value(self): if charging_data: charging_status = getattr(charging_data, "bmsChrgSts", None) + # --- Car says this value isn't live: hold, don't publish --- + if self._is_invalidated(charging_data): + return self._last_valid_value + # --- Fields that return explicit 0 when not charging --- if self._field in self._NOT_CHARGING_ZERO_FIELDS: if charging_status in self._INACTIVE_CHARGING_STATUSES: @@ -2310,21 +2392,7 @@ def native_value(self): return self._last_valid_value return None if raw_value is not None: - result = raw_value * self._factor - # lastChargeEndingPower / powerUsageSinceLastCharge: some - # models (e.g. HS PHEV) report these energy fields inflated - # by ~3× relative to the true kWh value. Apply the profile's - # charging_capacity_correction factor when set so the - # displayed value matches the real battery. - if self._field in ( - "lastChargeEndingPower", - "powerUsageSinceLastCharge", - ): - correction = getattr( - self.coordinator, "charging_capacity_correction", None - ) - if correction is not None: - result = result * correction + result = self._apply_energy_correction(raw_value * self._factor) self._last_valid_value = result return result return None @@ -2403,7 +2471,12 @@ def native_value(self): return self._last_valid_value return None if raw_value is not None: - result = raw_value * self._factor if self._factor is not None else raw_value + result = ( + raw_value * self._factor + if self._factor is not None + else raw_value + ) + result = self._apply_energy_correction(result) self._last_valid_value = result return result # raw_value is None — fall through to retention below @@ -3106,6 +3179,10 @@ def native_value(self): _TRIP_ATTR_KEYS = ( "distance_km", "distance_mi", + "distance_km_counter", + "distance_mi_counter", + "distance_km_odometer", + "distance_mi_odometer", "duration_s", "soc_used_pct", "energy_kWh", @@ -3113,6 +3190,16 @@ def native_value(self): "efficiency_mi_per_kWh", "consumption_kWh_per_100km", "consumption_kWh_per_100mi", + "energy_kWh_counter", + "efficiency_km_per_kWh_counter", + "efficiency_mi_per_kWh_counter", + "consumption_kWh_per_100km_counter", + "consumption_kWh_per_100mi_counter", + "energy_kWh_soc", + "efficiency_km_per_kWh_soc", + "efficiency_mi_per_kWh_soc", + "consumption_kWh_per_100km_soc", + "consumption_kWh_per_100mi_soc", "fuel_used_pct", "fuel_used_litres", "fuel_consumption_L_per_100km", @@ -3271,3 +3358,231 @@ def native_value(self): @property def extra_state_attributes(self): return self._compute() + + +class SAICMGLastChargeEnergySensor(CoordinatorEntity, SensorEntity): + """Energy delivered into the battery by the last completed charge (#262). + + The API reports charging power live, and ``powerUsageSinceLastCharge`` + (energy taken *out* since the charge), but has no field for how much a + charge put *in* — there is no ``lastChargeStartingPower`` to subtract + from ``lastChargeEndingPower``. So the session is measured across its + start/end boundaries by the coordinator; see + trip_stats.compute_charge_session for the two methods and their tradeoffs. + + This is energy measured at the *battery*, so it will read lower than a + wall meter or a smart charger, which also pay for charger and cable + losses. Handy when charging away from home and settling up with whoever's + electricity you borrowed. + """ + + _CHARGE_ATTR_KEYS = ( + "energy_added_kWh", + "energy_added_kWh_soc", + "energy_added_kWh_counter", + "method", + "soc_start_pct", + "soc_end_pct", + "soc_added_pct", + "range_added_km", + "range_start_km", + "range_end_km", + "duration_s", + "average_power_kW", + "odometer_km", + "start_ts", + "end_ts", + ) + + def __init__(self, coordinator, entry): + super().__init__(coordinator) + self._name = "Last Charge Energy" + self._attr_icon = "mdi:battery-charging-medium" + self._attr_device_class = SensorDeviceClass.ENERGY + self._attr_native_unit_of_measurement = UnitOfEnergy.KILO_WATT_HOUR + # "measurement", not total_increasing: this is a per-session figure + # that goes up and down with each charge, not a running total. + self._attr_state_class = "measurement" + vin_info = coordinator.vin_info + self._unique_id = f"{entry.entry_id}_{vin_info.vin}_last_charge_energy" + self._device_info = create_device_info(coordinator, entry.entry_id) + + @property + def unique_id(self): + return self._unique_id + + @property + def name(self): + vin_info = self.coordinator.vin_info + return f"{vin_info.brandName} {vin_info.modelName} {self._name}" + + @property + def device_info(self): + return self._device_info + + def _charge(self): + stats = getattr(self.coordinator, "trip_stats", None) + return stats.last_charge if stats is not None else None + + @property + def available(self): + # Show "unknown" rather than dropping out before the first charge has + # been observed end-to-end — the sensor works, it just has nothing yet. + return True + + @property + def native_value(self): + charge = self._charge() + return charge.get("energy_added_kWh") if charge else None + + @property + def extra_state_attributes(self): + charge = self._charge() + if not charge: + return None + return { + k: charge.get(k) for k in self._CHARGE_ATTR_KEYS if charge.get(k) is not None + } + + +class SAICMGLastChargeRangeSensor(CoordinatorEntity, SensorEntity): + """Electric range added by the last completed charge (#262). + + A first-class sensor rather than only an attribute on Last Charge Energy, + because attributes are never unit-converted by Home Assistant. Declaring + DISTANCE in kilometres lets HA present miles to users whose system is + imperial — which most of the people asking for this figure are — instead + of a raw km number sitting next to an Electric Range sensor showing miles. + """ + + def __init__(self, coordinator, entry): + super().__init__(coordinator) + self._name = "Last Charge Range Added" + self._attr_icon = "mdi:map-marker-distance" + self._attr_device_class = SensorDeviceClass.DISTANCE + self._attr_native_unit_of_measurement = UnitOfLength.KILOMETERS + self._attr_state_class = "measurement" + vin_info = coordinator.vin_info + self._unique_id = f"{entry.entry_id}_{vin_info.vin}_last_charge_range_added" + self._device_info = create_device_info(coordinator, entry.entry_id) + + @property + def unique_id(self): + return self._unique_id + + @property + def name(self): + vin_info = self.coordinator.vin_info + return f"{vin_info.brandName} {vin_info.modelName} {self._name}" + + @property + def device_info(self): + return self._device_info + + @property + def available(self): + return True + + def _charge(self): + stats = getattr(self.coordinator, "trip_stats", None) + return stats.last_charge if stats is not None else None + + @property + def native_value(self): + charge = self._charge() + return charge.get("range_added_km") if charge else None + + @property + def extra_state_attributes(self): + charge = self._charge() + if not charge: + return None + return { + k: charge.get(k) + for k in ("range_start_km", "range_end_km", "start_ts", "end_ts") + if charge.get(k) is not None + } + + +class SAICMGEfficiencySinceResetSensor(CoordinatorEntity, SensorEntity): + """Electric efficiency since the SOC-detected reset point (#301). + + An SOC/odometer-only alternative to Efficiency Since Last Charge — + entirely independent of the API's ``mileageSinceLastCharge`` / + ``powerUsageSinceLastCharge`` fields. Added because, per field reports: + those fields reset spuriously without an actual charge on some cars (e.g. + MG4), and are permanently unpopulated (``Unknown``) on others (e.g. some + MGS5s) — this sensor works either way, and lets the two be compared + directly where both are present. + + The "reset point" is whenever SOC was last seen to rise while parked (a + charge); see TripStatsManager.note_soc_reset_baseline. As with the + counter-based sensor's baseline, this isn't necessarily "since a full + charge to 100%" (a partial charge also resets it), so it's a genuinely + "since reset" figure rather than a charge-accurate one — but it uses SOC + (reported to 0.1%) and the odometer (never resets), both of which are + more precise/reliable inputs than the fields it's complementing. + """ + + def __init__(self, coordinator, entry): + super().__init__(coordinator) + self._name = "Efficiency Since Charge (SOC)" + self._attr_icon = "mdi:gauge" + self._attr_device_class = ENERGY_DISTANCE_DEVICE_CLASS + self._attr_native_unit_of_measurement = "km/kWh" + self._attr_state_class = "measurement" + vin_info = coordinator.vin_info + self._unique_id = f"{entry.entry_id}_{vin_info.vin}_efficiency_since_reset_soc" + self._device_info = create_device_info(coordinator, entry.entry_id) + + @property + def unique_id(self): + return self._unique_id + + @property + def name(self): + vin_info = self.coordinator.vin_info + return f"{vin_info.brandName} {vin_info.modelName} {self._name}" + + @property + def device_info(self): + return self._device_info + + def _compute(self): + trip_stats = self.coordinator.trip_stats + baseline = trip_stats.soc_reset_baseline if trip_stats else None + if not baseline: + return None + # NB: the extractors take basicVehicleStatus, not the top-level status + # object — passing the latter made the odometer lookup miss (mileage + # lives on basicVehicleStatus), which returned None and left this + # sensor permanently Unknown on every car. + status = self.coordinator.data.get("status") + basic_status = getattr(status, "basicVehicleStatus", None) + charging = self.coordinator.data.get("charging") + current_soc = self.coordinator._extract_soc_pct(basic_status, charging) + current_odometer = self.coordinator._extract_odometer_km(basic_status, charging) + return compute_soc_since_reset_efficiency( + baseline.get("soc_pct"), + current_soc, + baseline.get("odometer_km"), + current_odometer, + self.coordinator.effective_battery_capacity_kwh, + ) + + @property + def available(self): + # Show "unknown" (not "unavailable") when there's no baseline yet (no + # charge observed since the sensor started tracking) or 0 km have been + # driven since. The sensor is working; it just has nothing to show yet. + return True + + @property + def native_value(self): + result = self._compute() + return None if result is None else result["efficiency_km_per_kWh"] + + @property + def extra_state_attributes(self): + return self._compute() + diff --git a/custom_components/mg_saic/trip_stats.py b/custom_components/mg_saic/trip_stats.py index 37913ea..1127570 100644 --- a/custom_components/mg_saic/trip_stats.py +++ b/custom_components/mg_saic/trip_stats.py @@ -41,6 +41,15 @@ from datetime import datetime from typing import Any +# Minimum SOC rise (%) for a plugged-in period to be recorded as a charge. +# Filters out a plug-in that delivered nothing and the small SOC rebound the +# pack reports after a drive. +MIN_CHARGE_SOC_PCT = 0.5 + +# Abandon (rather than record) a charge session left open longer than this — +# a missed charge-stop shouldn't produce a nonsense figure days later. +MAX_OPEN_CHARGE_SECONDS = 48 * 3600 + # Reject an odometer delta larger than this (km) as a single trip — protects # against odometer rollover, the uint16 saturation sentinel slipping through, # or a garbage reading. A genuine single drive won't exceed this. @@ -117,6 +126,48 @@ def _f(key): return None +@dataclass +class ChargeSnapshot: + """A reading taken at the start or end of a charging session (#262). + + ``pack_energy_kwh`` is the car's own estimate of the energy sitting in the + pack, derived as ``lastChargeEndingPower - powerUsageSinceLastCharge`` + (both already decimal-corrected, and scaled by the per-model energy + correction where one applies). That identity holds at both boundaries: at + the end of a charge the since-charge counter is ~0, so the expression + collapses to lastChargeEndingPower itself. + """ + + ts: str # ISO-8601 timestamp string (storage-friendly) + soc_pct: float | None = None + pack_energy_kwh: float | None = None + odometer_km: float | None = None + range_km: float | None = None # remaining electric range at the boundary + + def to_dict(self) -> dict[str, Any]: + return asdict(self) + + @classmethod + def from_dict(cls, d: dict[str, Any] | None) -> "ChargeSnapshot | None": + if not d: + return None + + def _f(key): + v = d.get(key) + return None if v is None else float(v) + + try: + return cls( + ts=d["ts"], + soc_pct=_f("soc_pct"), + pack_energy_kwh=_f("pack_energy_kwh"), + odometer_km=_f("odometer_km"), + range_km=_f("range_km"), + ) + except (KeyError, TypeError, ValueError): + return None + + def _duration_seconds(start_ts: str, end_ts: str) -> int | None: try: start = datetime.fromisoformat(start_ts) @@ -142,6 +193,34 @@ def _counter_delta(current, baseline_value): return round(current - baseline_value, 3) +def _efficiency_block(distance_km, distance_mi, energy_kwh): + """The 5-key energy/efficiency block for one (distance, energy) pairing. + Shared by the primary, _counter, and _soc figures so all three stay + consistent. Returns all-None when either input is missing/non-positive. + """ + if ( + energy_kwh is None + or energy_kwh <= 0 + or distance_km is None + or distance_km <= 0 + ): + return { + "energy_kWh": None, + "efficiency_km_per_kWh": None, + "efficiency_mi_per_kWh": None, + "consumption_kWh_per_100km": None, + "consumption_kWh_per_100mi": None, + } + distance_mi = distance_mi if distance_mi is not None else distance_km / KM_PER_MILE + return { + "energy_kWh": round(energy_kwh, 3), + "efficiency_km_per_kWh": round(distance_km / energy_kwh, 2), + "efficiency_mi_per_kWh": round(distance_mi / energy_kwh, 2), + "consumption_kWh_per_100km": round(energy_kwh / distance_km * 100.0, 2), + "consumption_kWh_per_100mi": round(energy_kwh / distance_mi * 100.0, 2), + } + + def compute_completed_trip( start: TripSnapshot, end: TripSnapshot, @@ -172,6 +251,17 @@ def compute_completed_trip( flagged ``retrospective: True`` / ``timing: approximate`` so it's distinguishable, and its timestamps bound the gap rather than the drive. + Beyond the primary (unprefixed) distance/energy/efficiency figures — which + keep picking counter-preferred-with-odometer/SOC-fallback exactly as + before, for backward compatibility — this also exposes the counter-only + and odometer+SOC-only figures independently as ``*_counter`` / ``*_soc`` + (energy) and ``distance_*_counter`` / ``distance_*_odometer`` (distance) + attributes, so both can be compared directly (#301: some cars' counters + appear to over-report energy even when not obviously reset). The counter + figures are raw/unfiltered here — shown even when ``counter_reset_detected`` + discarded them from the primary selection, since a bogus counter reading is + itself useful to see. + Returns ``None`` when no plausible distance can be established. Individual electric/fuel figures are ``None`` when their inputs are missing. """ @@ -182,20 +272,49 @@ def compute_completed_trip( base_kwh = baseline.get("since_charge_kwh") if baseline else None # Odometer delta is always computable and never resets mid-trip — used as - # the fallback distance, and as the sanity check against the counter below. + # the fallback distance, the odometer-side of the *_soc figures, and the + # sanity check against the counter below. odometer_delta_km = round(end.odometer_km - start.odometer_km, 2) + odometer_delta_mi = odometer_delta_km / KM_PER_MILE + + # Raw counter-derived distance/energy — unfiltered by the reset sanity + # check, so the *_counter attributes show what the counter actually said + # even when it's discarded from the primary figures below. + raw_counter_km = None if retrospective else _counter_delta(end.since_charge_km, base_km) + raw_counter_kwh = ( + None + if retrospective or not is_electric + else _counter_delta(end.since_charge_kwh, base_kwh) + ) + + # SOC-derived energy, computed independently whenever SOC data allows it — + # not just as a fallback for when the counter is missing. Paired with the + # odometer distance (not the counter distance) for the *_soc figures, so + # it's a fully self-consistent "odometer + SOC only" view. + soc_used_pct = None + soc_energy_kwh = None + charged_during_park = False + if is_electric and start.soc_pct is not None and end.soc_pct is not None: + soc_delta = round(start.soc_pct - end.soc_pct, 1) + if soc_delta < 0: + charged_during_park = True + else: + soc_used_pct = soc_delta + if capacity_kwh: + soc_energy_kwh = round(soc_delta / 100.0 * capacity_kwh, 3) # ── Distance: prefer the since-charge counter, else the odometer delta ──── # Retrospective trips always use the odometer (the counter may have reset in # the unobserved gap). - counter_km = None if retrospective else _counter_delta(end.since_charge_km, base_km) + counter_km = raw_counter_km # Sanity check: if the odometer shows a real drive but the counter says # (near) nothing, the counter reset mid-trip without an actual charge — a # known SAIC data-quality quirk, not tied to any one model. Trusting a # bogus ~0 counter value here would silently drop the whole trip (0 looks # like valid data, not "missing"), so discard the counter for BOTH distance - # and energy and fall back to the odometer/SOC path instead. + # and energy and fall back to the odometer/SOC path instead. (This only + # affects the primary figures — the raw *_counter attributes still show it.) counter_reset_detected = ( counter_km is not None and odometer_delta_km >= ODOMETER_SANITY_MIN_KM @@ -214,6 +333,14 @@ def compute_completed_trip( trip: dict[str, Any] = { "distance_km": round(distance_km, 2), "distance_mi": round(distance_mi, 2), + # Always available regardless of which source is primary — lets any + # trip's distance be checked against the other source directly. + "distance_km_counter": round(raw_counter_km, 2) if raw_counter_km is not None else None, + "distance_mi_counter": ( + round(raw_counter_km / KM_PER_MILE, 2) if raw_counter_km is not None else None + ), + "distance_km_odometer": odometer_delta_km, + "distance_mi_odometer": round(odometer_delta_mi, 2), "start_ts": start.ts, "end_ts": end.ts, "duration_s": _duration_seconds(start.ts, end.ts), @@ -242,29 +369,30 @@ def compute_completed_trip( # ── Electric energy (BEV/PHEV) ─────────────────────────────────────────── if is_electric: - # Retrospective trips, and trips where the counter reset mid-trip (see - # counter_reset_detected above), skip the counter and use SOC instead. - energy = ( - None - if retrospective or counter_reset_detected - else _counter_delta(end.since_charge_kwh, base_kwh) - ) - if energy is None and start.soc_pct is not None and end.soc_pct is not None: - # Derive from SOC change (coarse; the only source for retrospective - # trips, and the fallback when no counter is available). - soc_used = round(start.soc_pct - end.soc_pct, 1) - if soc_used < 0: - trip["charged_during_park"] = True - else: - trip["soc_used_pct"] = soc_used - if capacity_kwh: - energy = round(soc_used / 100.0 * capacity_kwh, 3) - if energy is not None and energy > 0: - trip["energy_kWh"] = round(energy, 3) - trip["efficiency_km_per_kWh"] = round(distance_km / energy, 2) - trip["efficiency_mi_per_kWh"] = round(distance_mi / energy, 2) - trip["consumption_kWh_per_100km"] = round(energy / distance_km * 100.0, 2) - trip["consumption_kWh_per_100mi"] = round(energy / distance_mi * 100.0, 2) + trip["charged_during_park"] = charged_during_park + trip["soc_used_pct"] = soc_used_pct + + # Primary (unprefixed): counter-preferred, SOC-fallback — unchanged + # behaviour from before this attribute expansion. + primary_energy = None if retrospective or counter_reset_detected else raw_counter_kwh + if primary_energy is None: + primary_energy = soc_energy_kwh + trip.update(_efficiency_block(distance_km, distance_mi, primary_energy)) + + # Counter-only view: counter distance + counter energy, both raw/ + # unfiltered — a fully self-consistent "trust the counter" figure. + for key, value in _efficiency_block( + raw_counter_km, None, raw_counter_kwh + ).items(): + trip[f"{key}_counter"] = value + + # Odometer+SOC-only view: odometer distance + SOC energy — a fully + # self-consistent "trust SOC" figure, computed independently of + # whether the counter was available or trusted for this trip. + for key, value in _efficiency_block( + odometer_delta_km, odometer_delta_mi, soc_energy_kwh + ).items(): + trip[f"{key}_soc"] = value # ── Fuel (ICE/HEV/PHEV) ────────────────────────────────────────────────── if is_combustion and start.fuel_pct is not None and end.fuel_pct is not None: @@ -317,13 +445,158 @@ def compute_since_charge_efficiency( } -# ── Persistent manager ─────────────────────────────────────────────────────── +def compute_soc_since_reset_efficiency( + baseline_soc_pct: float | None, + current_soc_pct: float | None, + baseline_odometer_km: float | None, + current_odometer_km: float | None, + capacity_kwh: float | None, +) -> dict[str, Any] | None: + """Efficiency since the SOC-detected reset point, as an SOC/odometer-only + alternative to compute_since_charge_efficiency's counter-only figure (#301). + + Independent of the car's own since-last-charge counters entirely — uses + only the odometer (never resets) and SOC×capacity (reported to 0.1%, so + accurate even over short distances). Requested because two of the fields + it replaces (mileageSinceLastCharge/powerUsageSinceLastCharge) are + reported unreliably on some cars (spurious resets) and not reported at all + on others (permanently Unknown, e.g. some MGS5s) — this sensor works on + both, since it never touches those fields. + + The "reset point" here is whenever SOC was last seen to rise while parked + (a charge) — see TripStatsManager.note_soc_reset_baseline, which only + evaluates this while parked so a mid-drive regen uptick can't trigger it. + Because the baseline isn't necessarily "at 100% right after a full + charge" (a partial charge, or any other SOC rise, also triggers it), this + is honestly a "since reset" figure rather than a charge-accurate one, but + it uses the same epoch boundary as the counter-based figure it's + replacing/complementing. + + Returns ``None`` when there's no baseline yet or SOC hasn't dropped. + """ + if ( + baseline_soc_pct is None + or current_soc_pct is None + or baseline_odometer_km is None + or current_odometer_km is None + ): + return None + distance_km = round(current_odometer_km - baseline_odometer_km, 2) + soc_used_pct = round(baseline_soc_pct - current_soc_pct, 1) + if distance_km <= 0 or soc_used_pct <= 0 or not capacity_kwh: + return None + energy_kwh = round(soc_used_pct / 100.0 * capacity_kwh, 3) + if energy_kwh <= 0: + return None + distance_mi = distance_km / KM_PER_MILE + return { + "distance_km": distance_km, + "distance_mi": round(distance_mi, 2), + "soc_used_pct": soc_used_pct, + "baseline_soc_pct": baseline_soc_pct, + "energy_kWh": energy_kwh, + "efficiency_km_per_kWh": round(distance_km / energy_kwh, 2), + "efficiency_mi_per_kWh": round(distance_mi / energy_kwh, 2), + "consumption_kWh_per_100km": round(energy_kwh / distance_km * 100.0, 2), + "consumption_kWh_per_100mi": round(energy_kwh / distance_mi * 100.0, 2), + } + + + + +def compute_charge_session( + start: "ChargeSnapshot", + end: "ChargeSnapshot", + *, + capacity_kwh: float | None, +) -> dict[str, Any] | None: + """Energy delivered into the battery during one charging session (#262). + + Requested by @HarryFlatter: the API reports charging *power* live and + ``powerUsageSinceLastCharge`` (energy taken *out* since the last charge), + but nothing for "how much did that charge put *in*" — which is what you + need when you're charging on someone else's supply and want to settle up. + There is no ``lastChargeStartingPower`` field to subtract, so it has to be + measured across the session. + + Two independent figures are produced, in the same + show-both-and-let-the-car-tell-us style as the trip sensors: + + * ``soc`` — (SOC rise) × usable capacity. Always available on a car + that reports SOC and has a known capacity, and SOC is reported to 0.1 %. + * ``counter`` — the delta of the car's own pack-energy figure + (``lastChargeEndingPower - powerUsageSinceLastCharge``). Independent of + the capacity we hold for the model, but it relies on the car refreshing + lastChargeEndingPower promptly at the end of the session. + + The SOC figure is the headline value because it is available on every + model; the counter figure rides along as an attribute so the two can be + compared on real cars. Both are *battery-side* energy — always less than + the energy drawn at the wall, which also covers charger and cable losses. + + Returns ``None`` when neither method can produce a plausible figure. + """ + if start is None or end is None: + return None + + result: dict[str, Any] = {"start_ts": start.ts, "end_ts": end.ts} + + duration_s = _duration_seconds(start.ts, end.ts) + if duration_s is not None: + result["duration_s"] = duration_s + + energy_soc = None + if start.soc_pct is not None and end.soc_pct is not None: + soc_added = round(end.soc_pct - start.soc_pct, 1) + result["soc_start_pct"] = start.soc_pct + result["soc_end_pct"] = end.soc_pct + result["soc_added_pct"] = soc_added + if soc_added >= MIN_CHARGE_SOC_PCT and capacity_kwh: + energy_soc = round(soc_added / 100.0 * capacity_kwh, 3) + + energy_counter = None + if start.pack_energy_kwh is not None and end.pack_energy_kwh is not None: + delta = round(end.pack_energy_kwh - start.pack_energy_kwh, 3) + # Guard against the car not having refreshed lastChargeEndingPower yet + # (delta <= 0) or reporting something larger than the pack can hold. + if delta > 0 and (capacity_kwh is None or delta <= capacity_kwh * 1.05): + energy_counter = delta + + if energy_soc is None and energy_counter is None: + return None + + energy = energy_soc if energy_soc is not None else energy_counter + result["energy_added_kWh"] = energy + result["method"] = "soc" if energy_soc is not None else "counter" + if energy_soc is not None: + result["energy_added_kWh_soc"] = energy_soc + if energy_counter is not None: + result["energy_added_kWh_counter"] = energy_counter + # Range added by the charge (#262, @HarryFlatter). The API's own + # chrgngAddedElecRng is a live during-session counter that resets, and on + # the cars seen so far it never leaves 0 even mid-charge — so the useful + # figure is the difference between the range at each boundary, from the + # field that demonstrably does work. + if start.range_km is not None and end.range_km is not None: + range_added = round(end.range_km - start.range_km, 1) + result["range_start_km"] = start.range_km + result["range_end_km"] = end.range_km + if range_added >= 0: + result["range_added_km"] = range_added + + if start.odometer_km is not None: + result["odometer_km"] = start.odometer_km + if duration_s and duration_s > 0 and energy: + result["average_power_kW"] = round(energy / (duration_s / 3600.0), 2) + return result + # HA imports are done lazily inside methods so the pure functions above can be # imported and unit-tested without Home Assistant installed. STORAGE_VERSION = 1 EVENT_TRIP_COMPLETED = "mg_saic_trip_completed" +EVENT_CHARGE_COMPLETED = "mg_saic_charge_completed" class TripStatsManager: @@ -356,6 +629,16 @@ def __init__(self, hass, entry_id: str, vin: str) -> None: # reconstruct trips that were never seen live (the car wasn't polled # while powered) — see detect_missed_trip. self.last_parked_snapshot: TripSnapshot | None = None + # SOC/odometer at the last-seen "since reset" epoch boundary — a charge + # (SOC rise) observed while parked. Powers the SOC-based Efficiency + # Since Charge (SOC) sensor, entirely independent of the since-charge + # counter fields — see note_soc_reset_baseline. + self.soc_reset_baseline: dict[str, Any] | None = None + # Charging-session tracking (#262): the snapshot taken when a charge + # started, and the last completed charge. Powers the Last Charge Energy + # sensor — the API has no "energy added by that charge" field. + self.open_charge: ChargeSnapshot | None = None + self.last_charge: dict[str, Any] | None = None async def async_load(self) -> None: from homeassistant.helpers.storage import Store @@ -370,6 +653,9 @@ async def async_load(self) -> None: self.last_parked_snapshot = TripSnapshot.from_dict( data.get("last_parked_snapshot") ) + self.soc_reset_baseline = data.get("soc_reset_baseline") + self.open_charge = ChargeSnapshot.from_dict(data.get("open_charge")) + self.last_charge = data.get("last_charge") async def async_save(self) -> None: """Persist current open/last-trip state and the since-charge baseline.""" @@ -387,6 +673,11 @@ async def async_save(self) -> None: if self.last_parked_snapshot else None ), + "soc_reset_baseline": self.soc_reset_baseline, + "open_charge": ( + self.open_charge.to_dict() if self.open_charge else None + ), + "last_charge": self.last_charge, } ) @@ -413,6 +704,76 @@ def note_since_charge(self, km, kwh) -> bool: return True return False + def note_soc_reset_baseline(self, soc_pct, odometer_km, ts) -> bool: + """Track SOC while parked to detect a charge (SOC rise) and rebase the + SOC-based "since reset" baseline — the odometer/SOC-only counterpart to + note_since_charge, entirely independent of the since-charge counter + fields (#301: those are unreliable on some cars, absent on others). + + Only ever called while parked (the coordinator gates this), so a + mid-drive regen SOC uptick can never be mistaken for a charge here. + Returns True if the baseline changed (caller may persist). + """ + if soc_pct is None or odometer_km is None: + return False + if self.soc_reset_baseline is None or soc_pct > self.soc_reset_baseline.get( + "soc_pct", -1.0 + ): + self.soc_reset_baseline = { + "soc_pct": round(soc_pct, 1), + "odometer_km": round(odometer_km, 3), + "ts": ts, + } + return True + return False + + def note_charge_state( + self, + is_charging: bool, + snapshot: "ChargeSnapshot | None", + *, + capacity_kwh: float | None, + now_iso: str, + ) -> tuple[dict[str, Any] | None, bool]: + """Open/close a charging session (#262). + + Returns ``(completed_charge_or_None, state_changed)``; the caller + persists when state_changed and fires an event for a completed charge. + + Called only on polls where charging data was actually returned — a + failed charging fetch drops charging_data to None and flips is_charging + to False, which would otherwise look exactly like the charge ending. + That matters here: on some cars (#262) the charging endpoint reliably + goes quiet the moment a session completes, so treating a dropout as an + end-of-charge would record a phantom session on every outage. + """ + if snapshot is None: + return None, False + + if is_charging: + if self.open_charge is None: + self.open_charge = snapshot + return None, True + # Already charging — nothing to do. The start snapshot stands. + return None, False + + if self.open_charge is None: + return None, False + + start = self.open_charge + self.open_charge = None + + age = _duration_seconds(start.ts, now_iso) + if age is not None and age > MAX_OPEN_CHARGE_SECONDS: + # A charge-stop we never saw. Abandon rather than invent a figure. + return None, True + + charge = compute_charge_session(start, snapshot, capacity_kwh=capacity_kwh) + if charge is None: + return None, True + self.last_charge = charge + return charge, True + def open(self, snapshot: TripSnapshot) -> bool: """Record the start-of-drive snapshot (synchronous). Returns True if a new trip was opened. @@ -561,3 +922,13 @@ def _fire_event(self, trip: dict[str, Any]) -> None: ) except Exception: # noqa: BLE001 - event firing must never break a poll pass + + def fire_charge_event(self, charge: dict[str, Any]) -> None: + """Fire mg_saic_charge_completed so automations can react to a finished + charge (#262) — the same contract as the trip event.""" + try: + self._hass.bus.async_fire( + EVENT_CHARGE_COMPLETED, {"vin": self._vin, **charge} + ) + except Exception: # noqa: BLE001 - event firing must never break a poll + pass diff --git a/reply-to-harry.md b/reply-to-harry.md new file mode 100644 index 0000000..1cc5d9e --- /dev/null +++ b/reply-to-harry.md @@ -0,0 +1,58 @@ +@HarryFlatter — all three of these turned out to be real, and two of them are worse than you thought (they affect every car, not just yours). Fixed in #330, going out as **1.2.7-beta5**. + +## 1. Efficiency Since Charge (SOC) — not a PHEV thing + +You said it looked "absent for PHEV". It isn't — the entity is created for BEV and PHEV alike, so you do have it. It just never produces a value **on any car**. I checked mine: charged Friday night, 61 km driven since, and it's been sat on `Unknown` the entire time too. + +Two bugs stacked on top of each other: + +- The sensor was passing the wrong object into its odometer and SOC lookups. SOC got away with it because it has a fallback to the charging data; the odometer didn't. +- That odometer fallback was then looking for `mileage` in the wrong part of the response. I checked the client library schema to be sure: `mileage`, `mileageOfDay` and `mileageSinceLastCharge` all live in `rvsChargeStatus`, not where we were looking. So the fallback could never have worked. + +No odometer, no distance, no value — every time. The charge-detection behind it was working perfectly all along, which is why it was so unobvious. Both fixed, and it'll populate as soon as you've driven after a charge. + +## 2. Power Usage Since Last Charge — you were right, and right about why + +Your instinct that this smelled like the earlier 3× bug was correct, though the mechanism was different this time. The correction **does** exist and it's the right number — it was just added in a place this particular sensor never reaches. So it was dead code and the sensor carried on showing the raw figure. + +Your numbers back it out exactly: 20.20 kWh corrected is **~6.9 kWh**, which fits 24 miles against a 53-of-75 mile range readout. Now applied wherever the value is read. + +Worth knowing which sensors were affected: `Efficiency Since Last Charge` and the `Last Trip` energy figures were already correcting this properly and were always right. Only the standalone Power Usage sensor was wrong — so if those looked inconsistent with each other on your dashboard, that's the explanation. + +On `lastChargeEndingPower=725` — good spot, and yes, that's the same 3× inflation (725 → 72.5 kWh → ~24.7 kWh real). To answer your question directly: **there is no `lastChargeStartingPower`.** I went through the full field list for both charging blocks; nothing of the sort exists. Which brings us to: + +## 3. Last Charge Energy — new sensor, your suggestion + +Since there's no starting figure to subtract, the only way to get this is to measure it across the charging session, so that's what it now does. It gives you two independent numbers, both in the attributes: + +- **From battery percentage** — SOC rise × usable capacity. This is the headline value since it works on every car. +- **From the car's own energy figures** — `lastChargeEndingPower` minus `Power Usage Since Last Charge`, which is effectively the energy sitting in the pack. Shown alongside for comparison, and dropped when it doesn't look trustworthy. + +Plus start/end/added percentage, duration, average power, and timestamps. There's also a `mg_saic_charge_completed` event firing at the end of each charge with the same data, which should suit your dashboard. + +One caveat for your granny-charging use case: **this is energy at the battery, not at the wall.** Your Zappi will always read higher, because it's also paying for charger and cable losses — typically 5–10%, more on a slow granny charge in the cold. So it'll get you close for settling up with your friend, but it's a floor, not a meter reading. + +Your car's charging dropout got specifically designed around, incidentally. Because your charging endpoint goes quiet the instant a session completes, a naive implementation would log a phantom charge every single time that happened. Session tracking now only acts on polls where the charging data actually came back. + +## On the capacity — you got there before I could ask + +I was going to ask you to check whether you had an override set, because 24.70 didn't match the profile. You've answered it: override deleted, now on 23.2. That's the right call and it makes your Last Charge Energy figures correct from the start. + +One correction though, because it matters for trusting the number: **SAIC hasn't started reporting 23.2.** The car still reports the same inflated 725 it always has. The 23.2 is a figure *we* hold in the vehicle profile for the AS33P — the real usable capacity, as opposed to the 24.7 nominal pack size from the brochure. So nothing changed at SAIC's end; you've just switched from the brochure number to the usable one, which is the more honest basis for energy maths (you can't actually use all 24.7). + +## And your arithmetic checks out + + 27.9% of 23.2 kWh = 6.47 kWh + 20.20 kWh / 3 = 6.73 kWh + +Close enough — yes, and usefully so. Those are two genuinely independent routes to the same number (battery percentage against known capacity, versus the car's own energy counter with the correction applied) landing within about 4% of each other. That's the first real-world confirmation I've had that the correction factor is right, so thank you — it's exactly the cross-check I couldn't do on my own car, since mine doesn't have the inflation. + +It's also mildly interesting that the SOC route reads slightly *lower*. With one data point I'm not going to start tuning the factor on the strength of it, but if you fancy jotting down both figures over your next few charges, that would tell us whether the 4% is a consistent bias worth correcting or just noise. No obligation. + +## On the version — sorry, that one's my fault + +Good catch, and the confusion is entirely mine. Here's what happened: when I released beta4 on the 27th I tagged and published the release, but didn't bump the version inside the integration's manifest, which was still reading beta3. So the release you installed says beta4 (that's what HACS shows you) while the code inside thinks it's beta3. + +I then wrote the PR against that manifest and bumped what I thought was beta3 → beta4 — straight into a version number that already existed and that you were already running. So no, beta4 hasn't been patched; my PR was about to create a second, different beta4, which would have been thoroughly confusing for everyone. + +**Fixed — these three changes will land as 1.2.7-beta5.** I'll post here when it's up. diff --git a/tests/test_backends.py b/tests/test_backends.py index ddb8e44..5f3fc77 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -16,9 +16,9 @@ # user's commands. # * Backend selection: region "India" -> IndiaBackend, everything else -> # the untouched global SAICMGAPIClient. -# * Capability sets: charging/alarm families must stay OUT of -# INDIA_FEATURES until confirmed on a real car; status-reported SOC and -# other confirmed features must stay IN. +# * Capability sets: charging-control and alarm families must stay OUT of +# INDIA_FEATURES until confirmed on a real car; status-reported SOC, +# charging telemetry and other confirmed features must stay IN. # * Legacy fallback: clients that declare no feature set are treated as # fully featured (pre-split global behaviour). @@ -199,11 +199,17 @@ class TestCapabilitySets(unittest.TestCase): Feature.CLIMATE, Feature.HEATED_SEATS, Feature.FIND_MY_CAR, + # Charging telemetry: decoded from the app-id 511 TAP frame and + # confirmed against captures spanning ~4-18 A and 45-100% SOC. + Feature.CHARGING_DATA, } # Must remain absent until decoded AND confirmed on a real India car. INDIA_FORBIDDEN = { - Feature.CHARGING_DATA, + # Not "unconfirmed" but structurally impossible: India reports fuel + # level in the field a non-BEV would read SOC from, so PHEV/HEV SOC + # cannot come from status there. + Feature.STATE_OF_CHARGE_NON_BEV, Feature.CHARGING_CONTROL, Feature.CHARGING_PORT_LOCK, Feature.SCHEDULED_CHARGING, @@ -247,6 +253,7 @@ def setUp(self): self.backend = india.IndiaBackend( username="9999999999", password="p", vin="VIN1", pin_hash="ABC" ) + self.backend._electric_vins.add("VIN1") self.fake = FakeIndiaClient() self.backend._client = self.fake @@ -302,6 +309,7 @@ def test_vehicle_metadata_preserves_model_series_and_colour(self): def test_status_is_saic_shaped_and_validator_safe(self): status = _run(self.backend.get_vehicle_status("VIN1")) + self.assertTrue(self.fake.status_include_charge) self.assertGreater(status.statusTime, 1_700_000_000) self.assertEqual(status.basicVehicleStatus.lockStatus, 1) self.assertEqual(status.basicVehicleStatus.driverDoor, 0) @@ -315,6 +323,31 @@ def test_status_is_saic_shaped_and_validator_safe(self): and status.basicVehicleStatus.mileage == 0 ) + def test_charging_info_reuses_charge_from_status_poll(self): + _run(self.backend.get_vehicle_status("VIN1")) + charging = _run(self.backend.get_charging_info("VIN1")) + + self.assertEqual(charging.chrgMgmtData.bmsPackVol, 1440) + self.assertEqual(charging.chrgMgmtData.bmsPackCrnt, 19680) + self.assertEqual(charging.chrgMgmtData.bmsPackSOCDsp, 625) + self.assertEqual(charging.chrgMgmtData.bmsChrgSts, 3) + self.assertEqual(charging.rvsChargeStatus.fuelRangeElec, 852) + self.assertEqual(charging.rvsChargeStatus.mileage, 12345) + self.assertEqual(charging.rvsChargeStatus.chargingDuration, 150) + self.assertEqual(charging.rvsChargeStatus.totalBatteryCapacity, 508) + self.assertEqual(charging.rvsChargeStatus.mileageSinceLastCharge, 456) + self.assertFalse( + hasattr(charging.rvsChargeStatus, "powerUsageSinceLastCharge") + ) + self.assertIsNone(_run(self.backend.get_charging_info("VIN1"))) + + def test_non_electric_status_does_not_wait_for_charge(self): + self.backend._electric_vins.clear() + + _run(self.backend.get_vehicle_status("VIN1")) + + self.assertFalse(self.fake.status_include_charge) + def test_controls_delegate_to_client(self): _run(self.backend.lock_vehicle("VIN1")) _run(self.backend.unlock_vehicle("VIN1")) @@ -353,6 +386,19 @@ def __init__(self): self.logged_in = False self.vin = "VIN1" self.calls = [] + self.status_include_charge = None + self.charge = types.SimpleNamespace( + is_charging=True, + is_plugged_in=True, + charging_voltage=360.0, + charging_current=16.0, + soc=62.5, + range_km=85.2, + odometer_km=1234.5, + charge_time_elapsed_s=90, + total_battery_capacity_kwh=50.8, + distance_since_last_charge_km=45.6, + ) async def login(self): self.logged_in = True @@ -369,7 +415,8 @@ async def vehicles(self): ) ] - async def status(self): + async def status(self, include_charge=False): + self.status_include_charge = include_charge return types.SimpleNamespace( status_time=1_800_000_000, locked=True, @@ -401,6 +448,7 @@ async def status(self): "frontRightSeatHeatLevel": 3, } }, + charge=self.charge, ) async def control_door_lock(self, lock): diff --git a/tests/test_charge_stats.py b/tests/test_charge_stats.py new file mode 100644 index 0000000..8db661c --- /dev/null +++ b/tests/test_charge_stats.py @@ -0,0 +1,231 @@ +# File: tests/test_charge_stats.py +"""Unit tests for the charge-session maths in trip_stats (#262). + +Covers the Last Charge Energy feature: how much energy a charge put *into* +the battery, which the API has no field for (there is no +``lastChargeStartingPower`` to subtract from ``lastChargeEndingPower``), so +it has to be measured across the session. + +Imports only the pure functions and the manager's session bookkeeping, both +of which are free of Home Assistant deps — matching python-tests.yaml CI. +""" + +import importlib.util +import sys +import unittest +from pathlib import Path + +REPO_ROOT = Path(__file__).resolve().parent.parent +PKG_DIR = REPO_ROOT / "custom_components" / "mg_saic" + + +def _load(name, path): + spec = importlib.util.spec_from_file_location(name, path) + module = importlib.util.module_from_spec(spec) + sys.modules[name] = module + spec.loader.exec_module(module) + return module + + +ts = _load("mg_saic_charge_stats_under_test", PKG_DIR / "trip_stats.py") +CSnap = ts.ChargeSnapshot + + +def csnap(soc=None, pack=None, odo=None, rng=None, t="2026-08-29T22:00:00+00:00"): + return CSnap( + ts=t, soc_pct=soc, pack_energy_kwh=pack, odometer_km=odo, range_km=rng + ) + + +class TestComputeChargeSession(unittest.TestCase): + def test_soc_based_energy_added(self): + start = csnap(soc=36.9, t="2026-08-28T18:30:00+00:00") + end = csnap(soc=80.0, t="2026-08-28T23:38:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.3) + self.assertIsNotNone(charge) + self.assertEqual(charge["soc_added_pct"], 43.1) + # 43.1 % of 74.3 kWh + self.assertAlmostEqual(charge["energy_added_kWh"], 32.023, places=3) + self.assertEqual(charge["method"], "soc") + self.assertEqual(charge["duration_s"], 18480) + + def test_counter_figure_exposed_alongside_soc(self): + start = csnap(soc=50.0, pack=30.0) + end = csnap(soc=80.0, pack=52.0, t="2026-08-29T23:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.3) + self.assertAlmostEqual(charge["energy_added_kWh_soc"], 22.29, places=2) + self.assertAlmostEqual(charge["energy_added_kWh_counter"], 22.0, places=2) + # SOC is the headline figure; the counter rides along for comparison. + self.assertEqual(charge["method"], "soc") + self.assertEqual(charge["energy_added_kWh"], charge["energy_added_kWh_soc"]) + + def test_counter_used_when_soc_unavailable(self): + start = csnap(pack=10.0) + end = csnap(pack=18.5, t="2026-08-29T23:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=24.7) + self.assertEqual(charge["method"], "counter") + self.assertAlmostEqual(charge["energy_added_kWh"], 8.5, places=2) + + def test_stale_ending_power_is_rejected(self): + """If the car hasn't refreshed lastChargeEndingPower yet the pack-energy + delta is <= 0 — that must not be reported as a charge.""" + start = csnap(soc=50.0, pack=40.0) + end = csnap(soc=80.0, pack=40.0, t="2026-08-29T23:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.3) + self.assertNotIn("energy_added_kWh_counter", charge) + self.assertEqual(charge["method"], "soc") + + def test_counter_larger_than_pack_is_rejected(self): + start = csnap(pack=10.0) + end = csnap(pack=200.0, t="2026-08-29T23:00:00+00:00") + self.assertIsNone( + ts.compute_charge_session(start, end, capacity_kwh=24.7) + ) + + def test_trivial_soc_rise_is_not_a_charge(self): + """The pack rebounds a fraction of a percent after a drive — that is + not a charge and must not produce an energy figure.""" + start = csnap(soc=66.0) + end = csnap(soc=66.2, t="2026-08-29T23:00:00+00:00") + self.assertIsNone( + ts.compute_charge_session(start, end, capacity_kwh=74.3) + ) + + def test_no_capacity_falls_back_to_counter(self): + start = csnap(soc=40.0, pack=12.0) + end = csnap(soc=90.0, pack=24.0, t="2026-08-29T23:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=None) + self.assertEqual(charge["method"], "counter") + self.assertAlmostEqual(charge["energy_added_kWh"], 12.0, places=2) + + def test_average_power(self): + start = csnap(soc=50.0, t="2026-08-29T20:00:00+00:00") + end = csnap(soc=60.0, t="2026-08-29T22:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.0) + # 7.4 kWh over 2 h + self.assertAlmostEqual(charge["average_power_kW"], 3.7, places=2) + + +class _Manager(ts.TripStatsManager): + """Manager with persistence/event plumbing bypassed for the pure logic.""" + + def __init__(self): + self.open_charge = None + self.last_charge = None + + +class TestNoteChargeState(unittest.TestCase): + def setUp(self): + self.m = _Manager() + + def _note(self, charging, snapshot, now="2026-08-29T23:00:00+00:00"): + return self.m.note_charge_state( + charging, snapshot, capacity_kwh=74.3, now_iso=now + ) + + def test_full_session_open_and_close(self): + charge, changed = self._note( + True, csnap(soc=40.0, t="2026-08-29T20:00:00+00:00") + ) + self.assertIsNone(charge) + self.assertTrue(changed) + self.assertIsNotNone(self.m.open_charge) + + charge, changed = self._note( + False, csnap(soc=80.0, t="2026-08-29T23:00:00+00:00") + ) + self.assertIsNotNone(charge) + self.assertTrue(changed) + self.assertIsNone(self.m.open_charge) + self.assertEqual(self.m.last_charge, charge) + self.assertAlmostEqual(charge["energy_added_kWh"], 29.72, places=2) + + def test_start_snapshot_is_not_overwritten_mid_charge(self): + self._note(True, csnap(soc=40.0, t="2026-08-29T20:00:00+00:00")) + charge, changed = self._note( + True, csnap(soc=60.0, t="2026-08-29T21:30:00+00:00") + ) + self.assertIsNone(charge) + self.assertFalse(changed) + self.assertEqual(self.m.open_charge.soc_pct, 40.0) + + def test_missing_snapshot_is_ignored(self): + """A poll with no usable reading must not open or close anything — + this is the charging-endpoint dropout guard.""" + self._note(True, csnap(soc=40.0, t="2026-08-29T20:00:00+00:00")) + charge, changed = self._note(False, None) + self.assertIsNone(charge) + self.assertFalse(changed) + self.assertIsNotNone(self.m.open_charge) + + def test_not_charging_with_no_open_session_is_a_no_op(self): + charge, changed = self._note(False, csnap(soc=66.0)) + self.assertIsNone(charge) + self.assertFalse(changed) + + def test_stale_open_session_is_abandoned(self): + self._note(True, csnap(soc=40.0, t="2026-08-25T20:00:00+00:00")) + charge, changed = self._note( + False, csnap(soc=80.0, t="2026-08-29T23:00:00+00:00") + ) + self.assertIsNone(charge) + self.assertTrue(changed) + self.assertIsNone(self.m.open_charge) + self.assertIsNone(self.m.last_charge) + + def test_plug_in_that_delivered_nothing_records_no_charge(self): + self._note(True, csnap(soc=66.0, t="2026-08-29T20:00:00+00:00")) + charge, changed = self._note( + False, csnap(soc=66.1, t="2026-08-29T21:00:00+00:00") + ) + self.assertIsNone(charge) + self.assertTrue(changed) + self.assertIsNone(self.m.last_charge) + + def test_snapshot_roundtrips_through_storage(self): + snap = csnap(soc=40.0, pack=30.0, odo=12345.6) + restored = CSnap.from_dict(snap.to_dict()) + self.assertEqual(restored, snap) + self.assertIsNone(CSnap.from_dict(None)) + + +class TestChargeSessionRangeAdded(unittest.TestCase): + """Range added by a charge (#262). + + The API's own chrgngAddedElecRng is a live counter that resets when the + session ends, and on the cars seen so far it reads 0 even mid-charge, so + the range delta across the session is measured here instead. + """ + + def test_range_added_across_a_charge(self): + start = csnap(soc=36.9, rng=27.0, t="2026-08-28T18:30:00+00:00") + end = csnap(soc=80.0, rng=120.0, t="2026-08-28T23:38:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.3) + self.assertEqual(charge["range_added_km"], 93.0) + self.assertEqual(charge["range_start_km"], 27.0) + self.assertEqual(charge["range_end_km"], 120.0) + + def test_range_omitted_when_unavailable(self): + charge = ts.compute_charge_session( + csnap(soc=40.0), csnap(soc=80.0, t="2026-08-29T23:00:00+00:00"), + capacity_kwh=74.3, + ) + self.assertNotIn("range_added_km", charge) + self.assertNotIn("range_start_km", charge) + + def test_negative_range_delta_is_dropped_but_endpoints_kept(self): + """Range can fall during a charge — a cold pack re-estimating, say. The + endpoints stay visible for diagnosis; the nonsense delta does not.""" + start = csnap(soc=40.0, rng=100.0) + end = csnap(soc=80.0, rng=95.0, t="2026-08-29T23:00:00+00:00") + charge = ts.compute_charge_session(start, end, capacity_kwh=74.3) + self.assertNotIn("range_added_km", charge) + self.assertEqual(charge["range_end_km"], 95.0) + + def test_range_survives_storage_roundtrip(self): + snap = csnap(soc=40.0, rng=27.0) + self.assertEqual(CSnap.from_dict(snap.to_dict()).range_km, 27.0) + + +if __name__ == "__main__": + unittest.main() diff --git a/tests/test_india_gps.py b/tests/test_india_gps.py index 7598b7d..58e801e 100644 --- a/tests/test_india_gps.py +++ b/tests/test_india_gps.py @@ -184,8 +184,12 @@ def __init__(self, *_args, **_kwargs): class _IndiaApiError(Exception): pass + class _ChargingStatusUnavailable(_IndiaApiError): + pass + _module( "mg_ismart_india_client", + ChargingStatusUnavailable=_ChargingStatusUnavailable, MgIndiaApiError=_IndiaApiError, MgIndiaClient=object, hash_control_pin=lambda pin: pin, diff --git a/tests/test_india_soc.py b/tests/test_india_soc.py index eeadd0c..bda52fc 100644 --- a/tests/test_india_soc.py +++ b/tests/test_india_soc.py @@ -133,8 +133,12 @@ def __init__(self, *_args, **_kwargs): class _IndiaApiError(Exception): pass + class _ChargingStatusUnavailable(_IndiaApiError): + pass + _module( "mg_ismart_india_client", + ChargingStatusUnavailable=_ChargingStatusUnavailable, MgIndiaApiError=_IndiaApiError, MgIndiaClient=object, hash_control_pin=lambda pin: pin, @@ -149,8 +153,9 @@ class _IndiaApiError(Exception): f"{PACKAGE}.backends.india", PKG_DIR / "backends" / "india.py", ) + logic = _load(f"{PACKAGE}.logic", PKG_DIR / "logic.py") sensor = _load(f"{PACKAGE}.sensor", PKG_DIR / "sensor.py") - return backends, india, sensor + return backends, india, sensor, logic finally: for name in LOADED_MODULE_NAMES: if name in previous_modules: @@ -159,7 +164,7 @@ class _IndiaApiError(Exception): sys.modules.pop(name, None) -BACKENDS, INDIA, SENSOR = _load_modules() +BACKENDS, INDIA, SENSOR, LOGIC = _load_modules() class IndiaBEVStateOfChargeTests(unittest.TestCase): @@ -194,6 +199,17 @@ def _setup_entities( supports_charging_current_limit=False, supports_target_soc=False, ) + # Mirror the coordinator's central capacity resolution (override > + # profile > API, placeholder rejected). These stubs have no profile + # and no override, so the API tier is what's under test here. + rcs = getattr(charging, "rvsChargeStatus", None) if charging else None + api_raw = getattr(rcs, "totalBatteryCapacity", None) if rcs else None + resolution = LOGIC.resolve_battery_capacity(None, None, api_raw, factor=0.1) + coordinator.battery_capacity_override = None + coordinator._profile_battery_capacity_kwh = None + coordinator.known_battery_capacity_kwh = resolution[0] + coordinator.battery_capacity_resolution = resolution + coordinator.effective_battery_capacity_kwh = resolution[0] coordinator.backend_supports = lambda feature: BACKENDS.backend_supports( backend, feature ) @@ -264,13 +280,18 @@ def test_status_soc_creates_only_soc_battery_entity(self): self.assertEqual(len(soc_entities), 1) self.assertEqual(soc_entities[0].native_value, 62) self.assertTrue(soc_entities[0].available) - self.assertFalse( - any( - isinstance(entity, SENSOR.SAICMGChargingSensor) - and entity._name == "Total Battery Capacity" - for entity in entities - ) - ) + # Total Battery Capacity is gated on CHARGING_DATA, which India now + # advertises, so the entity is created — but the India charging frame + # leaves totalBatteryCapacity absent in every capture seen so far, so + # it reads unknown rather than a fabricated number. + capacity = [ + entity + for entity in entities + if isinstance(entity, SENSOR.SAICMGChargingSensor) + and entity._name == "Total Battery Capacity" + ] + self.assertEqual(len(capacity), 1) + self.assertIsNone(capacity[0].native_value) def test_status_soc_accepts_initial_zero(self): backend = INDIA.IndiaBackend("user", "password", vin="VIN1") @@ -377,6 +398,74 @@ def test_global_phev_keeps_charging_soc_and_battery_capacity(self): ) ) + def test_global_soc_falls_back_to_extended_data_when_charging_soc_is_zero(self): + backend = SimpleNamespace(supported_features=BACKENDS.GLOBAL_FEATURES) + status = SimpleNamespace( + basicVehicleStatus=SimpleNamespace(extendedData1=61) + ) + charging = SimpleNamespace( + chrgMgmtData=SimpleNamespace(bmsPackSOCDsp=0), + rvsChargeStatus=SimpleNamespace(totalBatteryCapacity=300), + ) + + entities = self._setup_entities(backend, "PHEV", status, charging) + soc = next( + entity for entity in entities if isinstance(entity, SENSOR.SAICMGSOCSensor) + ) + + # bmsPackSOCDsp=0 is treated as a stale/unpopulated reading, not a + # real 0% SoC, so the sensor should fall back to extendedData1. + self.assertEqual(soc.native_value, 61) + self.assertTrue(soc.available) + + def test_global_soc_falls_back_to_extended_data_when_charging_data_missing(self): + backend = SimpleNamespace(supported_features=BACKENDS.GLOBAL_FEATURES) + status = SimpleNamespace( + basicVehicleStatus=SimpleNamespace(extendedData1=61) + ) + charging = SimpleNamespace( + chrgMgmtData=SimpleNamespace(bmsPackSOCDsp=None), + rvsChargeStatus=SimpleNamespace(totalBatteryCapacity=300), + ) + + entities = self._setup_entities(backend, "PHEV", status, charging) + soc = next( + entity for entity in entities if isinstance(entity, SENSOR.SAICMGSOCSensor) + ) + + self.assertEqual(soc.native_value, 61) + self.assertTrue(soc.available) + + def test_global_hev_gets_soc_sensor_from_extended_data(self): + # MG3 Hybrid+ (issue #318): a self-charging HEV with no charge port. + # No charging data is present, but basicVehicleStatus.extendedData1 + # independently tracks HV battery SoC and should populate the sensor. + backend = SimpleNamespace(supported_features=BACKENDS.GLOBAL_FEATURES) + status = SimpleNamespace( + basicVehicleStatus=SimpleNamespace(extendedData1=73) + ) + + entities = self._setup_entities(backend, "HEV", status, charging=None) + soc = next( + entity for entity in entities if isinstance(entity, SENSOR.SAICMGSOCSensor) + ) + + self.assertEqual(soc.native_value, 73) + self.assertTrue(soc.available) + + def test_india_hev_gets_no_soc_sensor(self): + # India's extendedData1 is repurposed to carry fuel_level, not + # battery SoC, and INDIA_FEATURES has no CHARGING_DATA — so HEV + # must NOT gain a SOC sensor there, unlike the global backend above. + backend = INDIA.IndiaBackend("user", "password", vin="VIN1") + status = self._india_status(backend, 47) + + entities = self._setup_entities(backend, "HEV", status) + + self.assertFalse( + any(isinstance(entity, SENSOR.SAICMGSOCSensor) for entity in entities) + ) + def test_india_non_bevs_keep_fuel_level_without_soc(self): backend = INDIA.IndiaBackend("user", "password", vin="VIN1") status = self._india_status(backend, 47) diff --git a/tests/test_logic.py b/tests/test_logic.py index 9006525..e436eaf 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -131,5 +131,185 @@ def test_preserves_user_default_interval_when_idle(self): self.assertEqual(interval, self.default_interval) +class ApplyEnergyCorrectionTests(unittest.TestCase): + """The ~3x energy inflation correction (#262, #310). + + Regression cover for a fix that was silently doing nothing: the correction + existed and was the right number, but sat in a code path + powerUsageSinceLastCharge never took, so the sensor kept reporting the raw + figure. Pinning the field list here means a future refactor that drops a + field fails loudly. + """ + + def test_corrects_power_usage_since_last_charge(self): + # Harry's HS PHEV: 20.20 kWh reported, ~6.73 kWh real (#262). + self.assertAlmostEqual( + LOGIC.apply_energy_correction("powerUsageSinceLastCharge", 20.20, 1 / 3), + 6.733, + places=3, + ) + + def test_corrects_last_charge_ending_power(self): + self.assertAlmostEqual( + LOGIC.apply_energy_correction("lastChargeEndingPower", 72.5, 1 / 3), + 24.167, + places=3, + ) + + def test_leaves_uncorrected_fields_alone(self): + # Distance is never inflated, only the energy fields. + self.assertEqual( + LOGIC.apply_energy_correction("mileageSinceLastCharge", 38.6, 1 / 3), 38.6 + ) + + def test_no_correction_configured_is_a_passthrough(self): + self.assertEqual( + LOGIC.apply_energy_correction("powerUsageSinceLastCharge", 10.0, None), 10.0 + ) + + def test_missing_value_stays_none(self): + self.assertIsNone( + LOGIC.apply_energy_correction("powerUsageSinceLastCharge", None, 1 / 3) + ) + + def test_zero_is_corrected_not_treated_as_missing(self): + self.assertEqual( + LOGIC.apply_energy_correction("powerUsageSinceLastCharge", 0.0, 1 / 3), 0.0 + ) + + +class OdometerKmTests(unittest.TestCase): + """Odometer source preference and fallback order (#262). + + The charging-data fallback used to read chrgMgmtData, which carries no + mileage field at all, so it could never fire — and nothing noticed because + the logic wasn't reachable from a test. It is now. + """ + + FACTOR = 0.1 + SATURATION = 65535 + + def _odo(self, basic=None, charging=None): + return LOGIC.odometer_km( + basic, charging, factor=self.FACTOR, saturation=self.SATURATION + ) + + def test_prefers_basic_vehicle_status(self): + basic = SimpleNamespace(mileage=123456) + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(mileage=999)) + self.assertAlmostEqual(self._odo(basic, charging), 12345.6) + + def test_falls_back_to_rvs_charge_status(self): + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(mileage=123456)) + self.assertAlmostEqual(self._odo(None, charging), 12345.6) + + def test_chrg_mgmt_data_carries_no_odometer(self): + # The original bug: chrgMgmtData has no mileage field, so a fallback + # pointed at it yields nothing. + charging = SimpleNamespace(chrgMgmtData=SimpleNamespace(bmsPackSOCDsp=661)) + self.assertIsNone(self._odo(None, charging)) + + def test_rejects_zero_and_negative(self): + self.assertIsNone(self._odo(SimpleNamespace(mileage=0))) + self.assertIsNone(self._odo(SimpleNamespace(mileage=-128))) + + def test_rejects_uint16_saturation(self): + self.assertIsNone(self._odo(SimpleNamespace(mileage=self.SATURATION))) + + def test_saturated_basic_status_falls_through_to_charging(self): + basic = SimpleNamespace(mileage=self.SATURATION) + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(mileage=123456)) + self.assertAlmostEqual(self._odo(basic, charging), 12345.6) + + def test_no_data_at_all(self): + self.assertIsNone(self._odo(None, None)) + + +class ElectricRangeKmTests(unittest.TestCase): + """Electric range extraction for the charge-session range delta (#262).""" + + FACTOR = 0.1 + + def _range(self, basic=None, charging=None): + return LOGIC.electric_range_km(basic, charging, factor=self.FACTOR) + + def test_prefers_charging_block(self): + basic = SimpleNamespace(fuelRangeElec=500) + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(fuelRangeElec=750)) + self.assertEqual(self._range(basic, charging), 75.0) + + def test_falls_back_to_basic_status(self): + self.assertEqual(self._range(SimpleNamespace(fuelRangeElec=530)), 53.0) + + def test_rejects_the_parked_sentinel(self): + basic = SimpleNamespace(fuelRangeElec=-128) + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(fuelRangeElec=-128)) + self.assertIsNone(self._range(basic, charging)) + + def test_sentinel_in_charging_block_falls_through(self): + basic = SimpleNamespace(fuelRangeElec=530) + charging = SimpleNamespace(rvsChargeStatus=SimpleNamespace(fuelRangeElec=-128)) + self.assertEqual(self._range(basic, charging), 53.0) + + def test_zero_range_is_a_real_value(self): + # A flat pack genuinely has no range left; that is not missing data. + self.assertEqual(self._range(SimpleNamespace(fuelRangeElec=0)), 0.0) + + def test_nothing_available(self): + self.assertIsNone(self._range(None, None)) +class ResolveBatteryCapacityTests(unittest.TestCase): + """Capacity precedence and the API-tier guards (#262, #302). + + The precedence was always documented as override > profile > API, and the + Total Battery Capacity sensor implemented all three — but the attribute + the energy maths read only ever saw the first two, so unprofiled cars got + a populated capacity sensor next to blank derived sensors. + """ + + FACTOR = 0.1 + + def _resolve(self, override=None, profile=None, api_raw=None): + return LOGIC.resolve_battery_capacity( + override, profile, api_raw, factor=self.FACTOR + ) + + def test_user_override_wins_over_everything(self): + self.assertEqual( + self._resolve(override=23.2, profile=64.0, api_raw=725), + (23.2, "user_override"), + ) + + def test_profile_wins_over_api(self): + self.assertEqual( + self._resolve(profile=23.2, api_raw=725), (23.2, "profile") + ) + + def test_falls_back_to_api_when_unprofiled(self): + # The gap this closes: an unprofiled car reporting a real capacity. + self.assertEqual(self._resolve(api_raw=383), (38.3, "api")) + + def test_rejects_the_placeholder(self): + # 725 -> 72.5 kWh is a documented placeholder, not a pack size, and is + # plausible enough that a range check alone would let it through. + self.assertEqual(self._resolve(api_raw=725), (None, None)) + + def test_placeholder_still_overridden_by_profile_and_user(self): + self.assertEqual(self._resolve(profile=23.2, api_raw=725)[1], "profile") + self.assertEqual( + self._resolve(override=24.7, api_raw=725)[1], "user_override" + ) + + def test_rejects_implausible_magnitudes(self): + self.assertEqual(self._resolve(api_raw=1)[0], None) # 0.1 kWh + self.assertEqual(self._resolve(api_raw=50000)[0], None) # 5000 kWh + + def test_nothing_available_yields_no_source(self): + self.assertEqual(self._resolve(), (None, None)) + + def test_source_is_reported_not_inferred(self): + # An API-derived value must not be labelled "profile". + self.assertEqual(self._resolve(api_raw=383)[1], "api") + + if __name__ == "__main__": unittest.main() diff --git a/tests/test_trip_stats.py b/tests/test_trip_stats.py index cb00c18..8a8d6e5 100644 --- a/tests/test_trip_stats.py +++ b/tests/test_trip_stats.py @@ -317,6 +317,132 @@ def test_small_counter_value_not_flagged_when_odometer_agrees(self): self.assertNotIn("counter_reset_detected", trip) +class TestParallelCounterSocFigures(unittest.TestCase): + """The *_counter / *_soc comparison attributes (#301).""" + + def _snap(self, odo, since_km=None, since_kwh=None, soc=None, t="2026-08-20T06:36:00+00:00"): + return Snap(ts=t, odometer_km=odo, soc_pct=soc, + since_charge_km=since_km, since_charge_kwh=since_kwh) + + def test_counter_and_soc_sets_both_present_and_independent(self): + # Reproduces SteveMSJ's report: the counter over-reports energy (17% + # high here) relative to the SOC-based figure, even with no reset — + # both should be exposed, self-consistently paired with their own + # distance source, alongside the existing primary (counter-preferred). + start = self._snap(1000.0, since_km=0.0, since_kwh=0.0, soc=100.0) + end = self._snap(1341.0, since_km=341.0, since_kwh=59.1, soc=18.4, + t="2026-08-20T12:00:00+00:00") + trip = ts.compute_completed_trip( + start, end, baseline={"since_charge_km": 0.0, "since_charge_kwh": 0.0}, + capacity_kwh=61.7, tank_litres=None, is_electric=True, is_combustion=False, + ) + # Primary stays counter-preferred (unchanged behaviour). + self.assertEqual(trip["distance_km"], 341.0) + self.assertEqual(trip["energy_kWh"], 59.1) + + # Counter-only set: counter distance + counter energy. + self.assertEqual(trip["distance_km_counter"], 341.0) + self.assertEqual(trip["energy_kWh_counter"], 59.1) + + # Odometer-only distance always present. + self.assertEqual(trip["distance_km_odometer"], 341.0) + self.assertAlmostEqual(trip["distance_mi_odometer"], 341.0 / ts.KM_PER_MILE, places=2) + + # SOC-only set: odometer distance + SOC×capacity energy — matches + # Steve's manual calc (81.6% x 61.7 = 50.3 kWh), independent of the + # counter's 59.1 kWh (a ~17% discrepancy, visible by comparing the two). + self.assertAlmostEqual(trip["energy_kWh_soc"], 50.35, places=1) + self.assertLess(trip["energy_kWh_soc"], trip["energy_kWh_counter"]) + + def test_counter_reset_still_exposes_raw_bogus_counter_value(self): + # Even when the primary figure discards a reset counter reading, the + # raw (bogus) counter value should still be visible in *_counter — + # seeing "the counter said 0" is itself useful, not something to hide. + start = self._snap(1100.0, since_km=56.5, since_kwh=14.6, soc=57.7) + end = self._snap(1191.0, since_km=0.0, since_kwh=0.0, soc=40.8, + t="2026-08-20T14:58:10+00:00") + trip = ts.compute_completed_trip( + start, end, baseline={"since_charge_km": 0.0, "since_charge_kwh": 0.0}, + capacity_kwh=64.0, tank_litres=None, is_electric=True, is_combustion=False, + ) + self.assertTrue(trip["counter_reset_detected"]) + self.assertEqual(trip["distance_km"], 91.0) # primary fell back to odometer + # Raw counter figures still shown (0 - 0 = 0), distinguishable via the flag. + self.assertEqual(trip["distance_km_counter"], 0.0) + self.assertIsNone(trip["energy_kWh_counter"]) # 0 energy -> no valid ratio + # SOC-based set is unaffected and gives a real figure. + self.assertAlmostEqual(trip["energy_kWh_soc"], 16.9 / 100 * 64.0, places=2) + + def test_no_soc_data_leaves_soc_set_none(self): + start = self._snap(1000.0, since_km=0.0, since_kwh=0.0) # no soc + end = self._snap(1020.0, since_km=20.0, since_kwh=3.0) + trip = ts.compute_completed_trip( + start, end, baseline={"since_charge_km": 0.0, "since_charge_kwh": 0.0}, + capacity_kwh=64.0, tank_litres=None, is_electric=True, is_combustion=False, + ) + self.assertIsNone(trip["energy_kWh_soc"]) + self.assertIsNone(trip["efficiency_km_per_kWh_soc"]) + + +class TestSocSinceResetEfficiency(unittest.TestCase): + """compute_soc_since_reset_efficiency — the pure SOC/odometer-only calc.""" + + def test_basic_calculation(self): + result = ts.compute_soc_since_reset_efficiency( + baseline_soc_pct=100.0, current_soc_pct=18.4, + baseline_odometer_km=1000.0, current_odometer_km=1341.0, + capacity_kwh=61.7, + ) + self.assertIsNotNone(result) + self.assertEqual(result["distance_km"], 341.0) + self.assertAlmostEqual(result["energy_kWh"], 50.35, places=1) + self.assertEqual(result["baseline_soc_pct"], 100.0) + + def test_no_baseline_returns_none(self): + self.assertIsNone(ts.compute_soc_since_reset_efficiency( + None, 50.0, None, 1000.0, 64.0 + )) + + def test_no_movement_returns_none(self): + self.assertIsNone(ts.compute_soc_since_reset_efficiency( + 80.0, 80.0, 1000.0, 1000.0, 64.0 + )) + + def test_soc_rose_since_baseline_returns_none(self): + # A further charge happened without the baseline being rebased yet — + # shouldn't report negative/nonsensical energy. + self.assertIsNone(ts.compute_soc_since_reset_efficiency( + 50.0, 60.0, 1000.0, 1010.0, 64.0 + )) + + +class TestNoteSocResetBaseline(unittest.TestCase): + def _mgr(self): + from unittest.mock import MagicMock + return ts.TripStatsManager(MagicMock(), "e", "V") + + def test_seeds_on_first_call(self): + m = self._mgr() + self.assertTrue(m.note_soc_reset_baseline(80.0, 1000.0, "t1")) + self.assertEqual(m.soc_reset_baseline["soc_pct"], 80.0) + + def test_rebases_on_soc_rise_only(self): + m = self._mgr() + m.note_soc_reset_baseline(50.0, 1000.0, "t1") + # SOC dropped (driving happened) -> no rebase. + self.assertFalse(m.note_soc_reset_baseline(40.0, 1010.0, "t2")) + self.assertEqual(m.soc_reset_baseline["soc_pct"], 50.0) + # SOC rose (a charge) -> rebase. + self.assertTrue(m.note_soc_reset_baseline(100.0, 1010.0, "t3")) + self.assertEqual(m.soc_reset_baseline["soc_pct"], 100.0) + self.assertEqual(m.soc_reset_baseline["odometer_km"], 1010.0) + + def test_none_soc_is_a_no_op(self): + m = self._mgr() + self.assertFalse(m.note_soc_reset_baseline(None, 1000.0, "t1")) + self.assertIsNone(m.soc_reset_baseline) + + class TestNoteSinceCharge(unittest.TestCase): def _mgr(self): from unittest.mock import MagicMock diff --git a/tests/test_vehicle_profiles.py b/tests/test_vehicle_profiles.py index 543ff24..ea62db3 100644 --- a/tests/test_vehicle_profiles.py +++ b/tests/test_vehicle_profiles.py @@ -93,7 +93,7 @@ def test_s12l_overrides_capacity_to_100(self): # The API reports totalBatteryCapacity=725 (→ 72.5 kWh); the profile must # override it with the real 100 kWh Platinum/Performance pack. self.assertEqual( - const.VEHICLE_PROFILES["S12L"]["battery_capacity_kwh"], 100.0 + const.VEHICLE_PROFILES["S12L"]["battery_capacity_kwh"], 96.5 ) def test_s12l_does_not_reuse_the_bogus_value(self): @@ -106,14 +106,14 @@ def test_real_world_series_string_resolves_to_the_profile(self): # VinInfo.series in the wild is exactly 'S12L' (see #53 log). key, profile = _resolve_profile("S12L") self.assertEqual(key, "S12L") - self.assertEqual(profile["battery_capacity_kwh"], 100.0) + self.assertEqual(profile["battery_capacity_kwh"], 96.5) def test_match_is_case_insensitive_substring(self): # The coordinator upper-cases the series before matching; make sure a # decorated/lower-case series string still resolves. key, profile = _resolve_profile("s12l l") self.assertEqual(key, "S12L") - self.assertEqual(profile["battery_capacity_kwh"], 100.0) + self.assertEqual(profile["battery_capacity_kwh"], 96.5) def test_only_battery_capacity_differs_from_default(self): # The fix must be surgical: relative to the default profile the IM6 used @@ -137,6 +137,124 @@ def test_only_battery_capacity_differs_from_default(self): ) +class TestP12LClimate(unittest.TestCase): + """MG IM5 (series P12L) climate profile — #326. + + Guards against the actual failure reported: unprofiled P12L fell to + DEFAULT's fan_speed scheme, which maps remoteClimateStatus=2 (the car's + real cooling status) to "fan_only" — the same #277 (MGS5) signature. + """ + + def test_p12l_profile_exists(self): + self.assertIn("P12L", const.VEHICLE_PROFILES) + + def test_real_world_series_string_resolves_to_the_profile(self): + # VinInfo.series in the #326 log is exactly 'P12L'. + key, profile = _resolve_profile("P12L") + self.assertEqual(key, "P12L") + self.assertEqual(profile["climate_control_scheme"], "mode_select") + + def test_match_is_case_insensitive_substring(self): + key, profile = _resolve_profile("p12l") + self.assertEqual(key, "P12L") + + def test_uses_mode_select_scheme_not_fan_speed(self): + # The reported app has no fan-speed control (temperature + AC on/off + # only), matching the MGS6/MGS5 mode_select scheme rather than a + # fan slider. + self.assertEqual( + const.VEHICLE_PROFILES["P12L"]["climate_control_scheme"], "mode_select" + ) + + def test_status_2_maps_to_cool_not_fan_only(self): + # This is the exact bug: status 2 must resolve to cooling, not + # fan-only, on this model. + p = const.VEHICLE_PROFILES["P12L"] + self.assertIn(2, p["climate_status_cool"]) + self.assertNotIn(2, p["climate_status_fan_only"]) + + def test_cool_and_fan_only_status_sets_are_disjoint(self): + p = const.VEHICLE_PROFILES["P12L"] + self.assertTrue(p["climate_status_cool"].isdisjoint(p["climate_status_fan_only"])) + + def test_climate_mode_cool_value_is_2(self): + # Confirmed via #326 screenshot + logs: sending mode 2 is what the + # app itself does to cool. + self.assertEqual(const.VEHICLE_PROFILES["P12L"]["climate_mode_cool"], 2) + + def test_uses_usable_capacity_for_confirmed_long_range(self): + # tabannis confirmed (#326) his P12L is the Long Range variant, i.e. + # the 100 kWh NCM/800V pack — resolving the earlier variant ambiguity. + # Profiles store USABLE capacity, not the nominal pack size (see + # AS33P: 23.2 usable against a 24.7 nominal), because every energy + # figure derived from it is energy you can actually use. + self.assertEqual( + const.VEHICLE_PROFILES["P12L"]["battery_capacity_kwh"], 96.5 + ) + + def test_does_not_use_the_nominal_pack_size(self): + # 100.0 would overstate every derived energy figure by ~3.5%. + self.assertNotAlmostEqual( + const.VEHICLE_PROFILES["P12L"]["battery_capacity_kwh"], 100.0, places=3 + ) + + def test_does_not_reuse_the_bogus_value(self): + # Guard against anyone "trusting the API" (None) or pinning the placeholder. + capacity = const.VEHICLE_PROFILES["P12L"]["battery_capacity_kwh"] + self.assertIsNotNone(capacity) + self.assertNotAlmostEqual(capacity, 72.5, places=3) + + def test_no_energy_correction_capacity_override_only(self): + # Mirrors S12L: this is a display-only capacity override, not an + # energy-scaling correction (unlike AS33P, which needs both). + self.assertIsNone(const.VEHICLE_PROFILES["P12L"]["charging_capacity_correction"]) + + def test_only_declared_fields_differ_from_default(self): + # Relative to the default profile P12L used while unprofiled, only the + # fields this profile deliberately sets may differ — everything else + # must stay identical so the fix cannot regress untested behaviour. + p12l = const.VEHICLE_PROFILES["P12L"] + default = const.DEFAULT_VEHICLE_PROFILE + changed_fields = { + "battery_capacity_kwh", + "climate_control_scheme", + "climate_mode_cool", + "climate_mode_fan_only", + "climate_mode_heat", + "climate_mode_max_cool", + "climate_mode_defrost", + "climate_status_cool", + "climate_status_fan_only", + "climate_status_heat", + "climate_status_defrost", + } + for field, default_value in default.items(): + if field in changed_fields: + continue + self.assertIn(field, p12l, msg=f"P12L is missing default field {field!r}") + self.assertEqual( + p12l[field], + default_value, + msg=f"P12L unexpectedly changes {field!r} vs the default profile", + ) + + def test_mirrors_mis3e_mode_values_as_best_effort(self): + # Fan-only/heat/defrost/max-cool are unconfirmed on this model; they + # should inherit the MIS3E values rather than invent new ones, so a + # future confirmation only has to update this profile, not redesign it. + p12l = const.VEHICLE_PROFILES["P12L"] + mis3e = const.VEHICLE_PROFILES["MIS3E"] + for field in ( + "climate_mode_fan_only", + "climate_mode_heat", + "climate_mode_max_cool", + "climate_mode_defrost", + "climate_status_heat", + "climate_status_defrost", + ): + self.assertEqual(p12l[field], mis3e[field], msg=f"{field} diverges from MIS3E") + + class TestBatteryCapacityOverridesAreSane(unittest.TestCase): """Every declared battery override must be a plausible real capacity."""