-
Notifications
You must be signed in to change notification settings - Fork 3
fix: consume config-update SSE events into the vehicle config record #27
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from 1 commit
f6f09dd
9d07e96
08080ff
790d9be
9bcee22
b431af9
744fd2b
28e9c61
b9fd803
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,13 @@ def __init__(self, stream: TeslemetryStream, vin: str): | |
| # Callers that arrive while it is running merge into `_config` and | ||
| # await it instead of starting their own PATCH. | ||
| self._flight = None | ||
| # Keeps `fields`/`preferTyped` current when the server applies a | ||
| # config change outside this client (another client, the console, | ||
| # a Teslemetry-side migration), so add_field/prefer_typed's no-op | ||
| # checks don't act on stale history. | ||
| self.stream.async_add_listener( | ||
| self._on_config_event, {Key.VIN: self.vin, Key.CONFIG: None} | ||
| ) | ||
|
Comment on lines
+95
to
+99
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a vehicle is constructed with the default Useful? React with 👍 / 👎. |
||
|
|
||
| @property | ||
| def config(self) -> dict[str, Any]: | ||
|
|
@@ -115,6 +122,42 @@ async def get_config(self) -> None: | |
|
|
||
| req.raise_for_status() | ||
|
|
||
| def _on_config_event(self, event: dict[str, Any]) -> None: | ||
| """Sync the record from a server-pushed config event. | ||
|
|
||
| Only well-typed pieces are applied; a bad piece is logged and | ||
| skipped so it can't corrupt the last-known-good record, and the | ||
| other piece (if well-typed) still applies. | ||
| """ | ||
| config = event.get(Key.CONFIG) | ||
| if not isinstance(config, dict): | ||
| LOGGER.warning( | ||
| "Ignoring malformed config event for %s: %r", self.vin, config | ||
| ) | ||
| return | ||
|
|
||
| if "fields" in config: | ||
| fields = config["fields"] | ||
| if isinstance(fields, dict): | ||
| self.fields = fields | ||
|
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a malformed config event has a dictionary-valued AGENTS.md reference: AGENTS.md:L19-L19 Useful? React with 👍 / 👎. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
When a public generic listener retains the same config event and later mutates or normalizes AGENTS.md reference: AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| else: | ||
| LOGGER.warning( | ||
| "Ignoring malformed fields in config event for %s: %r", | ||
| self.vin, | ||
| fields, | ||
| ) | ||
|
|
||
| if "prefer_typed" in config: | ||
| prefer_typed = config["prefer_typed"] | ||
| if isinstance(prefer_typed, bool): | ||
| self.preferTyped = prefer_typed | ||
| else: | ||
| LOGGER.warning( | ||
| "Ignoring malformed prefer_typed in config event for %s: %r", | ||
| self.vin, | ||
| prefer_typed, | ||
| ) | ||
|
|
||
| async def update_config(self, config: dict[str, Any]) -> None: | ||
| """Request a configuration update for the vehicle. | ||
|
|
||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,186 @@ | ||
| """Config-update SSE events keep the internal config record fresh. | ||
|
|
||
| The server pushes a ``config`` event (``Key.CONFIG`` / ``SseTopic.CONFIG``) | ||
| shaped like ``{"vin": ..., "config": {"fields": {...}, "prefer_typed": bool}}``, | ||
| mirroring the REST ``get_config`` response body. ``TeslemetryStreamVehicle`` | ||
| registers an internal listener for it at construction so ``fields``/ | ||
| ``preferTyped`` - and therefore the ``add_field``/``prefer_typed`` no-op | ||
| checks - reflect current server truth rather than only what this client has | ||
| itself requested or observed at connect. | ||
| """ | ||
| from __future__ import annotations | ||
|
|
||
| import asyncio | ||
| import logging | ||
| from typing import Any, Callable | ||
|
|
||
| from teslemetry_stream.const import Key | ||
| from teslemetry_stream.vehicle import TeslemetryStreamVehicle | ||
|
|
||
| VIN = "TESTVIN0000000001" | ||
|
|
||
|
|
||
| class FakeStream: | ||
| """Minimal stand-in for TeslemetryStream that captures the config listener.""" | ||
|
|
||
| manual = True | ||
|
|
||
| def __init__(self) -> None: | ||
| self.config_listener: Callable[[dict[str, Any]], None] | None = None | ||
|
|
||
| def async_add_listener( | ||
| self, | ||
| callback: Callable[[dict[str, Any]], None], | ||
| filters: dict[str, Any] | None = None, | ||
| ) -> Callable[[], None]: | ||
| assert filters is not None | ||
| if Key.CONFIG in filters: | ||
| self.config_listener = callback | ||
| return lambda: None | ||
|
|
||
|
|
||
| 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 | ||
|
|
||
|
|
||
| def make_vehicle() -> tuple[TeslemetryStreamVehicle, FakeStream]: | ||
| """Build a vehicle that records PATCH payloads instead of sending them.""" | ||
| stream = FakeStream() | ||
| vehicle = TeslemetryStreamVehicle(stream, 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 {"updated_vehicles": 1} | ||
|
|
||
| vehicle.patch_config = patch_config # type: ignore[assignment,method-assign] | ||
| return vehicle, stream | ||
|
|
||
|
|
||
| async def main() -> None: | ||
| results = [] | ||
|
|
||
| # A config event replaces the record with current server truth. | ||
| vehicle, stream = make_vehicle() | ||
| assert stream.config_listener is not None | ||
| stream.config_listener( | ||
| { | ||
| "vin": VIN, | ||
| "config": { | ||
| "fields": {"BatteryLevel": {"interval_seconds": 60}}, | ||
| "prefer_typed": True, | ||
| }, | ||
| } | ||
| ) | ||
| results.append( | ||
| check( | ||
| "a config event updates fields and prefer_typed", | ||
| vehicle.fields == {"BatteryLevel": {"interval_seconds": 60}} | ||
| and vehicle.preferTyped is True, | ||
| f"fields {vehicle.fields}, prefer_typed {vehicle.preferTyped}", | ||
| ) | ||
| ) | ||
|
|
||
| # A request that now matches the updated record is skipped - no PATCH sent. | ||
| await vehicle.add_field("BatteryLevel", 60) | ||
| results.append( | ||
| check( | ||
| "a request matching the updated record is skipped", | ||
| vehicle.sent == [], | ||
| f"sent {vehicle.sent}", | ||
| ) | ||
| ) | ||
|
|
||
| # A request that differs from the updated record is still sent. | ||
| await vehicle.add_field("BatteryLevel", 30) | ||
| results.append( | ||
| check( | ||
| "a request differing from the updated record is sent", | ||
| len(vehicle.sent) == 1 | ||
| and vehicle.sent[0]["fields"]["BatteryLevel"] == {"interval_seconds": 30}, | ||
| f"sent {vehicle.sent}", | ||
| ) | ||
| ) | ||
|
|
||
| # A malformed/partial config event never corrupts the record. | ||
| handler = CaptureWarnings() | ||
| logger = logging.getLogger("teslemetry_stream") | ||
| logger.addHandler(handler) | ||
| try: | ||
| vehicle, stream = make_vehicle() | ||
| assert stream.config_listener is not None | ||
| stream.config_listener( | ||
| { | ||
| "vin": VIN, | ||
| "config": { | ||
| "fields": {"BatteryLevel": {}}, | ||
| "prefer_typed": False, | ||
| }, | ||
| } | ||
| ) | ||
| good_fields, good_typed = dict(vehicle.fields), vehicle.preferTyped | ||
|
|
||
| # A non-dict "config" body is entirely rejected and logged. | ||
| stream.config_listener({"vin": VIN, "config": "not-a-dict"}) | ||
| results.append( | ||
| check( | ||
| "a non-dict config event is ignored, logged, and keeps last-good", | ||
| vehicle.fields == good_fields | ||
| and vehicle.preferTyped == good_typed | ||
| and any("malformed" in m.lower() for m in handler.messages), | ||
| f"fields {vehicle.fields}, warnings {handler.messages}", | ||
| ) | ||
| ) | ||
|
|
||
| # A partially malformed body applies the well-typed piece and keeps | ||
| # the last-good value for the malformed piece. | ||
| handler.messages.clear() | ||
| stream.config_listener( | ||
| {"vin": VIN, "config": {"fields": "not-a-dict", "prefer_typed": True}} | ||
| ) | ||
| results.append( | ||
| check( | ||
| "a partial config event applies the good field, keeps the bad one", | ||
| vehicle.fields == good_fields | ||
| and vehicle.preferTyped is True | ||
| and any("malformed" in m.lower() for m in handler.messages), | ||
| f"fields {vehicle.fields}, prefer_typed {vehicle.preferTyped}, " | ||
| f"warnings {handler.messages}", | ||
| ) | ||
| ) | ||
|
|
||
| # A config event missing a key entirely leaves that piece untouched. | ||
| handler.messages.clear() | ||
| stream.config_listener( | ||
| {"vin": VIN, "config": {"fields": {"CarType": {}}}} | ||
| ) | ||
| results.append( | ||
| check( | ||
| "a config event omitting prefer_typed leaves it unchanged", | ||
| vehicle.fields == {"CarType": {}} and vehicle.preferTyped is True, | ||
| f"fields {vehicle.fields}, prefer_typed {vehicle.preferTyped}", | ||
| ) | ||
| ) | ||
| finally: | ||
| logger.removeHandler(handler) | ||
|
|
||
| print("-" * 72) | ||
| print("ALL PASS" if all(results) else "FAILURES PRESENT") | ||
| if not all(results): | ||
| raise SystemExit(1) | ||
|
|
||
|
|
||
| if __name__ == "__main__": | ||
| asyncio.run(main()) |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
For every non-manual vehicle stream, this listener remains permanently in
stream._listeners. Consequently, removing the last public or typed listener can never satisfy the empty-listener check inTeslemetryStream.async_add_listener()'s remover, soclose()is not called and the SSE connection and owned listen task continue running indefinitely after consumers unsubscribe.Useful? React with 👍 / 👎.