fix: HA Number/Select/Text optimistic-mode + slow-SoC slider UX#440
Open
fix: HA Number/Select/Text optimistic-mode + slow-SoC slider UX#440
Conversation
4 tasks
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.
8bfc57f to
bde1bd7
Compare
acd43a8 to
dd1e975
Compare
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
dd1e975 to
d0607aa
Compare
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
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)
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:
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:
`pytest tests` → 177 passed.
Closes #375