diff --git a/CHANGELOG.md b/CHANGELOG.md index dcddda11..e9602077 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 dbfd84c9..17347cfb 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 3816ae32..7173f372 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 1341d49c..86cd5b4f 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 ae9a84f3..99456ab5 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 cfac6343..f955e930 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 781f2cab..7c0778e3 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 00000000..270cac74 --- /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 2282cf81..2f5ae632 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 a8864a55..fb2ac5c6 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")