Conversation
) * 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.
* fix: forward retain flag through all typed publish methods The MQTT broker accepts retain on every PUBLISH frame, but the typed publisher API only honored it for `publish_json`. Calls to `publish_str/int/bool/float` silently retained regardless of caller intent — there was no way to opt out for non-JSON payloads. Add `retain: bool = True` (kwarg-only) to every typed publish method on the `Publisher` ABC and forward it through `MqttPublisher` to the gmqtt client. `ConsolePublisher` accepts and ignores it (no retention semantics for log output). This is API-additive: existing call sites continue to work unchanged since `retain` defaults to `True`. * fix: log retain flag in ConsolePublisher debug output Surface the retain value in the debug log instead of discarding it via `del retain`. The console publisher exists to mirror what would have been published over MQTT — retention is part of that picture. `internal_publish` (and the test mock override) gain a kwarg-only `retain: bool = True` that gets formatted into the debug line. * chore: have pre-commit invoke mypy without filenames The mypy hook was failing on tests-only commits because pre-commit passes the staged file paths and `mypy <file>` runs without the project's `files = ["./src", "./tests"]` config in effect, so imports from `src/` can't be resolved. Set `pass_filenames: false` to match how CI invokes mypy (`poetry run mypy` with no args). mypy's incremental cache keeps this fast on repeat runs. * test: round out non-regression coverage for retain flag Pin both the default-retained behavior and explicit retain=False forwarding for every typed publish method (str/int/bool/float/json) plus clear_topic's retained None publish. Existing tests covered str (default + forward) and int/bool/float (forward only); a future change to a default would have slipped through. Add the missing default-retained tests for int/bool/float, both directions for json, and clear_topic.
* feat: add Publisher.publish dispatching method
Adds a single non-abstract `publish(key, value, no_prefix=False)` method on
the `Publisher` ABC that dispatches via isinstance to the existing typed
`publish_{bool,int,float,str}` methods, plus a `PublishedValue` type alias
for the union. The `bool`-before-`int` ordering is load-bearing because
`isinstance(True, int)` is `True` in Python.
Conformance tests cover every concrete subclass (`MqttPublisher`,
`ConsolePublisher`, `MessageCapturingConsolePublisher`) plus an
ABC-level minimal subclass, and explicitly lock the bool-vs-int ordering.
* refactor: unify Publishable union and collapse duplicate dispatch sites
Renames `PublishedValue` -> `Publishable` and widens it from
`bool | int | float | str` to also include `dict[str, Any] | datetime`,
matching the full set of value shapes the gateway publishes. Extends
`Publisher.publish` to dispatch the wider union: dicts forward to
`publish_json` (with `retain` plumbed through), datetimes are stringified
via `datetime_to_str` and routed through `publish_str`. An unsupported
runtime type now raises `TypeError` instead of silently no-op-ing.
This subsumes the two duplicate `Publishable` constrained-TypeVar
declarations (in `status_publisher/__init__.py` and `vehicle.py`) and
their two near-identical `_publish_directly` chains, which now collapse
to a single `self.publisher.publish(...)` call. Methods that used to
parametrize over the constrained TypeVar (`_publish`,
`_transform_and_publish`, `__publish`) switch to PEP 695 bounded
generics: `[V: Publishable]`, `[T, V: Publishable]`. This is a small
semantic loosening (subclasses of e.g. `dict` are now valid `V`) but
runtime dispatch is `isinstance`-based and handles subclasses correctly.
Tests grow new conformance cases for the dict (with `retain` forwarding)
and datetime arms plus a regression test for the `TypeError` arm; the
one test patching the deleted `_publish_directly` now patches the
underlying publisher's `publish` instead.
* feat: add publish_datetime and forward retain through dispatcher
Complete the typed publish API by giving `datetime` its own narrow
entry point — `publish_datetime` — that stringifies via
`datetime_to_str` and forwards to `publish_str` (and now `retain` too).
Removes the inline transformation in the dispatch chain.
`Publisher.publish()` now forwards `retain` to every arm of the typed
API, not just `publish_json`. Previously it was silently dropped for
str/int/bool/float; with #443 those typed methods now accept `retain`,
so the dispatcher can finally honor the kwarg uniformly.
Also folds the two remaining `datetime_to_str(...)` call sites in
`vehicle.py` (notify_car_activity, last_failed_refresh setter) through
the typed/Publishable APIs, dropping the now-unused import.
* refactor: type ConsolePublisher.internal_publish with Publishable
`Any` was overly permissive — at runtime `internal_publish` only ever
sees what the typed publish methods route to it (str/int/bool/float)
plus None from `clear_topic`. Reuse the `Publishable | None` alias to
make the contract explicit.
The `MessageCapturingConsolePublisher.map` test inspection store stays
typed `Any` so consumers can `json.loads(...)` serialized payloads
without per-call narrowing.
* refactor: type MqttPublisher.__publish payload with Publishable
Same narrowing as ConsolePublisher.internal_publish: the private
`__publish` only receives Publishable | None at runtime (str/int/bool/
float from the typed methods, str from publish_json after JSON
serialization, None from clear_topic). Replace `Any` with the explicit
alias.
* refactor: introduce WirePayload alias for transport-level publish helpers
Replace `Publishable | None` on `MqttPublisher.__publish` and
`ConsolePublisher.internal_publish` with `WirePayload | None`, where
`WirePayload = bool | int | float | str` — the precise set of values
that crosses the publisher/transport boundary after the typed
publish_* methods do their stringification.
This catches accidental misuse if a future caller tried to hand a raw
`dict` or `datetime` to a wire-level helper, and matches what gmqtt
can actually serialize without surprises.
…444) The SAIC API has been observed to return wrong DST offsets (e.g. GMT+11 for an Australian account that is currently on AEST/+10), which caused scheduled charging cron jobs to fire an hour off. Forcing an IANA zone like Australia/Sydney lets the scheduler honour DST transitions natively. Refs #438
* fix: republish Total Battery Capacity after HA number update The HA "Total Battery Capacity" number and sensor share the same MQTT state topic. The `_set` handler used to mutate the in-memory override on `VehicleInfo.custom_battery_capacity` and return `RESULT_DO_NOTHING`, relying on the next charge-status poll to refresh the shared sensor topic. As a result the HA number widget displayed the user's retained `/set` value while the sensor (and any UI binding to it) remained stuck on the previous value — typically the per-model default published from `rvs_charge_status.get_actual_battery_capacity`. Republish `vehicle.real_battery_capacity` to the state topic from the handler itself so the sensor reflects the change immediately, without forcing an extra vehicle poll. Reading through `real_battery_capacity` keeps the `payload == 0` "clear override" path consistent: it falls back to the per-model default instead of publishing `0`. The kWh-derived sensors (`SoC_kWh`, `Last Charge SoC kWh`, the two `Power Usage` sensors) still use the previous correction factor until the next charge-status update, same staleness window as before. * test: cover the Total Battery Capacity republish path Stub `vehicle.real_battery_capacity` on the mock so the existing retained- replay test exercises the new state-topic publish, and add coverage for the two interesting branches: * payload `0` clears the override and republishes the per-model default * unknown model (`real_battery_capacity is None`) skips the publish
* feat: add SOC kWh fallback for vehicles without realtimePower For car models (e.g. MGS5) where RvsChargeStatus.realtimePower is always None or 0, DRIVETRAIN_SOC_KWH was never published. Add extract_soc_kwh(charge_status, soc) to the extractors module, following the same pattern as extract_soc / extract_electric_range. It prefers the realtimePower-derived value when valid (> 0), and falls back to soc% * real_total_battery_capacity otherwise. Publishing moves from RvsChargeStatusPublisher to update_data_conflicting_in_vehicle_and_bms in VehicleState, where both the charge result and the resolved SoC% are in scope. * chore: add pre-commit dev dependency and apply linting fixes
Known variant: AH4EM L (Standard Range, 43kWh LFP). The Long Range 54kWh also uses LFP so supports_target_soc cannot differentiate it; returns None until a real device report confirms its series code.
AH4EM S (and any other non-L Urban variant) defaults to 54kWh LFP. Closes #452.
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
1 participant
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
AH4EMseries recognition toreal_battery_capacityinVehicleInfoAH4EM L(Standard Range, LFP) → 43 kWhAH4EMvariants (e.g.AH4EM S) → 54 kWh LFPtests/test_vehicle_info.pycovering MG4 and MG4 Urban battery capacity casesCloses #455
Closes #452
Test plan
poetry run pytest tests/test_vehicle_info.py— all 6 tests passpoetry run pytest— 293 tests passAH4EM L) no longer logs the "Unknown battery capacity" warningAH4EM S) no longer logs the "Unknown battery capacity" warning