Skip to content

fix: HA Number/Select/Text optimistic-mode + slow-SoC slider UX#440

Open
nanomad wants to merge 1 commit intodevelopfrom
fix/ha-number-optimistic-state
Open

fix: HA Number/Select/Text optimistic-mode + slow-SoC slider UX#440
nanomad wants to merge 1 commit intodevelopfrom
fix/ha-number-optimistic-state

Conversation

@nanomad
Copy link
Copy Markdown
Contributor

@nanomad nanomad commented May 9, 2026

Summary

HA's MQTT Number entity defaulted to optimistic mode in our discovery payload, so on restart HA treats its locally cached value as authoritative and republishes it to the command topic — the gateway then forwards it to the vehicle. That's how target SoC ends up reset to the user's last cached HA value (#375), and the same mechanism explains the analogous battery-capacity report. `_publish_select` and `_publish_text` had the same gap.

Changes

Discovery (structural fix)

  • Add `"optimistic": false` to `_publish_number`, `_publish_select`, `_publish_text` payloads so HA reads state from `state_topic` instead of treating its cache as authoritative.
  • Switches and locks already had this set; binary_sensor / sensor / event are read-only so don't apply.

Dispatcher (UX + correctness)

With `optimistic: false`, HA waits for `state_topic` to confirm before updating the slider. SAIC commands can take minutes when the car is asleep, so a naive `optimistic: false` would feel broken. The dispatcher now does:

  1. Eager echo on receipt — publish the requested value to the state topic before calling the SAIC API. The slider updates instantly.
  2. Rollback on failure — if the SAIC call (or any handler exception) fails, republish the previously captured value so the UI snaps back to the prior setting and the user sees the rejection.
  3. Skip echo when payload is invalid — `echo_payload` returns None for unparseable input, suppressing both the eager echo and the rollback.

Handlers opt in via three optional methods on `CommandHandlerBase` (`echo_state_topic`, `capture_current_state`, `echo_payload`). The 6 number-entity handlers (target SoC, total battery capacity, 4 refresh periods) implement them; everything else is unchanged.

Test plan

New tests:

  • `tests/test_ha_discovery_optimistic.py` — discovery payload regression: number / select / text all carry `optimistic: false` (and switches still do, as a guard).
  • `tests/handlers/test_vehicle_command.py::TestNumberEntityEagerEcho`:
    • eager echo publishes the new value to the state topic
    • SAIC failure rolls back to the prior value
    • no rollback when no prior state was captured (target_soc is None)
    • invalid payload skips echo (and therefore skips rollback)
    • switches don't trigger echo

`pytest tests` → 177 passed.

Closes #375

nanomad added a commit that referenced this pull request May 9, 2026
)

* fix: persist HA-driven gateway settings via retained /set commands

Adds retain=true to HA discovery for the four refresh_period_* numbers,
refresh_mode, and totalBatteryCapacity, so the broker keeps the user's
last `/set` value across gateway restarts. Plumbs the gmqtt retain flag
through the command-dispatch path and drops a retained one-shot refresh
mode (force, charging_detection) so a single-shot poll does not loop on
every restart. Also suppresses clear_command on retained replays so the
broker does not erase the user's intent.

Unblocks PR #440 (fix/ha-number-optimistic-state).

* fix: log dropped retained replays at WARN, not INFO

A retained /set arriving for an unknown VIN means we lost the user's
intent — surface it at WARN so it shows up in default log views.

* fix: preserve retained OFF refresh mode across restarts

Drops RefreshMode.OFF from INVALID_STARTUP_REFRESH_MODES and changes the
constructor default for refresh_mode from OFF to PERIODIC. Before, OFF
was in the INVALID set because OFF was the constructor default and a
gateway booting in OFF would never poll. Now that retained `/set off` is
replayed on reconnect (via the parent commit), OFF is a legitimate
persistent user choice and must be preserved by configure_missing.

FORCE / CHARGING_DETECTION remain in INVALID_STARTUP_REFRESH_MODES as a
belt-and-braces guard alongside the primary drop in RefreshModeCommand.

Also addresses two review nits:
- refresh_mode.handle empty-payload guard now mirrors the parent's
  `not self.supports_empty_payload` clause for contract parity
- drops a redundant `if _properties` defensive check; gmqtt always
  populates the properties dict with the retain flag

* fix: drop retained replays for non-replayable commands at dispatcher

Adds an opt-in CommandHandlerBase.is_replayable_when_retained() classmethod
(default False) and gates the dispatcher: any retained `/set` for a handler
that hasn't opted in is dropped with a WARN log before reaching the handler.

Defense-in-depth against non-HA producers (node-RED, custom scripts,
mosquitto_pub) that may mistakenly publish action-bearing commands with
retain=true. Without this guard such a stale retained command (e.g. a
retained `charging/set true`) would re-fire the SAIC API call on every
gateway restart.

Opted in: RefreshMode, the four RefreshPeriod_*, and TotalBatteryCapacity
— exactly the six entities whose HA discovery payload also declares
retain=true. Single source of truth lives next to the handler logic.
@nanomad nanomad force-pushed the fix/ha-number-optimistic-state branch from 8bfc57f to bde1bd7 Compare May 9, 2026 16:24
@nanomad nanomad changed the title fix: prevent stale HA values from overwriting writable entities fix: HA Number/Select/Text optimistic-mode + slow-SoC slider UX May 9, 2026
@nanomad nanomad force-pushed the fix/ha-number-optimistic-state branch 6 times, most recently from acd43a8 to dd1e975 Compare May 9, 2026 16:50
HA MQTT Number / Select / Text discovery defaults to optimistic mode,
so HA treats its locally cached value as authoritative even when the
gateway publishes a different state on state_topic — the trigger for
target SoC resetting on restart (#375). PR #441 narrowed the symptom
for the six retained entities by replaying the user's last /set on
reconnect; this PR closes the underlying optimistic-mode gap.

Discovery: add `optimistic: false` to _publish_number, _publish_select,
and _publish_text. Switches already had it.

Dispatcher: with optimistic: false the slider waits for state_topic
to confirm before updating, which on a sleeping car can take minutes
for SAIC-API-backed values. Add three optional hooks on
CommandHandlerBase (echo_state_topic / capture_current_state /
echo_payload) and an eager-echo + rollback path in the dispatcher:
echo the requested value to state_topic on receipt, roll back on
SAIC failure. Skipped on retained replays — no user is watching the
slider on startup.

Only DrivetrainSoCTargetCommand opts in: it's the one writable entity
that is API-backed (so #441's retained-replay path doesn't cover it)
and slow enough that the slider would otherwise freeze.

Closes #375
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant