From fb34a6afdecf0625c9c4e55e301b6753496c2f24 Mon Sep 17 00:00:00 2001 From: claudeMB Date: Fri, 18 Sep 2026 18:24:49 +0200 Subject: [PATCH] tyres: use HA's unit metadata instead of assuming kPa (#39) @eclass told petrus his tyres were at "thirty bar" on 11 Sep. Thirty bar is ten times a car tyre, and it had been saying so for a week. The fetch already asks Home Assistant for every entity's attributes, which carry unit_of_measurement, and then dropped it: _norm_value only ever received the state string. Whatever HA reported - psi, bar, kPa - was filed under the hardcoded key "tires_kpa". The prompt then handed the model a bare number and a unit word and left it to convert, which is where a decade can go missing. - _pressure_kpa() converts from bar/psi/kPa/mbar/hPa explicitly, and returns None on an unknown or missing unit rather than assuming kPa. An unlabelled pressure is not a kPa reading. - the fetch passes attributes.unit_of_measurement through for tyre entities. - fmt_tyres_bar() renders bar to one decimal, so the model never converts at all, and flags a physically implausible reading (outside 1.0-4.5 bar) as a sensor fault instead of stating it as the pressure. - webchat was interpolating the raw dict into the prompt ("tyres {'front_left': 250, ...} kPa"); it now uses the same formatter as the agent. Tests cover each unit, the refusal on an unknown unit, 30 psi no longer becoming thirty-anything, wheel ordering, and the implausibility flag. Not yet confirmed against the live car: the Mini's snapshot is 59h stale and the local one is empty, so which unit this particular HA reports is still unverified. The fix removes the assumption either way. Co-Authored-By: Claude Opus 5 --- carwatch/agent.py | 7 +++-- carwatch/mercedesme.py | 68 ++++++++++++++++++++++++++++++++++++++-- carwatch/webchat.py | 5 ++- tests/test_tyre_units.py | 64 +++++++++++++++++++++++++++++++++++++ 4 files changed, 137 insertions(+), 7 deletions(-) create mode 100644 tests/test_tyre_units.py diff --git a/carwatch/agent.py b/carwatch/agent.py index 9f3712e..0488dbc 100644 --- a/carwatch/agent.py +++ b/carwatch/agent.py @@ -364,9 +364,10 @@ def _think(question: str, asker: str) -> str: f"({_fu.get('range_km', 0):.0f} km range)") _ty = _me.get("tires_kpa") or {} if _ty: - _bits.append("tyre pressures kPa " + ", ".join( - f"{k.replace('_', ' ')} {v:.0f}" - for k, v in _ty.items())) + from carwatch.mercedesme import fmt_tyres_bar + _tt = fmt_tyres_bar(_ty) + if _tt: + _bits.append("tyre pressures " + _tt) _lk = (_me.get("lock") or {}).get("locked") if _lk: _bits.append(f"doors {_lk}") diff --git a/carwatch/mercedesme.py b/carwatch/mercedesme.py index cdd3bcb..e9bd622 100644 --- a/carwatch/mercedesme.py +++ b/carwatch/mercedesme.py @@ -223,8 +223,66 @@ def _numeric(state: str): return None -def _norm_value(entity_id: str, state: str): - """HA state string -> honest normalized value.""" +# Tyre pressure arrives in whatever unit the owner's Home Assistant is set to. +# We used to file it under "tires_kpa" regardless, so a psi reading became a +# "kPa" number and the agent told petrus his tyres were at THIRTY BAR +# (issue #39). The unit is in the entity attributes and we were throwing it +# away. Convert explicitly, and refuse rather than guess on an unknown unit. +_PRESSURE_TO_KPA = { + "kpa": 1.0, + "bar": 100.0, + "psi": 6.894757, + "mbar": 0.1, + "hpa": 0.1, +} + + +def _pressure_kpa(value: float, unit): + """Pressure in ANY reported unit -> kPa. None when the unit is unknown.""" + if value is None: + return None + factor = _PRESSURE_TO_KPA.get(str(unit or "").strip().lower()) + if factor is None: + return None # fail closed: an unlabelled pressure is not a kPa + return round(value * factor, 1) + + +_WHEEL_ORDER = ("front_left", "front_right", "rear_left", "rear_right") +# A passenger-car tyre lives around 2.0-3.0 bar. Anything outside this is a +# sensor or unit problem, and saying it flatly is how "thirty bar" reached the +# driver. Flag it instead of stating it as fact. +_TYRE_BAR_MIN, _TYRE_BAR_MAX = 1.0, 4.5 + + +def fmt_tyres_bar(tires_kpa: dict) -> str: + """kPa readings -> 'front left 2.4 bar, ...'. + + We do the conversion so the model never has to. Handing it a raw number + and a unit name is what produced a ten-times-too-large answer. + """ + if not tires_kpa: + return "" + keys = [k for k in _WHEEL_ORDER if k in tires_kpa] + keys += [k for k in tires_kpa if k not in _WHEEL_ORDER] + parts = [] + for k in keys: + v = tires_kpa.get(k) + if v is None: + continue + bar = v / 100.0 + txt = f"{k.replace('_', ' ')} {bar:.1f} bar" + if not (_TYRE_BAR_MIN <= bar <= _TYRE_BAR_MAX): + txt += " (IMPLAUSIBLE - report as a sensor fault, do not state it as the pressure)" + parts.append(txt) + return ", ".join(parts) + + +def _norm_value(entity_id: str, state: str, unit=None, to_kpa: bool = False): + """HA state string -> honest normalized value. + + `to_kpa` marks a pressure entity, whose numeric state is only meaningful + together with its unit_of_measurement. + """ if state in ("unknown", "unavailable", "", None): return None if entity_id.startswith(("binary_sensor.", "lock.")): @@ -232,6 +290,8 @@ def _norm_value(entity_id: str, state: str): # dashboard renders words, inventing booleans loses "jammed" etc. return state n = _numeric(state) + if to_kpa: + return _pressure_kpa(n, unit) return state if n is None else n @@ -339,7 +399,9 @@ def status(self) -> dict: str(attrs.get("friendly_name", "car")).split()[0].lower() car = cars.setdefault(slug, {"label": slug}) hits_per_slug[slug] = hits_per_slug.get(slug, 0) + 1 - val = _norm_value(eid, ent.get("state")) + val = _norm_value(eid, ent.get("state"), + attrs.get("unit_of_measurement"), + to_kpa=(grp == "tires_kpa")) if val is None: continue if grp == "_flat": diff --git a/carwatch/webchat.py b/carwatch/webchat.py index 3c2063f..3c4e12a 100644 --- a/carwatch/webchat.py +++ b/carwatch/webchat.py @@ -1165,7 +1165,10 @@ def answer(question: str, use_manual: bool = True) -> str: _label = f"{_label} (the household's OTHER car, not you)" _t = _car.get("tires_kpa") if _t: - _bits.append(f"{_label}: tyres {_t} kPa") + from carwatch.mercedesme import fmt_tyres_bar + _tt = fmt_tyres_bar(_t) + if _tt: + _bits.append(f"{_label}: tyres {_tt}") _fu = _car.get("fuel") or {} _ev = _car.get("ev") or {} _fparts = [] diff --git a/tests/test_tyre_units.py b/tests/test_tyre_units.py new file mode 100644 index 0000000..7cf023f --- /dev/null +++ b/tests/test_tyre_units.py @@ -0,0 +1,64 @@ +"""Issue #39: the car agent told petrus his tyres were at THIRTY BAR. + +Root cause: Home Assistant reports tyre pressure in whatever unit the owner's +HA is configured for. The fetch read `attributes.unit_of_measurement` and then +threw it away, filing the value under `tires_kpa` regardless, and the prompt +handed the model a bare number plus a unit word to convert for itself. +""" +import os +import sys +import unittest + +sys.path.insert(0, os.path.dirname(os.path.dirname(os.path.abspath(__file__)))) + +from carwatch.mercedesme import ( # noqa: E402 + _norm_value, _pressure_kpa, fmt_tyres_bar, +) + + +class TyrePressureUnits(unittest.TestCase): + + def test_converts_from_every_unit_ha_might_report(self): + self.assertEqual(_pressure_kpa(2.4, "bar"), 240.0) + self.assertEqual(_pressure_kpa(240, "kPa"), 240.0) + self.assertEqual(round(_pressure_kpa(35, "psi")), 241) + self.assertEqual(_pressure_kpa(2400, "mbar"), 240.0) + + def test_unknown_unit_refuses_rather_than_assuming_kpa(self): + # The old code assumed kPa, which is how a psi reading became a "kPa" + # value. An unlabelled pressure must not be invented. + self.assertIsNone(_pressure_kpa(30, None)) + self.assertIsNone(_pressure_kpa(30, "")) + self.assertIsNone(_pressure_kpa(30, "furlongs")) + + def test_psi_entity_no_longer_becomes_ten_times_too_large(self): + kpa = _norm_value("sensor.eclass_tirepressure_front_left", "30", + "psi", to_kpa=True) + bar = kpa / 100.0 + self.assertTrue(1.9 < bar < 2.2, f"30 psi should be ~2.1 bar, got {bar}") + self.assertIn("2.1 bar", fmt_tyres_bar({"front_left": kpa})) + + def test_output_is_bar_one_decimal_and_model_never_converts(self): + out = fmt_tyres_bar({"front_left": 250, "front_right": 250, + "rear_left": 260, "rear_right": 260}) + self.assertEqual(out, "front left 2.5 bar, front right 2.5 bar, " + "rear left 2.6 bar, rear right 2.6 bar") + self.assertNotIn("kPa", out) + + def test_implausible_pressure_is_flagged_not_stated(self): + self.assertIn("IMPLAUSIBLE", fmt_tyres_bar({"front_left": 3000})) + self.assertIn("IMPLAUSIBLE", fmt_tyres_bar({"front_left": 30})) + + def test_wheels_ordered_and_missing_values_skipped(self): + out = fmt_tyres_bar({"rear_right": 260, "front_left": 250, + "front_right": None}) + self.assertTrue(out.startswith("front left")) + self.assertNotIn("front right", out) + + def test_non_pressure_entities_are_untouched(self): + self.assertEqual(_norm_value("lock.eclass", "locked"), "locked") + self.assertEqual(_norm_value("sensor.eclass_odometer", "48211"), 48211.0) + + +if __name__ == "__main__": + unittest.main()