Skip to content

fix: gate config no-op skip on a populated flag, not connection state - #32

Merged
Bre77 merged 1 commit into
mainfrom
fm/pts-config-simplify
Aug 12, 2026
Merged

fix: gate config no-op skip on a populated flag, not connection state#32
Bre77 merged 1 commit into
mainfrom
fm/pts-config-simplify

Conversation

@Bre77

@Bre77 Bre77 commented Aug 12, 2026

Copy link
Copy Markdown
Member

Intent

  • add_field/prefer_typed's no-op skip trusted fields/preferTyped based on connection/topic state (_record_is_live()), not on whether the record had ever actually been confirmed against the server - leaving two races: a vehicle discovered mid-dispatch of its own config event never sees that event and sits unseeded, and a field-config call landing between connect() and that connection's config snapshot can match against a stale pre-disconnect record.
  • Fix: gate the no-op skip on whether the record has ever been populated, not on connection/topic state.
    • TeslemetryStreamVehicle tracks _populated, set by a successful get_config() (200 or 404 - both are an authoritative answer) and by every _on_config_event push. add_field/prefer_typed await _ensure_populated() first - a no-op if already populated, otherwise a single-flight get_config() fetch that concurrent callers (e.g. a batch of listen_* calls at HA integration setup) join instead of each starting their own GET.
    • A new _on_connection_event listener (registered via async_add_connection_listener at construction, alongside the existing config-sync listener) unpopulates the record on disconnect, so a call landing in the reconnect window re-fetches instead of trusting stale data.
    • _record_is_live() and its connection/topic coupling are gone entirely - optimistic updates from push events, plus the lazy fetch for the unpopulated gap, are the whole mechanism now.
  • tests/test_stream_lifecycle.py adapts fix: seed a vehicle discovered mid-dispatch from its triggering config event #30's scenario: the mid-dispatch-discovered vehicle stays unpopulated (no seeding trick needed) and self-corrects via the lazy fetch on its first field-config call. tests/test_reconnect_config_window.py adapts fix: close reconnect-window gap in add_field/prefer_typed no-op skip #31's scenario against the new design. tests/test_config_listener_lifecycle.py covers the populated/unpopulated gating directly.

… fetch

add_field/prefer_typed no longer gate their no-op skip on connection/topic
state (_record_is_live). Instead, TeslemetryStreamVehicle tracks whether
its record has ever been populated (by a push event or a REST fetch); an
unpopulated vehicle awaits a single-flight get_config() before deciding,
a populated one trusts the optimistically-updated record outright. A
disconnect notification unpopulates the record so a call landing in the
reconnect window (before the next connection's config snapshot arrives)
refetches instead of trusting stale pre-disconnect data.

This replaces the mechanisms proposed in #30 (a second dispatch pass for
listeners added mid-event) and #31 (per-connection snapshot tracking) with
a smaller, self-contained fix scoped to the vehicle itself.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: a5fc9a777f

ℹ️ 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 +160 to +161
if self._populated:
return

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P1 Badge Revalidate config when its SSE topic is excluded

When TeslemetryStream is constructed with an explicit topic allowlist that omits config (for example, only vehicle_data), the first REST fetch sets _populated and this early return remains effective for the entire connection. Because stream.py sends that allowlist unchanged, the internal listener cannot observe subsequent config changes from another client, so add_field or prefer_typed can silently skip a required PATCH using stale state. Preserve topic-awareness in this gate or re-fetch when config events are not subscribed.

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

Useful? React with 👍 / 👎.

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

Copy link
Copy Markdown

Choose a reason for hiding this comment

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

P2 Badge Clear cached config after an authoritative 404

If this vehicle has cached fields or preferTyped, is disconnected, and the lazy config GET then returns 404 because its server-side config was removed, this marks the old record populated without clearing it. The following no-op check can therefore skip recreating a missing field or restoring prefer_typed based on stale pre-disconnect values; reset both cached pieces to the authoritative empty/default state before setting _populated.

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

Useful? React with 👍 / 👎.

@Bre77
Bre77 merged commit 1651f2d into main Aug 12, 2026
14 checks passed
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

fm Opened by a Firstmate crewmate

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant