-
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 2 commits
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 |
|---|---|---|
|
|
@@ -66,7 +66,8 @@ def __init__( | |
| else: | ||
| self.topics = None | ||
| self._listeners: dict[ | ||
| Callable[..., Any], tuple[Callable[[dict[str, Any]], None], dict[str, Any] | None] | ||
| Callable[..., Any], | ||
| tuple[Callable[[dict[str, Any]], None], dict[str, Any] | None, bool], | ||
| ] = {} | ||
| self._connection_listeners: dict[Callable[..., Any], Callable[[bool], None]] = {} | ||
| self._listen_task: asyncio.Task[None] | None = None | ||
|
|
@@ -361,13 +362,21 @@ async def __anext__(self) -> dict[str, Any]: | |
| raise StopAsyncIteration | ||
|
|
||
| def async_add_listener( | ||
| self, callback: Callable[[dict[str, Any]], None], filters: dict[str, Any] | None = None | ||
| self, | ||
| callback: Callable[[dict[str, Any]], None], | ||
| filters: dict[str, Any] | None = None, | ||
| internal: bool = False, | ||
| ) -> Callable[[], None]: | ||
| """ | ||
| Listen for data updates. | ||
|
|
||
| :param callback: Callback function to handle updates. | ||
| :param filters: Filters to apply to the updates. | ||
| :param internal: True for a listener that keeps the client's own | ||
| state fresh (e.g. a vehicle's config-sync listener) rather than | ||
| serving a consumer callback. Excluded from the "last listener | ||
| removed" auto-close check, so a bookkeeping-only listener can't | ||
| pin the connection open forever once every real listener is gone. | ||
| :return: Function to remove the listener. | ||
| """ | ||
| schedule_refresh = not self._listeners | ||
|
|
@@ -377,11 +386,11 @@ def remove_listener() -> None: | |
| Remove update listener. | ||
| """ | ||
| self._listeners.pop(remove_listener) | ||
| if not self._listeners: | ||
| if not any(not is_internal for _, _, is_internal in self._listeners.values()): | ||
| LOGGER.info("Shutting down stream as there are no more listeners") | ||
| self.close() | ||
|
|
||
| self._listeners[remove_listener] = (callback, filters) | ||
| self._listeners[remove_listener] = (callback, filters, internal) | ||
|
|
||
| # This is the first listener - start the owned listen task, unless | ||
| # one is already running or manual mode delegates that to the caller. | ||
|
|
@@ -415,7 +424,7 @@ async def listen(self) -> None: | |
| try: | ||
| async for event in self: | ||
| if event: | ||
| for listener, filters in self._listeners.values(): | ||
| for listener, filters, _internal in self._listeners.values(): | ||
|
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 callback on an active multi-vehicle stream calls AGENTS.md reference: AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| if recursive_match(filters, event): | ||
| try: | ||
| listener(event) | ||
|
|
||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -86,6 +86,11 @@ 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 | ||
| # Registration is deferred to first use (see _ensure_config_listener) - | ||
| # eagerly calling async_add_listener here would create the stream's | ||
| # owned task via asyncio.create_task before a loop necessarily exists, | ||
| # breaking a synchronous `TeslemetryStream(vin=...)`/vehicle construction. | ||
| self._config_listener_registered = False | ||
|
|
||
| @property | ||
| def config(self) -> dict[str, Any]: | ||
|
|
@@ -115,13 +120,68 @@ async def get_config(self) -> None: | |
|
|
||
| req.raise_for_status() | ||
|
|
||
| def _ensure_config_listener(self) -> None: | ||
| """Register the internal config-sync listener once, lazily. | ||
|
|
||
| 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. Marked `internal` so it doesn't | ||
| pin the stream open once every public listener is removed. | ||
| """ | ||
| if self._config_listener_registered: | ||
| return | ||
| self._config_listener_registered = True | ||
| self.stream.async_add_listener( | ||
| self._on_config_event, | ||
| {Key.VIN: self.vin, Key.CONFIG: None}, | ||
| internal=True, | ||
| ) | ||
|
|
||
| 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. | ||
|
|
||
| Merges into the pending desired config and joins the single in-flight | ||
| flush for this vehicle rather than starting a new one, so that a | ||
| batch of listeners scheduled at the same time produces one PATCH. | ||
| """ | ||
| self._ensure_config_listener() | ||
|
|
||
| async with self.lock: | ||
| self._config = merge(config, self._config) | ||
|
|
@@ -231,6 +291,7 @@ async def post_config(self, config: dict[str, Any]) -> dict[str, Any]: | |
|
|
||
| async def add_field(self, field: Signal | str, interval: int | None = None) -> None: | ||
| """Handle vehicle data from the stream.""" | ||
| self._ensure_config_listener() | ||
|
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 the stream has been closed after its last public listener was removed, external config changes cannot reach this lazily registered internal listener. On the next AGENTS.md reference: AGENTS.md:L19-L19 Useful? React with 👍 / 👎. |
||
| if isinstance(field, Signal): | ||
| field = field.value | ||
|
|
||
|
|
@@ -249,6 +310,7 @@ async def add_field(self, field: Signal | str, interval: int | None = None) -> N | |
|
|
||
| async def prefer_typed(self, prefer_typed: bool) -> None: | ||
| """Set prefer typed.""" | ||
| self._ensure_config_listener() | ||
| if self.preferTyped == prefer_typed: | ||
| return | ||
| await self.update_config({"prefer_typed": prefer_typed}) | ||
|
|
||
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.
When a typed vehicle listener has registered the permanent internal config listener, removing the last public listener leaves that internal entry here and closes the stream. With the default
manual=False, subsequently adding a public listener computesschedule_refresh = not self._listenersas false, so no new_listen_taskis created and the re-added callback receives no SSE events. Determine startup from the transition in non-internal listeners rather than whether the entire registry is empty.AGENTS.md reference: AGENTS.md:L19-L20
Useful? React with 👍 / 👎.