From d93b39fc35562530493e4ad1ec60c5c1c9629540 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 16:31:33 +0200 Subject: [PATCH 01/12] chore: add pre-commit config (#439) --- .pre-commit-config.yaml | 43 +++++++++++++++++++++++++++++++++++++++++ 1 file changed, 43 insertions(+) create mode 100644 .pre-commit-config.yaml diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml new file mode 100644 index 0000000..6ae070e --- /dev/null +++ b/.pre-commit-config.yaml @@ -0,0 +1,43 @@ +ci: + autofix_prs: false + skip: + # These steps run in the CI workflow already. Keep in sync. + - mypy + +default_language_version: + python: python3.13 + +repos: + - repo: https://github.com/python-poetry/poetry + rev: '2.1.3' + hooks: + - id: poetry-check + - id: poetry-lock + - repo: https://github.com/astral-sh/ruff-pre-commit + rev: v0.12.0 + hooks: + - id: ruff + args: + - --fix + - --unsafe-fixes + - id: ruff-format + - repo: local + hooks: + - id: mypy + name: Check with mypy + entry: poetry run mypy + language: system + types: + - python + require_serial: true + - id: pytest + name: Run pytest + entry: poetry run pytest tests + language: system + pass_filenames: false + stages: [pre-push] + - repo: https://github.com/pre-commit/pre-commit-hooks + rev: v5.0.0 + hooks: + - id: check-yaml + - id: check-added-large-files From a4239e30f7525519ed9dadd1479550ed5c37168c Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sat, 9 May 2026 18:11:25 +0200 Subject: [PATCH 02/12] fix: persist HA-driven gateway settings via retained /set commands (#441) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- CHANGELOG.md | 16 +++ src/handlers/command/base.py | 38 +++++- .../drivetrain_total_battery_capacity.py | 5 + src/handlers/command/gateway/refresh_mode.py | 33 ++++- .../command/gateway/refresh_period.py | 20 +++ src/handlers/vehicle.py | 16 ++- src/handlers/vehicle_command.py | 31 ++++- src/integrations/home_assistant/base.py | 4 +- src/integrations/home_assistant/discovery.py | 6 + src/mqtt_gateway.py | 15 ++- src/publisher/core.py | 6 +- src/publisher/mqtt_publisher.py | 11 +- src/vehicle.py | 62 +++++---- tests/handlers/test_vehicle_command.py | 125 +++++++++++++++++- tests/integrations/home_assistant/__init__.py | 0 .../home_assistant/test_discovery_retain.py | 122 +++++++++++++++++ tests/test_mqtt_publisher.py | 4 +- tests/test_vehicle_state.py | 47 +++++-- 18 files changed, 493 insertions(+), 68 deletions(-) create mode 100644 tests/integrations/home_assistant/__init__.py create mode 100644 tests/integrations/home_assistant/test_discovery_retain.py diff --git a/CHANGELOG.md b/CHANGELOG.md index db1eb6e..dcddda1 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -1,5 +1,21 @@ # Change Log +## Unreleased + +### Fixed + +* Persist user-set HA gateway entities across gateway restarts by retaining + their `/set` commands on the MQTT broker (refresh mode, all four refresh + periods, and total battery capacity). On reconnect the existing command- + dispatch path replays the retained value before `configure_missing()` would + apply config defaults. A retained one-shot refresh mode (`force`, + `charging_detection`) is dropped on replay so a single-shot poll does not + fire on every restart. + + Note: on first upgrade only entities you change *after* the upgrade become + persistent. Existing retained STATE values on the broker are not converted + into retained `/set` commands. + ## 0.11.0 ### Added diff --git a/src/handlers/command/base.py b/src/handlers/command/base.py index 8b2bae4..888be4f 100644 --- a/src/handlers/command/base.py +++ b/src/handlers/command/base.py @@ -44,13 +44,28 @@ def __init__(self, saic_api: SaicApi, vehicle_state: VehicleState) -> None: def name(cls) -> str: return cls.__name__ + @classmethod + def is_replayable_when_retained(cls) -> bool: + """Whether the dispatcher may invoke this handler with retained=True. + + Default False: action-bearing commands (charging start/stop, locks, + climate, FORCE refresh, etc.) would re-fire on every gateway restart + if their `/set` topic was retained on the broker, so the dispatcher + drops the replay before the handler runs. Override to True only on + idempotent value-bearing handlers whose HA discovery payload also + declares `retain: true`. + """ + return False + @classmethod @abstractmethod def topic(cls) -> str: raise NotImplementedError @abstractmethod - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, payload: str, *, retained: bool = False + ) -> CommandProcessingResult: raise NotImplementedError @property @@ -84,7 +99,12 @@ def supports_empty_payload(self) -> bool: return False @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: normalized_payload = payload.strip().lower() if len(normalized_payload) == 0 and not self.supports_empty_payload: @@ -113,7 +133,12 @@ async def _get_action_result(self, _action_result: T) -> CommandProcessingResult pass @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: normalized_payload = payload.strip().lower() if len(normalized_payload) == 0: @@ -145,7 +170,12 @@ def supports_empty_payload(self) -> bool: return False @override - async def handle(self, payload: str) -> CommandProcessingResult: + async def handle( + self, + payload: str, + *, + retained: bool = False, + ) -> CommandProcessingResult: if len(payload.strip()) == 0 and not self.supports_empty_payload: return RESULT_DO_NOTHING diff --git a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py index e4ac585..849f16a 100644 --- a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py +++ b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py @@ -14,6 +14,11 @@ class DrivetrainTotalBatteryCapacitySetCommand(FloatCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: diff --git a/src/handlers/command/gateway/refresh_mode.py b/src/handlers/command/gateway/refresh_mode.py index 870cdfc..57851ea 100644 --- a/src/handlers/command/gateway/refresh_mode.py +++ b/src/handlers/command/gateway/refresh_mode.py @@ -1,17 +1,27 @@ from __future__ import annotations +import logging from typing import override +from exceptions import MqttGatewayException from handlers.command.base import ( RESULT_DO_NOTHING, CommandProcessingResult, PayloadConvertingCommandHandler, ) import mqtt_topics -from vehicle import RefreshMode +from vehicle import ONE_SHOT_REFRESH_MODES, RefreshMode + +LOG = logging.getLogger(__name__) class RefreshModeCommand(PayloadConvertingCommandHandler[RefreshMode]): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + # OFF / PERIODIC are persistent user choices; one-shots dropped in handle(). + return True + @classmethod @override def topic(cls) -> str: @@ -23,6 +33,27 @@ def convert_payload(payload: str) -> RefreshMode: normalized_payload = payload.strip().lower() return RefreshMode.get(normalized_payload) + @override + async def handle( + self, payload: str, *, retained: bool = False + ) -> CommandProcessingResult: + if len(payload.strip()) == 0 and not self.supports_empty_payload: + return RESULT_DO_NOTHING + try: + refresh_mode = self.convert_payload(payload) + except Exception as e: + msg = f"Error converting payload {payload} for command {self.name()}" + raise MqttGatewayException(msg) from e + if retained and refresh_mode in ONE_SHOT_REFRESH_MODES: + # Retained one-shot modes would re-fire on every gateway restart. + LOG.info( + "Dropping retained one-shot refresh mode %s for VIN %s", + refresh_mode.value, + self.vin, + ) + return RESULT_DO_NOTHING + return await self.handle_typed_payload(refresh_mode) + @override async def handle_typed_payload( self, refresh_mode: RefreshMode diff --git a/src/handlers/command/gateway/refresh_period.py b/src/handlers/command/gateway/refresh_period.py index 5723b46..91cae45 100644 --- a/src/handlers/command/gateway/refresh_period.py +++ b/src/handlers/command/gateway/refresh_period.py @@ -11,6 +11,11 @@ class RefreshPeriodActiveCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -23,6 +28,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodInactiveCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -35,6 +45,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodInactiveGraceCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: @@ -47,6 +62,11 @@ async def handle_typed_payload(self, payload: int) -> CommandProcessingResult: class RefreshPeriodAfterShutdownCommand(IntCommandHandler): + @classmethod + @override + def is_replayable_when_retained(cls) -> bool: + return True + @classmethod @override def topic(cls) -> str: diff --git a/src/handlers/vehicle.py b/src/handlers/vehicle.py index 2607f7a..7f18908 100644 --- a/src/handlers/vehicle.py +++ b/src/handlers/vehicle.py @@ -241,10 +241,9 @@ def __should_poll(self) -> bool: ) def __should_complete_configuration(self, start_time: datetime.datetime) -> bool: - return ( - not self.vehicle_state.is_complete() - and datetime.datetime.now(tz=datetime.UTC) > start_time + datetime.timedelta(seconds=10) - ) + return not self.vehicle_state.is_complete() and datetime.datetime.now( + tz=datetime.UTC + ) > start_time + datetime.timedelta(seconds=10) def __refresh_openwb( self, @@ -334,8 +333,12 @@ async def update_scheduled_battery_heating_status( ) return scheduled_battery_heating_status - async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: - await self.__command_handler.handle_mqtt_command(topic=topic, payload=payload) + async def handle_mqtt_command( + self, *, topic: str, payload: str, retained: bool = False + ) -> None: + await self.__command_handler.handle_mqtt_command( + topic=topic, payload=payload, retained=retained + ) def __setup_ha_discovery( self, vehicle_state: VehicleState, vin_info: VehicleInfo, config: Configuration @@ -344,7 +347,6 @@ def __setup_ha_discovery( return HomeAssistantDiscovery(vehicle_state, vin_info, config) return None - def handle_charging_station_energy_imported( self, imported_energy_wh: float ) -> None: diff --git a/src/handlers/vehicle_command.py b/src/handlers/vehicle_command.py index f06596e..e7f994c 100644 --- a/src/handlers/vehicle_command.py +++ b/src/handlers/vehicle_command.py @@ -91,7 +91,9 @@ def __report_command_failure( command, ) - async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: + async def handle_mqtt_command( + self, *, topic: str, payload: str, retained: bool = False + ) -> None: analyzed_topic = self.__get_command_topics(topic) handler = self.__command_handlers.get(analyzed_topic.command_no_vin) if not handler: @@ -103,7 +105,10 @@ async def handle_mqtt_command(self, *, topic: str, payload: str) -> None: ) else: await self.__execute_mqtt_command_handler( - handler=handler, payload=payload, analyzed_topic=analyzed_topic + handler=handler, + payload=payload, + analyzed_topic=analyzed_topic, + retained=retained, ) async def __execute_mqtt_command_handler( @@ -112,19 +117,33 @@ async def __execute_mqtt_command_handler( handler: CommandHandlerBase, payload: str, analyzed_topic: _MqttCommandTopic, + retained: bool, ) -> None: topic = analyzed_topic.command_no_vin topic_no_global = analyzed_topic.command_no_global result_topic = analyzed_topic.response_no_global + if retained and not handler.is_replayable_when_retained(): + # A retained `/set` for an action-bearing command would re-fire the + # action on every gateway restart. Drop it before invoking the + # handler. Only handlers that explicitly opt in via + # ``replayable_when_retained = True`` see retained replays. + LOG.warning( + "Dropping retained replay for non-replayable command %s on %s; " + "this command should not have been published with retain=true", + handler.name(), + topic, + ) + return + try: - execution_result = await handler.handle(payload) + execution_result = await handler.handle(payload, retained=retained) self.publisher.publish_str(result_topic, "Success") if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, f"after command execution on topic {topic}" ) - if execution_result.clear_command: + if execution_result.clear_command and not retained: self.publisher.clear_topic(topic_no_global) except MqttGatewayException as e: self.__report_command_failure( @@ -145,14 +164,14 @@ async def __execute_mqtt_command_handler( ) return try: - execution_result = await handler.handle(payload) + execution_result = await handler.handle(payload, retained=retained) self.publisher.publish_str(result_topic, "Success") if execution_result.force_refresh: self.vehicle_state.set_refresh_mode( RefreshMode.FORCE, f"after command execution on topic {topic}", ) - if execution_result.clear_command: + if execution_result.clear_command and not retained: self.publisher.clear_topic(topic_no_global) except Exception as retry_err: self.__report_command_failure( diff --git a/src/integrations/home_assistant/base.py b/src/integrations/home_assistant/base.py index 848eef6..a7603dd 100644 --- a/src/integrations/home_assistant/base.py +++ b/src/integrations/home_assistant/base.py @@ -46,14 +46,16 @@ def _publish_select( enabled: bool = True, value_template: str = "{{ value }}", command_template: str = "{{ value }}", + retain: bool = False, icon: str | None = None, custom_availability: HaCustomAvailabilityConfig | None = None, ) -> str: - payload = { + payload: dict[str, Any] = { "state_topic": self._get_state_topic(topic), "command_topic": self._get_command_topic(topic), "value_template": value_template, "command_template": command_template, + "retain": str(retain).lower(), "options": options, "enabled_by_default": enabled, } diff --git a/src/integrations/home_assistant/discovery.py b/src/integrations/home_assistant/discovery.py index d7c7320..1a8cafe 100644 --- a/src/integrations/home_assistant/discovery.py +++ b/src/integrations/home_assistant/discovery.py @@ -167,6 +167,7 @@ def __publish_ha_discovery_messages_real(self) -> None: mode="box", min_value=0.0, step=0.001, + retain=True, ) self._publish_sensor( @@ -640,6 +641,7 @@ def __publish_gateway_sensors(self) -> None: value_template="{{ value }}", command_template="{{ value }}", icon="mdi:refresh", + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -651,6 +653,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -662,6 +665,7 @@ def __publish_gateway_sensors(self) -> None: min_value=1 * 60 * 60, max_value=5 * 24 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -673,6 +677,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=12 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_number( @@ -684,6 +689,7 @@ def __publish_gateway_sensors(self) -> None: min_value=30, max_value=12 * 60 * 60, step=1, + retain=True, custom_availability=self.__system_availability_config, ) self._publish_sensor( diff --git a/src/mqtt_gateway.py b/src/mqtt_gateway.py index e5bf676..cfac634 100644 --- a/src/mqtt_gateway.py +++ b/src/mqtt_gateway.py @@ -255,7 +255,9 @@ async def __register_alarm_switches( def __create_vehicle_handler(self, vin_info: VinInfo) -> VehicleHandler: vin = vin_info.vin - total_battery_capacity = self.configuration.battery_capacity_map.get(vin, None) if vin else None + total_battery_capacity = ( + self.configuration.battery_capacity_map.get(vin, None) if vin else None + ) info = VehicleInfo(vin_info, total_battery_capacity) account_prefix = f"{self.configuration.saic_user}/{mqtt_topics.VEHICLES}/{vin}" vehicle_state = VehicleState( @@ -356,11 +358,18 @@ def vehicle_handlers(self) -> dict[str, VehicleHandler]: @override async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: vehicle_handler = self.get_vehicle_handler(vin) if vehicle_handler: - await vehicle_handler.handle_mqtt_command(topic=topic, payload=payload) + await vehicle_handler.handle_mqtt_command( + topic=topic, payload=payload, retained=retained + ) + elif retained: + LOG.warning( + f"Retained command for unknown vin {vin} received on {topic};" + f" handler not yet registered, dropping replay" + ) else: LOG.debug(f"Command for unknown vin {vin} received") diff --git a/src/publisher/core.py b/src/publisher/core.py index b6628d8..50fdf6b 100644 --- a/src/publisher/core.py +++ b/src/publisher/core.py @@ -16,7 +16,7 @@ class MqttCommandListener(ABC): @abstractmethod async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: raise NotImplementedError("Should have implemented this") @@ -187,7 +187,9 @@ def anonymize_str(value: str) -> str: def anonymize_device_id(self, device_id: str) -> str: elements = device_id.split("###", maxsplit=1) if len(elements) == 2: - return f"{self.anonymize_str(elements[0])}###{self.anonymize_str(elements[1])}" + return ( + f"{self.anonymize_str(elements[0])}###{self.anonymize_str(elements[1])}" + ) return self.anonymize_str(device_id) @staticmethod diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index 4f079fc..b441fef 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -151,11 +151,16 @@ async def __on_message( payload = payload.decode("utf-8") else: payload = str(payload) - await self.__on_message_real(topic=topic, payload=payload) + retained = bool(_properties.get("retain", 0)) + await self.__on_message_real( + topic=topic, payload=payload, retained=retained + ) except Exception as e: LOG.exception(f"Error while processing MQTT message: {e}") - async def __on_message_real(self, *, topic: str, payload: str) -> None: + async def __on_message_real( + self, *, topic: str, payload: str, retained: bool + ) -> None: if topic in self.vin_by_charge_state_topic: LOG.debug(f"Received message over topic {topic} with payload {payload}") vin = self.vin_by_charge_state_topic[topic] @@ -194,7 +199,7 @@ async def __on_message_real(self, *, topic: str, payload: str) -> None: vin = self.get_vin_from_topic(topic) if self.command_listener is not None: await self.command_listener.on_mqtt_command_received( - vin=vin, topic=topic, payload=payload + vin=vin, topic=topic, payload=payload, retained=retained ) async def __handle_imported_energy(self, topic: str, payload: str) -> None: diff --git a/src/vehicle.py b/src/vehicle.py index 669e1a5..d8f7b7a 100644 --- a/src/vehicle.py +++ b/src/vehicle.py @@ -71,8 +71,10 @@ def get(mode: str) -> RefreshMode: ) #: Refresh modes that are not valid at startup and must be replaced with PERIODIC. +#: Only one-shot modes are coerced; OFF is a legitimate persistent user choice +#: (restored from retained `/set` on reconnect) and must be preserved. INVALID_STARTUP_REFRESH_MODES: Final[frozenset[RefreshMode]] = frozenset( - {RefreshMode.OFF, RefreshMode.FORCE, RefreshMode.CHARGING_DETECTION} + {RefreshMode.FORCE, RefreshMode.CHARGING_DETECTION} ) @@ -108,13 +110,21 @@ def __init__( ) self.vehicle: Final[VehicleInfo] = vin_info self.mqtt_vin_prefix = account_prefix - self.last_car_activity: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) - self.last_successful_refresh: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) + self.last_car_activity: datetime.datetime = datetime.datetime.min.replace( + tzinfo=datetime.UTC + ) + self.last_successful_refresh: datetime.datetime = datetime.datetime.min.replace( + tzinfo=datetime.UTC + ) self.__last_failed_refresh: datetime.datetime | None = None self.__failed_refresh_counter = 0 self.__refresh_period_error = 30 - self.last_car_shutdown: datetime.datetime = datetime.datetime.now(tz=datetime.UTC) - self.last_car_vehicle_message: datetime.datetime = datetime.datetime.min.replace(tzinfo=datetime.UTC) + self.last_car_shutdown: datetime.datetime = datetime.datetime.now( + tz=datetime.UTC + ) + self.last_car_vehicle_message: datetime.datetime = ( + datetime.datetime.min.replace(tzinfo=datetime.UTC) + ) # treat high voltage battery as active, if we don't have any other information self.__hv_battery_active = True self.__hv_battery_active_from_car = True @@ -128,8 +138,8 @@ def __init__( self.charge_current_limit: ChargeCurrentLimitCode | None = None self.refresh_period_charging = 0 self.charge_polling_min_percent = charge_polling_min_percent - self.refresh_mode = RefreshMode.OFF - self.previous_refresh_mode = RefreshMode.OFF + self.refresh_mode = RefreshMode.PERIODIC + self.previous_refresh_mode = RefreshMode.PERIODIC self.__polling_phase: PollingPhase | None = None self.__remote_ac_temp: int | None = None self.__remote_ac_running: bool = False @@ -243,7 +253,11 @@ def update_scheduled_charging( tz = self.__user_timezone if self.refresh_period_inactive_grace > 0: # Add a grace period to the start time, so that the car is not woken up too early - now = datetime.datetime.now(tz=tz) if tz else datetime.datetime.now().astimezone() + now = ( + datetime.datetime.now(tz=tz) + if tz + else datetime.datetime.now().astimezone() + ) dt = now.replace( hour=start_time.hour, minute=start_time.minute, @@ -426,20 +440,16 @@ def __should_do_periodic_refresh(self) -> bool: self.__publish_polling_phase(PollingPhase.ERROR_RECOVERY) return result if self.is_charging and self.refresh_period_charging > 0: - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_charging)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_charging)) LOG.debug(f"HV battery is charging. Should refresh: {result}") self.__publish_polling_phase(PollingPhase.CHARGING) return result if self.hv_battery_active: - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_active)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_active)) LOG.debug(f"HV battery is active. Should refresh: {result}") self.__publish_polling_phase(PollingPhase.ACTIVE) return result @@ -447,21 +457,17 @@ def __should_do_periodic_refresh(self) -> bool: seconds=float(self.refresh_period_inactive_grace) ) if last_shutdown_plus_refresh > datetime.datetime.now(tz=datetime.UTC): - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_after_shutdown)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_after_shutdown)) LOG.debug( f"Refresh grace period after shutdown has not passed. Should refresh: {result}" ) self.__publish_polling_phase(PollingPhase.AFTER_SHUTDOWN) return result - result = ( - self.last_successful_refresh - < datetime.datetime.now(tz=datetime.UTC) - - datetime.timedelta(seconds=float(self.refresh_period_inactive)) - ) + result = self.last_successful_refresh < datetime.datetime.now( + tz=datetime.UTC + ) - datetime.timedelta(seconds=float(self.refresh_period_inactive)) LOG.debug( f"HV battery is inactive and refresh period after shutdown is over. Should refresh: {result}" ) diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py index ecc568f..d900983 100644 --- a/tests/handlers/test_vehicle_command.py +++ b/tests/handlers/test_vehicle_command.py @@ -1,5 +1,6 @@ from __future__ import annotations +from typing import cast import unittest from unittest.mock import AsyncMock, MagicMock, patch @@ -7,11 +8,14 @@ from handlers.vehicle_command import VehicleCommandHandler import mqtt_topics +from vehicle import RefreshMode MQTT_TOPIC = "saic" VIN = "vin_test_000000000" VEHICLE_PREFIX = f"vehicles/{VIN}" -CHARGING_SET_TOPIC = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING_SET}" +CHARGING_SET_TOPIC = ( + f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING_SET}" +) CHARGING_RESULT_TOPIC = ( f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_CHARGING}/{mqtt_topics.RESULT_SUFFIX}" ) @@ -182,9 +186,7 @@ async def test_retry_failure_publishes_error_event(self) -> None: await handler.handle_mqtt_command(topic=CHARGING_SET_TOPIC, payload="true") - pub.publish_str.assert_any_call( - CHARGING_RESULT_TOPIC, "Failed: retry boom" - ) + pub.publish_str.assert_any_call(CHARGING_RESULT_TOPIC, "Failed: retry boom") pub.publish_json.assert_called_once() event = pub.publish_json.call_args[0][1] assert event["detail"] == "retry boom" @@ -239,3 +241,118 @@ async def test_payload_structure(self) -> None: assert event["event_type"] == "command_error" assert event["command"] == mqtt_topics.DRIVETRAIN_CHARGING_SET assert "operation too frequent" in event["detail"] + + +REFRESH_MODE_SET_TOPIC = f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.REFRESH_MODE_SET}" +REFRESH_MODE_RESULT_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.REFRESH_MODE}/{mqtt_topics.RESULT_SUFFIX}" +) +TOTAL_BATTERY_CAPACITY_SET_TOPIC = ( + f"{MQTT_TOPIC}/{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY_SET}" +) +TOTAL_BATTERY_CAPACITY_RESULT_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY}" + f"/{mqtt_topics.RESULT_SUFFIX}" +) + + +class TestRetainedReplay(unittest.IsolatedAsyncioTestCase): + """Behavior for retained `/set` commands replayed on broker reconnect. + + Idempotent values (refresh periods, OFF/PERIODIC mode, battery capacity) + must seed in-memory state. One-shot refresh modes (FORCE / + CHARGING_DETECTION) must be dropped to avoid looping a poll on every + gateway restart. + """ + + async def test_retained_force_refresh_mode_dropped(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="force", retained=True + ) + + vehicle_state.set_refresh_mode.assert_not_called() + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + + async def test_retained_charging_detection_refresh_mode_dropped(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, + payload="charging_detection", + retained=True, + ) + + vehicle_state.set_refresh_mode.assert_not_called() + pub.publish_str.assert_any_call(REFRESH_MODE_RESULT_TOPIC, "Success") + + async def test_retained_periodic_refresh_mode_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="periodic", retained=True + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.PERIODIC + + async def test_retained_off_refresh_mode_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="off", retained=True + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.OFF + + async def test_non_retained_force_still_applied(self) -> None: + handler, _pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=REFRESH_MODE_SET_TOPIC, payload="force", retained=False + ) + + vehicle_state.set_refresh_mode.assert_called_once() + mode_arg = vehicle_state.set_refresh_mode.call_args[0][0] + assert mode_arg is RefreshMode.FORCE + + async def test_retained_battery_capacity_replays_to_vehicle_info(self) -> None: + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + + await handler.handle_mqtt_command( + topic=TOTAL_BATTERY_CAPACITY_SET_TOPIC, payload="50.0", retained=True + ) + + vehicle_state.update_battery_capacity.assert_called_once_with(50.0) + pub.publish_str.assert_any_call(TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success") + + async def test_retained_action_command_dropped_at_dispatcher(self) -> None: + """Retained `/set` for an action-bearing command is dropped at the dispatcher. + + DrivetrainChargingCommand has not opted in via + is_replayable_when_retained(). A retained replay of `charging/set` (e.g. + from a non-HA client that mistakenly retained the topic) must NOT + invoke the handler — otherwise the SAIC charging API call would re-fire + on every gateway restart. + """ + saic_api = AsyncMock() + handler, pub = _build(saic_api=saic_api) + + await handler.handle_mqtt_command( + topic=CHARGING_SET_TOPIC, payload="true", retained=True + ) + + # Handler never ran: no API call, no Success/result publish, no clear_topic + saic_api.control_charging.assert_not_called() + pub.publish_str.assert_not_called() + pub.clear_topic.assert_not_called() diff --git a/tests/integrations/home_assistant/__init__.py b/tests/integrations/home_assistant/__init__.py new file mode 100644 index 0000000..e69de29 diff --git a/tests/integrations/home_assistant/test_discovery_retain.py b/tests/integrations/home_assistant/test_discovery_retain.py new file mode 100644 index 0000000..45fa789 --- /dev/null +++ b/tests/integrations/home_assistant/test_discovery_retain.py @@ -0,0 +1,122 @@ +from __future__ import annotations + +import json +import unittest + +from apscheduler.schedulers.blocking import BlockingScheduler +from saic_ismart_client_ng.api.vehicle.schema import ( + VehicleModelConfiguration, + VinInfo, +) + +from configuration import Configuration +from integrations.home_assistant.discovery import HomeAssistantDiscovery +import mqtt_topics +from tests.common_mocks import VIN +from tests.mocks import MessageCapturingConsolePublisher +from vehicle import RefreshMode, VehicleState +from vehicle_info import VehicleInfo + +# Six entities whose `/set` commands HA must retain so the user's last value +# survives a gateway restart. Stored as the path suffix of the entity's state +# topic so we can match against published HA discovery payloads. +RETAINED_ENTITY_TOPICS = { + mqtt_topics.REFRESH_MODE, + mqtt_topics.REFRESH_PERIOD_ACTIVE, + mqtt_topics.REFRESH_PERIOD_INACTIVE, + mqtt_topics.REFRESH_PERIOD_AFTER_SHUTDOWN, + mqtt_topics.REFRESH_PERIOD_INACTIVE_GRACE, + mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY, +} + +# Sample of writable entities that must NOT be retained. SOC target / charge +# current are API-backed; charging is action-bearing. +NON_RETAINED_ENTITY_TOPICS = { + mqtt_topics.DRIVETRAIN_SOC_TARGET, + mqtt_topics.DRIVETRAIN_CHARGECURRENT_LIMIT, +} + + +def _make_discovery() -> tuple[ + HomeAssistantDiscovery, MessageCapturingConsolePublisher +]: + config = Configuration() + config.anonymized_publishing = False + config.ha_discovery_prefix = "homeassistant" + publisher = MessageCapturingConsolePublisher(config) + vin_info = VinInfo() + vin_info.vin = VIN + vin_info.series = "EH32 S" + vin_info.modelName = "MG4 Electric" + vin_info.modelYear = "2022" + vin_info.vehicleModelConfiguration = [ + VehicleModelConfiguration("BATTERY", "BATTERY", "1"), + VehicleModelConfiguration("BType", "Battery", "1"), + ] + vehicle_info = VehicleInfo(vin_info, None) + account_prefix = f"/vehicles/{VIN}" + scheduler = BlockingScheduler() + vehicle_state = VehicleState(publisher, scheduler, account_prefix, vehicle_info) + vehicle_state.refresh_period_active = 30 + vehicle_state.refresh_period_inactive = 120 + vehicle_state.refresh_period_after_shutdown = 60 + vehicle_state.refresh_period_inactive_grace = 600 + vehicle_state.refresh_mode = RefreshMode.PERIODIC + discovery = HomeAssistantDiscovery(vehicle_state, vehicle_info, config) + return discovery, publisher + + +def _writable_payloads( + publisher: MessageCapturingConsolePublisher, +) -> list[dict[str, object]]: + """Return every published discovery payload that has a `command_topic`.""" + payloads: list[dict[str, object]] = [] + for raw in publisher.map.values(): + try: + payload = json.loads(raw) + except (TypeError, json.JSONDecodeError): + continue + if isinstance(payload, dict) and "command_topic" in payload: + payloads.append(payload) + return payloads + + +def _payload_for_state_topic_suffix( + payloads: list[dict[str, object]], suffix: str +) -> dict[str, object] | None: + for payload in payloads: + state_topic = payload.get("state_topic") + if isinstance(state_topic, str) and state_topic.endswith(f"/{suffix}"): + return payload + return None + + +class TestDiscoveryRetainFlag(unittest.TestCase): + """The six idempotent persistence-relevant entities must be retained.""" + + def test_required_entities_have_retain_true(self) -> None: + discovery, publisher = _make_discovery() + discovery.publish_ha_discovery_messages() + payloads = _writable_payloads(publisher) + + for topic in RETAINED_ENTITY_TOPICS: + payload = _payload_for_state_topic_suffix(payloads, topic) + assert payload is not None, ( + f"No writable HA discovery payload found for topic {topic}" + ) + assert payload.get("retain") == "true", ( + f"Expected retain=true for {topic}, got {payload.get('retain')!r}" + ) + + def test_non_retained_entities_keep_retain_false(self) -> None: + discovery, publisher = _make_discovery() + discovery.publish_ha_discovery_messages() + payloads = _writable_payloads(publisher) + + for topic in NON_RETAINED_ENTITY_TOPICS: + payload = _payload_for_state_topic_suffix(payloads, topic) + if payload is None: + continue # entity not published for this vehicle config + assert payload.get("retain") in ("false", None), ( + f"Expected retain!=true for {topic}, got {payload.get('retain')!r}" + ) diff --git a/tests/test_mqtt_publisher.py b/tests/test_mqtt_publisher.py index fa65b29..2416dc9 100644 --- a/tests/test_mqtt_publisher.py +++ b/tests/test_mqtt_publisher.py @@ -24,10 +24,11 @@ async def on_mqtt_global_command_received( @override async def on_mqtt_command_received( - self, *, vin: str, topic: str, payload: str + self, *, vin: str, topic: str, payload: str, retained: bool = False ) -> None: self.received_vin = vin self.received_payload = payload.strip().lower() + self.received_retained = retained @override def setUp(self) -> None: @@ -39,6 +40,7 @@ def setUp(self) -> None: self.mqtt_client.command_listener = self self.received_vin = "" self.received_payload = "" + self.received_retained = False self.vehicle_base_topic = ( f"{self.mqtt_client.configuration.mqtt_topic}/{USER}/vehicles/{VIN}" ) diff --git a/tests/test_vehicle_state.py b/tests/test_vehicle_state.py index e0a47cd..faff700 100644 --- a/tests/test_vehicle_state.py +++ b/tests/test_vehicle_state.py @@ -192,11 +192,38 @@ def test_republish_command_states_skips_unset_values(self) -> None: self.get_topic(mqtt_topics.CLIMATE_REMOTE_TEMPERATURE) not in self.publisher.map ) - # refresh_mode defaults to RefreshMode.OFF (never None), so it IS always published + # refresh_mode defaults to RefreshMode.PERIODIC (never None), so it IS always published self.assert_mqtt_topic( - self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.OFF.value + self.get_topic(mqtt_topics.REFRESH_MODE), RefreshMode.PERIODIC.value ) + def test_configure_missing_skips_when_retained_value_present(self) -> None: + """Sentinel guards in configure_missing preserve retained-replay values. + + Retained `/set` replay seeds in-memory state before configure_missing + runs; configure_missing must then leave those values alone. + """ + self.vehicle_state.set_refresh_period_active(45) + self.vehicle_state.update_battery_capacity(50.0) + + self.vehicle_state.configure_missing() + + assert self.vehicle_state.refresh_period_active == 45 + assert self.vehicle_state.vehicle.custom_battery_capacity == 50.0 + + def test_configure_missing_applies_defaults_when_no_retained(self) -> None: + assert self.vehicle_state.refresh_period_active == -1 + self.vehicle_state.configure_missing() + assert self.vehicle_state.refresh_period_active == 30 + assert self.vehicle_state.refresh_period_inactive == 86400 + assert self.vehicle_state.refresh_period_after_shutdown == 120 + assert self.vehicle_state.refresh_period_inactive_grace == 600 + + def test_configure_missing_preserves_retained_off_refresh_mode(self) -> None: + self.vehicle_state.set_refresh_mode(RefreshMode.OFF, "retained replay") + self.vehicle_state.configure_missing() + assert self.vehicle_state.refresh_mode == RefreshMode.OFF + def test_republish_command_states_includes_api_values(self) -> None: self.vehicle_state.configure_missing() self.vehicle_state.update_target_soc(TargetBatteryCode.P_80) @@ -280,7 +307,9 @@ def test_handle_vehicle_status_rejects_max_int32_timestamp(self) -> None: def test_handle_vehicle_status_rejects_drifted_timestamp(self) -> None: resp = get_mock_vehicle_status_resp() resp.statusTime = 1000000000 # 2001-09-09, well outside 15 min window - with pytest.raises(VehicleStatusDriftException, match="drifted more than 15 minutes"): + with pytest.raises( + VehicleStatusDriftException, match="drifted more than 15 minutes" + ): self.vehicle_state.handle_vehicle_status(resp) def test_mileage_of_day_published_when_valid(self) -> None: @@ -303,7 +332,9 @@ def test_mileage_of_day_skipped_when_exceeds_total(self) -> None: chrg_mgmt_data_resp = get_mock_charge_management_data_resp() # Set mileageOfDay raw value higher than total mileage raw value assert chrg_mgmt_data_resp.rvsChargeStatus is not None - chrg_mgmt_data_resp.rvsChargeStatus.mileageOfDay = (DRIVETRAIN_MILEAGE + 100) * 10 + chrg_mgmt_data_resp.rvsChargeStatus.mileageOfDay = ( + DRIVETRAIN_MILEAGE + 100 + ) * 10 self.vehicle_state.handle_charge_status(chrg_mgmt_data_resp) assert ( @@ -318,7 +349,9 @@ def test_mileage_since_last_charge_skipped_when_exceeds_total(self) -> None: chrg_mgmt_data_resp = get_mock_charge_management_data_resp() assert chrg_mgmt_data_resp.rvsChargeStatus is not None - chrg_mgmt_data_resp.rvsChargeStatus.mileageSinceLastCharge = (DRIVETRAIN_MILEAGE + 100) * 10 + chrg_mgmt_data_resp.rvsChargeStatus.mileageSinceLastCharge = ( + DRIVETRAIN_MILEAGE + 100 + ) * 10 self.vehicle_state.handle_charge_status(chrg_mgmt_data_resp) assert ( @@ -442,9 +475,7 @@ def test_periodic_after_shutdown_publishes_after_shutdown_phase(self) -> None: self.vehicle_state.configure_missing() # Car just shut down, grace period active self.vehicle_state.hv_battery_active = False - self.vehicle_state.last_car_shutdown = datetime.datetime.now( - tz=datetime.UTC - ) + self.vehicle_state.last_car_shutdown = datetime.datetime.now(tz=datetime.UTC) self.vehicle_state.last_car_activity = datetime.datetime.min.replace( tzinfo=datetime.UTC ) From 8d4db97b8d24d4629a7c9b239a8e4291b3f06a46 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:44:39 +0200 Subject: [PATCH 03/12] fix: forward retain flag through all typed publish methods (#443) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 ` 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. --- .pre-commit-config.yaml | 1 + src/publisher/core.py | 16 ++++++--- src/publisher/log_publisher.py | 30 ++++++++++------ src/publisher/mqtt_publisher.py | 32 ++++++++++++----- tests/mocks/__init__.py | 4 +-- tests/test_mqtt_publisher.py | 62 +++++++++++++++++++++++++++++++++ 6 files changed, 120 insertions(+), 25 deletions(-) diff --git a/.pre-commit-config.yaml b/.pre-commit-config.yaml index 6ae070e..d62febd 100644 --- a/.pre-commit-config.yaml +++ b/.pre-commit-config.yaml @@ -29,6 +29,7 @@ repos: language: system types: - python + pass_filenames: false require_serial: true - id: pytest name: Run pytest diff --git a/src/publisher/core.py b/src/publisher/core.py index 50fdf6b..5fa8295 100644 --- a/src/publisher/core.py +++ b/src/publisher/core.py @@ -85,19 +85,27 @@ def publish_json( raise NotImplementedError @abstractmethod - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: raise NotImplementedError @abstractmethod diff --git a/src/publisher/log_publisher.py b/src/publisher/log_publisher.py index 00deb67..45e30d2 100644 --- a/src/publisher/log_publisher.py +++ b/src/publisher/log_publisher.py @@ -32,27 +32,35 @@ def publish_json( retain: bool = True, ) -> None: anonymized_json = self.dict_to_anonymized_json(data) - self.internal_publish(key, anonymized_json) + self.internal_publish(key, anonymized_json, retain=retain) @override - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: - self.internal_publish(key, value) + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.internal_publish(key, value, retain=retain) @override - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: - self.internal_publish(key, value) + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.internal_publish(key, value, retain=retain) @override - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: - self.internal_publish(key, value) + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.internal_publish(key, value, retain=retain) @override - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: - self.internal_publish(key, value) + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.internal_publish(key, value, retain=retain) @override def clear_topic(self, key: str, no_prefix: bool = False) -> None: self.internal_publish(key, None) - def internal_publish(self, key: str, value: Any) -> None: - LOG.debug(f"{key}: {value}") + def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: + LOG.debug(f"{key}: {value} (retain={retain})") diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index b441fef..6d3256f 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -248,20 +248,36 @@ def publish_json( ) @override - def publish_str(self, key: str, value: str, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_int(self, key: str, value: int, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_bool(self, key: str, value: bool, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override - def publish_float(self, key: str, value: float, no_prefix: bool = False) -> None: - self.__publish(topic=self.get_topic(key, no_prefix), payload=value) + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: + self.__publish( + topic=self.get_topic(key, no_prefix), payload=value, retain=retain + ) @override def clear_topic(self, key: str, no_prefix: bool = False) -> None: diff --git a/tests/mocks/__init__.py b/tests/mocks/__init__.py index 9790d3a..de6db2d 100644 --- a/tests/mocks/__init__.py +++ b/tests/mocks/__init__.py @@ -18,7 +18,7 @@ def __init__(self, configuration: Configuration) -> None: self.publish_count: dict[str, int] = {} @override - def internal_publish(self, key: str, value: Any) -> None: + def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: self.map[key] = value self.publish_count[key] = self.publish_count.get(key, 0) + 1 - LOG.debug(f"{key}: {value}") + LOG.debug(f"{key}: {value} (retain={retain})") diff --git a/tests/test_mqtt_publisher.py b/tests/test_mqtt_publisher.py index 2416dc9..1fdeba3 100644 --- a/tests/test_mqtt_publisher.py +++ b/tests/test_mqtt_publisher.py @@ -2,6 +2,7 @@ from typing import Any, override import unittest +from unittest.mock import patch from configuration import Configuration, TransportProtocol from publisher.core import MqttCommandListener @@ -98,3 +99,64 @@ async def on_charger_connection_state_changed( self, vin: str, connected: bool ) -> None: pass + + def test_publish_str_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_str("foo", "bar") + m_pub.assert_called_once_with("saic/foo", "bar", retain=True) + + def test_publish_str_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_str("foo", "bar", retain=False) + m_pub.assert_called_once_with("saic/foo", "bar", retain=False) + + def test_publish_int_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_int("foo", 42) + m_pub.assert_called_once_with("saic/foo", 42, retain=True) + + def test_publish_int_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_int("foo", 42, retain=False) + m_pub.assert_called_once_with("saic/foo", 42, retain=False) + + def test_publish_bool_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_bool("foo", True) + m_pub.assert_called_once_with("saic/foo", True, retain=True) + + def test_publish_bool_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_bool("foo", True, retain=False) + m_pub.assert_called_once_with("saic/foo", True, retain=False) + + def test_publish_float_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_float("foo", 1.5) + m_pub.assert_called_once_with("saic/foo", 1.5, retain=True) + + def test_publish_float_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_float("foo", 1.5, retain=False) + m_pub.assert_called_once_with("saic/foo", 1.5, retain=False) + + def test_publish_json_default_is_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_json("foo", {"a": 1}) + m_pub.assert_called_once() + args, kwargs = m_pub.call_args + assert args[0] == "saic/foo" + assert kwargs == {"retain": True} + + def test_publish_json_forwards_retain_false(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.publish_json("foo", {"a": 1}, retain=False) + m_pub.assert_called_once() + args, kwargs = m_pub.call_args + assert args[0] == "saic/foo" + assert kwargs == {"retain": False} + + def test_clear_topic_publishes_none_retained(self) -> None: + with patch.object(self.mqtt_client.client, "publish") as m_pub: + self.mqtt_client.clear_topic("foo") + m_pub.assert_called_once_with("saic/foo", None, retain=True) From 653da84e1b4074980de18cf362b9bc8ae70e7733 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 11:56:38 +0200 Subject: [PATCH 04/12] feat: add Publisher.publish dispatching method (#442) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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. --- src/publisher/core.py | 64 +++ src/publisher/log_publisher.py | 6 +- src/publisher/mqtt_publisher.py | 5 +- src/status_publisher/__init__.py | 58 +-- src/vehicle.py | 49 +-- tests/mocks/__init__.py | 7 +- tests/publisher/__init__.py | 1 + tests/publisher/test_publish_dispatch.py | 372 ++++++++++++++++++ .../test_message_publisher.py | 18 +- 9 files changed, 487 insertions(+), 93 deletions(-) create mode 100644 tests/publisher/__init__.py create mode 100644 tests/publisher/test_publish_dispatch.py diff --git a/src/publisher/core.py b/src/publisher/core.py index 5fa8295..6e7e926 100644 --- a/src/publisher/core.py +++ b/src/publisher/core.py @@ -1,17 +1,38 @@ from __future__ import annotations from abc import ABC, abstractmethod +from datetime import datetime import json import re from typing import TYPE_CHECKING, Any, TypeVar import mqtt_topics +from utils import datetime_to_str if TYPE_CHECKING: from configuration import Configuration T = TypeVar("T") +type Publishable = bool | int | float | str | dict[str, Any] | datetime +"""Closed union of value types this gateway knows how to publish to MQTT. + +Mirrors the typed `publish_*` methods on :class:`Publisher` plus the `dict` +shape handled by `publish_json`, and `datetime`, which is stringified via +:func:`utils.datetime_to_str`. Use it at signature boundaries when a caller +holds "something publishable" without statically knowing which arm. +""" + +type WirePayload = bool | int | float | str +"""Primitive subset of :data:`Publishable` that reaches the transport layer. + +After the typed `publish_*` methods do their work (`publish_json` serializes +dicts to JSON strings, `publish_datetime` stringifies via +:func:`utils.datetime_to_str`), only these scalar arms cross the +publisher/transport boundary. Use `WirePayload | None` for wire-level helpers +where `None` means "clear the retained message." +""" + class MqttCommandListener(ABC): @abstractmethod @@ -108,6 +129,49 @@ def publish_float( ) -> None: raise NotImplementedError + def publish_datetime( + self, + key: str, + value: datetime, + no_prefix: bool = False, + *, + retain: bool = True, + ) -> None: + """Stringify a datetime via :func:`utils.datetime_to_str` and publish.""" + self.publish_str(key, datetime_to_str(value), no_prefix, retain=retain) + + def publish( + self, + key: str, + value: Publishable, + no_prefix: bool = False, + *, + retain: bool = True, + ) -> None: + """Dispatch to the appropriate typed publish_* based on value type. + + For callers that hold a `Publishable` without statically knowing + which arm of the union it is. `retain` is forwarded to every arm. + """ + # bool must precede int: isinstance(True, int) is True in Python. + if isinstance(value, bool): + self.publish_bool(key, value, no_prefix, retain=retain) + elif isinstance(value, int): + self.publish_int(key, value, no_prefix, retain=retain) + elif isinstance(value, float): + self.publish_float(key, value, no_prefix, retain=retain) + elif isinstance(value, str): + self.publish_str(key, value, no_prefix, retain=retain) + elif isinstance(value, dict): + self.publish_json(key, value, no_prefix, retain=retain) + elif isinstance(value, datetime): + self.publish_datetime(key, value, no_prefix, retain=retain) + else: + # Defensive: type system rules this out, but `Any` callers can sneak + # an unsupported runtime type through; raise rather than silently no-op. + msg = f"Unsupported value type: {type(value).__name__}" # type: ignore[unreachable] + raise TypeError(msg) + @abstractmethod def clear_topic(self, key: str, no_prefix: bool = False) -> None: raise NotImplementedError diff --git a/src/publisher/log_publisher.py b/src/publisher/log_publisher.py index 45e30d2..7969c61 100644 --- a/src/publisher/log_publisher.py +++ b/src/publisher/log_publisher.py @@ -3,7 +3,7 @@ import logging from typing import Any, override -from publisher.core import Publisher +from publisher.core import Publisher, WirePayload LOG = logging.getLogger(__name__) LOG.setLevel(level="DEBUG") @@ -62,5 +62,7 @@ def publish_float( def clear_topic(self, key: str, no_prefix: bool = False) -> None: self.internal_publish(key, None) - def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: + def internal_publish( + self, key: str, value: WirePayload | None, *, retain: bool = True + ) -> None: LOG.debug(f"{key}: {value} (retain={retain})") diff --git a/src/publisher/mqtt_publisher.py b/src/publisher/mqtt_publisher.py index 6d3256f..ed535d4 100644 --- a/src/publisher/mqtt_publisher.py +++ b/src/publisher/mqtt_publisher.py @@ -13,6 +13,7 @@ if TYPE_CHECKING: from configuration import Configuration from integrations.openwb.charging_station import ChargingStation + from publisher.core import WirePayload LOG = logging.getLogger(__name__) @@ -226,7 +227,9 @@ async def __handle_imported_energy(self, topic: str, payload: str) -> None: vin, imported_energy_wh ) - def __publish(self, topic: str, payload: Any, *, retain: bool = True) -> None: + def __publish( + self, topic: str, payload: WirePayload | None, *, retain: bool = True + ) -> None: self.client.publish(topic, payload, retain=retain) @override diff --git a/src/status_publisher/__init__.py b/src/status_publisher/__init__.py index 4fffbba..387f13f 100644 --- a/src/status_publisher/__init__.py +++ b/src/status_publisher/__init__.py @@ -1,10 +1,9 @@ from __future__ import annotations from abc import ABCMeta, abstractmethod -from datetime import datetime -from typing import TYPE_CHECKING, Any, Final, TypeVar +from typing import TYPE_CHECKING, Final -from utils import datetime_to_str +from publisher.core import Publishable if TYPE_CHECKING: from collections.abc import Callable @@ -12,9 +11,6 @@ from publisher.core import Publisher from vehicle_info import VehicleInfo -T = TypeVar("T") -Publishable = TypeVar("Publishable", str, int, float, bool, dict[str, Any], datetime) - class VehicleDataPublisher[I, O](metaclass=ABCMeta): def __init__( @@ -28,65 +24,37 @@ def __init__( def publish(self, data: I) -> O: raise NotImplementedError - def _publish( + def _publish[V: Publishable]( self, *, topic: str, - value: Publishable | None, - validator: Callable[[Publishable], bool] = lambda _: True, + value: V | None, + validator: Callable[[V], bool] = lambda _: True, no_prefix: bool = False, retain: bool = True, - ) -> tuple[bool, Publishable | None]: + ) -> tuple[bool, V | None]: if value is None or not validator(value): return False, None actual_topic = topic if no_prefix else self.__get_topic(topic) - published = self._publish_directly( - topic=actual_topic, value=value, retain=retain - ) - return published, value + self.__publisher.publish(actual_topic, value, retain=retain) + return True, value - def _transform_and_publish( + def _transform_and_publish[T, V: Publishable]( self, *, topic: str, value: T | None, validator: Callable[[T], bool] = lambda _: True, - transform: Callable[[T], Publishable], + transform: Callable[[T], V], no_prefix: bool = False, retain: bool = True, - ) -> tuple[bool, Publishable | None]: + ) -> tuple[bool, V | None]: if value is None or not validator(value): return False, None actual_topic = topic if no_prefix else self.__get_topic(topic) transformed_value = transform(value) - published = self._publish_directly( - topic=actual_topic, value=transformed_value, retain=retain - ) - return published, transformed_value - - def _publish_directly( - self, *, topic: str, value: Publishable, retain: bool = True - ) -> bool: - published = False - if isinstance(value, bool): - self.__publisher.publish_bool(topic, value) - published = True - elif isinstance(value, int): - self.__publisher.publish_int(topic, value) - published = True - elif isinstance(value, float): - self.__publisher.publish_float(topic, value) - published = True - elif isinstance(value, str): - self.__publisher.publish_str(topic, value) - published = True - elif isinstance(value, dict): - self.__publisher.publish_json(topic, value, retain=retain) - published = True - elif isinstance(value, datetime): - self.__publisher.publish_str(topic, datetime_to_str(value)) - published = True - return published + self.__publisher.publish(actual_topic, transformed_value, retain=retain) + return True, transformed_value def __get_topic(self, sub_topic: str) -> str: return f"{self.__mqtt_vehicle_prefix}/{sub_topic}" diff --git a/src/vehicle.py b/src/vehicle.py index d8f7b7a..731293d 100644 --- a/src/vehicle.py +++ b/src/vehicle.py @@ -4,7 +4,7 @@ from enum import Enum, unique import logging import math -from typing import TYPE_CHECKING, Any, Final, TypeVar +from typing import TYPE_CHECKING, Final from apscheduler.triggers.cron import CronTrigger from saic_ismart_client_ng.api.vehicle_charging import ( @@ -17,6 +17,7 @@ from extractors import extract_electric_range, extract_soc import mqtt_topics +from publisher.core import Publishable from status_publisher.charge.chrg_mgmt_data_resp import ( ChrgMgmtDataRespProcessingResult, ChrgMgmtDataRespPublisher, @@ -26,7 +27,6 @@ VehicleStatusRespProcessingResult, VehicleStatusRespPublisher, ) -from utils import datetime_to_str if TYPE_CHECKING: from collections.abc import Callable @@ -42,11 +42,6 @@ from publisher.core import Publisher from vehicle_info import VehicleInfo - T = TypeVar("T") - Publishable = TypeVar( - "Publishable", str, int, float, bool, dict[str, Any], datetime.datetime - ) - DEFAULT_AC_TEMP = 22 PRESSURE_TO_BAR_FACTOR = 0.04 @@ -378,7 +373,7 @@ def notify_car_activity(self) -> None: self.last_car_activity = datetime.datetime.now(tz=datetime.UTC) self.__publish( topic=mqtt_topics.REFRESH_LAST_ACTIVITY, - value=datetime_to_str(self.last_car_activity), + value=self.last_car_activity, ) def notify_message(self, message: MessageEntity) -> None: @@ -505,8 +500,8 @@ def last_failed_refresh(self, value: datetime.datetime | None) -> None: ) ) self.__failed_refresh_counter = self.__failed_refresh_counter + 1 - self.publisher.publish_str( - self.get_topic(mqtt_topics.REFRESH_LAST_ERROR), datetime_to_str(value) + self.publisher.publish_datetime( + self.get_topic(mqtt_topics.REFRESH_LAST_ERROR), value ) self.publisher.publish_int( self.get_topic(mqtt_topics.REFRESH_PERIOD_ERROR), @@ -806,41 +801,19 @@ def update_battery_capacity(self, new_capacity: float) -> None: def is_remote_ac_running(self) -> bool: return self.__remote_ac_running - def __publish( + def __publish[V: Publishable]( self, *, topic: str, - value: Publishable | None, - validator: Callable[[Publishable], bool] = lambda _: True, + value: V | None, + validator: Callable[[V], bool] = lambda _: True, no_prefix: bool = False, - ) -> tuple[bool, Publishable | None]: + ) -> tuple[bool, V | None]: if value is None or not validator(value): return False, None actual_topic = topic if no_prefix else self.get_topic(topic) - published = self.__publish_directly(topic=actual_topic, value=value) - return published, value - - def __publish_directly(self, *, topic: str, value: Publishable) -> bool: - published = False - if isinstance(value, bool): - self.publisher.publish_bool(topic, value) - published = True - elif isinstance(value, int): - self.publisher.publish_int(topic, value) - published = True - elif isinstance(value, float): - self.publisher.publish_float(topic, value) - published = True - elif isinstance(value, str): - self.publisher.publish_str(topic, value) - published = True - elif isinstance(value, dict): - self.publisher.publish_json(topic, value) - published = True - elif isinstance(value, datetime.datetime): - self.publisher.publish_str(topic, datetime_to_str(value)) - published = True - return published + self.publisher.publish(actual_topic, value) + return True, value @property def vin(self) -> str: diff --git a/tests/mocks/__init__.py b/tests/mocks/__init__.py index de6db2d..b47d18b 100644 --- a/tests/mocks/__init__.py +++ b/tests/mocks/__init__.py @@ -7,6 +7,7 @@ if TYPE_CHECKING: from configuration import Configuration + from publisher.core import WirePayload LOG = logging.getLogger(__name__) @@ -14,11 +15,15 @@ class MessageCapturingConsolePublisher(ConsolePublisher): def __init__(self, configuration: Configuration) -> None: super().__init__(configuration) + # Test inspection map; consumers narrow per-key (e.g. json.loads on + # serialized dict topics), so keep the value type permissive here. self.map: dict[str, Any] = {} self.publish_count: dict[str, int] = {} @override - def internal_publish(self, key: str, value: Any, *, retain: bool = True) -> None: + def internal_publish( + self, key: str, value: WirePayload | None, *, retain: bool = True + ) -> None: self.map[key] = value self.publish_count[key] = self.publish_count.get(key, 0) + 1 LOG.debug(f"{key}: {value} (retain={retain})") diff --git a/tests/publisher/__init__.py b/tests/publisher/__init__.py new file mode 100644 index 0000000..9d48db4 --- /dev/null +++ b/tests/publisher/__init__.py @@ -0,0 +1 @@ +from __future__ import annotations diff --git a/tests/publisher/test_publish_dispatch.py b/tests/publisher/test_publish_dispatch.py new file mode 100644 index 0000000..7902771 --- /dev/null +++ b/tests/publisher/test_publish_dispatch.py @@ -0,0 +1,372 @@ +"""Conformance tests for `Publisher.publish` dispatch across all subclasses. + +`Publisher.publish` is a single non-abstract method on the ABC that dispatches +based on the runtime type of `value` to the corresponding typed +`publish_{bool,int,float,str,datetime,json}` method. `publish_datetime` is itself +a concrete ABC-level method that stringifies via :func:`utils.datetime_to_str` +and forwards to `publish_str`. The tests below exercise that dispatch directly +on every concrete `Publisher` subclass shipped by the project, plus a minimal +in-test subclass that locks the contract at the ABC level. + +The critical regression these tests guard against: `bool` is a subclass of +`int` in Python, so `isinstance(True, int)` is `True`. The dispatch must check +`bool` *before* `int` so that `publish(key, True)` reaches `publish_bool` (not +`publish_int`). +""" + +from __future__ import annotations + +from datetime import UTC, datetime +from typing import TYPE_CHECKING, Any, override +from unittest.mock import MagicMock, patch + +import pytest + +from configuration import Configuration, TransportProtocol +from publisher.core import Publishable, Publisher +from publisher.log_publisher import ConsolePublisher +from publisher.mqtt_publisher import MqttPublisher +from tests.mocks import MessageCapturingConsolePublisher +from utils import datetime_to_str + +if TYPE_CHECKING: + from collections.abc import Callable + + +KEY = "some/topic" + + +def _make_configuration() -> Configuration: + config = Configuration() + config.mqtt_topic = "saic" + config.saic_user = "user@example.com" + config.mqtt_transport_protocol = TransportProtocol.TCP + return config + + +# Each entry: (label, factory) where factory returns a fresh concrete Publisher. +PUBLISHER_FACTORIES: list[tuple[str, Callable[[], Publisher]]] = [ + ("MqttPublisher", lambda: MqttPublisher(_make_configuration())), + ("ConsolePublisher", lambda: ConsolePublisher(_make_configuration())), + ( + "MessageCapturingConsolePublisher", + lambda: MessageCapturingConsolePublisher(_make_configuration()), + ), +] + + +# (label, value, expected typed-method name) for arms where the value is +# forwarded to the typed method unchanged. +PASSTHROUGH_CASES: list[tuple[str, Publishable, str]] = [ + ("bool_true", True, "publish_bool"), + ("bool_false", False, "publish_bool"), + ("int_value", 5, "publish_int"), + ("int_zero", 0, "publish_int"), + ("float_value", 5.0, "publish_float"), + ("str_value", "hi", "publish_str"), + ( + "datetime_value", + datetime(2026, 5, 9, 12, 34, 56, tzinfo=UTC), + "publish_datetime", + ), +] + +TYPED_METHODS = ( + "publish_bool", + "publish_int", + "publish_float", + "publish_str", + "publish_datetime", + "publish_json", +) + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +@pytest.mark.parametrize( + ("case_label", "value", "expected_method"), + PASSTHROUGH_CASES, + ids=[label for label, _, _ in PASSTHROUGH_CASES], +) +def test_publish_dispatches_to_correct_typed_method( + publisher_label: str, + factory: Callable[[], Publisher], + case_label: str, + value: Publishable, + expected_method: str, +) -> None: + del publisher_label, case_label # only used as test ids + publisher = factory() + with ( + patch.object(publisher, "publish_bool") as m_bool, + patch.object(publisher, "publish_int") as m_int, + patch.object(publisher, "publish_float") as m_float, + patch.object(publisher, "publish_str") as m_str, + patch.object(publisher, "publish_datetime") as m_dt, + patch.object(publisher, "publish_json") as m_json, + ): + spies = { + "publish_bool": m_bool, + "publish_int": m_int, + "publish_float": m_float, + "publish_str": m_str, + "publish_datetime": m_dt, + "publish_json": m_json, + } + publisher.publish(KEY, value) + + spies[expected_method].assert_called_once_with(KEY, value, False, retain=True) + for name in TYPED_METHODS: + if name != expected_method: + spies[name].assert_not_called() + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_dict_routes_to_publish_json_with_retain( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + """`dict` values dispatch to `publish_json`, forwarding `retain`.""" + del publisher_label + publisher = factory() + payload: dict[str, Any] = {"a": 1, "b": "two"} + with patch.object(publisher, "publish_json") as m_json: + publisher.publish(KEY, payload, retain=False) + m_json.assert_called_once_with(KEY, payload, False, retain=False) + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +@pytest.mark.parametrize( + ("case_label", "value", "expected_method"), + PASSTHROUGH_CASES, + ids=[label for label, _, _ in PASSTHROUGH_CASES], +) +def test_publish_forwards_retain_false_to_every_arm( + publisher_label: str, + factory: Callable[[], Publisher], + case_label: str, + value: Publishable, + expected_method: str, +) -> None: + """`retain=False` reaches every typed dispatch target, not just `publish_json`.""" + del publisher_label, case_label + publisher = factory() + with patch.object(publisher, expected_method) as m: + publisher.publish(KEY, value, retain=False) + m.assert_called_once_with(KEY, value, False, retain=False) + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_datetime_stringifies_via_publish_str( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + """`publish_datetime` stringifies via `datetime_to_str` and forwards to `publish_str`.""" + del publisher_label + publisher = factory() + when = datetime(2026, 5, 9, 12, 34, 56, tzinfo=UTC) + with patch.object(publisher, "publish_str") as m_str: + publisher.publish_datetime(KEY, when) + m_str.assert_called_once_with(KEY, datetime_to_str(when), False, retain=True) + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_forwards_no_prefix_flag( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + del publisher_label + publisher = factory() + with patch.object(publisher, "publish_str") as m_str: + publisher.publish(KEY, "hello", no_prefix=True) + m_str.assert_called_once_with(KEY, "hello", True, retain=True) + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_true_routes_to_bool_not_int( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + """Locks in the bool-before-int dispatch ordering. + + `isinstance(True, int)` is `True` in Python, so a naive `int` check first + would silently route `True`/`False` to `publish_int`. + """ + del publisher_label + publisher = factory() + with ( + patch.object(publisher, "publish_bool") as m_bool, + patch.object(publisher, "publish_int") as m_int, + ): + publisher.publish(KEY, True) + m_bool.assert_called_once_with(KEY, True, False, retain=True) + m_int.assert_not_called() + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_int_does_not_route_to_bool( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + del publisher_label + publisher = factory() + with ( + patch.object(publisher, "publish_bool") as m_bool, + patch.object(publisher, "publish_int") as m_int, + ): + publisher.publish(KEY, 5) + m_int.assert_called_once_with(KEY, 5, False, retain=True) + m_bool.assert_not_called() + + +@pytest.mark.parametrize( + ("publisher_label", "factory"), + PUBLISHER_FACTORIES, + ids=[label for label, _ in PUBLISHER_FACTORIES], +) +def test_publish_unsupported_type_raises( + publisher_label: str, + factory: Callable[[], Publisher], +) -> None: + """Unsupported runtime types raise rather than silently no-op.""" + del publisher_label + publisher = factory() + with pytest.raises(TypeError, match="Unsupported value type"): + publisher.publish(KEY, b"bytes-not-supported") # type: ignore[arg-type] + + +class _MinimalPublisher(Publisher): + """ABC-level publisher that mocks only the typed publish methods. + + Keeps the dispatch contract pinned even if all concrete subclasses were + to override `publish` in the future. + """ + + def __init__(self, config: Configuration) -> None: + super().__init__(config) + self.publish_bool = MagicMock() # type: ignore[method-assign] + self.publish_int = MagicMock() # type: ignore[method-assign] + self.publish_float = MagicMock() # type: ignore[method-assign] + self.publish_str = MagicMock() # type: ignore[method-assign] + self.publish_datetime = MagicMock() # type: ignore[method-assign] + self.publish_json = MagicMock() # type: ignore[method-assign] + self.clear_topic = MagicMock() # type: ignore[method-assign] + + @override + async def connect(self) -> None: + pass + + @override + def enable_commands(self) -> None: + pass + + @override + def is_connected(self) -> bool: + return True + + @override + def publish_json( + self, + key: str, + data: dict[str, Any], + no_prefix: bool = False, + *, + retain: bool = True, + ) -> None: + pass + + @override + def publish_str( + self, key: str, value: str, no_prefix: bool = False, *, retain: bool = True + ) -> None: + pass + + @override + def publish_int( + self, key: str, value: int, no_prefix: bool = False, *, retain: bool = True + ) -> None: + pass + + @override + def publish_bool( + self, key: str, value: bool, no_prefix: bool = False, *, retain: bool = True + ) -> None: + pass + + @override + def publish_float( + self, key: str, value: float, no_prefix: bool = False, *, retain: bool = True + ) -> None: + pass + + @override + def clear_topic(self, key: str, no_prefix: bool = False) -> None: + pass + + +@pytest.mark.parametrize( + ("case_label", "value", "expected_method"), + PASSTHROUGH_CASES, + ids=[label for label, _, _ in PASSTHROUGH_CASES], +) +def test_abc_level_publish_dispatch( + case_label: str, + value: Publishable, + expected_method: str, +) -> None: + del case_label + publisher = _MinimalPublisher(_make_configuration()) + publisher.publish(KEY, value) + spies: dict[str, MagicMock] = { + "publish_bool": publisher.publish_bool, # type: ignore[dict-item] + "publish_int": publisher.publish_int, # type: ignore[dict-item] + "publish_float": publisher.publish_float, # type: ignore[dict-item] + "publish_str": publisher.publish_str, # type: ignore[dict-item] + "publish_datetime": publisher.publish_datetime, # type: ignore[dict-item] + "publish_json": publisher.publish_json, # type: ignore[dict-item] + } + spies[expected_method].assert_called_once_with(KEY, value, False, retain=True) + for name in TYPED_METHODS: + if name != expected_method: + spies[name].assert_not_called() + + +def test_abc_level_publish_dict_with_retain() -> None: + publisher = _MinimalPublisher(_make_configuration()) + payload: dict[str, Any] = {"x": 1} + publisher.publish(KEY, payload, retain=False) + publisher.publish_json.assert_called_once_with(KEY, payload, False, retain=False) # type: ignore[attr-defined] + + +def test_abc_level_publish_datetime_routes_to_publish_datetime() -> None: + publisher = _MinimalPublisher(_make_configuration()) + when = datetime(2026, 5, 9, 12, 34, 56, tzinfo=UTC) + publisher.publish(KEY, when) + publisher.publish_datetime.assert_called_once_with(KEY, when, False, retain=True) # type: ignore[attr-defined] diff --git a/tests/status_publisher/test_message_publisher.py b/tests/status_publisher/test_message_publisher.py index 8cdc598..a042375 100644 --- a/tests/status_publisher/test_message_publisher.py +++ b/tests/status_publisher/test_message_publisher.py @@ -156,14 +156,20 @@ def test_event_payload_keys(self) -> None: class TestMessageEventResilience(unittest.TestCase): def test_event_publish_failure_does_not_break_processing(self) -> None: publisher, capturing = _make_publisher() - original_publish = publisher._publish_directly - - def failing_publish(**kwargs: Any) -> bool: - if mqtt_topics.EVENTS_VEHICLE_MESSAGE in kwargs["topic"]: + original_publish = capturing.publish + + def failing_publish( + key: str, + value: Any, + no_prefix: bool = False, + *, + retain: bool = True, + ) -> None: + if mqtt_topics.EVENTS_VEHICLE_MESSAGE in key: raise RuntimeError("MQTT down") - return original_publish(**kwargs) + original_publish(key, value, no_prefix, retain=retain) - with patch.object(publisher, "_publish_directly", side_effect=failing_publish): + with patch.object(capturing, "publish", side_effect=failing_publish): result = publisher.publish(_make_message()) assert result.processed is True From 2688f81252da78fc04849cc7ddcb89713450c53a Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 15:50:40 +0200 Subject: [PATCH 05/12] feat: allow forcing the account timezone instead of trusting the API (#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 --- CHANGELOG.md | 8 ++ README.md | 1 + src/configuration/__init__.py | 3 + src/configuration/argparse_extensions.py | 11 ++ src/configuration/parser.py | 15 +++ src/mqtt_gateway.py | 54 +++++---- src/utils.py | 33 ++++++ tests/test_gateway_timezone.py | 136 +++++++++++++++++++++++ tests/test_parser.py | 57 +++++++++- tests/test_utils.py | 20 +++- 10 files changed, 308 insertions(+), 30 deletions(-) create mode 100644 tests/test_gateway_timezone.py diff --git a/CHANGELOG.md b/CHANGELOG.md index dcddda1..e960207 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -2,6 +2,14 @@ ## Unreleased +### Added + +* Add `--saic-user-timezone` / `SAIC_USER_TIMEZONE` config option to force + the account timezone instead of relying on the SAIC API value. Useful when + the API reports a wrong DST offset (#438). Discrepancies between the forced + zone and the API value are detected by comparing the current UTC offset and + logged at WARNING level. + ### Fixed * Persist user-set HA gateway entities across gateway restarts by retaining diff --git a/README.md b/README.md index dbfd84c..17347cf 100644 --- a/README.md +++ b/README.md @@ -41,6 +41,7 @@ When using combinations of configuration methods, the order of precedence is as | --battery-capacity-mapping | BATTERY_CAPACITY_MAPPING | Mapping of VIN to full battery capacity. Multiple mappings can be provided separated by ',' Example: LSJXXXX=54.0,LSJYYYY=64.0 | | --charge-min-percentage | CHARGE_MIN_PERCENTAGE | How many % points we should try to refresh the charge state. 1.0 by default | | --account-refresh-interval | ACCOUNT_REFRESH_INTERVAL | Interval in seconds for refreshing account-level data (vehicle list, timezone). Default is 86400 (24 hours). | +| --saic-user-timezone | SAIC_USER_TIMEZONE | Force the account timezone instead of trusting the SAIC API value. Accepts an IANA name (e.g. `Australia/Sydney`) or `GMT+HH:MM`. Mismatches with the API offset are logged. | | --publish-raw-api-data | PUBLISH_RAW_API_DATA_ENABLED | Publish raw SAIC API request/response to MQTT. Disabled (False) by default. | #### API Endpoints diff --git a/src/configuration/__init__.py b/src/configuration/__init__.py index 3816ae3..7173f37 100644 --- a/src/configuration/__init__.py +++ b/src/configuration/__init__.py @@ -4,6 +4,8 @@ from typing import TYPE_CHECKING if TYPE_CHECKING: + from zoneinfo import ZoneInfo + from integrations.openwb.charging_station import ChargingStation @@ -27,6 +29,7 @@ def __init__(self) -> None: self.saic_tenant_id: str = "459771" self.saic_relogin_delay: int = 15 * 60 # in seconds self.saic_read_timeout: float = 10.0 # in seconds + self.saic_user_timezone: ZoneInfo | None = None self.battery_capacity_map: dict[str, float] = {} self.mqtt_host: str | None = None self.mqtt_port: int = 1883 diff --git a/src/configuration/argparse_extensions.py b/src/configuration/argparse_extensions.py index 1341d49..86cd5b4 100644 --- a/src/configuration/argparse_extensions.py +++ b/src/configuration/argparse_extensions.py @@ -8,8 +8,11 @@ from dotenv import dotenv_values +from utils import parse_timezone + if TYPE_CHECKING: from collections.abc import Callable, Sequence + from zoneinfo import ZoneInfo # load .env file and merge with os.environ @@ -105,3 +108,11 @@ def check_positive_float(value: str) -> float: def check_bool(value: str) -> bool: return str(value).lower() in ["true", "1", "yes", "y"] + + +def check_timezone(value: str) -> ZoneInfo: + try: + return parse_timezone(value) + except (ValueError, KeyError, ModuleNotFoundError) as e: + msg = f"{value!r} is not a valid timezone" + raise argparse.ArgumentTypeError(msg) from e diff --git a/src/configuration/parser.py b/src/configuration/parser.py index ae9a84f..99456ab 100644 --- a/src/configuration/parser.py +++ b/src/configuration/parser.py @@ -15,6 +15,7 @@ check_bool, check_positive, check_positive_float, + check_timezone, ) from exceptions import MqttGatewayException from integrations.openwb.charging_station import ChargingStation @@ -127,6 +128,8 @@ def __setup_saic_api(args: Namespace, config: Configuration) -> None: config.saic_relogin_delay = args.saic_relogin_delay if args.saic_read_timeout: config.saic_read_timeout = args.saic_read_timeout + if args.saic_user_timezone is not None: + config.saic_user_timezone = args.saic_user_timezone def __setup_home_assistant(args: Namespace, config: Configuration) -> None: @@ -361,6 +364,18 @@ def __add_saic_api_argument_group( envvar="SAIC_READ_TIMEOUT", type=check_positive_float, ) + saic_api.add_argument( + "--saic-user-timezone", + help="""Force the account timezone instead of trusting the SAIC API value. + Accepts an IANA timezone name (e.g. Australia/Sydney) or the + GMT+HH:MM format. Any discrepancy between this value and the + timezone reported by the API is logged.""", + dest="saic_user_timezone", + required=False, + action=EnvDefault, + envvar="SAIC_USER_TIMEZONE", + type=check_timezone, + ) saic_api.add_argument( "--messages-request-interval", help="""The interval for retrieving messages in seconds.""", diff --git a/src/mqtt_gateway.py b/src/mqtt_gateway.py index cfac634..f955e93 100644 --- a/src/mqtt_gateway.py +++ b/src/mqtt_gateway.py @@ -6,9 +6,7 @@ import datetime import logging from random import uniform -import re from typing import TYPE_CHECKING, Any, override -from zoneinfo import ZoneInfo import apscheduler.schedulers.asyncio from saic_ismart_client_ng import SaicApi @@ -25,11 +23,13 @@ from publisher.log_publisher import ConsolePublisher from publisher.mqtt_publisher import MqttPublisher from saic_api_listener import MqttGatewaySaicApiListener -from utils import datetime_to_str, get_gateway_version +from utils import datetime_to_str, get_gateway_version, parse_timezone from vehicle import VehicleState from vehicle_info import VehicleInfo if TYPE_CHECKING: + from zoneinfo import ZoneInfo + from saic_ismart_client_ng.api.vehicle import VinInfo from configuration import Configuration @@ -45,7 +45,7 @@ def __init__(self, config: Configuration) -> None: self.configuration = config self.__vehicle_handlers: dict[str, VehicleHandler] = {} self.__vehicle_tasks: list[Task[Any]] = [] - self.__user_timezone: ZoneInfo | None = None + self.__user_timezone: ZoneInfo | None = config.saic_user_timezone self.publisher = self.__select_publisher() self.publisher.command_listener = self if config.publish_raw_api_data: @@ -136,33 +136,11 @@ async def run(self) -> None: LOG.info("Entering main loop") await self.__run_until_all_tasks_done() - @staticmethod - def __parse_timezone(tz_str: str) -> ZoneInfo: - try: - return ZoneInfo(tz_str) - except (KeyError, ModuleNotFoundError): - pass - - # Handle GMT+HH:MM / GMT-HH:MM format from the SAIC API. - # POSIX Etc/GMT zones use inverted signs: GMT+01:00 → Etc/GMT-1 - m = re.fullmatch(r"GMT([+-])(\d{2}):(\d{2})", tz_str) - if m: - sign, hours, minutes = m.group(1), int(m.group(2)), int(m.group(3)) - if minutes != 0: - LOG.warning( - "Timezone %s has non-zero minutes, rounding to whole hour", tz_str - ) - posix_sign = "-" if sign == "+" else "+" - return ZoneInfo(f"Etc/GMT{posix_sign}{hours}") - - msg = f"Unrecognized timezone format: {tz_str}" - raise ValueError(msg) - async def __fetch_user_timezone(self) -> ZoneInfo | None: try: resp = await self.saic_api.get_user_timezone() if resp.timezone: - tz = self.__parse_timezone(resp.timezone) + tz = parse_timezone(resp.timezone) LOG.info("User timezone from API: %s → %s", resp.timezone, tz) return tz LOG.warning("API returned no timezone, using system default") @@ -182,7 +160,27 @@ def __publish_account_int(self, topic: str, value: int) -> None: self.publisher.publish_int(self.__get_account_topic(topic), value) async def __refresh_user_timezone(self) -> None: - tz = await self.__fetch_user_timezone() + forced_tz = self.configuration.saic_user_timezone + api_tz = await self.__fetch_user_timezone() + tz: ZoneInfo | None + if forced_tz is not None: + if api_tz is not None: + # Compare offsets at "now": IANA zones (Europe/Rome) and the + # API's fixed Etc/GMT zones never compare equal by identity, + # but their current UTC offset will match when DST aligns. + now = datetime.datetime.now(tz=datetime.UTC) + if forced_tz.utcoffset(now) != api_tz.utcoffset(now): + LOG.warning( + "Forced user timezone %s (offset %s) differs from " + "API value %s (offset %s); using forced value", + forced_tz, + forced_tz.utcoffset(now), + api_tz, + api_tz.utcoffset(now), + ) + tz = forced_tz + else: + tz = api_tz if tz is not None: self.__user_timezone = tz for vh in self.vehicle_handlers.values(): diff --git a/src/utils.py b/src/utils.py index 781f2ca..7c0778e 100644 --- a/src/utils.py +++ b/src/utils.py @@ -2,14 +2,47 @@ from datetime import UTC, datetime, timedelta from importlib.metadata import PackageNotFoundError, version +import logging import os +import re from typing import TYPE_CHECKING +from zoneinfo import ZoneInfo from saic_ismart_client_ng.api.schema import GpsStatus if TYPE_CHECKING: from saic_ismart_client_ng.api.vehicle import VehicleStatusResp +LOG = logging.getLogger(__name__) + + +def parse_timezone(tz_str: str) -> ZoneInfo: + """Parse a timezone string into a :class:`ZoneInfo`. + + Accepts both IANA names (``Australia/Sydney``) and the ``GMT+HH:MM`` + offset format returned by the SAIC API. + """ + try: + return ZoneInfo(tz_str) + except (KeyError, ModuleNotFoundError): + pass + + # Handle GMT+HH:MM / GMT-HH:MM format from the SAIC API. + # POSIX Etc/GMT zones use inverted signs: GMT+01:00 → Etc/GMT-1 + m = re.fullmatch(r"GMT([+-])(\d{2}):(\d{2})", tz_str) + if m: + sign, hours, minutes = m.group(1), int(m.group(2)), int(m.group(3)) + if minutes != 0: + LOG.warning( + "Timezone %s has non-zero minutes, rounding to whole hour", tz_str + ) + posix_sign = "-" if sign == "+" else "+" + return ZoneInfo(f"Etc/GMT{posix_sign}{hours}") + + msg = f"Unrecognized timezone format: {tz_str}" + raise ValueError(msg) + + def value_in_range[Numeric: (int, float)]( value: Numeric, min_value: Numeric, diff --git a/tests/test_gateway_timezone.py b/tests/test_gateway_timezone.py new file mode 100644 index 0000000..270cac7 --- /dev/null +++ b/tests/test_gateway_timezone.py @@ -0,0 +1,136 @@ +from __future__ import annotations + +import logging +import unittest +from unittest.mock import AsyncMock, patch +from zoneinfo import ZoneInfo + +from saic_ismart_client_ng.api.user import UserTimezoneResp + +from configuration import Configuration +from mqtt_gateway import MqttGateway +import mqtt_topics + +from .mocks import MessageCapturingConsolePublisher + + +def _make_gateway(config: Configuration) -> MqttGateway: + with patch( + "mqtt_gateway.MqttGateway._MqttGateway__select_publisher", + return_value=MessageCapturingConsolePublisher(config), + ): + return MqttGateway(config) + + +def _make_config(*, forced_tz: ZoneInfo | None) -> Configuration: + config = Configuration() + config.saic_user = "user@example.com" + config.saic_password = "secret" # noqa: S105 + config.saic_user_timezone = forced_tz + return config + + +# Name-mangled access helpers — mypy does not see private dunder names. +async def _refresh(gateway: MqttGateway) -> None: + await gateway._MqttGateway__refresh_user_timezone() # type: ignore[attr-defined] + + +def _user_tz(gateway: MqttGateway) -> ZoneInfo | None: + tz: ZoneInfo | None = gateway._MqttGateway__user_timezone # type: ignore[attr-defined] + return tz + + +class TestGatewayTimezoneRefresh(unittest.IsolatedAsyncioTestCase): + async def test_uses_api_timezone_when_no_override(self) -> None: + config = _make_config(forced_tz=None) + gateway = _make_gateway(config) + publisher = gateway.publisher + assert isinstance(publisher, MessageCapturingConsolePublisher) + + with patch.object( + gateway.saic_api, + "get_user_timezone", + new=AsyncMock(return_value=UserTimezoneResp(timezone="GMT+10:00")), + ): + await _refresh(gateway) + + assert ( + publisher.map[f"user@example.com/{mqtt_topics.ACCOUNT_USER_TIMEZONE}"] + == "Etc/GMT-10" + ) + + async def test_forced_timezone_overrides_api_with_offset_mismatch(self) -> None: + # Sydney is currently at GMT+11 (DST) or GMT+10; pick a forced zone + # whose current offset does not match what the API returned. + forced = ZoneInfo("Europe/Rome") + config = _make_config(forced_tz=forced) + gateway = _make_gateway(config) + publisher = gateway.publisher + assert isinstance(publisher, MessageCapturingConsolePublisher) + + with ( + patch.object( + gateway.saic_api, + "get_user_timezone", + new=AsyncMock(return_value=UserTimezoneResp(timezone="GMT+11:00")), + ), + self.assertLogs("mqtt_gateway", level=logging.WARNING) as cm, + ): + await _refresh(gateway) + + assert ( + publisher.map[f"user@example.com/{mqtt_topics.ACCOUNT_USER_TIMEZONE}"] + == "Europe/Rome" + ) + joined = "\n".join(cm.output) + assert "Europe/Rome" in joined + assert "Etc/GMT-11" in joined + assert "differs from API value" in joined + + async def test_forced_timezone_used_when_api_fails(self) -> None: + forced = ZoneInfo("Australia/Sydney") + config = _make_config(forced_tz=forced) + gateway = _make_gateway(config) + publisher = gateway.publisher + assert isinstance(publisher, MessageCapturingConsolePublisher) + + with patch.object( + gateway.saic_api, + "get_user_timezone", + new=AsyncMock(side_effect=RuntimeError("boom")), + ): + await _refresh(gateway) + + assert ( + publisher.map[f"user@example.com/{mqtt_topics.ACCOUNT_USER_TIMEZONE}"] + == "Australia/Sydney" + ) + + def test_forced_timezone_primed_at_construction(self) -> None: + forced = ZoneInfo("Australia/Sydney") + config = _make_config(forced_tz=forced) + gateway = _make_gateway(config) + # Vehicles created during initial discovery (before + # __refresh_user_timezone runs) must already see the forced zone. + assert _user_tz(gateway) == forced + + async def test_no_warning_when_iana_zone_matches_api_offset(self) -> None: + # Same instant: Etc/GMT-10 has offset +10:00; an IANA zone fixed at +10 + # year-round (no DST) should be considered equivalent. + forced = ZoneInfo("Australia/Brisbane") # AEST, +10 year-round + config = _make_config(forced_tz=forced) + gateway = _make_gateway(config) + + logger = logging.getLogger("mqtt_gateway") + with ( + patch.object( + gateway.saic_api, + "get_user_timezone", + new=AsyncMock(return_value=UserTimezoneResp(timezone="GMT+10:00")), + ), + patch.object(logger, "warning") as mock_warning, + ): + await _refresh(gateway) + + for call in mock_warning.call_args_list: + assert "differs from API value" not in str(call) diff --git a/tests/test_parser.py b/tests/test_parser.py index 2282cf8..2f5ae63 100644 --- a/tests/test_parser.py +++ b/tests/test_parser.py @@ -1,8 +1,63 @@ from __future__ import annotations -from configuration.parser import setup_parser +import argparse +from zoneinfo import ZoneInfo + +import pytest + +from configuration.argparse_extensions import check_timezone +from configuration.parser import process_command_line, setup_parser def test_setup_parser_should_generate_a_valid_parser() -> None: parser = setup_parser() parser.print_help() + + +def test_check_timezone_accepts_iana_name() -> None: + assert check_timezone("Australia/Sydney") == ZoneInfo("Australia/Sydney") + + +def test_check_timezone_accepts_gmt_offset() -> None: + assert check_timezone("GMT+10:00") == ZoneInfo("Etc/GMT-10") + + +def test_check_timezone_rejects_invalid_value() -> None: + with pytest.raises(argparse.ArgumentTypeError, match="not a valid timezone"): + check_timezone("Not/A_Timezone") + + +def test_process_command_line_sets_forced_timezone( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + "sys.argv", + [ + "prog", + "--saic-user", + "user@example.com", + "--saic-password", + "secret", + "--saic-user-timezone", + "Australia/Sydney", + ], + ) + config = process_command_line() + assert config.saic_user_timezone == ZoneInfo("Australia/Sydney") + + +def test_process_command_line_defaults_forced_timezone_to_none( + monkeypatch: pytest.MonkeyPatch, +) -> None: + monkeypatch.setattr( + "sys.argv", + [ + "prog", + "--saic-user", + "user@example.com", + "--saic-password", + "secret", + ], + ) + config = process_command_line() + assert config.saic_user_timezone is None diff --git a/tests/test_utils.py b/tests/test_utils.py index a8864a5..fb2ac5c 100644 --- a/tests/test_utils.py +++ b/tests/test_utils.py @@ -2,11 +2,13 @@ import datetime from unittest import TestCase +from zoneinfo import ZoneInfo +import pytest from saic_ismart_client_ng.api.schema import GpsPosition, GpsStatus from saic_ismart_client_ng.api.vehicle import VehicleStatusResp -from utils import get_update_timestamp +from utils import get_update_timestamp, parse_timezone class Test(TestCase): @@ -118,3 +120,19 @@ def test_get_update_should_return_now_if_no_other_info_is_there_v3(self) -> None result = get_update_timestamp(vehicle_status_resp) assert result <= datetime.datetime.now(tz=datetime.UTC) + + +class TestParseTimezone(TestCase): + def test_parses_iana_name(self) -> None: + assert parse_timezone("Australia/Sydney") == ZoneInfo("Australia/Sydney") + + def test_parses_gmt_positive_offset(self) -> None: + # POSIX Etc/GMT zones use inverted signs: GMT+10:00 → Etc/GMT-10 + assert parse_timezone("GMT+10:00") == ZoneInfo("Etc/GMT-10") + + def test_parses_gmt_negative_offset(self) -> None: + assert parse_timezone("GMT-05:00") == ZoneInfo("Etc/GMT+5") + + def test_rejects_unknown_format(self) -> None: + with pytest.raises(ValueError, match="Unrecognized timezone format"): + parse_timezone("not-a-timezone") From fe86989ecd85de852ad5d4d4d29641cd61a541c1 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Sun, 10 May 2026 16:31:28 +0200 Subject: [PATCH 06/12] fix: republish Total Battery Capacity after HA number update (#445) MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit * 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 --- CHANGELOG.md | 8 +++++ .../drivetrain_total_battery_capacity.py | 13 ++++++++ tests/handlers/test_vehicle_command.py | 33 +++++++++++++++++++ 3 files changed, 54 insertions(+) diff --git a/CHANGELOG.md b/CHANGELOG.md index e960207..e5d1fa2 100644 --- a/CHANGELOG.md +++ b/CHANGELOG.md @@ -24,6 +24,14 @@ persistent. Existing retained STATE values on the broker are not converted into retained `/set` commands. +* Republish the effective Total Battery Capacity to its state topic right after + the user updates the HA number. The `_set` handler used to only mutate the + in-memory override and rely on the next vehicle poll to refresh the shared + sensor topic, leaving the HA sensor stuck on the previous (often hardcoded + per-model default) value while the number widget already showed the new + setting. A payload of `0` re-publishes the per-model default via + `real_battery_capacity`. + ## 0.11.0 ### Added diff --git a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py index 849f16a..1c0ea43 100644 --- a/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py +++ b/src/handlers/command/drivetrain/drivetrain_total_battery_capacity.py @@ -29,5 +29,18 @@ async def handle_typed_payload(self, payload: float) -> CommandProcessingResult: LOG.info("Setting Total Battery Capacity to %f", payload) self.vehicle_state.update_battery_capacity(payload) + # The HA number and sensor entities share the same state topic. + # Republish the effective capacity locally so the sensor reflects + # the change immediately instead of waiting for the next vehicle poll + # (payload of 0 falls back to the per-model default in real_battery_capacity). + effective_capacity = self.vehicle_state.vehicle.real_battery_capacity + if effective_capacity is not None and effective_capacity > 0: + self.publisher.publish_float( + self.vehicle_state.get_topic( + mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY + ), + effective_capacity, + ) + # No need to force a refresh return RESULT_DO_NOTHING diff --git a/tests/handlers/test_vehicle_command.py b/tests/handlers/test_vehicle_command.py index d900983..4815b5e 100644 --- a/tests/handlers/test_vehicle_command.py +++ b/tests/handlers/test_vehicle_command.py @@ -254,6 +254,9 @@ async def test_payload_structure(self) -> None: f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY}" f"/{mqtt_topics.RESULT_SUFFIX}" ) +TOTAL_BATTERY_CAPACITY_STATE_TOPIC = ( + f"{VEHICLE_PREFIX}/{mqtt_topics.DRIVETRAIN_TOTAL_BATTERY_CAPACITY}" +) class TestRetainedReplay(unittest.IsolatedAsyncioTestCase): @@ -328,14 +331,44 @@ async def test_non_retained_force_still_applied(self) -> None: async def test_retained_battery_capacity_replays_to_vehicle_info(self) -> None: handler, pub = _build() vehicle_state = cast("MagicMock", handler.vehicle_state) + vehicle_state.vehicle.real_battery_capacity = 50.0 await handler.handle_mqtt_command( topic=TOTAL_BATTERY_CAPACITY_SET_TOPIC, payload="50.0", retained=True ) vehicle_state.update_battery_capacity.assert_called_once_with(50.0) + pub.publish_float.assert_any_call(TOTAL_BATTERY_CAPACITY_STATE_TOPIC, 50.0) pub.publish_str.assert_any_call(TOTAL_BATTERY_CAPACITY_RESULT_TOPIC, "Success") + async def test_battery_capacity_zero_payload_publishes_model_default(self) -> None: + """Payload `0` clears the override; the per-model default is republished.""" + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + # update_battery_capacity(0) clears the override; real_battery_capacity then + # falls back to the per-model default (e.g. 64 kWh for an MG4 NMC). + vehicle_state.vehicle.real_battery_capacity = 64.0 + + await handler.handle_mqtt_command( + topic=TOTAL_BATTERY_CAPACITY_SET_TOPIC, payload="0", retained=False + ) + + vehicle_state.update_battery_capacity.assert_called_once_with(0.0) + pub.publish_float.assert_any_call(TOTAL_BATTERY_CAPACITY_STATE_TOPIC, 64.0) + + async def test_battery_capacity_skips_publish_when_no_default(self) -> None: + """When `real_battery_capacity` returns None (unknown model), skip the publish.""" + handler, pub = _build() + vehicle_state = cast("MagicMock", handler.vehicle_state) + vehicle_state.vehicle.real_battery_capacity = None + + await handler.handle_mqtt_command( + topic=TOTAL_BATTERY_CAPACITY_SET_TOPIC, payload="0", retained=False + ) + + vehicle_state.update_battery_capacity.assert_called_once_with(0.0) + pub.publish_float.assert_not_called() + async def test_retained_action_command_dropped_at_dispatcher(self) -> None: """Retained `/set` for an action-bearing command is dropped at the dispatcher. From 98c677a92f558b41612833880ab5cab6f06e635c Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Mon, 11 May 2026 21:37:49 +0200 Subject: [PATCH 07/12] feat: add SOC kWh fallback for vehicles without realtimePower (#448) * 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 --- poetry.lock | 207 +++++++++++++++++- pyproject.toml | 1 + src/extractors/__init__.py | 39 ++++ src/handlers/message.py | 22 +- src/integrations/openwb/__init__.py | 4 +- .../charge/chrg_mgmt_data_resp.py | 4 + .../charge/rvs_charge_status.py | 21 +- src/vehicle.py | 9 +- src/vehicle_info.py | 18 +- .../test_rvs_charge_status.py | 69 ++++++ tests/test_extractors.py | 55 +++++ tests/test_ha_discovery_windows.py | 12 +- tests/test_vehicle_handler.py | 58 ++++- tests/test_vehicle_state.py | 1 + 14 files changed, 484 insertions(+), 36 deletions(-) create mode 100644 tests/status_publisher/test_rvs_charge_status.py create mode 100644 tests/test_extractors.py diff --git a/poetry.lock b/poetry.lock index 3e6be2d..04bd68a 100644 --- a/poetry.lock +++ b/poetry.lock @@ -1,4 +1,4 @@ -# This file is automatically @generated by Poetry 2.3.2 and should not be changed by hand. +# This file is automatically @generated by Poetry 2.4.1 and should not be changed by hand. [[package]] name = "anyio" @@ -71,6 +71,18 @@ files = [ {file = "certifi-2026.2.25.tar.gz", hash = "sha256:e887ab5cee78ea814d3472169153c2d12cd43b14bd03329a39a9c6e2e80bfba7"}, ] +[[package]] +name = "cfgv" +version = "3.5.0" +description = "Validate configuration and produce human readable error messages." +optional = false +python-versions = ">=3.10" +groups = ["dev"] +files = [ + {file = "cfgv-3.5.0-py2.py3-none-any.whl", hash = "sha256:a8dc6b26ad22ff227d2634a65cb388215ce6cc96bbcc5cfde7641ae87e8dacc0"}, + {file = "cfgv-3.5.0.tar.gz", hash = "sha256:d5b1034354820651caa73ede66a6294d6e95c1b00acc5e9b098e917404669132"}, +] + [[package]] name = "colorama" version = "0.4.6" @@ -234,6 +246,30 @@ files = [ graph = ["objgraph (>=1.7.2)"] profile = ["gprof2dot (>=2022.7.29)"] +[[package]] +name = "distlib" +version = "0.4.0" +description = "Distribution utilities" +optional = false +python-versions = "*" +groups = ["dev"] +files = [ + {file = "distlib-0.4.0-py2.py3-none-any.whl", hash = "sha256:9659f7d87e46584a30b5780e43ac7a2143098441670ff0a49d5f9034c54a6c16"}, + {file = "distlib-0.4.0.tar.gz", hash = "sha256:feec40075be03a04501a973d81f633735b4b69f98b05450592310c0f401a4e0d"}, +] + +[[package]] +name = "filelock" +version = "3.29.0" +description = "A platform independent file lock." +optional = false +python-versions = ">=3.10" +groups = ["dev"] +files = [ + {file = "filelock-3.29.0-py3-none-any.whl", hash = "sha256:96f5f6344709aa1572bbf631c640e4ebeeb519e08da902c39a001882f30ac258"}, + {file = "filelock-3.29.0.tar.gz", hash = "sha256:69974355e960702e789734cb4871f884ea6fe50bd8404051a3530bc07809cf90"}, +] + [[package]] name = "gmqtt" version = "0.7.0" @@ -308,6 +344,21 @@ http2 = ["h2 (>=3,<5)"] socks = ["socksio (==1.*)"] zstd = ["zstandard (>=0.18.0)"] +[[package]] +name = "identify" +version = "2.6.19" +description = "File identification library for Python" +optional = false +python-versions = ">=3.10" +groups = ["dev"] +files = [ + {file = "identify-2.6.19-py2.py3-none-any.whl", hash = "sha256:20e6a87f786f768c092a721ad107fc9df0eb89347be9396cadf3f4abbd1fb78a"}, + {file = "identify-2.6.19.tar.gz", hash = "sha256:6be5020c38fcb07da56c53733538a3081ea5aa70d36a156f83044bfbf9173842"}, +] + +[package.extras] +license = ["ukkonen"] + [[package]] name = "idna" version = "3.11" @@ -566,6 +617,18 @@ files = [ {file = "mypy_extensions-1.1.0.tar.gz", hash = "sha256:52e68efc3284861e772bbcd66823fde5ae21fd2fdb51c62a211403730b916558"}, ] +[[package]] +name = "nodeenv" +version = "1.10.0" +description = "Node.js virtual environment builder" +optional = false +python-versions = "!=3.0.*,!=3.1.*,!=3.2.*,!=3.3.*,!=3.4.*,!=3.5.*,!=3.6.*,>=2.7" +groups = ["dev"] +files = [ + {file = "nodeenv-1.10.0-py2.py3-none-any.whl", hash = "sha256:5bb13e3eed2923615535339b3c620e76779af4cb4c6a90deccc9e36b274d3827"}, + {file = "nodeenv-1.10.0.tar.gz", hash = "sha256:996c191ad80897d076bdfba80a41994c2b47c68e224c542b48feba42ba00f8bb"}, +] + [[package]] name = "packaging" version = "26.0" @@ -624,6 +687,25 @@ files = [ dev = ["pre-commit", "tox"] testing = ["coverage", "pytest", "pytest-benchmark"] +[[package]] +name = "pre-commit" +version = "4.6.0" +description = "A framework for managing and maintaining multi-language pre-commit hooks." +optional = false +python-versions = ">=3.10" +groups = ["dev"] +files = [ + {file = "pre_commit-4.6.0-py2.py3-none-any.whl", hash = "sha256:e2cf246f7299edcabcf15f9b0571fdce06058527f0a06535068a86d38089f29b"}, + {file = "pre_commit-4.6.0.tar.gz", hash = "sha256:718d2208cef53fdc38206e40524a6d4d9576d103eb16f0fec11c875e7716e9d9"}, +] + +[package.dependencies] +cfgv = ">=2.0.0" +identify = ">=1.0.0" +nodeenv = ">=0.11.1" +pyyaml = ">=5.1" +virtualenv = ">=20.10.0" + [[package]] name = "pycryptodome" version = "3.23.0" @@ -795,6 +877,26 @@ pytest = ">=6.2.5" [package.extras] dev = ["pre-commit", "pytest-asyncio", "tox"] +[[package]] +name = "python-discovery" +version = "1.3.0" +description = "Python interpreter discovery" +optional = false +python-versions = ">=3.8" +groups = ["dev"] +files = [ + {file = "python_discovery-1.3.0-py3-none-any.whl", hash = "sha256:441d9ced3dfce36e113beb35ca302c71c7ef06f3c0f9c227a0b9bb3bd49b9e9f"}, + {file = "python_discovery-1.3.0.tar.gz", hash = "sha256:d098f1e86be5d45fe4d14bf1029294aabbd332f4321179dec85e76cddce834b0"}, +] + +[package.dependencies] +filelock = ">=3.15.4" +platformdirs = ">=4.3.6,<5" + +[package.extras] +docs = ["furo (>=2025.12.19)", "sphinx (>=9.1)", "sphinx-autodoc-typehints (>=3.6.3)", "sphinxcontrib-mermaid (>=2)"] +testing = ["covdefaults (>=2.3)", "coverage (>=7.5.4)", "pytest (>=8.3.5)", "pytest-mock (>=3.14)", "setuptools (>=75.1)"] + [[package]] name = "python-dotenv" version = "1.2.2" @@ -810,6 +912,89 @@ files = [ [package.extras] cli = ["click (>=5.0)"] +[[package]] +name = "pyyaml" +version = "6.0.3" +description = "YAML parser and emitter for Python" +optional = false +python-versions = ">=3.8" +groups = ["dev"] +files = [ + {file = "PyYAML-6.0.3-cp38-cp38-macosx_10_13_x86_64.whl", hash = "sha256:c2514fceb77bc5e7a2f7adfaa1feb2fb311607c9cb518dbc378688ec73d8292f"}, + {file = "PyYAML-6.0.3-cp38-cp38-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:9c57bb8c96f6d1808c030b1687b9b5fb476abaa47f0db9c0101f5e9f394e97f4"}, + {file = "PyYAML-6.0.3-cp38-cp38-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:efd7b85f94a6f21e4932043973a7ba2613b059c4a000551892ac9f1d11f5baf3"}, + {file = "PyYAML-6.0.3-cp38-cp38-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:22ba7cfcad58ef3ecddc7ed1db3409af68d023b7f940da23c6c2a1890976eda6"}, + {file = "PyYAML-6.0.3-cp38-cp38-musllinux_1_2_x86_64.whl", hash = "sha256:6344df0d5755a2c9a276d4473ae6b90647e216ab4757f8426893b5dd2ac3f369"}, + {file = "PyYAML-6.0.3-cp38-cp38-win32.whl", hash = "sha256:3ff07ec89bae51176c0549bc4c63aa6202991da2d9a6129d7aef7f1407d3f295"}, + {file = "PyYAML-6.0.3-cp38-cp38-win_amd64.whl", hash = "sha256:5cf4e27da7e3fbed4d6c3d8e797387aaad68102272f8f9752883bc32d61cb87b"}, + {file = "pyyaml-6.0.3-cp310-cp310-macosx_10_13_x86_64.whl", hash = "sha256:214ed4befebe12df36bcc8bc2b64b396ca31be9304b8f59e25c11cf94a4c033b"}, + {file = "pyyaml-6.0.3-cp310-cp310-macosx_11_0_arm64.whl", hash = "sha256:02ea2dfa234451bbb8772601d7b8e426c2bfa197136796224e50e35a78777956"}, + {file = "pyyaml-6.0.3-cp310-cp310-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:b30236e45cf30d2b8e7b3e85881719e98507abed1011bf463a8fa23e9c3e98a8"}, + {file = "pyyaml-6.0.3-cp310-cp310-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:66291b10affd76d76f54fad28e22e51719ef9ba22b29e1d7d03d6777a9174198"}, + {file = "pyyaml-6.0.3-cp310-cp310-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:9c7708761fccb9397fe64bbc0395abcae8c4bf7b0eac081e12b809bf47700d0b"}, + {file = "pyyaml-6.0.3-cp310-cp310-musllinux_1_2_aarch64.whl", hash = "sha256:418cf3f2111bc80e0933b2cd8cd04f286338bb88bdc7bc8e6dd775ebde60b5e0"}, + {file = "pyyaml-6.0.3-cp310-cp310-musllinux_1_2_x86_64.whl", hash = "sha256:5e0b74767e5f8c593e8c9b5912019159ed0533c70051e9cce3e8b6aa699fcd69"}, + {file = "pyyaml-6.0.3-cp310-cp310-win32.whl", hash = "sha256:28c8d926f98f432f88adc23edf2e6d4921ac26fb084b028c733d01868d19007e"}, + {file = "pyyaml-6.0.3-cp310-cp310-win_amd64.whl", hash = "sha256:bdb2c67c6c1390b63c6ff89f210c8fd09d9a1217a465701eac7316313c915e4c"}, + {file = "pyyaml-6.0.3-cp311-cp311-macosx_10_13_x86_64.whl", hash = "sha256:44edc647873928551a01e7a563d7452ccdebee747728c1080d881d68af7b997e"}, + {file = "pyyaml-6.0.3-cp311-cp311-macosx_11_0_arm64.whl", hash = "sha256:652cb6edd41e718550aad172851962662ff2681490a8a711af6a4d288dd96824"}, + {file = "pyyaml-6.0.3-cp311-cp311-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:10892704fc220243f5305762e276552a0395f7beb4dbf9b14ec8fd43b57f126c"}, + {file = "pyyaml-6.0.3-cp311-cp311-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:850774a7879607d3a6f50d36d04f00ee69e7fc816450e5f7e58d7f17f1ae5c00"}, + {file = "pyyaml-6.0.3-cp311-cp311-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:b8bb0864c5a28024fac8a632c443c87c5aa6f215c0b126c449ae1a150412f31d"}, + {file = "pyyaml-6.0.3-cp311-cp311-musllinux_1_2_aarch64.whl", hash = "sha256:1d37d57ad971609cf3c53ba6a7e365e40660e3be0e5175fa9f2365a379d6095a"}, + {file = "pyyaml-6.0.3-cp311-cp311-musllinux_1_2_x86_64.whl", hash = "sha256:37503bfbfc9d2c40b344d06b2199cf0e96e97957ab1c1b546fd4f87e53e5d3e4"}, + {file = "pyyaml-6.0.3-cp311-cp311-win32.whl", hash = "sha256:8098f252adfa6c80ab48096053f512f2321f0b998f98150cea9bd23d83e1467b"}, + {file = "pyyaml-6.0.3-cp311-cp311-win_amd64.whl", hash = "sha256:9f3bfb4965eb874431221a3ff3fdcddc7e74e3b07799e0e84ca4a0f867d449bf"}, + {file = "pyyaml-6.0.3-cp312-cp312-macosx_10_13_x86_64.whl", hash = "sha256:7f047e29dcae44602496db43be01ad42fc6f1cc0d8cd6c83d342306c32270196"}, + {file = "pyyaml-6.0.3-cp312-cp312-macosx_11_0_arm64.whl", hash = "sha256:fc09d0aa354569bc501d4e787133afc08552722d3ab34836a80547331bb5d4a0"}, + {file = "pyyaml-6.0.3-cp312-cp312-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:9149cad251584d5fb4981be1ecde53a1ca46c891a79788c0df828d2f166bda28"}, + {file = "pyyaml-6.0.3-cp312-cp312-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:5fdec68f91a0c6739b380c83b951e2c72ac0197ace422360e6d5a959d8d97b2c"}, + {file = "pyyaml-6.0.3-cp312-cp312-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:ba1cc08a7ccde2d2ec775841541641e4548226580ab850948cbfda66a1befcdc"}, + {file = "pyyaml-6.0.3-cp312-cp312-musllinux_1_2_aarch64.whl", hash = "sha256:8dc52c23056b9ddd46818a57b78404882310fb473d63f17b07d5c40421e47f8e"}, + {file = "pyyaml-6.0.3-cp312-cp312-musllinux_1_2_x86_64.whl", hash = "sha256:41715c910c881bc081f1e8872880d3c650acf13dfa8214bad49ed4cede7c34ea"}, + {file = "pyyaml-6.0.3-cp312-cp312-win32.whl", hash = "sha256:96b533f0e99f6579b3d4d4995707cf36df9100d67e0c8303a0c55b27b5f99bc5"}, + {file = "pyyaml-6.0.3-cp312-cp312-win_amd64.whl", hash = "sha256:5fcd34e47f6e0b794d17de1b4ff496c00986e1c83f7ab2fb8fcfe9616ff7477b"}, + {file = "pyyaml-6.0.3-cp312-cp312-win_arm64.whl", hash = "sha256:64386e5e707d03a7e172c0701abfb7e10f0fb753ee1d773128192742712a98fd"}, + {file = "pyyaml-6.0.3-cp313-cp313-macosx_10_13_x86_64.whl", hash = "sha256:8da9669d359f02c0b91ccc01cac4a67f16afec0dac22c2ad09f46bee0697eba8"}, + {file = "pyyaml-6.0.3-cp313-cp313-macosx_11_0_arm64.whl", hash = "sha256:2283a07e2c21a2aa78d9c4442724ec1eb15f5e42a723b99cb3d822d48f5f7ad1"}, + {file = "pyyaml-6.0.3-cp313-cp313-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:ee2922902c45ae8ccada2c5b501ab86c36525b883eff4255313a253a3160861c"}, + {file = "pyyaml-6.0.3-cp313-cp313-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:a33284e20b78bd4a18c8c2282d549d10bc8408a2a7ff57653c0cf0b9be0afce5"}, + {file = "pyyaml-6.0.3-cp313-cp313-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0f29edc409a6392443abf94b9cf89ce99889a1dd5376d94316ae5145dfedd5d6"}, + {file = "pyyaml-6.0.3-cp313-cp313-musllinux_1_2_aarch64.whl", hash = "sha256:f7057c9a337546edc7973c0d3ba84ddcdf0daa14533c2065749c9075001090e6"}, + {file = "pyyaml-6.0.3-cp313-cp313-musllinux_1_2_x86_64.whl", hash = "sha256:eda16858a3cab07b80edaf74336ece1f986ba330fdb8ee0d6c0d68fe82bc96be"}, + {file = "pyyaml-6.0.3-cp313-cp313-win32.whl", hash = "sha256:d0eae10f8159e8fdad514efdc92d74fd8d682c933a6dd088030f3834bc8e6b26"}, + {file = "pyyaml-6.0.3-cp313-cp313-win_amd64.whl", hash = "sha256:79005a0d97d5ddabfeeea4cf676af11e647e41d81c9a7722a193022accdb6b7c"}, + {file = "pyyaml-6.0.3-cp313-cp313-win_arm64.whl", hash = "sha256:5498cd1645aa724a7c71c8f378eb29ebe23da2fc0d7a08071d89469bf1d2defb"}, + {file = "pyyaml-6.0.3-cp314-cp314-macosx_10_13_x86_64.whl", hash = "sha256:8d1fab6bb153a416f9aeb4b8763bc0f22a5586065f86f7664fc23339fc1c1fac"}, + {file = "pyyaml-6.0.3-cp314-cp314-macosx_11_0_arm64.whl", hash = "sha256:34d5fcd24b8445fadc33f9cf348c1047101756fd760b4dacb5c3e99755703310"}, + {file = "pyyaml-6.0.3-cp314-cp314-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:501a031947e3a9025ed4405a168e6ef5ae3126c59f90ce0cd6f2bfc477be31b7"}, + {file = "pyyaml-6.0.3-cp314-cp314-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:b3bc83488de33889877a0f2543ade9f70c67d66d9ebb4ac959502e12de895788"}, + {file = "pyyaml-6.0.3-cp314-cp314-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:c458b6d084f9b935061bc36216e8a69a7e293a2f1e68bf956dcd9e6cbcd143f5"}, + {file = "pyyaml-6.0.3-cp314-cp314-musllinux_1_2_aarch64.whl", hash = "sha256:7c6610def4f163542a622a73fb39f534f8c101d690126992300bf3207eab9764"}, + {file = "pyyaml-6.0.3-cp314-cp314-musllinux_1_2_x86_64.whl", hash = "sha256:5190d403f121660ce8d1d2c1bb2ef1bd05b5f68533fc5c2ea899bd15f4399b35"}, + {file = "pyyaml-6.0.3-cp314-cp314-win_amd64.whl", hash = "sha256:4a2e8cebe2ff6ab7d1050ecd59c25d4c8bd7e6f400f5f82b96557ac0abafd0ac"}, + {file = "pyyaml-6.0.3-cp314-cp314-win_arm64.whl", hash = "sha256:93dda82c9c22deb0a405ea4dc5f2d0cda384168e466364dec6255b293923b2f3"}, + {file = "pyyaml-6.0.3-cp314-cp314t-macosx_10_13_x86_64.whl", hash = "sha256:02893d100e99e03eda1c8fd5c441d8c60103fd175728e23e431db1b589cf5ab3"}, + {file = "pyyaml-6.0.3-cp314-cp314t-macosx_11_0_arm64.whl", hash = "sha256:c1ff362665ae507275af2853520967820d9124984e0f7466736aea23d8611fba"}, + {file = "pyyaml-6.0.3-cp314-cp314t-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:6adc77889b628398debc7b65c073bcb99c4a0237b248cacaf3fe8a557563ef6c"}, + {file = "pyyaml-6.0.3-cp314-cp314t-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:a80cb027f6b349846a3bf6d73b5e95e782175e52f22108cfa17876aaeff93702"}, + {file = "pyyaml-6.0.3-cp314-cp314t-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:00c4bdeba853cc34e7dd471f16b4114f4162dc03e6b7afcc2128711f0eca823c"}, + {file = "pyyaml-6.0.3-cp314-cp314t-musllinux_1_2_aarch64.whl", hash = "sha256:66e1674c3ef6f541c35191caae2d429b967b99e02040f5ba928632d9a7f0f065"}, + {file = "pyyaml-6.0.3-cp314-cp314t-musllinux_1_2_x86_64.whl", hash = "sha256:16249ee61e95f858e83976573de0f5b2893b3677ba71c9dd36b9cf8be9ac6d65"}, + {file = "pyyaml-6.0.3-cp314-cp314t-win_amd64.whl", hash = "sha256:4ad1906908f2f5ae4e5a8ddfce73c320c2a1429ec52eafd27138b7f1cbe341c9"}, + {file = "pyyaml-6.0.3-cp314-cp314t-win_arm64.whl", hash = "sha256:ebc55a14a21cb14062aa4162f906cd962b28e2e9ea38f9b4391244cd8de4ae0b"}, + {file = "pyyaml-6.0.3-cp39-cp39-macosx_10_13_x86_64.whl", hash = "sha256:b865addae83924361678b652338317d1bd7e79b1f4596f96b96c77a5a34b34da"}, + {file = "pyyaml-6.0.3-cp39-cp39-macosx_11_0_arm64.whl", hash = "sha256:c3355370a2c156cffb25e876646f149d5d68f5e0a3ce86a5084dd0b64a994917"}, + {file = "pyyaml-6.0.3-cp39-cp39-manylinux2014_aarch64.manylinux_2_17_aarch64.manylinux_2_28_aarch64.whl", hash = "sha256:3c5677e12444c15717b902a5798264fa7909e41153cdf9ef7ad571b704a63dd9"}, + {file = "pyyaml-6.0.3-cp39-cp39-manylinux2014_s390x.manylinux_2_17_s390x.manylinux_2_28_s390x.whl", hash = "sha256:5ed875a24292240029e4483f9d4a4b8a1ae08843b9c54f43fcc11e404532a8a5"}, + {file = "pyyaml-6.0.3-cp39-cp39-manylinux2014_x86_64.manylinux_2_17_x86_64.manylinux_2_28_x86_64.whl", hash = "sha256:0150219816b6a1fa26fb4699fb7daa9caf09eb1999f3b70fb6e786805e80375a"}, + {file = "pyyaml-6.0.3-cp39-cp39-musllinux_1_2_aarch64.whl", hash = "sha256:fa160448684b4e94d80416c0fa4aac48967a969efe22931448d853ada8baf926"}, + {file = "pyyaml-6.0.3-cp39-cp39-musllinux_1_2_x86_64.whl", hash = "sha256:27c0abcb4a5dac13684a37f76e701e054692a9b2d3064b70f5e4eb54810553d7"}, + {file = "pyyaml-6.0.3-cp39-cp39-win32.whl", hash = "sha256:1ebe39cb5fc479422b83de611d14e2c0d3bb2a18bbcb01f229ab3cfbd8fee7a0"}, + {file = "pyyaml-6.0.3-cp39-cp39-win_amd64.whl", hash = "sha256:2e71d11abed7344e42a8849600193d15b6def118602c4c176f748e4583246007"}, + {file = "pyyaml-6.0.3.tar.gz", hash = "sha256:d76623373421df22fb4cf8817020cbb7ef15c725b9d5e45f17e189bfc384190f"}, +] + [[package]] name = "ruff" version = "0.15.5" @@ -928,7 +1113,25 @@ tzdata = {version = "*", markers = "platform_system == \"Windows\""} [package.extras] devenv = ["check-manifest", "pytest (>=4.3)", "pytest-cov", "pytest-mock (>=3.3)", "zest.releaser"] +[[package]] +name = "virtualenv" +version = "21.3.1" +description = "Virtual Python Environment builder" +optional = false +python-versions = ">=3.8" +groups = ["dev"] +files = [ + {file = "virtualenv-21.3.1-py3-none-any.whl", hash = "sha256:d1a71cf58f2f9228fff23a1f6ec15d39785c6b32e03658d104974247145edd35"}, + {file = "virtualenv-21.3.1.tar.gz", hash = "sha256:c2305bc1fddeec40699b8370d13f8d431b0701f00ce895061ce493aeded4426b"}, +] + +[package.dependencies] +distlib = ">=0.3.7,<1" +filelock = {version = ">=3.24.2,<4", markers = "python_version >= \"3.10\""} +platformdirs = ">=3.9.1,<5" +python-discovery = ">=1.2.2" + [metadata] lock-version = "2.1" python-versions = ">=3.12,<4.0" -content-hash = "895b2fe59d35a1326b819dc68c6a88aac237d81fd60c16076a13c4801e17dee2" +content-hash = "ba4f444f330a6cb58a06926a1c2701c544ffeaeda02107641361b7abaa61e4af" diff --git a/pyproject.toml b/pyproject.toml index c191824..afda254 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -41,6 +41,7 @@ pytest-asyncio = "^1.2.0" pytest-mock = "^3.14.0" mypy = "^1.15.0" pylint = "^4.0.0" +pre-commit = "^4.6.0" [tool.poetry.dependencies] saic-ismart-client-ng = { develop = true } diff --git a/src/extractors/__init__.py b/src/extractors/__init__.py index 4eb2fd0..2ebd343 100644 --- a/src/extractors/__init__.py +++ b/src/extractors/__init__.py @@ -39,6 +39,39 @@ def extract_electric_range( return None +def extract_soc_kwh( + charge_status: ChrgMgmtDataRespProcessingResult | None, + soc: float | None, +) -> float | None: + if ( + charge_status is not None + and (raw_soc_kwh := charge_status.soc_kwh) is not None + and (soc_kwh := __validate_and_convert_soc_kwh(raw_soc_kwh)) is not None + ): + LOG.debug("SoC kWh derived from realtimePower") + return soc_kwh + + if ( + soc is not None + and charge_status is not None + and ( + capacity := __validate_and_convert_soc_kwh( + charge_status.real_total_battery_capacity + ) + ) + is not None + ): + LOG.debug( + "SoC kWh computed from SoC%%=%s and capacity=%s kWh", + soc, + capacity, + ) + return round(soc / 100.0 * capacity, 2) + + LOG.warning("Could not extract a valid SoC kWh") + return None + + def extract_soc( vehicle_status: VehicleStatusRespProcessingResult, charge_status: ChrgMgmtDataRespProcessingResult | None, @@ -71,3 +104,9 @@ def __validate_and_convert_soc(raw_value: float) -> float | None: if value_in_range(raw_value, 0, 100.0, is_max_excl=False): return raw_value return None + + +def __validate_and_convert_soc_kwh(raw_value: float) -> float | None: + if raw_value > 0: + return raw_value + return None diff --git a/src/handlers/message.py b/src/handlers/message.py index ea43b6f..64bf508 100644 --- a/src/handlers/message.py +++ b/src/handlers/message.py @@ -54,10 +54,13 @@ async def __polling(self) -> None: if ( latest_message is not None and latest_message.messageId != self.last_message_id - and ensure_datetime_aware(latest_message.message_time) > self.last_message_ts + and ensure_datetime_aware(latest_message.message_time) + > self.last_message_ts ): self.last_message_id = latest_message.messageId - self.last_message_ts = ensure_datetime_aware(latest_message.message_time) + self.last_message_ts = ensure_datetime_aware( + latest_message.message_time + ) LOG.info( f"{latest_message.title} detected at {latest_message.message_time}" ) @@ -107,7 +110,8 @@ async def __get_all_alarm_messages(self) -> list[MessageEntity]: oldest_message = self.__get_oldest_message(all_messages) if ( oldest_message is not None - and ensure_datetime_aware(oldest_message.message_time) < self.last_message_ts + and ensure_datetime_aware(oldest_message.message_time) + < self.last_message_ts ): return all_messages except SaicLogoutException: @@ -121,7 +125,9 @@ async def __get_all_alarm_messages(self) -> list[MessageEntity]: return all_messages finally: idx = idx + 1 - LOG.warning("Reached max page limit (%d) while fetching alarm messages", max_pages) + LOG.warning( + "Reached max page limit (%d) while fetching alarm messages", max_pages + ) return all_messages async def __delete_message(self, message: MessageEntity) -> None: @@ -174,7 +180,9 @@ def __get_latest_message( ) -> MessageEntity | None: if len(vehicle_start_messages) == 0: return None - return max(vehicle_start_messages, key=lambda m: ensure_datetime_aware(m.message_time)) + return max( + vehicle_start_messages, key=lambda m: ensure_datetime_aware(m.message_time) + ) @staticmethod def __get_oldest_message( @@ -182,4 +190,6 @@ def __get_oldest_message( ) -> MessageEntity | None: if len(vehicle_start_messages) == 0: return None - return min(vehicle_start_messages, key=lambda m: ensure_datetime_aware(m.message_time)) + return min( + vehicle_start_messages, key=lambda m: ensure_datetime_aware(m.message_time) + ) diff --git a/src/integrations/openwb/__init__.py b/src/integrations/openwb/__init__.py index fbf7ea3..ece6583 100644 --- a/src/integrations/openwb/__init__.py +++ b/src/integrations/openwb/__init__.py @@ -65,7 +65,9 @@ def update_openwb( soc_ts_topic = self.__charging_station.soc_ts_topic if soc_ts_topic is not None: soc_ts = int(datetime.datetime.now(tz=datetime.UTC).timestamp()) - LOG.info("OpenWB Integration published SoC timestamp to %s", soc_ts_topic) + LOG.info( + "OpenWB Integration published SoC timestamp to %s", soc_ts_topic + ) self.__publisher.publish_int( key=soc_ts_topic, value=soc_ts, diff --git a/src/status_publisher/charge/chrg_mgmt_data_resp.py b/src/status_publisher/charge/chrg_mgmt_data_resp.py index 867bd38..a8066f8 100644 --- a/src/status_publisher/charge/chrg_mgmt_data_resp.py +++ b/src/status_publisher/charge/chrg_mgmt_data_resp.py @@ -41,6 +41,7 @@ class ChrgMgmtDataRespProcessingResult: real_total_battery_capacity: float raw_soc: int | None raw_fuel_range_elec: int | None + soc_kwh: float | None class ChrgMgmtDataRespPublisher( @@ -112,4 +113,7 @@ def publish( raw_fuel_range_elec=charge_status_result.raw_fuel_range_elec if charge_status_result is not None else None, + soc_kwh=charge_status_result.soc_kwh + if charge_status_result is not None + else None, ) diff --git a/src/status_publisher/charge/rvs_charge_status.py b/src/status_publisher/charge/rvs_charge_status.py index 98898cd..be709ba 100644 --- a/src/status_publisher/charge/rvs_charge_status.py +++ b/src/status_publisher/charge/rvs_charge_status.py @@ -21,6 +21,7 @@ class RvsChargeStatusProcessingResult: real_total_battery_capacity: float raw_fuel_range_elec: int | None + soc_kwh: float | None class RvsChargeStatusPublisher( @@ -38,7 +39,10 @@ def update_total_mileage(self, raw_mileage: int) -> None: def _is_valid_partial_mileage(self, raw_value: int) -> bool: if not value_in_range(raw_value, 0, 65535): return False - if self._last_total_mileage_raw is not None and raw_value > self._last_total_mileage_raw: + if ( + self._last_total_mileage_raw is not None + and raw_value > self._last_total_mileage_raw + ): LOG.warning( "Partial mileage %d exceeds total mileage %d, skipping", raw_value, @@ -98,13 +102,13 @@ def publish( validator=lambda x: x > 0, ) - self._transform_and_publish( - topic=mqtt_topics.DRIVETRAIN_SOC_KWH, - value=charge_status.realtimePower, - transform=lambda p: round( - (battery_capacity_correction_factor * p) / 10.0, 2 - ), - ) + soc_kwh: float | None = None + if charge_status.realtimePower: + soc_kwh = round( + (battery_capacity_correction_factor * charge_status.realtimePower) + / 10.0, + 2, + ) self._transform_and_publish( topic=mqtt_topics.DRIVETRAIN_LAST_CHARGE_ENDING_POWER, @@ -136,6 +140,7 @@ def publish( return RvsChargeStatusProcessingResult( real_total_battery_capacity=real_total_battery_capacity, raw_fuel_range_elec=charge_status.fuelRangeElec, + soc_kwh=soc_kwh, ) def get_actual_battery_capacity( diff --git a/src/vehicle.py b/src/vehicle.py index 731293d..8a36e40 100644 --- a/src/vehicle.py +++ b/src/vehicle.py @@ -15,7 +15,7 @@ TargetBatteryCode, ) -from extractors import extract_electric_range, extract_soc +from extractors import extract_electric_range, extract_soc, extract_soc_kwh import mqtt_topics from publisher.core import Publishable from status_publisher.charge.chrg_mgmt_data_resp import ( @@ -659,6 +659,13 @@ def update_data_conflicting_in_vehicle_and_bms( value=soc, ) + soc_kwh = extract_soc_kwh(charge_status, soc) + if soc_kwh is not None: + self.__publish( + topic=mqtt_topics.DRIVETRAIN_SOC_KWH, + value=soc_kwh, + ) + @property def user_timezone(self) -> ZoneInfo | None: return self.__user_timezone diff --git a/src/vehicle_info.py b/src/vehicle_info.py index bd174bf..2e4e2f9 100644 --- a/src/vehicle_info.py +++ b/src/vehicle_info.py @@ -64,12 +64,14 @@ def get_ac_temperature_idx(self, remote_ac_temperature: int) -> int: def min_ac_temperature(self) -> int: if self.series.startswith("EH32"): return 17 + if self.series.startswith("MZS3E"): + return 16 return 16 @property def max_ac_temperature(self) -> int: - if self.series.startswith("EH32"): - return 33 + if self.series.startswith("EH32") or self.series.startswith("MZS3E"): + return 31 return 28 def __get_property_by_code(self, property_name: str) -> str | None: @@ -122,6 +124,8 @@ def real_battery_capacity(self) -> float | None: result = self.__mg5_real_battery_capacity elif self.series.startswith("ZS EV"): result = self.__zs_ev_real_battery_capacity + elif self.series.startswith("MZS3E"): + result = self.__mgs5_real_battery_capacity if result is None: LOG.warning( @@ -141,6 +145,16 @@ def __mg4_real_battery_capacity(self) -> float | None: # MG4 with LFP battery return 51.0 + @property + def __mgs5_real_battery_capacity(self) -> float | None: + # From the datasheet + # Battery pack type 1 (49kWh) + # Battery pack type 2 (62.2kWh) + # Battery pack type 3 (64kWh) + if self.supports_target_soc: + return 64.0 + return 49.0 + @property def __cyberster_real_battery_capacity(self) -> float | None: # Model: MG Cyberster diff --git a/tests/status_publisher/test_rvs_charge_status.py b/tests/status_publisher/test_rvs_charge_status.py new file mode 100644 index 0000000..27378e1 --- /dev/null +++ b/tests/status_publisher/test_rvs_charge_status.py @@ -0,0 +1,69 @@ +from __future__ import annotations + +import unittest + +import pytest +from saic_ismart_client_ng.api.vehicle.schema import VehicleModelConfiguration, VinInfo +from saic_ismart_client_ng.api.vehicle_charging.schema import RvsChargeStatus + +from configuration import Configuration +from status_publisher.charge.rvs_charge_status import RvsChargeStatusPublisher +from tests.common_mocks import VIN +from tests.mocks import MessageCapturingConsolePublisher +from vehicle_info import VehicleInfo + +# EH32 S with BType=1 → real_battery_capacity = 64.0, raw=72.5 kWh +REAL_CAPACITY = 64.0 +RAW_CAPACITY = 72.5 +CORRECTION = REAL_CAPACITY / RAW_CAPACITY + + +def _make_publisher() -> tuple[ + RvsChargeStatusPublisher, MessageCapturingConsolePublisher +]: + config = Configuration() + config.anonymized_publishing = False + pub = MessageCapturingConsolePublisher(config) + vin_info = VinInfo() + vin_info.vin = VIN + vin_info.series = "EH32 S" + vin_info.modelName = "MG4 Electric" + vin_info.modelYear = "2022" + vin_info.vehicleModelConfiguration = [ + VehicleModelConfiguration("BType", "Battery", "1"), + ] + vehicle_info = VehicleInfo(vin_info, None) + return RvsChargeStatusPublisher(vehicle_info, pub, f"/vehicles/{VIN}"), pub + + +class TestRvsChargeStatusSocKwh(unittest.TestCase): + def setUp(self) -> None: + self.publisher, _ = _make_publisher() + + def test_soc_kwh_present_when_realtime_power_set(self) -> None: + charge_status = RvsChargeStatus( + realtimePower=int((42.0 / CORRECTION) * 10), + totalBatteryCapacity=int(RAW_CAPACITY * 10), + ) + result = self.publisher.publish(charge_status) + + assert result.soc_kwh is not None + assert result.soc_kwh == pytest.approx(42.0, abs=0.1) + + def test_soc_kwh_none_when_realtime_power_is_none(self) -> None: + charge_status = RvsChargeStatus( + realtimePower=None, + totalBatteryCapacity=int(RAW_CAPACITY * 10), + ) + result = self.publisher.publish(charge_status) + + assert result.soc_kwh is None + + def test_soc_kwh_none_when_realtime_power_is_zero(self) -> None: + charge_status = RvsChargeStatus( + realtimePower=0, + totalBatteryCapacity=int(RAW_CAPACITY * 10), + ) + result = self.publisher.publish(charge_status) + + assert result.soc_kwh is None diff --git a/tests/test_extractors.py b/tests/test_extractors.py new file mode 100644 index 0000000..e44be3b --- /dev/null +++ b/tests/test_extractors.py @@ -0,0 +1,55 @@ +from __future__ import annotations + +import pytest + +from extractors import extract_soc_kwh +from status_publisher.charge.chrg_mgmt_data_resp import ChrgMgmtDataRespProcessingResult + + +def _make_charge_result( + *, + soc_kwh: float | None = None, + real_total_battery_capacity: float = 64.0, +) -> ChrgMgmtDataRespProcessingResult: + return ChrgMgmtDataRespProcessingResult( + charge_current_limit=None, + target_soc=None, + scheduled_charging=None, + is_charging=None, + remaining_charging_time=None, + power=None, + real_total_battery_capacity=real_total_battery_capacity, + raw_soc=None, + raw_fuel_range_elec=None, + soc_kwh=soc_kwh, + ) + + +class TestExtractSocKwh: + def test_prefers_realtime_power_soc_kwh(self) -> None: + result = extract_soc_kwh(_make_charge_result(soc_kwh=42.0), soc=80.0) + assert result == pytest.approx(42.0) + + def test_fallback_to_soc_times_capacity(self) -> None: + # 80% of 64 kWh = 51.2 kWh + result = extract_soc_kwh(_make_charge_result(soc_kwh=None), soc=80.0) + assert result == pytest.approx(51.2) + + def test_fallback_returns_none_when_soc_is_none(self) -> None: + result = extract_soc_kwh(_make_charge_result(soc_kwh=None), soc=None) + assert result is None + + def test_fallback_returns_none_when_charge_status_is_none(self) -> None: + result = extract_soc_kwh(None, soc=80.0) + assert result is None + + def test_fallback_returns_none_when_capacity_is_zero(self) -> None: + result = extract_soc_kwh( + _make_charge_result(soc_kwh=None, real_total_battery_capacity=0.0), soc=80.0 + ) + assert result is None + + def test_fallback_used_when_soc_kwh_is_zero(self) -> None: + # soc_kwh=0 is not a valid primary reading; fall back to soc * capacity + result = extract_soc_kwh(_make_charge_result(soc_kwh=0.0), soc=80.0) + assert result == pytest.approx(51.2) diff --git a/tests/test_ha_discovery_windows.py b/tests/test_ha_discovery_windows.py index 8d2be70..cff3e6d 100644 --- a/tests/test_ha_discovery_windows.py +++ b/tests/test_ha_discovery_windows.py @@ -43,9 +43,7 @@ def _make_discovery( configs = [ VehicleModelConfiguration("BATTERY", "BATTERY", "1"), VehicleModelConfiguration("BType", "Battery", "1"), - VehicleModelConfiguration( - "S35", "Sunroof", "1" if has_sunroof else "0" - ), + VehicleModelConfiguration("S35", "Sunroof", "1" if has_sunroof else "0"), ] vin_info.vehicleModelConfiguration = configs vehicle_info = VehicleInfo(vin_info, None) @@ -97,9 +95,7 @@ def test_sunroof_published_as_binary_sensor_when_supported(self) -> None: discovery, publisher = self._make_discovery(has_sunroof=True) discovery.publish_ha_discovery_messages() - sunroof_topic = ( - f"homeassistant/binary_sensor/{VIN}_mg/{VIN}_sun_roof/config" - ) + sunroof_topic = f"homeassistant/binary_sensor/{VIN}_mg/{VIN}_sun_roof/config" assert sunroof_topic in publisher.map payload = json.loads(publisher.map[sunroof_topic]) assert "command_topic" not in payload @@ -109,9 +105,7 @@ def test_sunroof_unpublished_when_not_supported(self) -> None: discovery, publisher = self._make_discovery(has_sunroof=False) discovery.publish_ha_discovery_messages() - sunroof_binary = ( - f"homeassistant/binary_sensor/{VIN}_mg/{VIN}_sun_roof/config" - ) + sunroof_binary = f"homeassistant/binary_sensor/{VIN}_mg/{VIN}_sun_roof/config" assert sunroof_binary in publisher.map, ( "Expected unpublish message for binary_sensor Sun roof" ) diff --git a/tests/test_vehicle_handler.py b/tests/test_vehicle_handler.py index 9d77622..b624766 100644 --- a/tests/test_vehicle_handler.py +++ b/tests/test_vehicle_handler.py @@ -96,7 +96,7 @@ def setUp(self) -> None: vehicle_info = VehicleInfo(vin_info, None) account_prefix = f"/vehicles/{VIN}" scheduler = BlockingScheduler() - vehicle_state = VehicleState( + self.vehicle_state = VehicleState( self.publisher, scheduler, account_prefix, vehicle_info ) mock_relogin_handler = ReloginHandler( @@ -108,7 +108,7 @@ def setUp(self) -> None: self.saicapi, self.publisher, vehicle_info, - vehicle_state, + self.vehicle_state, ) async def test_update_vehicle_status(self) -> None: @@ -308,10 +308,6 @@ async def test_update_charge_status(self) -> None: ), DRIVETRAIN_MILEAGE_SINCE_LAST_CHARGE, ) - self.assert_mqtt_topic( - TestVehicleHandler.get_topic(mqtt_topics.DRIVETRAIN_SOC_KWH), - DRIVETRAIN_SOC_KWH, - ) self.assert_mqtt_topic( TestVehicleHandler.get_topic(mqtt_topics.DRIVETRAIN_CHARGING_TYPE), DRIVETRAIN_CHARGING_TYPE, @@ -359,7 +355,6 @@ async def test_update_charge_status(self) -> None: "/vehicles/vin10000000000000/drivetrain/remainingChargingTime", "/vehicles/vin10000000000000/refresh/lastChargeState", "/vehicles/vin10000000000000/drivetrain/totalBatteryCapacity", - "/vehicles/vin10000000000000/drivetrain/soc_kwh", "/vehicles/vin10000000000000/drivetrain/lastChargeEndingPower", "/vehicles/vin10000000000000/drivetrain/batteryHeating", "/vehicles/vin10000000000000/drivetrain/chargingCableLock", @@ -397,6 +392,55 @@ async def test_should_not_publish_same_data_twice(self) -> None: f"Some topics have been published from both car state and BMS state: {common_data!s}" ) + async def test_soc_kwh_published_after_full_cycle(self) -> None: + with ( + patch.object(self.saicapi, "get_vehicle_status") as mock_get_vehicle_status, + patch.object( + self.saicapi, "get_vehicle_charging_management_data" + ) as mock_get_charge, + ): + mock_get_vehicle_status.return_value = get_mock_vehicle_status_resp() + mock_get_charge.return_value = get_mock_charge_management_data_resp() + + _, vehicle_result = await self.vehicle_handler.update_vehicle_status() + _, charge_result = await self.vehicle_handler.update_charge_status() + + self.publisher.map.clear() + self.vehicle_state.update_data_conflicting_in_vehicle_and_bms( + vehicle_result, charge_result + ) + + self.assert_mqtt_topic( + TestVehicleHandler.get_topic(mqtt_topics.DRIVETRAIN_SOC_KWH), + DRIVETRAIN_SOC_KWH, + ) + + async def test_soc_kwh_fallback_when_realtime_power_missing(self) -> None: + charge_resp = get_mock_charge_management_data_resp() + charge_resp.rvsChargeStatus.realtimePower = None # type: ignore[union-attr] + + with ( + patch.object(self.saicapi, "get_vehicle_status") as mock_get_vehicle_status, + patch.object( + self.saicapi, "get_vehicle_charging_management_data" + ) as mock_get_charge, + ): + mock_get_vehicle_status.return_value = get_mock_vehicle_status_resp() + mock_get_charge.return_value = charge_resp + + _, vehicle_result = await self.vehicle_handler.update_vehicle_status() + _, charge_result = await self.vehicle_handler.update_charge_status() + + self.publisher.map.clear() + self.vehicle_state.update_data_conflicting_in_vehicle_and_bms( + vehicle_result, charge_result + ) + + soc_kwh_topic = TestVehicleHandler.get_topic(mqtt_topics.DRIVETRAIN_SOC_KWH) + assert soc_kwh_topic in self.publisher.map + # Fallback: DRIVETRAIN_SOC_BMS (96.3%) * 64.0 kWh ≈ 61.6 kWh + assert self.publisher.map[soc_kwh_topic] == pytest.approx(61.6, abs=0.1) + def assert_mqtt_topic(self, topic: str, value: Any) -> None: mqtt_map = self.publisher.map if topic in mqtt_map: diff --git a/tests/test_vehicle_state.py b/tests/test_vehicle_state.py index faff700..b2c9d07 100644 --- a/tests/test_vehicle_state.py +++ b/tests/test_vehicle_state.py @@ -112,6 +112,7 @@ async def test_update_soc_with_bms_data(self) -> None: "/vehicles/vin10000000000000/refresh/lastActivity", "/vehicles/vin10000000000000/drivetrain/soc", "/vehicles/vin10000000000000/drivetrain/range", + "/vehicles/vin10000000000000/drivetrain/soc_kwh", } assert expected_topics == set(self.publisher.map.keys()) From 6060deaa46b2a20ace4190d36789db1640a6ad61 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 19:57:50 +0200 Subject: [PATCH 08/12] feat: add battery capacity support for MG4 Urban (AH4EM series) (#455) 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. --- src/vehicle_info.py | 11 +++++++++ tests/test_vehicle_info.py | 49 ++++++++++++++++++++++++++++++++++++++ 2 files changed, 60 insertions(+) create mode 100644 tests/test_vehicle_info.py diff --git a/src/vehicle_info.py b/src/vehicle_info.py index 2e4e2f9..1c2bec8 100644 --- a/src/vehicle_info.py +++ b/src/vehicle_info.py @@ -118,6 +118,8 @@ def real_battery_capacity(self) -> float | None: if self.series.startswith("EH32"): result = self.__mg4_real_battery_capacity + elif self.series.startswith("AH4EM"): + result = self.__mg4_urban_real_battery_capacity elif self.series.startswith("EC32"): result = self.__cyberster_real_battery_capacity elif self.series.startswith("EP2"): @@ -145,6 +147,15 @@ def __mg4_real_battery_capacity(self) -> float | None: # MG4 with LFP battery return 51.0 + @property + def __mg4_urban_real_battery_capacity(self) -> float | None: + # MG4 Urban Standard Range (LFP, 43kWh) — series AH4EM L + # Long Range (LFP, 54kWh) also uses LFP so supports_target_soc is False for both; + # return None until a series code for the 54kWh variant is confirmed. + if self.series == "AH4EM L": + return 43.0 + return None + @property def __mgs5_real_battery_capacity(self) -> float | None: # From the datasheet diff --git a/tests/test_vehicle_info.py b/tests/test_vehicle_info.py new file mode 100644 index 0000000..f21c6ac --- /dev/null +++ b/tests/test_vehicle_info.py @@ -0,0 +1,49 @@ +from __future__ import annotations + +import pytest +from saic_ismart_client_ng.api.vehicle.schema import VehicleModelConfiguration, VinInfo + +from tests.common_mocks import VIN +from vehicle_info import VehicleInfo + + +def _make_vehicle_info( + series: str, + model: str = "", + btype: str | None = None, + custom_battery_capacity: float | None = None, +) -> VehicleInfo: + vin_info = VinInfo() + vin_info.vin = VIN + vin_info.series = series + vin_info.modelName = model + if btype is not None: + vin_info.vehicleModelConfiguration = [ + VehicleModelConfiguration("BType", "Battery", btype), + ] + return VehicleInfo(vin_info, custom_battery_capacity) + + +class TestMg4RealBatteryCapacity: + def test_mg4_lfp_51kwh(self) -> None: + assert _make_vehicle_info("EH32 S", btype=None).real_battery_capacity == 51.0 + + def test_mg4_nmc_64kwh(self) -> None: + assert _make_vehicle_info("EH32 S", btype="1").real_battery_capacity == 64.0 + + def test_mg4_trophy_extended_range_77kwh(self) -> None: + assert _make_vehicle_info("EH32 S", model="EH32 X3").real_battery_capacity == 77.0 + + +class TestMg4UrbanRealBatteryCapacity: + def test_standard_range_43kwh(self) -> None: + assert _make_vehicle_info("AH4EM L", model="MG4 EV URBAN").real_battery_capacity == 43.0 + + def test_unknown_urban_variant_returns_none(self) -> None: + # 54kWh Long Range series code not yet confirmed — should return None until a + # real device report is available so the user is prompted to set a custom capacity. + assert _make_vehicle_info("AH4EM LL", model="MG4 EV URBAN LR").real_battery_capacity is None + + def test_custom_capacity_overrides_lookup(self) -> None: + vi = _make_vehicle_info("AH4EM L", model="MG4 EV URBAN", custom_battery_capacity=54.0) + assert vi.real_battery_capacity == 54.0 From 15d0b68737839f4cb599513ec4ad87293d9fa28e Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 19:59:10 +0200 Subject: [PATCH 09/12] feat: default MG4 Urban non-L variants to 54kWh (#452) AH4EM S (and any other non-L Urban variant) defaults to 54kWh LFP. Closes #452. --- src/vehicle_info.py | 7 +++---- tests/test_vehicle_info.py | 7 +++---- 2 files changed, 6 insertions(+), 8 deletions(-) diff --git a/src/vehicle_info.py b/src/vehicle_info.py index 1c2bec8..e98ffbb 100644 --- a/src/vehicle_info.py +++ b/src/vehicle_info.py @@ -149,12 +149,11 @@ def __mg4_real_battery_capacity(self) -> float | None: @property def __mg4_urban_real_battery_capacity(self) -> float | None: - # MG4 Urban Standard Range (LFP, 43kWh) — series AH4EM L - # Long Range (LFP, 54kWh) also uses LFP so supports_target_soc is False for both; - # return None until a series code for the 54kWh variant is confirmed. + # MG4 Urban: "L" suffix = low/standard trim (43kWh LFP, series AH4EM L) + # All other variants (e.g. AH4EM S) default to 54kWh LFP if self.series == "AH4EM L": return 43.0 - return None + return 54.0 @property def __mgs5_real_battery_capacity(self) -> float | None: diff --git a/tests/test_vehicle_info.py b/tests/test_vehicle_info.py index f21c6ac..af9d56f 100644 --- a/tests/test_vehicle_info.py +++ b/tests/test_vehicle_info.py @@ -39,10 +39,9 @@ class TestMg4UrbanRealBatteryCapacity: def test_standard_range_43kwh(self) -> None: assert _make_vehicle_info("AH4EM L", model="MG4 EV URBAN").real_battery_capacity == 43.0 - def test_unknown_urban_variant_returns_none(self) -> None: - # 54kWh Long Range series code not yet confirmed — should return None until a - # real device report is available so the user is prompted to set a custom capacity. - assert _make_vehicle_info("AH4EM LL", model="MG4 EV URBAN LR").real_battery_capacity is None + def test_long_range_54kwh(self) -> None: + # AH4EM S = Long Range (54kWh LFP), confirmed via issue #452 + assert _make_vehicle_info("AH4EM S", model="MG4 EV URBAN").real_battery_capacity == 54.0 def test_custom_capacity_overrides_lookup(self) -> None: vi = _make_vehicle_info("AH4EM L", model="MG4 EV URBAN", custom_battery_capacity=54.0) From 9a522592b5ce0dac7a8418ac1427ffc149b0f81f Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 20:00:14 +0200 Subject: [PATCH 10/12] fix: remove unused pytest import, apply ruff formatting --- tests/test_vehicle_info.py | 19 ++++++++++++++----- 1 file changed, 14 insertions(+), 5 deletions(-) diff --git a/tests/test_vehicle_info.py b/tests/test_vehicle_info.py index af9d56f..d1d4121 100644 --- a/tests/test_vehicle_info.py +++ b/tests/test_vehicle_info.py @@ -1,6 +1,5 @@ from __future__ import annotations -import pytest from saic_ismart_client_ng.api.vehicle.schema import VehicleModelConfiguration, VinInfo from tests.common_mocks import VIN @@ -32,17 +31,27 @@ def test_mg4_nmc_64kwh(self) -> None: assert _make_vehicle_info("EH32 S", btype="1").real_battery_capacity == 64.0 def test_mg4_trophy_extended_range_77kwh(self) -> None: - assert _make_vehicle_info("EH32 S", model="EH32 X3").real_battery_capacity == 77.0 + assert ( + _make_vehicle_info("EH32 S", model="EH32 X3").real_battery_capacity == 77.0 + ) class TestMg4UrbanRealBatteryCapacity: def test_standard_range_43kwh(self) -> None: - assert _make_vehicle_info("AH4EM L", model="MG4 EV URBAN").real_battery_capacity == 43.0 + assert ( + _make_vehicle_info("AH4EM L", model="MG4 EV URBAN").real_battery_capacity + == 43.0 + ) def test_long_range_54kwh(self) -> None: # AH4EM S = Long Range (54kWh LFP), confirmed via issue #452 - assert _make_vehicle_info("AH4EM S", model="MG4 EV URBAN").real_battery_capacity == 54.0 + assert ( + _make_vehicle_info("AH4EM S", model="MG4 EV URBAN").real_battery_capacity + == 54.0 + ) def test_custom_capacity_overrides_lookup(self) -> None: - vi = _make_vehicle_info("AH4EM L", model="MG4 EV URBAN", custom_battery_capacity=54.0) + vi = _make_vehicle_info( + "AH4EM L", model="MG4 EV URBAN", custom_battery_capacity=54.0 + ) assert vi.real_battery_capacity == 54.0 From fb93d596ec4a98b2b995878302acf05b797bf910 Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 20:05:26 +0200 Subject: [PATCH 11/12] chore: add CLAUDE.md with project guidance for Claude Code --- CLAUDE.md | 96 +++++++++++++++++++++++++++++++++++++++++++++++++++++++ 1 file changed, 96 insertions(+) create mode 100644 CLAUDE.md diff --git a/CLAUDE.md b/CLAUDE.md new file mode 100644 index 0000000..4ec7bd0 --- /dev/null +++ b/CLAUDE.md @@ -0,0 +1,96 @@ +# CLAUDE.md + +This file provides guidance to Claude Code (claude.ai/code) when working with code in this repository. + +## Branching strategy + +- `main` — stable releases only +- `develop` — beta/integration branch; the default merge target for all feature and bugfix work + +**Always branch from `develop` for features and bugfixes.** PRs must target `develop`, not `main`. The only exception is a hotfix that must go directly to `main`. + +## Commands + +```bash +# Install dependencies (first time or after lockfile changes) +poetry install --no-root + +# Type check +poetry run mypy + +# Lint (ruff runs with --fix --unsafe-fixes in pre-commit) +poetry run ruff check . +poetry run ruff format . + +# Run all tests with coverage +poetry run pytest tests --cov + +# Run a single test file or test +poetry run pytest tests/test_vehicle_info.py +poetry run pytest tests/test_vehicle_info.py::TestMg4UrbanRealBatteryCapacity::test_standard_range_43kwh -v +``` + +Pre-commit hooks run `ruff`, `ruff-format`, `mypy`, and `poetry-check` on every commit. Pytest runs as a **pre-push** hook. Always run mypy and ruff before committing to avoid fixup commits. + +## Architecture + +### Data flow + +The gateway polls the SAIC cloud API on a per-vehicle schedule and bridges results to an MQTT broker. Incoming MQTT `/set` commands are forwarded back to the SAIC API. + +``` +SAIC Cloud API + ↓ (VehicleState.should_refresh() controls timing) +VehicleHandler.__polling() + ↓ +VehicleState.handle_vehicle_status() → VehicleStatusRespPublisher → MQTT +VehicleState.handle_charge_status() → ChrgMgmtDataRespPublisher → MQTT + ↓ +extractors.extract_soc/range() (cross-fuses BMS + vehicle status values) + ↓ +AbrpApi / OsmAndApi / OpenWBIntegration (optional side-effects) + +MQTT broker (/set topics) + ↓ +MqttGateway → VehicleHandler → VehicleCommandHandler → SAIC API +``` + +### Key modules + +**`src/mqtt_gateway.py`** — top-level orchestrator. Implements `MqttCommandListener` (MQTT callbacks) and `VehicleHandlerLocator` (VIN → handler lookup). + +**`src/vehicle.py` — `VehicleState`** — the polling state machine. Controls refresh timing via `PollingPhase` and `RefreshMode` enums. Exponential backoff on errors (doubles up to `refresh_period_inactive`). Polling is gated by `is_complete()` — all four refresh periods must be populated before the first poll. They are restored from retained MQTT messages on reconnect or defaulted by `configure_missing()` after a 10-second startup delay. + +**`src/handlers/vehicle.py` — `VehicleHandler`** — per-VIN lifecycle. Owns `VehicleState`, `VehicleCommandHandler`, all integrations, and HA discovery. The `handle_vehicle()` coroutine is a long-lived asyncio task. + +**`src/vehicle_info.py` — `VehicleInfo`** — static metadata derived from `VinInfo`. Holds series/model identity, vehicle configuration properties (e.g. `BType` for NMC/LFP battery type), battery capacity lookup, AC temperature mapping, and feature flags (`is_ev`, `has_sunroof`, etc.). `is_ev` is determined by series **not** starting with `"ZP22"`. + +**`src/publisher/core.py` — `Publisher`** — abstract base with typed publish methods. Handles topic sanitization, data anonymization, and LWT. The `publish(key, Publishable)` dispatcher checks `bool` before `int` (Python's `isinstance(True, int)` is `True`). + +**`src/handlers/command/`** — one `CommandHandlerBase` subclass per writable MQTT topic. All registered in `handlers/command/__init__.py::ALL_COMMAND_HANDLERS`. + +**`src/status_publisher/`** — stateless publishers for each API response type. Return frozen dataclasses that carry extracted values back up to `VehicleState` for cross-cutting decisions (e.g. BMS vs vehicle SoC reconciliation). + +**`src/extractors/__init__.py`** — pure functions that reconcile values present in both API responses. BMS values take precedence over vehicle status. + +### Battery capacity (`src/vehicle_info.py`) + +`real_battery_capacity` dispatches by `series` prefix to a vehicle-specific property. When adding a new model, add an `elif self.series.startswith(...)` branch and a corresponding `___real_battery_capacity` property. `supports_target_soc` (`BType == "1"`) distinguishes NMC from LFP where both share a series prefix. Custom capacity via `BATTERY_CAPACITY_MAPPING` (`VIN=kWh`) always overrides the lookup. + +### Integrations (`src/integrations/`) + +All optional, instantiated per-VIN: +- **Home Assistant**: MQTT auto-discovery. Re-published on broker reconnect or HA `online` LWT. +- **OpenWB**: subscribes to charger MQTT topics; triggers forced vehicle refresh on charge start; publishes SoC/range back to the charger. +- **ABRP / OsmAnd**: REST/HTTP telemetry push after each successful poll. + +### MQTT topic structure + +``` +//vehicles/// # status +//vehicles////set # writable +//account/... # account-level +/_internal/api/... # raw API debug +``` + +All topic constants are in `src/mqtt_topics.py`. From 35f1dcfb828780b6c602f1db086b925d13ee486e Mon Sep 17 00:00:00 2001 From: Giovanni Condello Date: Wed, 24 Jun 2026 20:06:30 +0200 Subject: [PATCH 12/12] chore: add /new-branch and /ship project skills --- .claude/commands/new-branch.md | 8 ++++++++ .claude/commands/ship.md | 11 +++++++++++ 2 files changed, 19 insertions(+) create mode 100644 .claude/commands/new-branch.md create mode 100644 .claude/commands/ship.md diff --git a/.claude/commands/new-branch.md b/.claude/commands/new-branch.md new file mode 100644 index 0000000..7eb9466 --- /dev/null +++ b/.claude/commands/new-branch.md @@ -0,0 +1,8 @@ +Create a new branch from the latest `develop` for a feature or bugfix. + +Steps: +1. Fetch and check out `develop`, pull the latest changes. +2. Create and check out a new branch named after the issue or feature. Use the format `feature/` for features and `fix/` for bugfixes, where the slug is a short kebab-case description. If the user mentioned an issue number, prefix it: e.g. `fix/455-mg4-urban-battery`. +3. Confirm the new branch name to the user. + +Do not push the branch yet. diff --git a/.claude/commands/ship.md b/.claude/commands/ship.md new file mode 100644 index 0000000..f6d1614 --- /dev/null +++ b/.claude/commands/ship.md @@ -0,0 +1,11 @@ +Run quality checks, commit staged changes, push, and open a PR targeting `develop`. + +Steps: +1. Run `poetry run ruff check . --fix --unsafe-fixes && poetry run ruff format .` and fix any remaining issues. +2. Run `poetry run mypy` — fix all type errors before continuing. +3. Run `poetry run pytest tests` — fix any failures before continuing. +4. Show a `git diff --staged` summary and ask the user to confirm the commit message, or draft one following the repo convention (`feat:`, `fix:`, `chore:`, etc.). +5. Commit, push the current branch, and open a PR with `gh pr create --base develop`. Include "Closes #" in the PR body if an issue number is known. +6. Return the PR URL. + +Do not proceed past any failing step — fix the issue first.