From 6c88f3ca2a064cb0dc9056d6c5e2f687e2bc3c66 Mon Sep 17 00:00:00 2001 From: Brett Date: Sun, 26 Jul 2026 13:04:54 +1000 Subject: [PATCH] fix(vehicle): keep config state per vehicle and surface ignored fields Pending config, known fields and prefer_typed were class attributes, so every vehicle shared one dict and one vehicle's pending change was sent for another. The success branch looked for updated_vehicles under a response key, but the config API reports success flat, so it never ran: fields were never recorded and the pending batch was never cleared. Read the real shape, and warn naming anything the API reports in ignoredFields. Also record prefer_typed by value - it was assigned the result of an 'in' test, so setting it False set it True. --- AGENTS.md | 3 + teslemetry_stream/vehicle.py | 46 +++++--- tests/test_config_update.py | 221 +++++++++++++++++++++++++++++++++++ 3 files changed, 256 insertions(+), 14 deletions(-) create mode 100644 tests/test_config_update.py diff --git a/AGENTS.md b/AGENTS.md index 414df99..f096676 100644 --- a/AGENTS.md +++ b/AGENTS.md @@ -6,6 +6,9 @@ This file is the project's committed home for project-intrinsic agent knowledge: - CI runs on push/PR: `.github/workflows/ci.yml`. Uses `uv` (see `uv.lock`). - `tests/` files are plain scripts (`if __name__ == "__main__"`), not pytest-based - pytest would collect zero tests here. Run each directly, e.g. `uv run python tests/test_field_type_coercion.py`. - `pyproject.toml` has a `[tool.mypy]` config but no dev-dependency group declares mypy, so `uv sync` alone won't install it. CI installs it ephemerally via `uv run --with mypy mypy teslemetry_stream`. +- `Signal` in `const.py` tracks ; the config route rejects names it does not know with `fst_err_validation`. Fields the API has retired are not rejected - it accepts the request and names them in a top-level `ignoredFields` list - so the library can lag the published list without breaking. +- Config responses are shaped inconsistently: success is flat, `{"updated_vehicles": n}` plus `ignoredFields` when some were dropped, while errors are wrapped, `{"response": null, "error": ...}`. Do not look for `updated_vehicles` under `response`; that lookup silently never matches. +- `update_config` debounces changes for 1s into one batch, and the API merges that batch into the vehicle's *full* config before forwarding it to Tesla. `tests/test_config_update.py` covers the response handling. ## Maintaining this file diff --git a/teslemetry_stream/vehicle.py b/teslemetry_stream/vehicle.py index c1f7c43..1b22d6b 100644 --- a/teslemetry_stream/vehicle.py +++ b/teslemetry_stream/vehicle.py @@ -64,15 +64,19 @@ class TeslemetryStreamVehicle: """Handle streaming field updates.""" - fields: dict[str, dict[str, int]] = {} - preferTyped: bool | None = None - _config: dict[str, Any] = {} + fields: dict[str, dict[str, int]] + preferTyped: bool | None + _config: dict[str, Any] def __init__(self, stream: TeslemetryStream, vin: str): # A dictionary of TelemetryField keys and null values self.stream = stream self.vin: str = vin self.lock = asyncio.Lock() + # Per-instance: class-level dicts would share pending config between vehicles + self.fields = {} + self.preferTyped = None + self._config = {} @property def config(self) -> dict[str, Any]: @@ -121,17 +125,31 @@ async def update_config(self, config: dict[str, Any]) -> None: "Error updating streaming config for %s: %s", self.vin, error ) return - elif data.get("response", {}).get("updated_vehicles"): - LOGGER.info("Updated vehicle streaming config for %s", self.vin) - if fields := self._config.get("fields"): - LOGGER.debug( - "Configured streaming fields %s", ", ".join(fields.keys()) - ) - self.fields = {**self.fields, **fields} - if prefer_typed := self._config.get("prefer_typed") in [True, False]: - LOGGER.debug("Configured streaming typed to %s", prefer_typed) - self.preferTyped = prefer_typed - self._config.clear() + + ignored = data.get("ignoredFields") or [] + if ignored: + LOGGER.warning( + "Streaming fields not available for %s and were ignored: %s", + self.vin, + ", ".join(ignored), + ) + + LOGGER.info("Updated vehicle streaming config for %s", self.vin) + if fields := self._config.get("fields"): + # The API dropped the ignored ones, so they are not configured. + # A requested default has no options; record it as an empty mapping. + applied = { + field: value or {} + for field, value in fields.items() + if field not in ignored + } + LOGGER.debug("Configured streaming fields %s", ", ".join(applied)) + self.fields = {**self.fields, **applied} + prefer_typed = self._config.get("prefer_typed") + if isinstance(prefer_typed, bool): + LOGGER.debug("Configured streaming typed to %s", prefer_typed) + self.preferTyped = prefer_typed + self._config.clear() async def patch_config(self, config: dict[str, Any]) -> dict[str, Any]: """Modify the configuration for the vehicle.""" diff --git a/tests/test_config_update.py b/tests/test_config_update.py new file mode 100644 index 0000000..62a3320 --- /dev/null +++ b/tests/test_config_update.py @@ -0,0 +1,221 @@ +"""Checks ``update_config`` against the shapes the config API really returns. + +Success is reported flat as ``{"updated_vehicles": n}``, optionally with an +``ignoredFields`` list naming fields the API dropped, and errors arrive as +``{"response": null, "error": ...}``. +""" +from __future__ import annotations + +import asyncio +import logging +from typing import Any + +from teslemetry_stream.vehicle import TeslemetryStreamVehicle + +VIN_A = "TESTVIN0000000001" +VIN_B = "TESTVIN0000000002" + +# Response bodies as observed against the live API +ACCEPTED: dict[str, Any] = {"updated_vehicles": 1} +IGNORED: dict[str, Any] = {"updated_vehicles": 1, "ignoredFields": ["RouteLastUpdated"]} +ALL_IGNORED: dict[str, Any] = { + "updated_vehicles": 0, + "ignoredFields": ["LifetimeEnergyUsedDrive", "RouteLastUpdated"], +} +FAILED: dict[str, Any] = {"response": None, "error": "upstream internal error"} + + +class FakeStream: + """Minimal stand-in for TeslemetryStream.""" + + manual = True + + +def make_vehicle(vin: str, responses: list[Any]) -> TeslemetryStreamVehicle: + """Build a vehicle that records payloads and replays canned responses.""" + vehicle = TeslemetryStreamVehicle(FakeStream(), vin) # type: ignore[arg-type] + vehicle.sent = [] # type: ignore[attr-defined] + + async def patch_config(config: dict[str, Any]) -> dict[str, Any]: + vehicle.sent.append(dict(config)) # type: ignore[attr-defined] + return responses.pop(0) if responses else ACCEPTED + + vehicle.patch_config = patch_config # type: ignore[assignment,method-assign] + return vehicle + + +class CaptureWarnings(logging.Handler): + """Collect formatted WARNING records emitted by the library.""" + + def __init__(self) -> None: + super().__init__(level=logging.WARNING) + self.messages: list[str] = [] + + def emit(self, record: logging.LogRecord) -> None: + self.messages.append(record.getMessage()) + + +def check(label: str, ok: bool, detail: str = "") -> bool: + print(f"{label:<56} {'PASS' if ok else 'FAIL'}{' ' + detail if detail else ''}") + return ok + + +async def main() -> None: + results = [] + + # A confirmed update records its fields and leaves nothing pending. + vehicle = make_vehicle(VIN_A, [ACCEPTED]) + await vehicle.update_config({"fields": {"BatteryLevel": {"interval_seconds": 60}}}) + results.append( + check( + "a confirmed update records its fields", + "BatteryLevel" in vehicle.fields and vehicle._config == {}, + f"fields {sorted(vehicle.fields)}, pending {vehicle._config}", + ) + ) + + # Ignored fields are named in a warning. + handler = CaptureWarnings() + logger = logging.getLogger("teslemetry_stream") + logger.addHandler(handler) + try: + vehicle = make_vehicle(VIN_A, [IGNORED]) + await vehicle.update_config( + {"fields": {"BatteryLevel": None, "RouteLastUpdated": None}} + ) + results.append( + check( + "ignored fields are named in a warning", + any("RouteLastUpdated" in m for m in handler.messages), + f"warnings {handler.messages}", + ) + ) + + handler.messages.clear() + vehicle = make_vehicle(VIN_A, [ALL_IGNORED]) + await vehicle.update_config({"fields": {"RouteLastUpdated": None}}) + results.append( + check( + "every ignored field is named", + any( + "RouteLastUpdated" in m and "LifetimeEnergyUsedDrive" in m + for m in handler.messages + ), + f"warnings {handler.messages}", + ) + ) + + handler.messages.clear() + vehicle = make_vehicle(VIN_A, [ACCEPTED]) + await vehicle.update_config({"fields": {"BatteryLevel": None}}) + results.append( + check( + "no warning when nothing was ignored", + not handler.messages, + f"warnings {handler.messages}", + ) + ) + finally: + logger.removeHandler(handler) + + # Ignored fields were dropped by the API, so they are not configured - and + # must not short-circuit a later add_field into reporting them as enabled. + vehicle = make_vehicle(VIN_A, [IGNORED]) + await vehicle.update_config( + {"fields": {"BatteryLevel": None, "RouteLastUpdated": None}} + ) + results.append( + check( + "an ignored field is not recorded as configured", + "RouteLastUpdated" not in vehicle.fields and "BatteryLevel" in vehicle.fields, + f"fields {sorted(vehicle.fields)}", + ) + ) + before = len(vehicle.sent) # type: ignore[attr-defined] + await vehicle.add_field("RouteLastUpdated") + results.append( + check( + "an ignored field is requested again, not short-circuited", + len(vehicle.sent) == before + 1, # type: ignore[attr-defined] + f"{len(vehicle.sent) - before} further request(s)", # type: ignore[attr-defined] + ) + ) + + # A default add_field stores an options mapping, so a second listener on the + # same signal does not call .get on None. + vehicle = make_vehicle(VIN_A, [ACCEPTED, ACCEPTED]) + await vehicle.add_field("BatteryLevel") + results.append( + check( + "a default add_field records an empty mapping", + vehicle.fields.get("BatteryLevel") == {}, + f"stored {vehicle.fields.get('BatteryLevel')!r}", + ) + ) + try: + await vehicle.add_field("BatteryLevel") + second_ok = True + except AttributeError: + second_ok = False + results.append(check("a second listener on the same signal does not raise", second_ok)) + results.append( + check( + "a later add_field with an interval still goes through", + (await vehicle.add_field("BatteryLevel", 60)) is None + and vehicle.sent[-1]["fields"]["BatteryLevel"] # type: ignore[attr-defined] + == {"interval_seconds": 60}, + f"sent {vehicle.sent[-1]}", # type: ignore[attr-defined] + ) + ) + + # A failed update does not record its fields as configured. + vehicle = make_vehicle(VIN_A, [FAILED]) + await vehicle.update_config({"fields": {"BatteryLevel": None}}) + results.append( + check( + "a failed update records no fields", + vehicle.fields == {}, + f"fields {sorted(vehicle.fields)}", + ) + ) + + # prefer_typed=False must be recorded as False, not coerced to True. + vehicle = make_vehicle(VIN_A, [ACCEPTED]) + await vehicle.prefer_typed(False) + results.append( + check( + "prefer_typed(False) recorded as False", + vehicle.preferTyped is False, + f"got {vehicle.preferTyped!r}", + ) + ) + + # Pending config must not be shared between vehicles. + first = make_vehicle(VIN_A, [FAILED]) + second = make_vehicle(VIN_B, [ACCEPTED]) + await first.update_config({"fields": {"RouteLastUpdated": None}}) + await second.update_config({"fields": {"BatteryLevel": None}}) + sent = second.sent[0]["fields"] # type: ignore[attr-defined] + results.append( + check( + "one vehicle's pending config does not leak to another", + "RouteLastUpdated" not in sent, + f"sent {sorted(sent)}", + ) + ) + results.append( + check( + "one vehicle's recorded fields do not leak to another", + "BatteryLevel" not in first.fields, + f"first {sorted(first.fields)}, second {sorted(second.fields)}", + ) + ) + + print("-" * 72) + print("ALL PASS" if all(results) else "FAILURES PRESENT") + if not all(results): + raise SystemExit(1) + + +if __name__ == "__main__": + asyncio.run(main())