Skip to content

refactor: remove prefer_typed - the server always streams typed values now - #33

Merged
Bre77 merged 5 commits into
mainfrom
fm/pts-prefertyped-always
Aug 12, 2026
Merged

refactor: remove prefer_typed - the server always streams typed values now#33
Bre77 merged 5 commits into
mainfrom
fm/pts-prefertyped-always

Conversation

@Bre77

@Bre77Bre77 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Intent

  • The API maintainer confirms prefer_typed is enabled by default server-side now, so the per-vehicle opt-in itself has no reason to exist - but it's a default, not a guarantee: some vehicles haven't picked it up and still stream string-encoded numeric/boolean values, so the wire-format coercion in make_int/make_float/make_bool stays (see below).
  • Removed the preference surface from TeslemetryStreamVehicle:
    • Public prefer_typed(bool) method and preferTyped attribute are gone.
    • config property no longer carries a prefer_typed key; get_config()/_on_config_event()/_flush() no longer read or write it from REST/SSE config bodies - fields is the only tracked piece now.
    • make_int/make_float/make_boolkeep their isinstance(data, str) coercion - per maintainer review, some vehicles still stream string-encoded values even with prefer_typed on by default, so tests/test_field_type_coercion.py (regression-tests this against real production telemetry) stays too.
  • Version bump: minor (0.10.1 → 0.11.0). This removes public API (prefer_typed(), preferTyped, the prefer_typed key in config) - user-facing breaking - but this repo's 0.x line uses the minor slot for changes of that scale (e.g. chore: bump version to 0.10.0 #18) rather than reserving a major bump.
  • Consumer impact (Home Assistant teslemetry integration, HACS beta channel) - anything referencing these needs a mechanical update on the next bump:
    • TeslemetryStreamVehicle.prefer_typed(...) - method removed.
    • TeslemetryStreamVehicle.preferTyped - attribute removed.
    • TeslemetryStreamVehicle.config["prefer_typed"] - key removed from the config property's return dict.
    • listen_* callbacks are unaffected otherwise - numeric/boolean coercion behavior is unchanged.
  • Two more fixes landed on this branch from review, both reproduced with a failing test before the fix:
    • get_config()'s 404 branch marked the vehicle populated without clearing fields, so a config deleted server-side while offline could leave add_field() no-op'ing forever against a stale record - now the 404 branch clears fields too.
    • A failed populating fetch (transport error, or a non-200/404 status) propagated out of _ensure_populated(), which every listen_* reaches through _enable_field()'s fire-and-forget task - the exception just became an unretrieved-task-exception log line and the field was never configured. Failed fetches aren't authoritative like a 404: now caught, logged, and the vehicle stays unpopulated so add_field() proceeds to the PATCH instead of stranding the request.
  • fix: gate config no-op skip on a populated flag, not connection state #32 (the _populated/lazy-fetch fix this branch coordinates with) was already merged to main before this branch was created, so this sits directly on top of it with no rebase needed - it keeps fix: gate config no-op skip on a populated flag, not connection state #32's add_field machinery intact and only removes the prefer_typed portions layered on top.
  • tests/test_config_events.py, tests/test_config_listener_lifecycle.py, tests/test_config_update.py, tests/test_reconnect_config_window.py, tests/test_stream_lifecycle.py adapted to drop prefer_typed fixtures/assertions while keeping their fields/typed-behavior coverage; tests/test_field_type_coercion.py unchanged.

…s now
The Teslemetry API maintainer confirms prefer_typed is always true
server-side, so the per-vehicle opt-in and its untyped (string-encoded)
wire-format handling no longer have a reason to exist.
- Drop TeslemetryStreamVehicle.prefer_typed()/preferTyped, and the
prefer_typed key from get_config/_on_config_event/_flush/config.
- Drop the isinstance(data, str) coercion in make_int/make_float/
make_bool - listen_* callbacks now receive whatever the server sends
straight through.
- Delete tests/test_field_type_coercion.py (its subject, string-to-
native coercion, no longer exists); adapt the remaining config/field
tests to drop prefer_typed fixtures and assertions.
@Bre77Bre77 added the fm Opened by a Firstmate crewmate label Aug 12, 2026
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

ifreq.status==404:
# No config exists for this vehicle yet - an authoritative
# answer (empty), not a missing one.
self._populated=True
return

P2 Badge Clear cached fields after an authoritative 404

When a vehicle retains fields from before a disconnect and its configuration is deleted elsewhere, the reconnect-window REST fetch returns 404 but leaves those stale fields intact. Because this branch marks the vehicle populated, add_field() can then incorrectly treat a stale field as already configured and skip the PATCH permanently. Clear self.fields when handling the authoritative 404 response.

AGENTS.md reference: AGENTS.md:L21-L21


self.stream.async_add_connection_listener(self._on_connection_event)

P2 Badge Avoid mutating live connection-listener iteration

When an existing connection-state callback calls get_vehicle() for an uncached VIN, this constructor registers another connection listener while stream.py's _update_connection_listeners() is iterating the live dictionary at line 227. Python then raises RuntimeError: dictionary changed size during iteration, interrupting connect() or disconnect notification delivery; iterate over a snapshot there or defer this registration.

AGENTS.md reference: AGENTS.md:L20-L20

ℹ️ 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".

Comment on lines -2975 to -2979
data = event["data"][signal]
if isinstance(data, str):
# Handle invalid and None?
data = int(data)
callback(data)

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

These protections should stay, while prefer_typed is enabled by default, there are some vehicles that still don't use it so we should still catch those.

Copy link
Copy Markdown
MemberAuthor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Confirmed - restored the string coercion in make_int/make_float/make_bool and tests/test_field_type_coercion.py in 767c019. prefer_typed removal (the method/attribute/config-sync surface) stays; only the wire-format fallback was reinstated.

get_config()'s 404 branch marked the vehicle populated without clearing
self.fields. A vehicle carrying fields from before a disconnect whose
config was then deleted server-side would keep those stale fields
forever, causing add_field() to permanently skip the PATCH for a field
the server no longer has configured.
Reported by Codex review on #33.
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

version = "0.10.1"

P1 Badge Bump the distribution version for this breaking release

The changed lock entry still declares version 0.10.1, matching the unchanged pyproject.toml, even though this commit removes the public prefer_typed() API and describes a 0.11.0 release. Tagging this tree as v0.11.0 would therefore build a 0.10.1 artifact; publishing will either be rejected as a duplicate or expose the breaking code under the wrong version. Update the project version and regenerate the lockfile before release.

AGENTS.md reference: AGENTS.md:L17-L17



P1 Badge Continue enabling a field when the initial config GET fails

When an unpopulated vehicle's config GET raises a transport/timeout error or returns a non-200/404 status, this await aborts the fire-and-forget task created by _enable_field() before update_config() is reached. The SSE connection may still succeed, but the requested field is never configured and its listener remains silent until another explicit registration retries it; handle the fetch failure by conservatively proceeding with the PATCH or by retrying instead of abandoning the enable operation.

AGENTS.md reference: AGENTS.md:L21-L21

ℹ️ 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".

Matches the minor bump this PR's body already describes for the
breaking removal of prefer_typed()/preferTyped - pyproject.toml and
uv.lock still said 0.10.1.
Reported by Codex review on #33.
_ensure_populated() let a failed populating GET (non-200/404 status,
or a transport/timeout error) propagate out of add_field(). Called
directly that's a normal exception a caller can handle, but every
listen_* method reaches add_field() through _enable_field()'s
fire-and-forget asyncio.create_task() - nobody awaits it, so the
exception just became an unretrieved-task-exception log line and the
requested field was never configured, silently.
A failed fetch isn't authoritative like a 404: catch it, log a
warning, and stay unpopulated so add_field() proceeds against whatever
fields it currently knows and sends the PATCH, rather than abandoning
the request.
Reported by Codex review on #33.
…_bool
prefer_typed is only a default now, not a guarantee: some vehicles
haven't picked it up and still stream string-encoded numeric/boolean
values. Dropping the isinstance(data, str) coercion in
make_int/make_float/make_bool would silently hand those vehicles'
consumers raw strings instead of int/float/bool.
Restores tests/test_field_type_coercion.py, which regression-tests
this coercion against real observed telemetry. The prefer_typed
preference surface itself (prefer_typed()/preferTyped, the config-sync
handling) stays removed - only the wire-format coercion is back.
Per maintainer review on #33.
@chatgpt-codex-connector

Copy link
Copy Markdown

💡 Codex Review

ifself._populated:
return

P2 Badge Refresh configs when the config topic is filtered out

When a stream uses an exact topic allowlist that omits config (for example, topics=[SseTopic.DATA]), an initial REST fetch sets _populated, but subsequent server-side config changes can never reach _on_config_event while the connection remains open. This early return then trusts the stale record indefinitely, so removing a field elsewhere and re-adding its listener locally can incorrectly skip the PATCH and leave telemetry disabled; either ensure the internal config topic is subscribed or refetch before no-op decisions when it is excluded.

AGENTS.md reference: AGENTS.md:L18-L21


self.stream.async_add_connection_listener(self._on_connection_event)

P2 Badge Snapshot connection listeners before callbacks create vehicles

When a connection-state callback calls get_vehicle() for an uncached VIN, construction now executes this registration while _update_connection_listeners() is iterating the live _connection_listeners.values() view. Python then raises RuntimeError: dictionary changed size during iteration; on a disconnect notification this escapes _close_response() and can terminate the owned listen task instead of reconnecting. Dispatch connection callbacks over a snapshot, as the event-listener dispatch already does.

AGENTS.md reference: AGENTS.md:L23-L23

ℹ️ 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".

@Bre77
Bre77 merged commit f52ff55 into mainAug 12, 2026
14 checks passed
Bre77 added a commit that referenced this pull request Aug 12, 2026
#34)
A connection listener that calls get_vehicle() for an uncached VIN (e.g.
an integration discovering a vehicle on reconnect) registers that
vehicle's own connection listener from inside TeslemetryStreamVehicle
__init__, mutating _connection_listeners while
_update_connection_listeners() is iterating it. That raised
RuntimeError: dictionary changed size during iteration, interrupting
connect()/disconnect notification delivery.
listen() already dispatches over a snapshot of _listeners for the same
reason; _update_connection_listeners() now does the same for
_connection_listeners.
Reported by Codex review on #33.
Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fmOpened by a Firstmate crewmate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant

@Bre77