From 99ae75802b411a06f1313437494f192fcca8b0d9 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Wed, 19 Aug 2026 00:25:35 +0530 Subject: [PATCH 01/27] feat(india): add charging data feature and implement get_charging_info method for India backend Co-Authored-By: Claude Opus 4.8 --- .../mg_saic/backends/__init__.py | 3 ++ custom_components/mg_saic/backends/india.py | 41 ++++++++++++++++++- 2 files changed, 43 insertions(+), 1 deletion(-) diff --git a/custom_components/mg_saic/backends/__init__.py b/custom_components/mg_saic/backends/__init__.py index 0f07003..4dd6519 100644 --- a/custom_components/mg_saic/backends/__init__.py +++ b/custom_components/mg_saic/backends/__init__.py @@ -95,6 +95,9 @@ class Feature(str, Enum): { 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 95fc1fb..dbe9da8 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 @@ -309,6 +309,45 @@ 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 frame onto the chrgMgmtData shape the + charging sensors read. + + The India frame reports charging voltage (volts) and current (amps) in + real units. The shared charging/power sensors expect raw fields on the + global SAIC scales (voltage = raw * CHARGING_VOLTAGE_FACTOR, current = + 1000 - raw * CHARGING_CURRENT_FACTOR), so we invert those scales here; + the sensors then decode straight back to the real volts/amps and derive + power from them. Returns None when no charging frame is seen, which the + coordinator handles gracefully. + """ + self._set_vin(vin) + charge = await (await self._ensure_client()).charge_status() + if not charge: + return None + voltage = charge.get("charging_voltage") + current = charge.get("charging_current") + if charge.get("is_charging"): + bms_chrg_sts = 3 # Charging + elif charge.get("charge_complete"): + bms_chrg_sts = 2 # Charging Finished (plugged in, battery full) + else: + bms_chrg_sts = 0 # Unplugged + chrg_mgmt = _ns( + bmsPackVol=( + round(voltage / CHARGING_VOLTAGE_FACTOR) + if voltage is not None + else None + ), + bmsPackCrnt=( + round((1000 - current) / CHARGING_CURRENT_FACTOR) + if current is not None + else None + ), + bmsChrgSts=bms_chrg_sts, + ) + return _ns(chrgMgmtData=chrg_mgmt, rvsChargeStatus=_ns()) + async def control_heated_seat(self, vin, seat, level): self._set_vin(vin) if seat not in self._seat_levels: From 27c6e4bafad3cce03a8a4ab9560c10105588ee79 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Wed, 19 Aug 2026 09:43:39 +0530 Subject: [PATCH 02/27] feat(india): enhance charging data handling and add charging duration conversion Co-Authored-By: Claude Opus 4.8 --- custom_components/mg_saic/backends/india.py | 96 +++++++++++++++------ 1 file changed, 68 insertions(+), 28 deletions(-) diff --git a/custom_components/mg_saic/backends/india.py b/custom_components/mg_saic/backends/india.py index dbe9da8..7282080 100644 --- a/custom_components/mg_saic/backends/india.py +++ b/custom_components/mg_saic/backends/india.py @@ -72,6 +72,34 @@ 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.""" + if seconds is None: + return None + return round( + seconds / _SECONDS_PER_MINUTE * _CHARGING_DURATION_UNITS_PER_MINUTE + ) + + def _vehicle_config(vehicle, code: str, default=None): raw = getattr(vehicle, "raw", None) if isinstance(raw, dict): @@ -310,43 +338,55 @@ async def stop_ac(self, vin): await (await self._ensure_client()).control_climate(False) async def get_charging_info(self, vin): - """Map the India EV charging frame onto the chrgMgmtData shape the - charging sensors read. - - The India frame reports charging voltage (volts) and current (amps) in - real units. The shared charging/power sensors expect raw fields on the - global SAIC scales (voltage = raw * CHARGING_VOLTAGE_FACTOR, current = - 1000 - raw * CHARGING_CURRENT_FACTOR), so we invert those scales here; - the sensors then decode straight back to the real volts/amps and derive - power from them. Returns None when no charging frame is seen, which the - coordinator handles gracefully. + """Map the India EV charging status onto the chrgMgmtData / rvsChargeStatus + shapes the shared charging sensors read. + + Voltage, current, SOC and range come from the client's declared-unit + ChargeStatus fields (volts, amps, percent, km) and are re-encoded onto the + global SAIC raw scales the shared sensors decode + (CHARGING_VOLTAGE_FACTOR / 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. Returns None when the vehicle sends + no charging frame, which the coordinator handles gracefully; session and + protocol failures propagate from the client so they are logged rather than + silently reported as "not charging". """ self._set_vin(vin) charge = await (await self._ensure_client()).charge_status() - if not charge: + if charge is None: return None - voltage = charge.get("charging_voltage") - current = charge.get("charging_current") - if charge.get("is_charging"): - bms_chrg_sts = 3 # Charging - elif charge.get("charge_complete"): - bms_chrg_sts = 2 # Charging Finished (plugged in, battery full) + 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 = 0 # Unplugged + bms_chrg_sts = _BMS_CHRG_STS_UNPLUGGED chrg_mgmt = _ns( - bmsPackVol=( - round(voltage / CHARGING_VOLTAGE_FACTOR) - if voltage is not None - else None - ), - bmsPackCrnt=( - round((1000 - current) / CHARGING_CURRENT_FACTOR) - if current is not None - else None + 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, ) - return _ns(chrgMgmtData=chrg_mgmt, rvsChargeStatus=_ns()) + 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), + # These three have no confirmed scale on the India frame, so the + # client hands back the vehicle's own integer and we forward it on + # the assumption that it matches the global protocol's scale. If a + # sensor reads wrong, this is the line to correct. + totalBatteryCapacity=charge.total_battery_capacity_raw, + mileageSinceLastCharge=charge.mileage_since_last_charge_raw, + powerUsageSinceLastCharge=charge.power_usage_since_last_charge_raw, + ) + return _ns(chrgMgmtData=chrg_mgmt, rvsChargeStatus=rvs) async def control_heated_seat(self, vin, seat, level): self._set_vin(vin) From 68a58b8b05a866040bcd62ad17563ecf2197bcf0 Mon Sep 17 00:00:00 2001 From: James Date: Mon, 24 Aug 2026 22:58:01 +0000 Subject: [PATCH 03/27] fix(sensor): treat bmsPackSOCDsp=0 as invalid, fall back to extendedData1 basicVehicleStatus.extendedData1 has been confirmed (issue #318, and cross-checked against a second vehicle's own logs) to independently track HV battery SoC as a truncated whole percent, matching bmsPackSOCDsp/10 closely. The State of Charge sensor already fell back from bmsPackSOCDsp to extendedData1 when the charging field was missing or the -128 sentinel, but did not treat an exact 0 as suspect. In practice 0 on this raw field has only been observed as a stale/unpopulated reading, not a genuine 0% SoC, so it is now added to the reject list alongside -128, triggering the existing extendedData1 fallback. Adds regression tests for: charging SoC = 0 falls back to extendedData1, and charging SoC = None (missing field) falls back to extendedData1. Existing India tests confirm 0 is still accepted as a legitimate value on the fallback field itself. --- custom_components/mg_saic/sensor.py | 11 +++++++-- tests/test_india_soc.py | 38 +++++++++++++++++++++++++++++ 2 files changed, 47 insertions(+), 2 deletions(-) diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index ec6f237..fb7b67c 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -1839,8 +1839,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 diff --git a/tests/test_india_soc.py b/tests/test_india_soc.py index eeadd0c..a42d881 100644 --- a/tests/test_india_soc.py +++ b/tests/test_india_soc.py @@ -377,6 +377,44 @@ 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_india_non_bevs_keep_fuel_level_without_soc(self): backend = INDIA.IndiaBackend("user", "password", vin="VIN1") status = self._india_status(backend, 47) From daf4b279f4e9b2af24ad6fc320beb864a2633a7c Mon Sep 17 00:00:00 2001 From: James Date: Tue, 25 Aug 2026 20:52:24 +0000 Subject: [PATCH 04/27] fix(sensor): create SOC sensor for HEV using extendedData1 (#318) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit nerlu's full logs (issue #318) show the MG3 Hybrid+ reports as vehicle_type=HEV, not PHEV. It's a self-charging hybrid with no charge port, so it correctly has no charging-endpoint data — but that also meant it fell outside the State of Charge sensor's eligibility gate entirely, which only covered BEV/PHEV. So the SOC sensor was never created for this vehicle, independent of the bmsPackSOCDsp=0 fallback fixed in the previous commit. Across nerlu's two log snippets, basicVehicleStatus.extendedData1 dropped from 78 to 73 while driving (currentJourneyDistance=50), consistent with genuine HV battery drain during hybrid operation rather than a static/sentinel value. HEV is now included in the SOC sensor's eligibility list, gated on Feature.CHARGING_DATA support (same requirement as PHEV) rather than added unconditionally. This keeps the sensor off the India backend: India's extendedData1 is repurposed to carry fuel_level rather than battery SoC, and INDIA_FEATURES does not advertise CHARGING_DATA, so the gate excludes it there. On the global backend, the sensor will have no charging data to read for a genuine HEV and will fall straight through to the extendedData1 fallback added previously. Adds regression tests: global HEV gets a SOC sensor sourced from extendedData1 with no charging data present; India HEV still gets no SOC sensor (fuel level only). 187 tests green. --- custom_components/mg_saic/sensor.py | 13 ++++++++++++- tests/test_india_soc.py | 30 +++++++++++++++++++++++++++++ 2 files changed, 42 insertions(+), 1 deletion(-) diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index fb7b67c..68bdba4 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -341,8 +341,19 @@ 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 on CHARGING_DATA support (same as PHEV) + # rather than adding it unconditionally keeps this off the + # India backend, where extendedData1 is repurposed to carry + # fuel_level rather than battery SoC and INDIA_FEATURES does + # not advertise 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" diff --git a/tests/test_india_soc.py b/tests/test_india_soc.py index a42d881..e9d99bc 100644 --- a/tests/test_india_soc.py +++ b/tests/test_india_soc.py @@ -415,6 +415,36 @@ def test_global_soc_falls_back_to_extended_data_when_charging_data_missing(self) 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) From b2f1b8bcb30863614cfa2931df6eb8b45435ec40 Mon Sep 17 00:00:00 2001 From: James Date: Tue, 25 Aug 2026 21:04:01 +0000 Subject: [PATCH 05/27] docs: document MG3 Hybrid+ HEV SOC support, bump to 1.2.7-beta1 Adds a Vehicle Profiles row for ZP22 EU (MG3 Hybrid+) explaining the self-charging HEV / extendedData1 SOC source added for #318, and qualifies the State of Charge sensor listing to note HEV availability on self-charging hybrids with no charge port. Bumps manifest version 1.2.6 -> 1.2.7-beta1 for the beta release. --- README.md | 3 ++- custom_components/mg_saic/manifest.json | 2 +- 2 files changed, 3 insertions(+), 2 deletions(-) diff --git a/README.md b/README.md index a83dacf..19fe315 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)* @@ -618,6 +618,7 @@ The integration includes built-in profiles for specific MG/SAIC models that corr | `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 | +| `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/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 0e288e6..2566379 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.5" ], - "version": "1.2.6" + "version": "1.2.7-beta1" } From 73acc7137787541074375bf10fe9f1385059b125 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Wed, 26 Aug 2026 13:26:03 +0530 Subject: [PATCH 06/27] fix(india): translate unavailable charge status into no charging data --- custom_components/mg_saic/backends/india.py | 24 ++++++++++++++++----- 1 file changed, 19 insertions(+), 5 deletions(-) diff --git a/custom_components/mg_saic/backends/india.py b/custom_components/mg_saic/backends/india.py index 0dd92db..844dde7 100644 --- a/custom_components/mg_saic/backends/india.py +++ b/custom_components/mg_saic/backends/india.py @@ -400,14 +400,28 @@ async def get_charging_info(self, vin): 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. Returns None when the vehicle sends - no charging frame, which the coordinator handles gracefully; session and - protocol failures propagate from the client so they are logged rather than + assumption. Returns None when the poll budget expires without a charging + frame, which the coordinator handles gracefully; session and protocol + failures propagate from the client so they are logged rather than silently reported as "not charging". + + The client raises MgIndiaApiError for the exhausted-budget case (an idle + vehicle sends a charging frame of its own, so a missing frame means the + data was unavailable, not that the car is idle). That is a routine poll + outcome here rather than a fault, so it is translated to None; every + other MgIndiaApiError still propagates. """ self._set_vin(vin) - charge = await (await self._ensure_client()).charge_status() - if charge is None: + try: + charge = await (await self._ensure_client()).charge_status() + except MgIndiaApiError as err: + # Message coupled to MgIndiaClient.charge_status's unavailable path. + if "not available after polling" not in str(err): + raise + LOGGER.debug( + "No charging frame for VIN %s after polling; reporting no charging data", + vin, + ) return None if charge.is_charging: bms_chrg_sts = _BMS_CHRG_STS_CHARGING From bb2403fe337e622a4b311f99360487afbac9550e Mon Sep 17 00:00:00 2001 From: townsmcp Date: Wed, 26 Aug 2026 13:19:08 +0000 Subject: [PATCH 07/27] feat(trip-stats): expose counter vs SOC/odometer figures side-by-side (#301) Field reports from SteveMSJ and joaommarques (MG4) on issue #301: - The since-charge counters (Mileage/Power Usage Since Last Charge) reset spuriously without an actual charge on some cars (already mitigated by #315's fallback), and are permanently 'Unknown' on others (e.g. some MGS5s). - More significantly: SteveMSJ compared the counter-derived energy against a SOC-drop x capacity calculation on a real 212-mile trip with no reset involved, and found the counter reads ~17% HIGH (59.1 kWh vs 50.3 kWh SOC- based; the car's own dash efficiency matched the SOC-based figure). joaommarques independently sees the same pattern on his MG4. Rather than picking a winner, this exposes both independently so users can compare across their own cars and trips before we decide whether to change the default. No behaviour change to any existing primary (unprefixed) value. 1. Last Trip Distance / Last Trip Efficiency: compute_completed_trip now always computes the counter-derived and odometer/SOC-derived figures independently (not just as a sequential fallback), and exposes full parallel attribute sets: - distance_km_counter / distance_mi_counter (raw counter delta, shown even when counter_reset_detected discarded it from the primary figure -- seeing the bogus value is itself useful) - distance_km_odometer / distance_mi_odometer (always available) - energy_kWh_counter + its 4 derived _counter efficiency/consumption figures (counter distance + counter energy, self-consistent) - energy_kWh_soc + its 4 derived _soc figures (odometer distance + SOC energy, self-consistent -- mirrors how Steve/Joao are already doing their own comparisons by hand) The existing primary distance_km/energy_kWh/efficiency_*/consumption_* keys are unchanged (still counter-preferred with odometer/SOC fallback), so nothing breaks for existing dashboards/automations. 2. New sensor: Efficiency Since Charge (SOC) -- an SOC/odometer-only alternative to Efficiency Since Last Charge, entirely independent of the mileageSinceLastCharge/powerUsageSinceLastCharge fields. Available on every BEV/PHEV regardless of whether those fields are reliable (MG4) or populated at all (some MGS5s) on a given car -- directly answers both users' reports. Its epoch boundary is 'battery % last seen to rise while parked' (a charge), tracked via new TripStatsManager.note_soc_reset_baseline, deliberately independent of the since-charge counter's own (unreliable) reset detection, and only evaluated while parked so a mid-drive regen SOC uptick can never be mistaken for a charge. New pure function compute_soc_since_reset_efficiency(); new persisted state trip_stats.soc_reset_baseline. Tests: parallel-figure computation (incl. Steve's exact 212mi numbers), raw-counter-value-shown-through-a-reset, missing-SOC-data, the new pure function (4 cases), and the new baseline-tracking method (3 cases). 197 tests green (was 187 before this change -- 10 new, 0 changed/removed). README updated. Bumps manifest to 1.2.7-beta2. Per discussion in #301: deliberately NOT changing which figure is primary -- just gathering comparable data. That decision is still open. --- README.md | 7 +- custom_components/mg_saic/coordinator.py | 6 + custom_components/mg_saic/manifest.json | 2 +- custom_components/mg_saic/sensor.py | 100 ++++++++++- custom_components/mg_saic/trip_stats.py | 218 ++++++++++++++++++++--- tests/test_trip_stats.py | 126 +++++++++++++ 6 files changed, 429 insertions(+), 30 deletions(-) diff --git a/README.md b/README.md index 19fe315..11d4011 100644 --- a/README.md +++ b/README.md @@ -158,6 +158,7 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - 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 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 +172,13 @@ 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 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. diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index df67ede..9e17b4c 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -1452,6 +1452,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) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 2566379..e8b303d 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.5" ], - "version": "1.2.7-beta1" + "version": "1.2.7-beta2" } diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 68bdba4..91b0bae 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -37,7 +37,7 @@ DATA_100_DECIMAL_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): @@ -687,6 +687,11 @@ async def async_setup_entry(hass, entry, async_add_entities): sensors.append( SAICMGEfficiencySinceChargeSensor(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( @@ -3124,6 +3129,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", @@ -3131,6 +3140,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", @@ -3289,3 +3308,82 @@ def native_value(self): @property def extra_state_attributes(self): return self._compute() + + +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 + basic_status = self.coordinator.data.get("status") + 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.known_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..8bb145e 100644 --- a/custom_components/mg_saic/trip_stats.py +++ b/custom_components/mg_saic/trip_stats.py @@ -142,6 +142,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 +200,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 +221,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 +282,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 +318,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,7 +394,64 @@ 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), + } + + + # HA imports are done lazily inside methods so the pure functions above can be # imported and unit-tested without Home Assistant installed. @@ -356,6 +490,11 @@ 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 async def async_load(self) -> None: from homeassistant.helpers.storage import Store @@ -370,6 +509,7 @@ 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") async def async_save(self) -> None: """Persist current open/last-trip state and the since-charge baseline.""" @@ -387,6 +527,7 @@ async def async_save(self) -> None: if self.last_parked_snapshot else None ), + "soc_reset_baseline": self.soc_reset_baseline, } ) @@ -413,6 +554,29 @@ 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 open(self, snapshot: TripSnapshot) -> bool: """Record the start-of-drive snapshot (synchronous). Returns True if a new trip was opened. 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 From 73756b7acfa4741fbb735f7b6e66477e9c454420 Mon Sep 17 00:00:00 2001 From: townsmcp Date: Wed, 26 Aug 2026 16:00:30 +0000 Subject: [PATCH 08/27] fix(sensor): Last Trip Distance wasn't exposing any attributes Pre-existing gap, unrelated to this branch's new keys: SAICMGLastTripSensor takes a with_attributes flag controlling whether it exposes the trip's full attribute dict at all. Last Trip Efficiency and Last Trip Fuel Economy were both instantiated with with_attributes=True; Last Trip Distance was not, so it has never carried ANY trip attributes (not just the new _counter/_soc ones added in this branch, but duration_s, start_ts/end_ts, everything). Found live: after this branch's changes, Last Trip Efficiency correctly showed the new distance_km_counter/_soc etc., but Last Trip Distance showed none of them -- same trip, so not a staleness issue, a genuine gap. One-line fix: with_attributes=True on the Last Trip Distance instantiation. 197 tests still green (no test previously covered this, none needed changing). --- custom_components/mg_saic/sensor.py | 1 + 1 file changed, 1 insertion(+) diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 91b0bae..3141307 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -667,6 +667,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"]: From 711e5695f3e14c3432355317d800280789a51b18 Mon Sep 17 00:00:00 2001 From: James Townsend Date: Wed, 26 Aug 2026 17:08:15 +0100 Subject: [PATCH 09/27] Update version to 1.2.7-beta3 --- custom_components/mg_saic/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index e8b303d..d99740c 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.5" ], - "version": "1.2.7-beta2" + "version": "1.2.7-beta3" } From f4c961131249435471f7d5c1b48171a2d3887d1c Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:24:34 +0530 Subject: [PATCH 10/27] fix(sensor): gate non-BEV SOC on its own feature MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Beta's #322 gates PHEV/HEV SOC on CHARGING_DATA, relying on a comment that India does not advertise it. India does now — charging telemetry landed earlier in this branch — and India repurposes basicVehicleStatus.extendedData1 to carry fuel level, so a PHEV/HEV there would have shown litres of petrol as battery percent. Three of beta's own India tests caught it. STATE_OF_CHARGE_NON_BEV carries that meaning on its own, leaving CHARGING_DATA to mean only "has a charging endpoint". India omits the new feature, global keeps it, global behaviour unchanged. India BEVs also gain a Total Battery Capacity entity, gated on CHARGING_DATA like every other charging sensor. The charging frame omits totalBatteryCapacity in every capture so far, so it reads unknown rather than a fabricated number. Co-Authored-By: Claude Opus 5 --- .../mg_saic/backends/__init__.py | 9 +++++++-- custom_components/mg_saic/sensor.py | 14 ++++++++------ tests/test_backends.py | 14 ++++++++++---- tests/test_india_soc.py | 19 ++++++++++++------- 4 files changed, 37 insertions(+), 19 deletions(-) diff --git a/custom_components/mg_saic/backends/__init__.py b/custom_components/mg_saic/backends/__init__.py index 4dd6519..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,10 +88,14 @@ 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, diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 3141307..3fd1a8e 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -347,17 +347,19 @@ async def async_setup_entry(hass, entry, async_add_entities): # 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 on CHARGING_DATA support (same as PHEV) - # rather than adding it unconditionally keeps this off the - # India backend, where extendedData1 is repurposed to carry - # fuel_level rather than battery SoC and INDIA_FEATURES does - # not advertise CHARGING_DATA. + # 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", "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( diff --git a/tests/test_backends.py b/tests/test_backends.py index ddb8e44..a31f90a 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, diff --git a/tests/test_india_soc.py b/tests/test_india_soc.py index e9d99bc..8172475 100644 --- a/tests/test_india_soc.py +++ b/tests/test_india_soc.py @@ -264,13 +264,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") From 394dea6c9b6d6920721671612f8d13c521b674d7 Mon Sep 17 00:00:00 2001 From: "Dr.Blank" <64108942+Dr-Blank@users.noreply.github.com> Date: Thu, 27 Aug 2026 10:25:43 +0530 Subject: [PATCH 11/27] feat(india): consume charging client 0.1.6 Swap the string-matched "not available after polling" check for the typed ChargingStatusUnavailable the client now exports (john-lazarus/mg-ismart-india-client@511884c). An exhausted poll budget is still translated to None; every other MgIndiaApiError now propagates instead of being caught by an incidental message match. The India test stubs gain ChargingStatusUnavailable, or india.py fails to import under them. Co-Authored-By: Claude Opus 5 --- custom_components/mg_saic/backends/india.py | 65 ++++++++++++--------- tests/test_india_gps.py | 4 ++ tests/test_india_soc.py | 4 ++ 3 files changed, 47 insertions(+), 26 deletions(-) diff --git a/custom_components/mg_saic/backends/india.py b/custom_components/mg_saic/backends/india.py index 844dde7..b8b65a2 100644 --- a/custom_components/mg_saic/backends/india.py +++ b/custom_components/mg_saic/backends/india.py @@ -9,7 +9,12 @@ from types import SimpleNamespace from aiohttp import ClientSession -from mg_ismart_india_client import MgIndiaApiError, MgIndiaClient, hash_control_pin +from mg_ismart_india_client import ( + ChargingStatusUnavailable, + MgIndiaApiError, + MgIndiaClient, + hash_control_pin, +) from ..const import CHARGING_CURRENT_FACTOR, CHARGING_VOLTAGE_FACTOR, LOGGER from . import INDIA_FEATURES @@ -92,7 +97,11 @@ def _tyre_pressure(status, attribute: str) -> float | None: def _charging_duration_units(seconds: int | None) -> int | None: - """Convert elapsed seconds into the hundredths-of-a-minute the sensor decodes.""" + """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( @@ -390,34 +399,38 @@ async def stop_ac(self, 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 client's declared-unit - ChargeStatus fields (volts, amps, percent, km) and are re-encoded onto the - global SAIC raw scales the shared sensors decode - (CHARGING_VOLTAGE_FACTOR / 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. Returns None when the poll budget expires without a charging - frame, which the coordinator handles gracefully; session and protocol - failures propagate from the client so they are logged rather than - silently reported as "not charging". - - The client raises MgIndiaApiError for the exhausted-budget case (an idle - vehicle sends a charging frame of its own, so a missing frame means the - data was unavailable, not that the car is idle). That is a routine poll - outcome here rather than a fault, so it is translated to None; every - other MgIndiaApiError still propagates. + """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. + + :param vin: VIN to report charging status for. + :returns: a namespace carrying ``chrgMgmtData`` and ``rvsChargeStatus``, + or ``None`` when the poll budget expires without a charging frame + (the coordinator handles that gracefully). + :raises MgIndiaApiError: on session and protocol failures, so they are + logged rather than silently reported as "not charging". + + :meth:`~mg_ismart_india_client.client.MgIndiaClient.charge_status` raises + :exc:`~mg_ismart_india_client.client.ChargingStatusUnavailable` for the + exhausted-budget case (an idle vehicle sends a charging frame of its own, + so a missing frame means the data was unavailable, not that the car is + idle). That is a routine poll outcome here rather than a fault, so it is + translated to ``None``; every other + :exc:`~mg_ismart_india_client.crypto.MgIndiaApiError` still propagates. """ self._set_vin(vin) try: charge = await (await self._ensure_client()).charge_status() - except MgIndiaApiError as err: - # Message coupled to MgIndiaClient.charge_status's unavailable path. - if "not available after polling" not in str(err): - raise + except ChargingStatusUnavailable: LOGGER.debug( "No charging frame for VIN %s after polling; reporting no charging data", vin, 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 8172475..f197da9 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, From dd7292a05ac74f27c61f93de8bd6ff8966e69da1 Mon Sep 17 00:00:00 2001 From: "Claude (townsmcp)" Date: Thu, 27 Aug 2026 19:25:09 +0000 Subject: [PATCH 12/27] feat(P12L): mode-select climate profile for MG IM5 (#326) Unprofiled P12L (MG IM5) was falling to DEFAULT_VEHICLE_PROFILE's fan_speed scheme, which maps remoteClimateStatus=2 (the car's actual cooling status) to fan_only -- the same #277 (MGS5) failure signature. Reported by tabannis: iSmart app showed AC on/20C/Auto (owner-confirmed in the car), while HA's Climate Ctrl card showed Mode 'Fan only' at 22C with an irrelevant Fan mode 'Medium' slider. Adds a P12L profile mirroring MIS3E's mode_select scheme: - climate_mode_cool / climate_status_cool = 2, CONFIRMED via the screenshot + logs (status-2-derived misread while genuinely cooling) - fan_only/heat/defrost/max_cool values inherited from MIS3E as best-effort, unconfirmed on this car - temperature range left at DEFAULT's 16-28 pending confirmation Deliberately does NOT set battery_capacity_kwh or charging_capacity_correction. The log shows the same bogus totalBatteryCapacity=725 placeholder seen on EC32/AS33P/S12L, and bmsPackVol suggests an 800V (100 kWh) pack, but the IM5 ships in three variants (75 kWh Standard Range/400V, 100 kWh Long Range/800V, 100 kWh Performance/800V) that 'P12L' alone can't distinguish -- same unresolved ambiguity as S12L/IM6 Premium vs Platinum (#53). Asked the reporter to confirm variant before adding a capacity/energy fix. Adds tests/test_vehicle_profiles.py::TestP12LClimate (profile resolution, mode_select scheme, status-2-is-cool regression guard, MIS3E-mirroring of unconfirmed values, and the deliberate absence of a capacity override). 165 tests green (2 pre-existing, unrelated import-ordering failures on beta HEAD reproduced before this change). Updates the Vehicle Profiles table in README.md. --- README.md | 1 + custom_components/mg_saic/const.py | 63 +++++++++++++++++++++++++++ tests/test_vehicle_profiles.py | 70 ++++++++++++++++++++++++++++++ 3 files changed, 134 insertions(+) diff --git a/README.md b/README.md index 11d4011..3ae9648 100644 --- a/README.md +++ b/README.md @@ -623,6 +623,7 @@ The integration includes built-in profiles for specific MG/SAIC models that corr | `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 | +| `P12L` | IM5 (IM by MG Motor) | Mode-select climate scheme mirroring the MGS6 (status code 2 = cool, #326) — fixes the car showing as "Fan only" while genuinely cooling. Fan-only/heat/defrost values are unconfirmed best-effort. ⚠️ Battery capacity is **not yet corrected**: the API's bogus `totalBatteryCapacity=725` placeholder is present here too, and pack voltage suggests a 100 kWh pack, but the IM5 ships in three variants (75 kWh Standard Range / 100 kWh Long Range / 100 kWh Performance) that this series code can't yet distinguish — affected owners, please open an issue confirming your variant | | `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/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index 3676b58..b5d7ea3 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -157,6 +157,69 @@ # 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). + # The iSmart app for this car was reported to expose 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 — UNCONFIRMED, inherited + # from MIS3E as best-effort. The app was not confirmed to expose a + # true fan-only or heat control on this car; these may be + # unreachable in practice until an owner confirms. + # + # Temperature range/offset: left at DEFAULT's 16-28/offset 2 pending + # confirmation — not yet independently verified for this model (unlike + # MIS3E's captured 16-30 non-linear index map). + # + # Battery capacity / energy correction: DELIBERATELY NOT SET. 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) is consistent with a + # 100 kWh Long Range/Performance pack. BUT the IM5 ships in THREE + # variants — 75 kWh Standard Range (LFP, 400V), 100 kWh Long Range + # (NCM, 800V), 100 kWh Performance (NCM, 800V) — and 'P12L' alone + # cannot yet distinguish them (same unresolved ambiguity as S12L/IM6 + # Premium vs Platinum, #53). Hardcoding 100.0 would badly misreport a + # Standard Range car. Asked tabannis for variant + bmsPackVol + # confirmation before adding a capacity override or energy correction. + "min_temp": 16, + "max_temp": 28, + "temp_offset": 2, + "battery_capacity_kwh": None, + "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 (no app control seen) + "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}, + "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, diff --git a/tests/test_vehicle_profiles.py b/tests/test_vehicle_profiles.py index 543ff24..ddbb08a 100644 --- a/tests/test_vehicle_profiles.py +++ b/tests/test_vehicle_profiles.py @@ -137,6 +137,76 @@ 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_no_capacity_override_pending_variant_confirmation(self): + # Deliberately not set: P12L covers three battery variants (75/100/100 + # kWh) that the series code alone can't distinguish. See #326 comments + # and the reply asking tabannis to confirm which variant this is. + p = const.VEHICLE_PROFILES["P12L"] + self.assertIsNone(p["battery_capacity_kwh"]) + self.assertIsNone(p["charging_capacity_correction"]) + + 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.""" From 794c20863fd960841421cc9e0731e342c8a5c8f2 Mon Sep 17 00:00:00 2001 From: "Claude (townsmcp)" Date: Thu, 27 Aug 2026 20:51:30 +0000 Subject: [PATCH 13/27] feat(P12L): confirm battery capacity 100kWh for Long Range (#326) tabannis confirmed (#326 comments) their IM5 is the Long Range variant -- the 100 kWh NCM/800V pack -- resolving the earlier variant ambiguity that blocked a capacity fix in #327. Also confirmed: - no fan speed control in the app (mode_select scheme was correct) - 16-28C temperature range (matches what was already set) - 'Low'/'High' buttons are one-tap max-cool/max-heat presets, not a distinct wire status -- climate_mode_max_cool stays unconfirmed - AC-gates-heat quirk mirrors the MGS6 (James, same thread) -- supporting, not wire-confirmed, evidence for the inherited values Changes: - battery_capacity_kwh: None -> 100.0, mirroring S12L's handling of the same bogus totalBatteryCapacity=725 placeholder. No charging_capacity_correction (display-only override, same as S12L). - Added fan_speed_low/medium/high (1/2/3) for consistency with other mode_select profiles (MIS3E/MZS3E) -- unused under mode_select (no FAN_MODE feature exposed) but kept as a safe, explicit default rather than relying on the coordinator's .get() fallback. - README row updated: capacity corrected, caveat narrowed to the still-untested 75kWh Standard Range. Still unconfirmed and NOT changed here: fan_only/heat/defrost/max_cool status codes. These need a debug log captured with the AC confirmed on, which tabannis is sending separately -- this branch is left open for that follow-up commit. Tests: extended TestP12LClimate with capacity assertions mirroring TestIM6BatteryCapacity, plus a full-profile diff test (only the deliberately-changed fields may differ from DEFAULT) which caught the missing fan_speed_* keys above. 137 tests total (2 pre-existing, unrelated import-ordering failures on beta HEAD, same as before). --- README.md | 2 +- custom_components/mg_saic/const.py | 63 +++++++++++++++++++----------- tests/test_vehicle_profiles.py | 53 +++++++++++++++++++++---- 3 files changed, 87 insertions(+), 31 deletions(-) diff --git a/README.md b/README.md index 3ae9648..da04db2 100644 --- a/README.md +++ b/README.md @@ -623,7 +623,7 @@ The integration includes built-in profiles for specific MG/SAIC models that corr | `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 | -| `P12L` | IM5 (IM by MG Motor) | Mode-select climate scheme mirroring the MGS6 (status code 2 = cool, #326) — fixes the car showing as "Fan only" while genuinely cooling. Fan-only/heat/defrost values are unconfirmed best-effort. ⚠️ Battery capacity is **not yet corrected**: the API's bogus `totalBatteryCapacity=725` placeholder is present here too, and pack voltage suggests a 100 kWh pack, but the IM5 ships in three variants (75 kWh Standard Range / 100 kWh Long Range / 100 kWh Performance) that this series code can't yet distinguish — affected owners, please open an issue confirming your variant | +| `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 corrected to 100 kWh for the confirmed Long Range/Performance pack (#326) — corrects the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh). ⚠️ If you have the 75 kWh Standard Range and see the same `P12L` series, please open an issue — this will need splitting | | `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/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index b5d7ea3..dfdca3c 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -169,48 +169,65 @@ # # The IM5/IM6 are a separate platform from the classic MG ICE-derived # range, architecturally contemporary with the MGS6/MGS5 (MIS3E/MZS3E). - # The iSmart app for this car was reported to expose Temperature + AC - # on/off with no fan-speed control, matching mode_select rather than + # 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 — UNCONFIRMED, inherited - # from MIS3E as best-effort. The app was not confirmed to expose a - # true fan-only or heat control on this car; these may be - # unreachable in practice until an owner confirms. + # 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: left at DEFAULT's 16-28/offset 2 pending - # confirmation — not yet independently verified for this model (unlike - # MIS3E's captured 16-30 non-linear index map). + # 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 / energy correction: DELIBERATELY NOT SET. 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) is consistent with a - # 100 kWh Long Range/Performance pack. BUT the IM5 ships in THREE - # variants — 75 kWh Standard Range (LFP, 400V), 100 kWh Long Range - # (NCM, 800V), 100 kWh Performance (NCM, 800V) — and 'P12L' alone - # cannot yet distinguish them (same unresolved ambiguity as S12L/IM6 - # Premium vs Platinum, #53). Hardcoding 100.0 would badly misreport a - # Standard Range car. Asked tabannis for variant + bmsPackVol - # confirmation before adding a capacity override or energy correction. + # Battery capacity: 100.0 kWh. 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": None, + "battery_capacity_kwh": 100.0, "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 (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, diff --git a/tests/test_vehicle_profiles.py b/tests/test_vehicle_profiles.py index ddbb08a..4f27606 100644 --- a/tests/test_vehicle_profiles.py +++ b/tests/test_vehicle_profiles.py @@ -182,13 +182,52 @@ def test_climate_mode_cool_value_is_2(self): # app itself does to cool. self.assertEqual(const.VEHICLE_PROFILES["P12L"]["climate_mode_cool"], 2) - def test_no_capacity_override_pending_variant_confirmation(self): - # Deliberately not set: P12L covers three battery variants (75/100/100 - # kWh) that the series code alone can't distinguish. See #326 comments - # and the reply asking tabannis to confirm which variant this is. - p = const.VEHICLE_PROFILES["P12L"] - self.assertIsNone(p["battery_capacity_kwh"]) - self.assertIsNone(p["charging_capacity_correction"]) + def test_overrides_capacity_to_100_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. + self.assertEqual( + const.VEHICLE_PROFILES["P12L"]["battery_capacity_kwh"], 100.0 + ) + + 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 From e392a0dad64c53d073a7b46c616483af57d0d828 Mon Sep 17 00:00:00 2001 From: townsmcp Date: Sun, 30 Aug 2026 01:14:03 +0000 Subject: [PATCH 14/27] fix(sensors): unbreak Efficiency Since Charge (SOC) and the 3x energy correction; add Last Charge Energy (#262) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Three issues reported by @HarryFlatter on an MG HS PHEV (AS33P), two of which reproduce on every car. 1. Efficiency Since Charge (SOC) was permanently Unknown — on all models, not just PHEVs (the entity is created for BEV and PHEV alike). Two stacked faults: the sensor passed the top-level status object into _extract_soc_pct/_extract_odometer_km, which expect basicVehicleStatus; SOC survived via its charging-data fallback, the odometer did not. That fallback then looked for `mileage` on chrgMgmtData, which has no such field — it lives on rvsChargeStatus — so it could never succeed. With the odometer always None the sensor returned no value at all. The soc_reset_baseline tracking itself was correct. 2. Power Usage Since Last Charge kept showing the raw ~3x figure on affected models. The charging_capacity_correction added in #310 sat inside the _NOT_CHARGING_ZERO_FIELDS branch, which powerUsageSinceLastCharge never enters (only lastChargeEndingPower is in that set), so it fell through to the generic numeric branch and was never applied. Hoisted into _apply_energy_correction(), called from both branches. The coordinator path and Efficiency Since Last Charge were already applying it correctly — only the standalone sensor was wrong. 3. New Last Charge Energy sensor: how much energy the last charge put INTO the battery. There is no lastChargeStartingPower in the API, so the session is measured across its boundaries, reporting the SOC-based figure as the headline value and the car's own pack-energy delta alongside it for comparison. A charging-data dropout is never treated as the end of a charge, since on some cars the charging endpoint goes quiet the instant a session completes. Fires mg_saic_charge_completed. --- README.md | 10 ++ custom_components/mg_saic/const.py | 5 + custom_components/mg_saic/coordinator.py | 88 ++++++++++- custom_components/mg_saic/manifest.json | 2 +- custom_components/mg_saic/sensor.py | 139 ++++++++++++++-- custom_components/mg_saic/trip_stats.py | 193 +++++++++++++++++++++++ tests/test_charge_stats.py | 191 ++++++++++++++++++++++ 7 files changed, 605 insertions(+), 23 deletions(-) create mode 100644 tests/test_charge_stats.py diff --git a/README.md b/README.md index 3ae9648..e85d07e 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,7 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - 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 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)* @@ -174,6 +175,15 @@ The integration derives per-trip and per-charge efficiency from data it already **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: `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. + +Note this is energy 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. 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. diff --git a/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index b5d7ea3..46c4892 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -1051,6 +1051,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 9e17b4c..7d730a6 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -12,7 +12,7 @@ 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 .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 +28,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, @@ -1377,10 +1378,14 @@ def _extract_odometer_km(basic_status, charging_data): 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: + # Fall back to the odometer in the charging data. This lives on + # rvsChargeStatus (the same block as mileageSinceLastCharge) — NOT on + # chrgMgmtData, which carries the BMS fields and has no mileage at all, + # so the previous lookup here could never succeed and this fallback was + # silently dead. + rcs = getattr(charging_data, "rvsChargeStatus", None) if charging_data else None + raw = getattr(rcs, "mileage", None) if rcs is not None else None + if raw is not None and raw > 0 and raw != MILEAGE_UINT16_SATURATION: return raw * DATA_DECIMAL_CORRECTION return None @@ -1474,6 +1479,71 @@ def _update_trip_state(self, power_mode, basic_status, charging_data): elif was_seeded: self._schedule_trip_save() + 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 = raw * DATA_DECIMAL_CORRECTION + if self.charging_capacity_correction is not None: + ending = ending * 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), + ) + + 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.known_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: @@ -1541,6 +1611,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/manifest.json b/custom_components/mg_saic/manifest.json index d99740c..66f3dce 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.5" ], - "version": "1.2.7-beta3" + "version": "1.2.7-beta4" } diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 3141307..e3913e8 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -688,6 +688,10 @@ 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)) # 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 @@ -2226,6 +2230,29 @@ class SAICMGChargingSensor(CoordinatorEntity, SensorEntity): # _NOT_CHARGING_ZERO_FIELDS above should return 0 explicitly. # V2X_DISCHARGING (13) is deliberately absent — it has live current/voltage data. _INACTIVE_CHARGING_STATUSES = frozenset({0, 5}) + # Energy fields that some models (e.g. MG HS PHEV / AS33P) report inflated + # by ~3× relative to the true kWh — the same quirk that makes + # totalBatteryCapacity read 72.5 kWh on a 24.7 kWh pack. The per-profile + # charging_capacity_correction factor brings them back to real kWh. + _CORRECTED_ENERGY_FIELDS = frozenset( + {"lastChargeEndingPower", "powerUsageSinceLastCharge"} + ) + + 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). + """ + if value is None or self._field not in self._CORRECTED_ENERGY_FIELDS: + return value + correction = getattr(self.coordinator, "charging_capacity_correction", None) + if correction is None: + return value + return value * correction def __init__( self, @@ -2334,21 +2361,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 @@ -2427,7 +2440,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 @@ -3311,6 +3329,88 @@ 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", + "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 SAICMGEfficiencySinceResetSensor(CoordinatorEntity, SensorEntity): """Electric efficiency since the SOC-detected reset point (#301). @@ -3360,7 +3460,12 @@ def _compute(self): baseline = trip_stats.soc_reset_baseline if trip_stats else None if not baseline: return None - basic_status = self.coordinator.data.get("status") + # 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) diff --git a/custom_components/mg_saic/trip_stats.py b/custom_components/mg_saic/trip_stats.py index 8bb145e..4a52568 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,46 @@ 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 + + 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"), + ) + except (KeyError, TypeError, ValueError): + return None + + def _duration_seconds(start_ts: str, end_ts: str) -> int | None: try: start = datetime.fromisoformat(start_ts) @@ -453,11 +502,87 @@ def compute_soc_since_reset_efficiency( +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 + 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: @@ -495,6 +620,11 @@ def __init__(self, hass, entry_id: str, vin: str) -> None: # 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 @@ -510,6 +640,8 @@ async def async_load(self) -> None: 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.""" @@ -528,6 +660,10 @@ async def async_save(self) -> None: 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, } ) @@ -577,6 +713,53 @@ def note_soc_reset_baseline(self, soc_pct, odometer_km, ts) -> bool: 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. @@ -725,3 +908,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/tests/test_charge_stats.py b/tests/test_charge_stats.py new file mode 100644 index 0000000..650bbd9 --- /dev/null +++ b/tests/test_charge_stats.py @@ -0,0 +1,191 @@ +# 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, t="2026-08-29T22:00:00+00:00"): + return CSnap(ts=t, soc_pct=soc, pack_energy_kwh=pack, odometer_km=odo) + + +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)) + + +if __name__ == "__main__": + unittest.main() From c2a70faa9539fd8526e325d7b429157dd90582c6 Mon Sep 17 00:00:00 2001 From: townsmcp Date: Sun, 30 Aug 2026 08:07:55 +0000 Subject: [PATCH 15/27] chore: bump to 1.2.7-beta5 MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1.2.7-beta4 was already tagged and published on 27 Aug, from a commit whose manifest still read 1.2.7-beta3 — so the manifest on beta was one behind the released tag, and this branch's bump to beta4 would have created a second, different beta4. Skip to beta5. --- RELEASE_NOTES_1.2.7-beta5.md | 38 ++++++++++++++++ custom_components/mg_saic/manifest.json | 2 +- reply-to-harry.md | 58 +++++++++++++++++++++++++ 3 files changed, 97 insertions(+), 1 deletion(-) create mode 100644 RELEASE_NOTES_1.2.7-beta5.md create mode 100644 reply-to-harry.md 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/manifest.json b/custom_components/mg_saic/manifest.json index 66f3dce..d15a2af 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.5" ], - "version": "1.2.7-beta4" + "version": "1.2.7-beta5" } 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. From 1403cfb321b370915655e3bf9b56fd617bc86a5b Mon Sep 17 00:00:00 2001 From: townsmcp Date: Sun, 30 Aug 2026 08:21:55 +0000 Subject: [PATCH 16/27] refactor: move the energy correction and odometer lookup into logic.py (#262) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Both of the bugs fixed in #330 shared a cause: rules that mattered were buried in code that no test could reach. The energy correction sat in one branch of a sensor's numeric handler, so it could be added to a branch the field never took and silently do nothing (#310). The odometer fallback lived inside a coordinator staticmethod, so it could point at a field that does not exist on that object and never fire, for as long as nobody noticed the sensor was blank. Moving both into logic.py — the existing pure, HA-free module — makes each rule directly testable and gives them a single home, so the coordinator's charge-session maths and the charging sensors can no longer drift apart. No behaviour change. 13 regression tests added covering the field list, the passthrough cases, the source preference and the fallback order, including an explicit test that chrgMgmtData carries no odometer. --- custom_components/mg_saic/coordinator.py | 43 +++++------ custom_components/mg_saic/logic.py | 51 +++++++++++++ custom_components/mg_saic/sensor.py | 24 +++--- tests/test_logic.py | 94 ++++++++++++++++++++++++ 4 files changed, 177 insertions(+), 35 deletions(-) diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index 7d730a6..ca76403 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -11,7 +11,7 @@ from .api import SAICMGAPIClient, CommandsLimitReachedException from .backends import Feature from .backends import backend_supports as _backend_supports -from .logic import select_update_interval +from .logic import apply_energy_correction, odometer_km, select_update_interval from .trip_stats import TripStatsManager, TripSnapshot, ChargeSnapshot # After the car turns off, fire extra refreshes at these intervals (seconds) @@ -1367,27 +1367,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 odometer in the charging data. This lives on - # rvsChargeStatus (the same block as mileageSinceLastCharge) — NOT on - # chrgMgmtData, which carries the BMS fields and has no mileage at all, - # so the previous lookup here could never succeed and this fallback was - # silently dead. - rcs = getattr(charging_data, "rvsChargeStatus", None) if charging_data else None - raw = getattr(rcs, "mileage", None) if rcs is not None else None - if raw is not None and raw > 0 and raw != MILEAGE_UINT16_SATURATION: - 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): @@ -1497,9 +1496,11 @@ def _extract_pack_energy_kwh(self, charging_data): raw = getattr(rcs, "lastChargeEndingPower", None) if raw is None or raw < 0: return None - ending = raw * DATA_DECIMAL_CORRECTION - if self.charging_capacity_correction is not None: - ending = ending * self.charging_capacity_correction + 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) diff --git a/custom_components/mg_saic/logic.py b/custom_components/mg_saic/logic.py index 2f304f8..131c4eb 100644 --- a/custom_components/mg_saic/logic.py +++ b/custom_components/mg_saic/logic.py @@ -92,3 +92,54 @@ 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 diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index e3913e8..c568ccb 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -36,6 +36,7 @@ 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, compute_soc_since_reset_efficiency @@ -2230,14 +2231,6 @@ class SAICMGChargingSensor(CoordinatorEntity, SensorEntity): # _NOT_CHARGING_ZERO_FIELDS above should return 0 explicitly. # V2X_DISCHARGING (13) is deliberately absent — it has live current/voltage data. _INACTIVE_CHARGING_STATUSES = frozenset({0, 5}) - # Energy fields that some models (e.g. MG HS PHEV / AS33P) report inflated - # by ~3× relative to the true kWh — the same quirk that makes - # totalBatteryCapacity read 72.5 kWh on a 24.7 kWh pack. The per-profile - # charging_capacity_correction factor brings them back to real kWh. - _CORRECTED_ENERGY_FIELDS = frozenset( - {"lastChargeEndingPower", "powerUsageSinceLastCharge"} - ) - def _apply_energy_correction(self, value): """Scale an inflated energy field by the profile's correction factor. @@ -2246,13 +2239,16 @@ def _apply_energy_correction(self, value): 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. """ - if value is None or self._field not in self._CORRECTED_ENERGY_FIELDS: - return value - correction = getattr(self.coordinator, "charging_capacity_correction", None) - if correction is None: - return value - return value * correction + return apply_energy_correction( + self._field, + value, + getattr(self.coordinator, "charging_capacity_correction", None), + ) def __init__( self, diff --git a/tests/test_logic.py b/tests/test_logic.py index 9006525..9fc1183 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -131,5 +131,99 @@ 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)) + + if __name__ == "__main__": unittest.main() From b50bb86ba7bc1b077221f00b0a6a2732a2b9c2cc Mon Sep 17 00:00:00 2001 From: John Lazarus <22115598+john-lazarus@users.noreply.github.com> Date: Sun, 30 Aug 2026 12:50:34 +0000 Subject: [PATCH 17/27] fix(india): consume charging data from status poll --- custom_components/mg_saic/backends/india.py | 50 ++++++++------------- custom_components/mg_saic/manifest.json | 2 +- tests/test_backends.py | 44 +++++++++++++++++- 3 files changed, 63 insertions(+), 33 deletions(-) diff --git a/custom_components/mg_saic/backends/india.py b/custom_components/mg_saic/backends/india.py index b8b65a2..7b5cf30 100644 --- a/custom_components/mg_saic/backends/india.py +++ b/custom_components/mg_saic/backends/india.py @@ -9,12 +9,7 @@ from types import SimpleNamespace from aiohttp import ClientSession -from mg_ismart_india_client import ( - ChargingStatusUnavailable, - MgIndiaApiError, - MgIndiaClient, - hash_control_pin, -) +from mg_ismart_india_client import MgIndiaApiError, MgIndiaClient, hash_control_pin from ..const import CHARGING_CURRENT_FACTOR, CHARGING_VOLTAGE_FACTOR, LOGGER from . import INDIA_FEATURES @@ -205,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: @@ -238,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 = ( @@ -412,29 +417,17 @@ async def get_charging_info(self, vin): 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 poll budget expires without a charging frame - (the coordinator handles that gracefully). - :raises MgIndiaApiError: on session and protocol failures, so they are - logged rather than silently reported as "not charging". - - :meth:`~mg_ismart_india_client.client.MgIndiaClient.charge_status` raises - :exc:`~mg_ismart_india_client.client.ChargingStatusUnavailable` for the - exhausted-budget case (an idle vehicle sends a charging frame of its own, - so a missing frame means the data was unavailable, not that the car is - idle). That is a routine poll outcome here rather than a fault, so it is - translated to ``None``; every other - :exc:`~mg_ismart_india_client.crypto.MgIndiaApiError` still propagates. + or ``None`` when the status poll did not receive a charging frame. """ self._set_vin(vin) - try: - charge = await (await self._ensure_client()).charge_status() - except ChargingStatusUnavailable: - LOGGER.debug( - "No charging frame for VIN %s after polling; reporting no charging data", - 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 @@ -459,11 +452,6 @@ async def get_charging_info(self, vin): chargingDuration=_charging_duration_units(charge.charge_time_elapsed_s), totalBatteryCapacity=_tenths(charge.total_battery_capacity_kwh), mileageSinceLastCharge=_tenths(charge.distance_since_last_charge_km), - # Energy-since-charge has no confirmed India scale yet, so the client - # hands back the vehicle's own integer and we forward it on the - # assumption it matches the global scale. If the sensor reads wrong, - # this is the line to correct. - powerUsageSinceLastCharge=charge.power_usage_since_last_charge_raw, ) return _ns(chrgMgmtData=chrg_mgmt, rvsChargeStatus=rvs) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index d15a2af..b0725da 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.7-beta5" } diff --git a/tests/test_backends.py b/tests/test_backends.py index a31f90a..5f3fc77 100644 --- a/tests/test_backends.py +++ b/tests/test_backends.py @@ -253,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 @@ -308,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) @@ -321,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")) @@ -359,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 @@ -375,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, @@ -407,6 +448,7 @@ async def status(self): "frontRightSeatHeatLevel": 3, } }, + charge=self.charge, ) async def control_door_lock(self, lock): From c74a47de6d0c7a2663123db1069c07111f0a443f Mon Sep 17 00:00:00 2001 From: James Townsend Date: Sun, 30 Aug 2026 21:35:55 +0100 Subject: [PATCH 18/27] Update version to 1.2.7-beta6 --- custom_components/mg_saic/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index b0725da..9451ab3 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.7" ], - "version": "1.2.7-beta5" + "version": "1.2.7-beta6" } From 29140a1e213da25fbf1bfc6ee6f105146a7a656e Mon Sep 17 00:00:00 2001 From: townsmcp Date: Sun, 30 Aug 2026 21:39:58 +0000 Subject: [PATCH 19/27] feat(capacity): resolve battery capacity once, with the API as a real third tier MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit The precedence has always been documented as user override > our profile > the API's own totalBatteryCapacity, and the Total Battery Capacity sensor implemented all three. But known_battery_capacity_kwh — which the charge-session maths, the SOC efficiency sensor and the SOC trip stats all read — only ever saw the first two. On a car with no profile entry that left a populated capacity sensor sitting next to three blank sensors derived from it. India made this visible (#302), but it was never an India problem: any unprofiled series behaved the same way. Capacity is now resolved in one place, logic.resolve_battery_capacity, returning both the value and its source. All four consumers read it, so the displayed pack size and the energy figures derived from it can no longer disagree. The API tier is guarded. 725 (-> 72.5 kWh) is a documented placeholder seen identically across EC32/AS33P/S12L, and it is plausible enough that a range check alone would pass it, so it is rejected by value; anything outside a wide plausibility band is rejected too. A rejected value yields no capacity rather than a fabricated one. This does change what an unprofiled car emitting the placeholder displays: 72.5 kWh becomes blank. That is the point — it was never a real pack size, and it was feeding energy figures. capacity_source is now reported rather than inferred. It previously guessed "profile" from known_battery_capacity_kwh being set, which would have mislabelled every API-derived value the moment that attribute gained an API tier — the exact confusion #301 added the attribute to prevent. 8 tests for the resolver. The India SOC test's stub coordinator now models the resolution the way the real one does. --- custom_components/mg_saic/coordinator.py | 40 ++++++++++++++++-- custom_components/mg_saic/logic.py | 52 +++++++++++++++++++++++ custom_components/mg_saic/manifest.json | 2 +- custom_components/mg_saic/sensor.py | 32 ++++++++------ tests/test_india_soc.py | 16 ++++++- tests/test_logic.py | 54 ++++++++++++++++++++++++ 6 files changed, 177 insertions(+), 19 deletions(-) diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index ca76403..9401175 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -11,7 +11,12 @@ from .api import SAICMGAPIClient, CommandsLimitReachedException from .backends import Feature from .backends import backend_supports as _backend_supports -from .logic import apply_energy_correction, odometer_km, select_update_interval +from .logic import ( + apply_energy_correction, + 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) @@ -1433,7 +1438,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"), @@ -1478,6 +1483,35 @@ 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. @@ -1536,7 +1570,7 @@ def _update_charge_state(self, basic_status, charging_data): charge, changed = self.trip_stats.note_charge_state( status in CHARGE_SESSION_STATUS_CODES, self._charge_snapshot(basic_status, charging_data), - capacity_kwh=self.known_battery_capacity_kwh, + capacity_kwh=self.effective_battery_capacity_kwh, now_iso=datetime.now(timezone.utc).isoformat(), ) if charge is not None: diff --git a/custom_components/mg_saic/logic.py b/custom_components/mg_saic/logic.py index 131c4eb..511815d 100644 --- a/custom_components/mg_saic/logic.py +++ b/custom_components/mg_saic/logic.py @@ -143,3 +143,55 @@ def odometer_km(basic_status, charging_data, *, factor, saturation): if raw is not None and raw > 0 and raw != saturation: return raw * factor 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 9451ab3..310e864 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.7" ], - "version": "1.2.7-beta6" + "version": "1.2.7-beta7" } diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index ab95207..d26f112 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -2321,24 +2321,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( @@ -3472,7 +3478,7 @@ def _compute(self): current_soc, baseline.get("odometer_km"), current_odometer, - self.coordinator.known_battery_capacity_kwh, + self.coordinator.effective_battery_capacity_kwh, ) @property diff --git a/tests/test_india_soc.py b/tests/test_india_soc.py index f197da9..bda52fc 100644 --- a/tests/test_india_soc.py +++ b/tests/test_india_soc.py @@ -153,8 +153,9 @@ class _ChargingStatusUnavailable(_IndiaApiError): 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: @@ -163,7 +164,7 @@ class _ChargingStatusUnavailable(_IndiaApiError): sys.modules.pop(name, None) -BACKENDS, INDIA, SENSOR = _load_modules() +BACKENDS, INDIA, SENSOR, LOGIC = _load_modules() class IndiaBEVStateOfChargeTests(unittest.TestCase): @@ -198,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 ) diff --git a/tests/test_logic.py b/tests/test_logic.py index 9fc1183..c6ba9cb 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -225,5 +225,59 @@ def test_no_data_at_all(self): self.assertIsNone(self._odo(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() From fb5adaa938c41232b3516543db3061d8127603b6 Mon Sep 17 00:00:00 2001 From: townsmcp Date: Mon, 31 Aug 2026 10:41:53 +0000 Subject: [PATCH 20/27] feat(charge): report the range a charge added (#262) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit @HarryFlatter asked why Added Electric Range never populates. It exposes the API's chrgngAddedElecRng, which sits in chrgMgmtData alongside chrgngRmnngTime and chrgngSpdngTime — the live during-session block — so it is a counter that runs while charging and resets afterwards, not a record of the last charge. On the cars seen so far it reads 0 even mid-charge: verified against a five-hour AC charge that took a BEV from 36.9% to 80% with the sensor flat at 0.0 throughout. It reads 0 rather than unknown because the field is in _NOT_CHARGING_ZERO_FIELDS, which is correct for a live counter but hides the fact that it never carries data. Rather than resuscitate a field the car does not fill, the charge session now records the electric range at each boundary and reports the difference, from fuelRangeElec — which demonstrably works, since it drives the Electric Range sensor. Exposed as range_added_km on Last Charge Energy, with range_start_km and range_end_km alongside, and carried on the mg_saic_charge_completed event. A negative delta is dropped rather than reported (range can fall across a charge when a cold pack re-estimates) while the endpoints are kept, so the attributes still show what happened. 10 tests: range extraction and its -128 sentinel handling in test_logic, the session delta and storage round-trip in test_charge_stats. --- README.md | 6 ++-- custom_components/mg_saic/coordinator.py | 3 ++ custom_components/mg_saic/logic.py | 23 +++++++++++++ custom_components/mg_saic/sensor.py | 3 ++ custom_components/mg_saic/trip_stats.py | 14 ++++++++ tests/test_charge_stats.py | 44 ++++++++++++++++++++++-- tests/test_logic.py | 34 ++++++++++++++++++ 7 files changed, 123 insertions(+), 4 deletions(-) diff --git a/README.md b/README.md index e85d07e..24bdc09 100644 --- a/README.md +++ b/README.md @@ -180,9 +180,11 @@ The integration derives per-trip and per-charge efficiency from data it already - `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: `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. +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. -Note this is energy 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. +`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. diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index ca76403..f9b2dd8 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -1511,6 +1511,9 @@ def _charge_snapshot(self, basic_status, charging_data): 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): diff --git a/custom_components/mg_saic/logic.py b/custom_components/mg_saic/logic.py index 131c4eb..d995269 100644 --- a/custom_components/mg_saic/logic.py +++ b/custom_components/mg_saic/logic.py @@ -143,3 +143,26 @@ def odometer_km(basic_status, charging_data, *, factor, saturation): 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 diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index ab95207..273601c 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -3351,6 +3351,9 @@ class SAICMGLastChargeEnergySensor(CoordinatorEntity, SensorEntity): "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", diff --git a/custom_components/mg_saic/trip_stats.py b/custom_components/mg_saic/trip_stats.py index 4a52568..1127570 100644 --- a/custom_components/mg_saic/trip_stats.py +++ b/custom_components/mg_saic/trip_stats.py @@ -142,6 +142,7 @@ class ChargeSnapshot: 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) @@ -161,6 +162,7 @@ def _f(key): 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 @@ -570,6 +572,18 @@ def compute_charge_session( 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: diff --git a/tests/test_charge_stats.py b/tests/test_charge_stats.py index 650bbd9..8db661c 100644 --- a/tests/test_charge_stats.py +++ b/tests/test_charge_stats.py @@ -31,8 +31,10 @@ def _load(name, path): CSnap = ts.ChargeSnapshot -def csnap(soc=None, pack=None, odo=None, t="2026-08-29T22:00:00+00:00"): - return CSnap(ts=t, soc_pct=soc, pack_energy_kwh=pack, odometer_km=odo) +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): @@ -187,5 +189,43 @@ def test_snapshot_roundtrips_through_storage(self): 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_logic.py b/tests/test_logic.py index 9fc1183..f58df39 100644 --- a/tests/test_logic.py +++ b/tests/test_logic.py @@ -225,5 +225,39 @@ 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)) + + if __name__ == "__main__": unittest.main() From c8b0089fa344459c56875ac3e519f36a88baa682 Mon Sep 17 00:00:00 2001 From: James Townsend Date: Mon, 31 Aug 2026 11:55:05 +0100 Subject: [PATCH 21/27] Update version to 1.2.7-beta8 in manifest.json --- custom_components/mg_saic/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 9451ab3..9565be9 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.7" ], - "version": "1.2.7-beta6" + "version": "1.2.7-beta8" } From e726f77d7d62ebd3930e4e6e6f80c782d829d7da Mon Sep 17 00:00:00 2001 From: townsmcp Date: Mon, 31 Aug 2026 11:01:47 +0000 Subject: [PATCH 22/27] docs: cover the capacity resolver (#332) and annotate Added Electric Range #332 shipped in beta7 without README changes. Three gaps: - capacity_source didn't mention that the figure can now be absent, which is what a car reporting the 725 placeholder will show. - The P12L/IM5 row said capacity was 'not yet corrected', implying 72.5 kWh is displayed. Since the guard landed it reads blank instead, which is a visible change for IM5 owners and needs saying. - Added Electric Range was listed with no indication that it is a live session counter that most cars never populate (#262). --- README.md | 8 +++++--- 1 file changed, 5 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 24bdc09..cf5b52b 100644 --- a/README.md +++ b/README.md @@ -154,7 +154,7 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - 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))* @@ -535,7 +535,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 @@ -635,7 +637,7 @@ The integration includes built-in profiles for specific MG/SAIC models that corr | `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 | -| `P12L` | IM5 (IM by MG Motor) | Mode-select climate scheme mirroring the MGS6 (status code 2 = cool, #326) — fixes the car showing as "Fan only" while genuinely cooling. Fan-only/heat/defrost values are unconfirmed best-effort. ⚠️ Battery capacity is **not yet corrected**: the API's bogus `totalBatteryCapacity=725` placeholder is present here too, and pack voltage suggests a 100 kWh pack, but the IM5 ships in three variants (75 kWh Standard Range / 100 kWh Long Range / 100 kWh Performance) that this series code can't yet distinguish — affected owners, please open an issue confirming your variant | +| `P12L` | IM5 (IM by MG Motor) | Mode-select climate scheme mirroring the MGS6 (status code 2 = cool, #326) — fixes the car showing as "Fan only" while genuinely cooling. Fan-only/heat/defrost values are unconfirmed best-effort. ⚠️ Battery capacity is **not yet profiled**: the API's bogus `totalBatteryCapacity=725` placeholder is present here too, and pack voltage suggests a 100 kWh pack, but the IM5 ships in three variants (75 kWh Standard Range / 100 kWh Long Range / 100 kWh Performance) that this series code can't yet distinguish. Since the placeholder is now rejected rather than displayed, Total Battery Capacity reads blank on this series and the energy sensors derived from it stay empty — set a [battery capacity override](#battery-capacity-override) for your variant in the meantime, and please open an issue confirming which one you have | | `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. From f876b6eec74db4ad87c30355ecdf82efa7412ecb Mon Sep 17 00:00:00 2001 From: townsmcp Date: Mon, 31 Aug 2026 16:30:39 +0000 Subject: [PATCH 23/27] feat: Last Charge Range Added sensor, and usable capacity for IM6 (#262, #53) Range added is now a first-class sensor, not only an attribute. Home Assistant converts sensor states to the user's unit system but never converts attribute values, so a UK user reading range_added_km got kilometres on a dashboard where every other range figure shows miles. The sensor declares DISTANCE in km and lets HA present it correctly; the attributes stay for templating and are documented as always-km. S12L (IM6) moves from 100.0 to 96.5, the usable figure for the 100 kWh NMC pack, matching the convention every other profile follows (AS33P is 23.2 usable against 24.7 nominal) and the P12L value set earlier on this branch. UK/EU sources put all IM6 variants on that pack; the Australian spec sheet's 75 kWh LFP Premium is noted in the profile comment, and if one ever reports S12L it needs 73.5 and its own split. --- README.md | 5 ++- custom_components/mg_saic/const.py | 8 +++- custom_components/mg_saic/sensor.py | 60 +++++++++++++++++++++++++++++ tests/test_vehicle_profiles.py | 6 +-- 4 files changed, 74 insertions(+), 5 deletions(-) diff --git a/README.md b/README.md index 3795e81..d8d21ff 100644 --- a/README.md +++ b/README.md @@ -159,6 +159,7 @@ The MG/SAIC Custom Integration provides the following sensors, binary sensors, a - 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)* @@ -182,6 +183,8 @@ The integration derives per-trip and per-charge efficiency from data it already 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. @@ -636,7 +639,7 @@ 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) — corrects the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh) for the Platinum/Performance pack (#53). ⚠️ Confirmed on the 96.5 kWh usable (100 kWh nominal) Platinum; if the 75 kWh LFP Premium reports the same series, this will need splitting — Premium owners, please open an issue with debug logs | | `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, #326), replacing the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh). ⚠️ If you have the 75 kWh Standard Range (73.5 kWh usable) and see the same `P12L` series, set a [battery capacity override](#battery-capacity-override) and please open an issue — this will need splitting | | `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) | diff --git a/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index a02ae55..b7178b4 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -673,7 +673,13 @@ "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). UK/EU sources put every IM6 variant on the 100 kWh NMC + # pack at 96.5 usable, which sits alongside the Australian spec + # sheet's 75 kWh LFP Premium noted above; if a Premium ever turns up + # reporting '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}, diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 58117fa..60ddd67 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -695,6 +695,7 @@ async def async_setup_entry(hass, entry, async_add_entities): # 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 @@ -3418,6 +3419,65 @@ def extra_state_attributes(self): } +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). diff --git a/tests/test_vehicle_profiles.py b/tests/test_vehicle_profiles.py index 0fa3711..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 From 467e39e73ff617db21048e759d9441d0063b068a Mon Sep 17 00:00:00 2001 From: townsmcp Date: Mon, 31 Aug 2026 16:41:12 +0000 Subject: [PATCH 24/27] docs: IM6 has no 75 kWh option in the UK/EU MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit UK sources confirm the IM6 is sold on the 100 kWh NMC pack only, in Long Range and Performance — so S12L is unambiguous in these markets and the inherited trim caveat overstated the risk. Kept, scoped to other markets, since the Australian spec sheet does list a 75 kWh LFP Premium. Both IM rows in the README now state usable vs nominal explicitly, and the P12L row leads with the Standard Range warning rather than burying it. --- README.md | 4 ++-- custom_components/mg_saic/const.py | 12 ++++++++---- 2 files changed, 10 insertions(+), 6 deletions(-) diff --git a/README.md b/README.md index d8d21ff..89d5851 100644 --- a/README.md +++ b/README.md @@ -639,8 +639,8 @@ 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 96.5 kWh usable (100 kWh nominal) — corrects the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh) for the Platinum/Performance pack (#53). ⚠️ Confirmed on the 96.5 kWh usable (100 kWh nominal) Platinum; if the 75 kWh LFP Premium reports the same series, this will need splitting — Premium owners, please open an issue with debug logs | -| `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, #326), replacing the API's bogus `totalBatteryCapacity=725` (→ 72.5 kWh). ⚠️ If you have the 75 kWh Standard Range (73.5 kWh usable) and see the same `P12L` series, set a [battery capacity override](#battery-capacity-override) and please open an issue — this will need splitting | +| `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/custom_components/mg_saic/const.py b/custom_components/mg_saic/const.py index b7178b4..9e147e1 100644 --- a/custom_components/mg_saic/const.py +++ b/custom_components/mg_saic/const.py @@ -675,10 +675,14 @@ "temp_offset": 2, # 96.5 kWh USABLE (100 kWh nominal NMC). Profiles store usable # capacity, not marketed pack size — see AS33P (23.2 usable / 24.7 - # nominal). UK/EU sources put every IM6 variant on the 100 kWh NMC - # pack at 96.5 usable, which sits alongside the Australian spec - # sheet's 75 kWh LFP Premium noted above; if a Premium ever turns up - # reporting 'S12L', it needs 73.5 usable and its own split. + # 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}, From d3d52e6f6e89fa264480bc0354f533c12d64c9fd Mon Sep 17 00:00:00 2001 From: townsmcp Date: Mon, 31 Aug 2026 17:27:29 +0000 Subject: [PATCH 25/27] fix: restore missing electric_range_km import, repoint Estimated Range After Charging MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit 1.2.7-beta8 raises NameError on every poll for BEV/PHEV owners: _charge_snapshot calls electric_range_km, but the import was never added. The edit that should have added it was a string replacement against an import block that did not yet have the shape it assumed, so it silently matched nothing. py_compile passes — a missing global is a runtime error — and no unit test reaches the coordinator's poll path, so the whole suite stayed green over a broken integration. Estimated Range After Charging moves from bmsEstdElecRng to imcuChrgngEstdElecRng. bmsEstdElecRng does not track a projected range: on an MGS6 at 57% SOC showing 285 km with an 80% target it reported 761 km, against a ~400 km projection and more than the car manages from empty. imcuChrgngEstdElecRng read 410 against a 398 km projection on the same car, and imcuVehElecRng matches Electric Range exactly, confirming these fields are whole kilometres like the existing factor assumes. Values are now gated on the companion validity flag: a mid-charge capture with current flowing showed V=0 on chrgngRmnngTime while it reported a healthy 300 minutes, so 0 is the valid state. A non-zero flag holds the last good reading rather than publishing a stale one. A CI step running pyflakes to fail the build on undefined names is supplied separately — the token used here lacks workflow scope. --- README.md | 2 +- custom_components/mg_saic/coordinator.py | 1 + custom_components/mg_saic/manifest.json | 2 +- custom_components/mg_saic/sensor.py | 28 +++++++++++++++++++++++- 4 files changed, 30 insertions(+), 3 deletions(-) diff --git a/README.md b/README.md index 89d5851..95b63c0 100644 --- a/README.md +++ b/README.md @@ -150,7 +150,7 @@ 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 diff --git a/custom_components/mg_saic/coordinator.py b/custom_components/mg_saic/coordinator.py index ac788f1..a13edb5 100644 --- a/custom_components/mg_saic/coordinator.py +++ b/custom_components/mg_saic/coordinator.py @@ -13,6 +13,7 @@ from .backends import backend_supports as _backend_supports from .logic import ( apply_energy_correction, + electric_range_km, odometer_km, resolve_battery_capacity, select_update_interval, diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 9565be9..9d01827 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.7" ], - "version": "1.2.7-beta8" + "version": "1.2.7-beta9" } diff --git a/custom_components/mg_saic/sensor.py b/custom_components/mg_saic/sensor.py index 60ddd67..eea7dfb 100644 --- a/custom_components/mg_saic/sensor.py +++ b/custom_components/mg_saic/sensor.py @@ -509,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", @@ -2234,6 +2240,22 @@ class SAICMGChargingSensor(CoordinatorEntity, SensorEntity): # _NOT_CHARGING_ZERO_FIELDS above should return 0 explicitly. # 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. @@ -2355,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: From 040e73da0b6aef434e03332663ce6cae5ea923f4 Mon Sep 17 00:00:00 2001 From: James Townsend Date: Mon, 31 Aug 2026 18:37:37 +0100 Subject: [PATCH 26/27] Add pyflakes check for undefined names Added a step to check for undefined names using pyflakes before running unit tests. --- .github/workflows/python-tests.yaml | 15 +++++++++++++++ 1 file changed, 15 insertions(+) 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" From c02c2cd10cb52c94a3c8480f4f10e2f280ad8691 Mon Sep 17 00:00:00 2001 From: James Townsend Date: Tue, 1 Sep 2026 08:32:56 +0100 Subject: [PATCH 27/27] Update version to 1.2.7 in manifest.json --- custom_components/mg_saic/manifest.json | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/custom_components/mg_saic/manifest.json b/custom_components/mg_saic/manifest.json index 9d01827..c9bf1ec 100644 --- a/custom_components/mg_saic/manifest.json +++ b/custom_components/mg_saic/manifest.json @@ -15,5 +15,5 @@ "mg-saic-client==0.9.4", "mg-ismart-india-client==0.1.7" ], - "version": "1.2.7-beta9" + "version": "1.2.7" }