feat(sse): support topics allowlist and tariff_content_v2 topic - #16
Conversation
Adds Pythonic client support for api PR 318 (tariff_content_v2 split out of site_info) and PR 319 (topics allowlist query param), fully additive - existing listener contracts are unchanged. - TeslemetryStream(topics=...) sends the exact comma-separated SSE topics allowlist; omitting it preserves legacy-all behavior forever. SseTopic plus SSE_VEHICLE_TOPICS/SSE_ENERGY_TOPICS/SSE_ALL_TOPICS presets expand client-side to exact wire names. - listen_TariffContentV2 exposes the V2 tariff document verbatim, with a null body surfaced as an explicit removal signal (None). - listen_SiteInfo keeps working against the now-slim site_info shape. - listen_ComposedSiteInfo merges the latest site_info with the last known tariff piece into a whole-document view. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: 67afbbcb13
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| if self.vin: | ||
| url += f"/{self.vin}" | ||
| headers = await self.headers() | ||
| params = {"topics": ",".join(self.topics)} if self.topics else None |
There was a problem hiding this comment.
When a caller dynamically constructs topics and the iterable is empty, this truthiness check treats it the same as None and omits the query parameter. Because omission explicitly enables legacy-all behavior, an exact empty allowlist silently receives every applicable event instead of none, potentially increasing traffic and exposing data the caller intended to filter. Distinguish None from an empty list and either send the server-supported empty representation or reject the empty list explicitly.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
None would have no value, no topics is no data?
There was a problem hiding this comment.
Fixed - topics=[] now raises ValueError at construction time instead of falling through to legacy-all. None still means omit the param (legacy-all); an explicit empty iterable is now rejected client-side, mirroring the server's 400 on an empty topics value. Covered in tests/test_sse_topics.py (f698194).
Per PR review: a truthiness check on self.topics treated an empty iterable the same as None, silently falling back to legacy-all instead of subscribing to nothing. Distinguish the two at construction time and raise ValueError on empty, mirroring the server's own 400. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f69819440e
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| until a value has been received, and again after an explicit | ||
| removal. | ||
| """ | ||
| state: dict[str, Any] = {"site_info": None, "tariff_content_v2": None} |
There was a problem hiding this comment.
For sites whose REST site_info contains the legacy tariff_content, this state only tracks the slim SSE document and tariff_content_v2, so listen_ComposedSiteInfo can never produce the promised whole REST-shaped document and silently drops v1 tariff data. Either supply the legacy tariff from another source or describe this API as composing only site_info plus V2 rather than as restoring the whole document.
AGENTS.md reference: AGENTS.md:L14-L14
Useful? React with 👍 / 👎.
Per PR review: the docstring/README claimed the composed view produces "the whole REST-shaped document" / "whole Tesla-shaped document," but the stream never carries the legacy V1 tariff_content - it has no SSE topic and stays REST-only by design. Reword to describe the helper as composing only what the stream itself carries (slim site_info + V2 tariff), and point consumers needing V1 at the REST endpoint. No behavior change. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re the composed-site-info tariff scoping finding (energysite.py:89): fixed by honest scoping, not by adding a REST fetch. |
| {Key.SITE_ID: self.site_id, Key.TARIFF_CONTENT_V2: None}, | ||
| ) | ||
|
|
||
| def listen_ComposedSiteInfo( |
| ) | ||
|
|
||
| # Energy site topics that are live-only - never part of a connect-time snapshot. | ||
| SSE_ENERGY_LIVE_ONLY_TOPICS: tuple[SseTopic, ...] = (SseTopic.ENERGY_TOTALS,) |
There was a problem hiding this comment.
This is snapshot now, but why is this distinction even useful in the library? This is upstream behaviour that shouldn't hardcode in the library
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: f656202c5f
ℹ️ About Codex in GitHub
Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you
- Open a pull request for review
- Mark a draft as ready
- Comment "@codex review".
If Codex has suggestions, it will comment; otherwise it will react with 👍.
Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".
| self.vin = vin | ||
| self.topics: list[str] | None | ||
| if topics is not None: | ||
| self.topics = list(topics) |
There was a problem hiding this comment.
When a caller passes topics="state" or topics=SseTopic.STATE, both values satisfy the advertised Iterable[str] type, but list(topics) splits them into characters, so connect() sends topics=s,t,a,t,e and the server rejects the connection instead of selecting state. Normalize string-like inputs to a one-element list or reject them explicitly at construction.
AGENTS.md reference: AGENTS.md:L15-L15
Useful? React with 👍 / 👎.
There was a problem hiding this comment.
I agree, this should enforce the enum types in a list, or handle the self crafted string.
…ive-only in topic presets Per captain review: - Drop listen_ComposedSiteInfo entirely. It could never actually produce the whole REST-shaped site_info document (legacy V1 tariff_content has no SSE topic and stays REST-only by design), so the promise it made was structurally unkeepable - not just a docs wording problem. A consumer wanting both tariffs together should use the REST site_info endpoint. - Collapse SSE_VEHICLE_SNAPSHOT_TOPICS/SSE_VEHICLE_LIVE_ONLY_TOPICS and their energy counterparts into flat SSE_VEHICLE_TOPICS/ SSE_ENERGY_TOPICS/SSE_ALL_TOPICS presets. Whether a topic happens to have a connect-time snapshot is server-side behavior the library has no reason to hardcode - the presets only need to know which topics apply to which product kind to expand client-side into exact names. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re "Drop this method" (energysite.py:75, |
|
Re the snapshot/live-only distinction (const.py:375): agreed, and fixed. Collapsed SSE_VEHICLE_SNAPSHOT_TOPICS/SSE_VEHICLE_LIVE_ONLY_TOPICS and their energy counterparts into flat SSE_VEHICLE_TOPICS/SSE_ENERGY_TOPICS/SSE_ALL_TOPICS presets grouped only by product kind (vehicle/energy/account) - which a topic inherently belongs to - not by whether the server happens to give it a connect-time snapshot, which is upstream implementation detail the library has no reason to hardcode. Pushed in da1130e. |
Per PR review: topics="state" (or a bare SseTopic member) satisfies Iterable[str], so list(topics) split it into ['s','t','a','t','e'], sent as a garbled topics query param the server would reject. Accept str | Iterable[str] | None and wrap a lone string as a single-element list before validating emptiness. Co-Authored-By: Claude Sonnet 5 <noreply@anthropic.com>
|
Re "Treat a single string as one SSE topic" (stream.py:56): fixed. |
1 participant
Intent
Adds Pythonic client support for Teslemetry/api PR 318 (tariff split out of
site_info) and PR 319 (SSEtopicsallowlist), both additive - no existing listener contract changes.TeslemetryStream(topics=...)sends the connection's exact comma-separated SSE topics allowlist (?topics=...); omitting it preserves today's legacy-all behavior forever.SseTopicinconst.pymirrors the api's closedSSE_TOPICSset, andSSE_VEHICLE_TOPICS/SSE_ENERGY_TOPICS/SSE_ALL_TOPICSare convenience presets that expand client-side to exact names.site_infoevents no longer carrytariff_content/tariff_content_v2(PR 318) -listen_SiteInfokeeps working unmodified against the slimmer shape.TeslemetryStreamEnergySite.listen_TariffContentV2is a new typed listener for the site's V2 tariff, carrying the document verbatim on change; aNonebody is the server's explicit removal signal, surfaced distinctly rather than conflated with "no data yet".listen_ComposedSiteInfomerges the latestsite_infowith the last knowntariff_content_v2piece into a whole-document view, so consumers who want both halves together don't have to hand-assemble them.site_infoandtariff_content_v2share the same silence-means-no-change contract: event cadence never signals freshness, only REST does. Documented in the README alongside the new topics-selection example.Contract taken from Teslemetry/api PRs 318/319 at their merged HEAD (
src/lib/sseTopics.ts,src/routes/sse/tariffContentV2Schema.ts,src/routes/sse/siteInfoSchema.ts).tests/test_sse_topics.pycoverstopicsURL construction (present, omitted, plain strings vsSseTopic), the tariff listener including null-as-removal,site_infoignoring tariff events, andlisten_ComposedSiteInfo's merge/removal/withhold-until-first-site_info/listener-teardown behavior.