From b226f3dc96b2d798af44e262245995bc326e6468 Mon Sep 17 00:00:00 2001 From: Colin L Rice Date: Mon, 1 Jun 2026 21:32:17 -0400 Subject: [PATCH 1/6] Add typing to everything --- openevsehttp/client.py | 55 +++++++++++++++++++++++--------------- openevsehttp/commands.py | 12 ++++----- openevsehttp/properties.py | 18 ++++++------- openevsehttp/websocket.py | 53 ++++++++++++++++++++---------------- pyproject.toml | 3 +++ tests/test_commands.py | 2 +- 6 files changed, 82 insertions(+), 61 deletions(-) diff --git a/openevsehttp/client.py b/openevsehttp/client.py index 6a799f9..02bc7da 100644 --- a/openevsehttp/client.py +++ b/openevsehttp/client.py @@ -7,10 +7,10 @@ import json import logging import threading -from collections.abc import Callable, Mapping +from collections.abc import Callable, Mapping, MutableMapping from typing import Any -import aiohttp # type: ignore +import aiohttp from aiohttp.client_exceptions import ContentTypeError, ServerTimeoutError from awesomeversion import AwesomeVersion from awesomeversion.exceptions import AwesomeVersionCompareException @@ -56,15 +56,15 @@ def __init__( self._user = user self._pwd = pwd self.url = f"http://{host}/" - self._status: dict = {} - self._config: dict = {} - self._override = None + self._status: dict[str, Any] = {} + self._config: dict[str, Any] = {} + self._override: Any = None self._ws_listening = False self.websocket: OpenEVSEWebsocket | None = None - self.callback: Callable | None = None + self.callback: Callable[[], Any] | None = None self._loop: asyncio.AbstractEventLoop | None = None - self._ws_listen_task: asyncio.Task | None = None - self._ws_keepalive_task: asyncio.Task | None = None + self._ws_listen_task: asyncio.Task[Any] | None = None + self._ws_keepalive_task: asyncio.Task[Any] | None = None self._owns_loop = False self._loop_thread: threading.Thread | None = None self._session = session @@ -130,16 +130,21 @@ async def _process_request_with_session( kwargs["json"] = data async with http_method(url, **kwargs) as resp: try: - message = await resp.text() + raw = await resp.text() except UnicodeDecodeError: _LOGGER.debug("Decoding error") - message = await resp.read() - message = message.decode(errors="replace") + raw = (await resp.read()).decode(errors="replace") + message: Mapping[str, Any] | list[Any] | str = raw try: - message = json.loads(message) + message = json.loads(raw) except ValueError: - _LOGGER.debug("Non JSON response: %s", message) + _LOGGER.debug("Non JSON response: %s", raw) + if not isinstance(message, dict | list | str): + _LOGGER.error( + "Unexpected JSON primitive response from %s: %r", url, message + ) + raise ParseJSONError if resp.status == 400: if isinstance(message, dict) and "msg" in message: @@ -170,7 +175,7 @@ async def _process_request_with_session( _LOGGER.error("Content error: %s", err.message) raise - async def send_command(self, command: str) -> tuple: + async def send_command(self, command: str) -> tuple[Any, Any]: """Send a RAPI command to the charger and parses the response.""" url = f"{self.url}r" data = {"json": 1, "rapi": command} @@ -220,13 +225,13 @@ async def update(self, force_status: bool = False) -> None: "Received non-JSON response from /config: %s", response ) - async def test_and_get(self) -> dict: + async def test_and_get(self) -> dict[str, Any]: """Test connection. Return model serial number as dict """ url = f"{self.url}config" - data = {} + data: dict[str, Any] = {} response = await self.process_request(url, method="get") if not isinstance(response, Mapping): @@ -276,7 +281,7 @@ def ws_start(self) -> None: self._start_listening() - def _start_listening(self): + def _start_listening(self) -> None: """Start the websocket listener.""" if not self._loop: try: @@ -300,7 +305,7 @@ def _start_listening(self): ) self._loop_thread.start() - async def _update_status(self, msgtype, data, error): + async def _update_status(self, msgtype: str, data: Any, error: Any) -> None: """Update data from websocket listener.""" if msgtype == SIGNAL_CONNECTION_STATE: uri = self.websocket.uri if self.websocket else "Unknown" @@ -327,7 +332,7 @@ async def _update_status(self, msgtype, data, error): elif msgtype == "data": _LOGGER.debug("Websocket data: %s", data) - if not isinstance(data, Mapping): + if not isinstance(data, MutableMapping): _LOGGER.warning("Received non-Mapping websocket data: %s", data) return @@ -354,7 +359,7 @@ async def _update_status(self, msgtype, data, error): if inspect.isawaitable(result): await result - async def _shutdown(self): + async def _shutdown(self) -> None: """Shutdown the websocket and tasks on the listener loop.""" tasks = [] if self._ws_keepalive_task: @@ -417,7 +422,7 @@ async def ws_disconnect(self) -> None: # Standard async disconnect for caller loop await self._shutdown() - def is_coroutine_function(self, callback): + def is_coroutine_function(self, callback: Any) -> bool: """Check if a callback is a coroutine function.""" return inspect.iscoroutinefunction(callback) @@ -428,7 +433,13 @@ def ws_state(self) -> Any | None: return STATE_STOPPED return self.websocket.state - async def repeat(self, interval, func, *args, **kwargs): + async def repeat( + self, + interval: float, + func: Callable[..., Any], + *args: Any, + **kwargs: Any, + ) -> None: """Run func every interval seconds. If func has not finished before *interval*, will run again diff --git a/openevsehttp/commands.py b/openevsehttp/commands.py index 2b3ff34..9355c85 100644 --- a/openevsehttp/commands.py +++ b/openevsehttp/commands.py @@ -7,7 +7,7 @@ from collections.abc import Mapping from typing import Any -import aiohttp # type: ignore +import aiohttp from aiohttp.client_exceptions import ContentTypeError, ServerTimeoutError from awesomeversion import AwesomeVersion from awesomeversion.exceptions import AwesomeVersionCompareException @@ -23,8 +23,8 @@ class CommandsMixin: """Mixin providing command methods for OpenEVSE.""" url: str - _status: dict - _config: dict + _status: dict[str, Any] + _config: dict[str, Any] _session: Any # These are defined in client.py @@ -36,7 +36,7 @@ async def process_request( ) -> Mapping[str, Any] | list[Any] | str: raise NotImplementedError - async def send_command(self, command: str) -> tuple: + async def send_command(self, command: str) -> tuple[Any, Any]: raise NotImplementedError async def update(self, force_status: bool = False) -> None: @@ -360,7 +360,7 @@ async def restart_evse(self) -> None: _LOGGER.debug("EVSE Restart response: %s", response) # Firmware version - async def firmware_check(self) -> dict | None: + async def firmware_check(self) -> dict[str, Any] | None: """Return the latest firmware version.""" if "version" not in self._config: # Throw warning if we can't find the version @@ -403,7 +403,7 @@ async def firmware_check(self) -> dict | None: async def _firmware_check_with_session( self, session: aiohttp.ClientSession, url: str, method: str - ) -> dict | None: + ) -> dict[str, Any] | None: """Process a firmware check request with a given session.""" http_method = getattr(session, method) _LOGGER.debug( diff --git a/openevsehttp/properties.py b/openevsehttp/properties.py index 4e3a574..b5a489a 100644 --- a/openevsehttp/properties.py +++ b/openevsehttp/properties.py @@ -17,8 +17,8 @@ class PropertiesMixin: """Mixin providing all @property accessors for OpenEVSE.""" - _status: dict - _config: dict + _status: dict[str, Any] + _config: dict[str, Any] # These are used by properties but defined in client.py def _version_check(self, min_version: str, max_version: str = "") -> bool: @@ -336,7 +336,7 @@ def protocol_version(self) -> str | None: @property def vehicle(self) -> bool: """Return if a vehicle is connected to the EVSE.""" - return self._status.get("vehicle", False) + return bool(self._status.get("vehicle", False)) @property def ota_update(self) -> bool: @@ -356,7 +356,7 @@ def ota_state(self) -> str | None: @property def manual_override(self) -> bool: """Return if Manual Override is set.""" - return self._status.get("manual_override", False) + return bool(self._status.get("manual_override", False)) @property def divertmode(self) -> str: @@ -472,12 +472,12 @@ def vehicle_eta(self) -> datetime | None: @property def min_amps(self) -> int: """Return the minimum amps.""" - return self._config.get("min_current_hard", MIN_AMPS) + return int(self._config.get("min_current_hard", MIN_AMPS)) @property def max_amps(self) -> int: """Return the maximum amps.""" - return self._config.get("max_current_hard", MAX_AMPS) + return int(self._config.get("max_current_hard", MAX_AMPS)) @property def mqtt_connected(self) -> bool: @@ -506,10 +506,10 @@ def freeram(self) -> int | None: # Safety counts @property - def checks_count(self) -> dict: + def checks_count(self) -> dict[str, Any]: """Return the safety checks counts.""" attributes = ("gfcicount", "nogndcount", "stuckcount") - counts = {} + counts: dict[str, Any] = {} if self._status is not None and set(attributes).issubset(self._status.keys()): counts["gfcicount"] = self._status["gfcicount"] counts["nogndcount"] = self._status["nogndcount"] @@ -534,4 +534,4 @@ def current_power(self) -> int: if not self._version_check("4.2.2"): _LOGGER.debug("Feature not supported for older firmware.") raise UnsupportedFeature - return self._status.get("power", 0) + return int(self._status.get("power", 0)) diff --git a/openevsehttp/websocket.py b/openevsehttp/websocket.py index 336fbe7..2c00c26 100644 --- a/openevsehttp/websocket.py +++ b/openevsehttp/websocket.py @@ -1,9 +1,13 @@ """Websocket class for OpenEVSE HTTP.""" +from __future__ import annotations + import asyncio import datetime import inspect import logging +from collections.abc import Awaitable, Callable +from typing import Any import aiohttp @@ -28,12 +32,12 @@ class OpenEVSEWebsocket: def __init__( self, - server, - callback, - user=None, - password=None, + server: str, + callback: Callable[[str, Any, Any], Any] | None, + user: str | None = None, + password: str | None = None, session: aiohttp.ClientSession | None = None, - ): + ) -> None: """Initialize a OpenEVSEWebsocket instance.""" self.session = session self._session_external = session is not None @@ -43,20 +47,20 @@ def __init__( self.callback = callback self._state = STATE_DISCONNECTED self.failed_attempts = 0 - self._error_reason = None - self._client = None - self._ping = None - self._pong = None - self._tasks: set[asyncio.Task] = set() + self._error_reason: Any = None + self._client: aiohttp.ClientWebSocketResponse | None = None + self._ping: datetime.datetime | None = None + self._pong: datetime.datetime | None = None + self._tasks: set[asyncio.Task[Any]] = set() self._listener_loop: asyncio.AbstractEventLoop | None = None @property - def state(self): + def state(self) -> str: """Return the current state.""" return self._state @state.setter - def state(self, value): + def state(self, value: str) -> None: """Setter that schedules the callback.""" self._state = value _LOGGER.debug("Websocket %s", value) @@ -90,7 +94,7 @@ def state(self, value): coro.close() self._error_reason = None - def _schedule_task(self, coro): + def _schedule_task(self, coro: Awaitable[Any]) -> None: """Schedule a task from a thread-safe context.""" try: task = asyncio.ensure_future(coro) @@ -103,7 +107,7 @@ def _schedule_task(self, coro): if hasattr(coro, "close"): coro.close() - async def _set_state(self, value): + async def _set_state(self, value: str) -> None: """Async helper to set the state and await the callback.""" self._state = value _LOGGER.debug("Websocket %s", value) @@ -114,11 +118,11 @@ async def _set_state(self, value): self._error_reason = None @staticmethod - def _get_uri(server): + def _get_uri(server: str) -> str: """Generate the websocket URI.""" return server[: server.rfind("/")].replace("http", "ws") + "/ws" - async def running(self): + async def running(self) -> None: """Open a persistent websocket connection and act on events.""" await self._ensure_session() await self._set_state(STATE_STARTING) @@ -128,6 +132,7 @@ async def running(self): auth = aiohttp.BasicAuth(self._user, self._password) try: + assert self.session is not None async with self.session.ws_connect( self.uri, heartbeat=15, @@ -156,7 +161,9 @@ async def running(self): await self._client.close() self._client = None - async def _handle_messages(self, ws_client): + async def _handle_messages( + self, ws_client: aiohttp.ClientWebSocketResponse + ) -> None: """Handle incoming websocket messages.""" async for message in ws_client: if self.state == STATE_STOPPED: @@ -183,7 +190,7 @@ async def _handle_messages(self, ws_client): _LOGGER.error("Websocket error") break - async def _handle_response_error(self, error): + async def _handle_response_error(self, error: aiohttp.ClientResponseError) -> None: """Handle ClientResponseError.""" if error.status == 401: _LOGGER.error("Credentials rejected: %s", error) @@ -193,7 +200,7 @@ async def _handle_response_error(self, error): self._error_reason = error await self._set_state(STATE_STOPPED) - async def _handle_connection_error(self, error): + async def _handle_connection_error(self, error: BaseException) -> None: """Handle connection errors.""" self.failed_attempts += 1 if self.failed_attempts > MAX_FAILED_ATTEMPTS: @@ -209,7 +216,7 @@ async def _handle_connection_error(self, error): await self._set_state(STATE_DISCONNECTED) await asyncio.sleep(retry_delay) - async def listen(self): + async def listen(self) -> None: """Start the listening websocket.""" await self._ensure_session() self.failed_attempts = 0 @@ -220,13 +227,13 @@ async def listen(self): finally: self._listener_loop = None - async def _ensure_session(self): + async def _ensure_session(self) -> None: """Ensure aiohttp.ClientSession exists.""" if self.session is None: self.session = aiohttp.ClientSession() self._session_external = False - async def close(self): + async def close(self) -> None: """Close the listening websocket.""" await self._set_state(STATE_STOPPED) @@ -244,7 +251,7 @@ async def close(self): await self.session.close() self.session = None - async def keepalive(self): + async def keepalive(self) -> None: """Send ping requests to websocket.""" if self._ping and self._pong: time_delta = self._pong - self._ping diff --git a/pyproject.toml b/pyproject.toml index a760362..80b1be0 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -36,3 +36,6 @@ requires = ["setuptools>=82.0.1", "setuptools-scm>=10.0.5"] build-backend = "setuptools.build_meta" [tool.setuptools_scm] + +[tool.mypy] +strict = true diff --git a/tests/test_commands.py b/tests/test_commands.py index 33b1e74..3565b5a 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -754,7 +754,7 @@ async def test_restart_evse_http_failure(test_charger, mock_aioclient): test_charger._config["version"] = "5.0.0" # 1. Test False reply - mock_aioclient.post(TEST_URL_RESTART, status=200, body="false") + mock_aioclient.post(TEST_URL_RESTART, status=200, body='{"msg": false}') with pytest.raises( RuntimeError, match=r"Failed to restart EVSE module via HTTP: \{'msg': False\}" ): From 3b67d2613ffa324f1377e7664251f3dc54ec6024 Mon Sep 17 00:00:00 2001 From: Colin L Rice Date: Mon, 1 Jun 2026 21:37:44 -0400 Subject: [PATCH 2/6] Fix typing issues with properties.py --- openevsehttp/properties.py | 49 ++++++++++++++++++++++---------------- openevsehttp/py.typed | 0 2 files changed, 28 insertions(+), 21 deletions(-) create mode 100644 openevsehttp/py.typed diff --git a/openevsehttp/properties.py b/openevsehttp/properties.py index b5a489a..ebcf2d7 100644 --- a/openevsehttp/properties.py +++ b/openevsehttp/properties.py @@ -5,7 +5,7 @@ import logging from collections.abc import Mapping from datetime import datetime, timedelta, timezone -from typing import Any +from typing import Any, cast from .const import MAX_AMPS, MIN_AMPS, states from .exceptions import UnsupportedFeature @@ -125,15 +125,15 @@ async def get_charge_current(self) -> int | None: @property def max_current(self) -> int | None: """Return the max current.""" - return self._status.get("max_current", None) + return cast("int | None", self._status.get("max_current", None)) @property def wifi_firmware(self) -> str | None: """Return the ESP firmware version.""" value = self._config.get("version") - if value is not None: - value = normalize_version(value) - return value + if value is None: + return None + return normalize_version(value) @property def ip_address(self) -> str | None: @@ -303,27 +303,30 @@ def usage_session(self) -> float | None: @property def total_day(self) -> float | None: """Get the total day energy usage.""" - return self._status.get("total_day", None) + return cast("float | None", self._status.get("total_day", None)) @property def total_week(self) -> float | None: """Get the total week energy usage.""" - return self._status.get("total_week", None) + return cast("float | None", self._status.get("total_week", None)) @property def total_month(self) -> float | None: """Get the total month energy usage.""" - return self._status.get("total_month", None) + return cast("float | None", self._status.get("total_month", None)) @property def total_year(self) -> float | None: """Get the total year energy usage.""" - return self._status.get("total_year", None) + return cast("float | None", self._status.get("total_year", None)) @property def has_limit(self) -> bool | None: """Return if a limit has been set.""" - return self._status.get("has_limit", self._status.get("limit", None)) + return cast( + "bool | None", + self._status.get("has_limit", self._status.get("limit", None)), + ) @property def protocol_version(self) -> str | None: @@ -394,7 +397,7 @@ def divert_active(self) -> bool: @property def wifi_serial(self) -> str | None: """Return wifi serial.""" - return self._config.get("wifi_serial", None) + return cast("str | None", self._config.get("wifi_serial", None)) @property def charging_power(self) -> float | None: @@ -415,12 +418,12 @@ def charging_power(self) -> float | None: @property def shaper_active(self) -> bool | None: """Return if shaper is active.""" - return self._status.get("shaper", None) + return cast("bool | None", self._status.get("shaper", None)) @property def shaper_live_power(self) -> int | None: """Return shaper live power reading.""" - return self._status.get("shaper_live_pwr", None) + return cast("int | None", self._status.get("shaper_live_pwr", None)) @property def shaper_available_current(self) -> float | None: @@ -433,7 +436,7 @@ def shaper_available_current(self) -> float | None: @property def shaper_max_power(self) -> int | None: """Return shaper live power reading.""" - return self._status.get("shaper_max_pwr", None) + return cast("int | None", self._status.get("shaper_max_pwr", None)) @property def shaper_updated(self) -> bool: @@ -444,13 +447,17 @@ def shaper_updated(self) -> bool: @property def vehicle_soc(self) -> int | None: """Return battery level.""" - return self._status.get("vehicle_soc", self._status.get("battery_level", None)) + return cast( + "int | None", + self._status.get("vehicle_soc", self._status.get("battery_level", None)), + ) @property def vehicle_range(self) -> int | None: """Return battery range.""" - return self._status.get( - "vehicle_range", self._status.get("battery_range", None) + return cast( + "int | None", + self._status.get("vehicle_range", self._status.get("battery_range", None)), ) @property @@ -487,22 +494,22 @@ def mqtt_connected(self) -> bool: @property def emoncms_connected(self) -> bool | None: """Return the status of the emoncms connection.""" - return self._status.get("emoncms_connected", None) + return cast("bool | None", self._status.get("emoncms_connected", None)) @property def ocpp_connected(self) -> bool | None: """Return the status of the ocpp connection.""" - return self._status.get("ocpp_connected", None) + return cast("bool | None", self._status.get("ocpp_connected", None)) @property def uptime(self) -> int | None: """Return the unit uptime.""" - return self._status.get("uptime", None) + return cast("int | None", self._status.get("uptime", None)) @property def freeram(self) -> int | None: """Return the unit freeram.""" - return self._status.get("freeram", None) + return cast("int | None", self._status.get("freeram", None)) # Safety counts @property diff --git a/openevsehttp/py.typed b/openevsehttp/py.typed new file mode 100644 index 0000000..e69de29 From 932db44133c685722d5b59851e8de0573a11da88 Mon Sep 17 00:00:00 2001 From: "firstof9@gmail.com" Date: Tue, 2 Jun 2026 13:07:56 -0700 Subject: [PATCH 3/6] Add tests to reach 100% coverage --- tests/test_client.py | 12 ++++++++++++ tests/test_properties.py | 7 +++++++ 2 files changed, 19 insertions(+) diff --git a/tests/test_client.py b/tests/test_client.py index 6bf6eaa..6f496be 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1723,3 +1723,15 @@ async def test_update_status_ota(): assert charger.ota_update is False assert charger.ota_progress is None assert charger.ota_state == "completed" + + +async def test_process_request_invalid_json_primitive(mock_aioclient): + """Test process_request with an unexpected JSON primitive (e.g., bool or int).""" + charger = OpenEVSE(SERVER_URL) + mock_aioclient.get( + TEST_URL_STATUS, + status=200, + body="123", + ) + with pytest.raises(ParseJSONError): + await charger.process_request(TEST_URL_STATUS, method="get") diff --git a/tests/test_properties.py b/tests/test_properties.py index b590dba..ddbc583 100644 --- a/tests/test_properties.py +++ b/tests/test_properties.py @@ -1227,3 +1227,10 @@ async def test_config_boolean_coercion(): assert charger.vent_required_enabled is True assert charger.ground_check_enabled is True assert charger.stuck_relay_check_enabled is True + + +async def test_wifi_firmware_none(): + """Test wifi_firmware returns None when version is missing.""" + charger = OpenEVSE(SERVER_URL) + charger._config = {} + assert charger.wifi_firmware is None From ec278df5aed6de3c7378bc846e559d5b749dce10 Mon Sep 17 00:00:00 2001 From: "firstof9@gmail.com" Date: Tue, 2 Jun 2026 14:25:35 -0700 Subject: [PATCH 4/6] Address CodeRabbit review feedback and clean up types --- openevsehttp/client.py | 6 ++++-- openevsehttp/properties.py | 26 +++++++++++++------------- openevsehttp/websocket.py | 11 +++-------- tests/test_client.py | 28 ++++++++++++++++++++++------ tests/test_websocket.py | 38 ++++---------------------------------- 5 files changed, 46 insertions(+), 63 deletions(-) diff --git a/openevsehttp/client.py b/openevsehttp/client.py index 02bc7da..6ffb173 100644 --- a/openevsehttp/client.py +++ b/openevsehttp/client.py @@ -447,10 +447,12 @@ async def repeat( *args and **kwargs are passed as the arguments to func. """ - while self.ws_state != STATE_STOPPED and self._ws_listening: + while self.ws_state != STATE_STOPPED: await asyncio.sleep(interval) - if self.ws_state == STATE_STOPPED or not self._ws_listening: + if self.ws_state == STATE_STOPPED: break + if not self._ws_listening: + continue result = func(*args, **kwargs) if inspect.isawaitable(result): await result diff --git a/openevsehttp/properties.py b/openevsehttp/properties.py index ebcf2d7..e26517b 100644 --- a/openevsehttp/properties.py +++ b/openevsehttp/properties.py @@ -125,7 +125,7 @@ async def get_charge_current(self) -> int | None: @property def max_current(self) -> int | None: """Return the max current.""" - return cast("int | None", self._status.get("max_current", None)) + return cast(int | None, self._status.get("max_current", None)) @property def wifi_firmware(self) -> str | None: @@ -303,22 +303,22 @@ def usage_session(self) -> float | None: @property def total_day(self) -> float | None: """Get the total day energy usage.""" - return cast("float | None", self._status.get("total_day", None)) + return cast(float | None, self._status.get("total_day", None)) @property def total_week(self) -> float | None: """Get the total week energy usage.""" - return cast("float | None", self._status.get("total_week", None)) + return cast(float | None, self._status.get("total_week", None)) @property def total_month(self) -> float | None: """Get the total month energy usage.""" - return cast("float | None", self._status.get("total_month", None)) + return cast(float | None, self._status.get("total_month", None)) @property def total_year(self) -> float | None: """Get the total year energy usage.""" - return cast("float | None", self._status.get("total_year", None)) + return cast(float | None, self._status.get("total_year", None)) @property def has_limit(self) -> bool | None: @@ -397,7 +397,7 @@ def divert_active(self) -> bool: @property def wifi_serial(self) -> str | None: """Return wifi serial.""" - return cast("str | None", self._config.get("wifi_serial", None)) + return cast(str | None, self._config.get("wifi_serial", None)) @property def charging_power(self) -> float | None: @@ -418,12 +418,12 @@ def charging_power(self) -> float | None: @property def shaper_active(self) -> bool | None: """Return if shaper is active.""" - return cast("bool | None", self._status.get("shaper", None)) + return cast(bool | None, self._status.get("shaper", None)) @property def shaper_live_power(self) -> int | None: """Return shaper live power reading.""" - return cast("int | None", self._status.get("shaper_live_pwr", None)) + return cast(int | None, self._status.get("shaper_live_pwr", None)) @property def shaper_available_current(self) -> float | None: @@ -436,7 +436,7 @@ def shaper_available_current(self) -> float | None: @property def shaper_max_power(self) -> int | None: """Return shaper live power reading.""" - return cast("int | None", self._status.get("shaper_max_pwr", None)) + return cast(int | None, self._status.get("shaper_max_pwr", None)) @property def shaper_updated(self) -> bool: @@ -494,22 +494,22 @@ def mqtt_connected(self) -> bool: @property def emoncms_connected(self) -> bool | None: """Return the status of the emoncms connection.""" - return cast("bool | None", self._status.get("emoncms_connected", None)) + return cast(bool | None, self._status.get("emoncms_connected", None)) @property def ocpp_connected(self) -> bool | None: """Return the status of the ocpp connection.""" - return cast("bool | None", self._status.get("ocpp_connected", None)) + return cast(bool | None, self._status.get("ocpp_connected", None)) @property def uptime(self) -> int | None: """Return the unit uptime.""" - return cast("int | None", self._status.get("uptime", None)) + return cast(int | None, self._status.get("uptime", None)) @property def freeram(self) -> int | None: """Return the unit freeram.""" - return cast("int | None", self._status.get("freeram", None)) + return cast(int | None, self._status.get("freeram", None)) # Safety counts @property diff --git a/openevsehttp/websocket.py b/openevsehttp/websocket.py index 2c00c26..8ea9db8 100644 --- a/openevsehttp/websocket.py +++ b/openevsehttp/websocket.py @@ -80,14 +80,9 @@ def state(self, value: str) -> None: if self._listener_loop: self._listener_loop.call_soon_threadsafe(self._schedule_task, coro) else: - try: - task = asyncio.ensure_future(coro) - self._tasks.add(task) - task.add_done_callback(self._tasks.discard) - except RuntimeError: - # Fallback to get_event_loop if ensure_future fails and no _listener_loop - loop = asyncio.get_event_loop() - loop.call_soon_threadsafe(self._schedule_task, coro) + task = asyncio.ensure_future(coro) + self._tasks.add(task) + task.add_done_callback(self._tasks.discard) except RuntimeError: _LOGGER.error("Failed to schedule callback from sync context: %s", coro) if hasattr(coro, "close"): diff --git a/tests/test_client.py b/tests/test_client.py index 6f496be..389432a 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -742,15 +742,30 @@ async def test_repeat(): """Test repeat helper.""" charger = OpenEVSE(SERVER_URL) charger.websocket = MagicMock() - charger._ws_listening = True - # Mock ws_state to stop after one iteration + charger._ws_listening = False + # Mock ws_state to stop after second iteration (one continue, one run) with patch( "openevsehttp.__main__.OpenEVSE.ws_state", new_callable=PropertyMock ) as mock_state: - mock_state.side_effect = ["connected", "connected", "stopped", "stopped"] + mock_state.side_effect = [ + "connected", + "connected", + "connected", + "connected", + "stopped", + "stopped", + ] mock_func = AsyncMock() - with patch("asyncio.sleep", AsyncMock()): + sleep_count = 0 + + async def mock_sleep(interval): + nonlocal sleep_count + sleep_count += 1 + if sleep_count > 1: + charger._ws_listening = True + + with patch("asyncio.sleep", side_effect=mock_sleep): await charger.repeat(1, mock_func, "test") mock_func.assert_called_once_with("test") @@ -1725,13 +1740,14 @@ async def test_update_status_ota(): assert charger.ota_state == "completed" -async def test_process_request_invalid_json_primitive(mock_aioclient): +@pytest.mark.parametrize("body", ["123", "false", "null"]) +async def test_process_request_invalid_json_primitive(mock_aioclient, body): """Test process_request with an unexpected JSON primitive (e.g., bool or int).""" charger = OpenEVSE(SERVER_URL) mock_aioclient.get( TEST_URL_STATUS, status=200, - body="123", + body=body, ) with pytest.raises(ParseJSONError): await charger.process_request(TEST_URL_STATUS, method="get") diff --git a/tests/test_websocket.py b/tests/test_websocket.py index eb4cb29..7468d50 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -289,37 +289,8 @@ async def test_keepalive_send_exceptions(ws_client_auth): @pytest.mark.asyncio -async def test_state_setter_threadsafe_fallback(ws_client): - """Test state setter falls back to call_soon_threadsafe on RuntimeError.""" - mock_loop = MagicMock() - ws_client._error_reason = "Previous Error" - - with ( - patch("asyncio.ensure_future", side_effect=RuntimeError("No running loop")), - patch("asyncio.get_event_loop", return_value=mock_loop), - ): - ws_client.state = STATE_CONNECTED - assert ws_client.state == STATE_CONNECTED - - mock_loop.call_soon_threadsafe.assert_called_once() - - args, _ = mock_loop.call_soon_threadsafe.call_args - assert args[0] == ws_client._schedule_task - # Cover _schedule_task by manual invocation - with patch("asyncio.ensure_future") as mock_ct: - task = mock_ct.return_value - args[0](args[1]) - mock_ct.assert_called_once_with(args[1]) - assert task in ws_client._tasks - # Trigger cleanup - mock_ct.call_args[0][0].close() # close mock coro to avoid warning - # Manually trigger the done callback to cover discard - task.add_done_callback.call_args[0][0](task) - assert task not in ws_client._tasks - - assert ws_client._error_reason is None - - # Test state setter without callback coverage +async def test_state_setter_no_callback(ws_client): + """Test state setter without callback coverage.""" ws_client.callback = None ws_client.state = STATE_STOPPED assert ws_client.state == STATE_STOPPED @@ -339,14 +310,13 @@ async def test_websocket_sync_callback(ws_client): @pytest.mark.asyncio async def test_websocket_schedule_failure_sync(ws_client, mock_callback): - """Test state setter handles RuntimeError during call_soon_threadsafe.""" + """Test state setter handles RuntimeError during scheduling.""" # Use AsyncMock to ensure it's awaitable and triggers the try...except block async_mock = AsyncMock() - # Trigger RuntimeError in both create_task and get_event_loop/call_soon_threadsafe + # Trigger RuntimeError in create_task with ( patch("asyncio.ensure_future", side_effect=RuntimeError("No loop")), - patch("asyncio.get_event_loop", side_effect=RuntimeError("Loop closed")), patch("openevsehttp.websocket._LOGGER") as mock_logger, ): ws_client.callback = async_mock From 0beb6e55cb8e886468b1a3a643a0221d137550ac Mon Sep 17 00:00:00 2001 From: "firstof9@gmail.com" Date: Tue, 2 Jun 2026 15:24:57 -0700 Subject: [PATCH 5/6] Ensure _ws_listening is cleared on stop and add schedule success test --- openevsehttp/client.py | 14 +++++++------- tests/test_websocket.py | 21 +++++++++++++++++++++ 2 files changed, 28 insertions(+), 7 deletions(-) diff --git a/openevsehttp/client.py b/openevsehttp/client.py index 6ffb173..76f13be 100644 --- a/openevsehttp/client.py +++ b/openevsehttp/client.py @@ -321,13 +321,13 @@ async def _update_status(self, msgtype: str, data: Any, error: Any) -> None: self._ws_listening = False # Stopped websockets without errors are expected during shutdown - # and ignored - elif data == STATE_STOPPED and error: - _LOGGER.debug( - "Websocket to %s failed, aborting [Error: %s]", - uri, - error, - ) + elif data == STATE_STOPPED: + if error: + _LOGGER.debug( + "Websocket to %s failed, aborting [Error: %s]", + uri, + error, + ) self._ws_listening = False elif msgtype == "data": diff --git a/tests/test_websocket.py b/tests/test_websocket.py index 7468d50..5095aa8 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -296,6 +296,27 @@ async def test_state_setter_no_callback(ws_client): assert ws_client.state == STATE_STOPPED +@pytest.mark.asyncio +async def test_websocket_schedule_success_sync(ws_client): + """Test state setter schedules the callback successfully when outside listener loop.""" + # Ensure no listener loop is set, so ensure_future path is taken + ws_client._listener_loop = None + + # Trigger state change, which schedules callback + ws_client.state = STATE_CONNECTED + + # We should have scheduled 1 task + assert len(ws_client._tasks) == 1 + + # Let the loop run to execute the callback + await asyncio.gather(*ws_client._tasks) + + ws_client.callback.assert_called_with( + SIGNAL_CONNECTION_STATE, STATE_CONNECTED, None + ) + assert len(ws_client._tasks) == 0 + + @pytest.mark.asyncio async def test_websocket_sync_callback(ws_client): """Test state setter with a synchronous callback.""" From e5fe8af09efc5523709165a170967c2e95e17f7d Mon Sep 17 00:00:00 2001 From: "firstof9@gmail.com" Date: Tue, 2 Jun 2026 16:15:33 -0700 Subject: [PATCH 6/6] Address review feedback and fix JSON boolean primitive response handling --- openevsehttp/client.py | 40 +++++++++++++++++++++++---------------- openevsehttp/commands.py | 4 ++-- openevsehttp/managers.py | 2 +- openevsehttp/sensors.py | 2 +- openevsehttp/websocket.py | 1 + tests/test_client.py | 16 ++++++++++++++-- tests/test_commands.py | 2 +- tests/test_websocket.py | 12 ++++++++++++ 8 files changed, 56 insertions(+), 23 deletions(-) diff --git a/openevsehttp/client.py b/openevsehttp/client.py index 76f13be..c703a85 100644 --- a/openevsehttp/client.py +++ b/openevsehttp/client.py @@ -76,7 +76,7 @@ async def process_request( method: str = "", data: Any = None, rapi: Any = None, - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: """Return result of processed HTTP request.""" auth = None allowed_methods = ["get", "post", "put", "delete", "patch", "head", "options"] @@ -112,7 +112,7 @@ async def _process_request_with_session( data: Any, rapi: Any, auth: Any, - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: """Process a request with a given session.""" if not hasattr(session, method): raise MissingMethod @@ -135,38 +135,46 @@ async def _process_request_with_session( _LOGGER.debug("Decoding error") raw = (await resp.read()).decode(errors="replace") - message: Mapping[str, Any] | list[Any] | str = raw + # JSON responses can sometimes be primitive values (like bools). + # If json.loads fails with ValueError (e.g. non-JSON text/html), + # we fall back to treating the raw response as a string. + response_content: Mapping[str, Any] | list[Any] | str | bool = raw try: - message = json.loads(raw) + response_content = json.loads(raw) except ValueError: _LOGGER.debug("Non JSON response: %s", raw) - if not isinstance(message, dict | list | str): + if not isinstance(response_content, dict | list | str | bool): _LOGGER.error( - "Unexpected JSON primitive response from %s: %r", url, message + "Unexpected JSON primitive response from %s: %r", + url, + response_content, ) raise ParseJSONError if resp.status == 400: - if isinstance(message, dict) and "msg" in message: - _LOGGER.error("Error 400: %s", message["msg"]) - elif isinstance(message, dict) and "error" in message: - _LOGGER.error("Error 400: %s", message["error"]) + if isinstance(response_content, dict) and "msg" in response_content: + _LOGGER.error("Error 400: %s", response_content["msg"]) + elif ( + isinstance(response_content, dict) + and "error" in response_content + ): + _LOGGER.error("Error 400: %s", response_content["error"]) else: - _LOGGER.error("Error 400: %s", message) + _LOGGER.error("Error 400: %s", response_content) raise ParseJSONError if resp.status == 401: - _LOGGER.error("Authentication error: %s", message) + _LOGGER.error("Authentication error: %s", response_content) raise AuthenticationError if resp.status in [404, 405, 500]: - _LOGGER.warning("%s", message) + _LOGGER.warning("%s", response_content) if ( method.lower() != "get" - and isinstance(message, dict) - and any(key in message for key in UPDATE_TRIGGERS) + and isinstance(response_content, dict) + and any(key in response_content for key in UPDATE_TRIGGERS) ): await self.update() - return message + return response_content except (TimeoutError, ServerTimeoutError): _LOGGER.error("%s: %s", ERROR_TIMEOUT, url) diff --git a/openevsehttp/commands.py b/openevsehttp/commands.py index 9355c85..c73faea 100644 --- a/openevsehttp/commands.py +++ b/openevsehttp/commands.py @@ -33,7 +33,7 @@ def _version_check(self, min_version: str, max_version: str = "") -> bool: async def process_request( self, url: str, method: str = "", data: Any = None, rapi: Any = None - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: raise NotImplementedError async def send_command(self, command: str) -> tuple[Any, Any]: @@ -448,7 +448,7 @@ async def update_firmware( firmware_url: str | None = None, firmware_bytes: bytes | None = None, filename: str = "firmware.bin", - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: """Instruct the device to update its firmware. You can either: diff --git a/openevsehttp/managers.py b/openevsehttp/managers.py index 079c5ea..5c2cc1f 100644 --- a/openevsehttp/managers.py +++ b/openevsehttp/managers.py @@ -23,7 +23,7 @@ def _version_check(self, min_version: str, max_version: str = "") -> bool: async def process_request( self, url: str, method: str = "", data: Any = None, rapi: Any = None - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: raise NotImplementedError def _normalize_response(self, response: Any) -> dict[str, Any] | list[Any]: diff --git a/openevsehttp/sensors.py b/openevsehttp/sensors.py index ba730ef..1693925 100644 --- a/openevsehttp/sensors.py +++ b/openevsehttp/sensors.py @@ -23,7 +23,7 @@ def _version_check(self, min_version: str, max_version: str = "") -> bool: async def process_request( self, url: str, method: str = "", data: Any = None, rapi: Any = None - ) -> Mapping[str, Any] | list[Any] | str: + ) -> Mapping[str, Any] | list[Any] | str | bool: raise NotImplementedError def _normalize_response(self, response: Any) -> dict[str, Any] | list[Any]: diff --git a/openevsehttp/websocket.py b/openevsehttp/websocket.py index 8ea9db8..ac37843 100644 --- a/openevsehttp/websocket.py +++ b/openevsehttp/websocket.py @@ -127,6 +127,7 @@ async def running(self) -> None: auth = aiohttp.BasicAuth(self._user, self._password) try: + # Narrow type for mypy since _ensure_session sets self.session assert self.session is not None async with self.session.ws_connect( self.uri, diff --git a/tests/test_client.py b/tests/test_client.py index 389432a..bff5a34 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -1740,9 +1740,9 @@ async def test_update_status_ota(): assert charger.ota_state == "completed" -@pytest.mark.parametrize("body", ["123", "false", "null"]) +@pytest.mark.parametrize("body", ["123", "null"]) async def test_process_request_invalid_json_primitive(mock_aioclient, body): - """Test process_request with an unexpected JSON primitive (e.g., bool or int).""" + """Test process_request with an unexpected JSON primitive (e.g., int or null).""" charger = OpenEVSE(SERVER_URL) mock_aioclient.get( TEST_URL_STATUS, @@ -1751,3 +1751,15 @@ async def test_process_request_invalid_json_primitive(mock_aioclient, body): ) with pytest.raises(ParseJSONError): await charger.process_request(TEST_URL_STATUS, method="get") + + +async def test_process_request_boolean_primitive(mock_aioclient): + """Test process_request allows boolean JSON primitives (e.g., false).""" + charger = OpenEVSE(SERVER_URL) + mock_aioclient.get( + TEST_URL_STATUS, + status=200, + body="false", + ) + result = await charger.process_request(TEST_URL_STATUS, method="get") + assert result is False diff --git a/tests/test_commands.py b/tests/test_commands.py index 3565b5a..33b1e74 100644 --- a/tests/test_commands.py +++ b/tests/test_commands.py @@ -754,7 +754,7 @@ async def test_restart_evse_http_failure(test_charger, mock_aioclient): test_charger._config["version"] = "5.0.0" # 1. Test False reply - mock_aioclient.post(TEST_URL_RESTART, status=200, body='{"msg": false}') + mock_aioclient.post(TEST_URL_RESTART, status=200, body="false") with pytest.raises( RuntimeError, match=r"Failed to restart EVSE module via HTTP: \{'msg': False\}" ): diff --git a/tests/test_websocket.py b/tests/test_websocket.py index 5095aa8..f4bc7a3 100644 --- a/tests/test_websocket.py +++ b/tests/test_websocket.py @@ -524,3 +524,15 @@ async def test_websocket_state_task_management(ws_client): # Wait for task to complete await asyncio.gather(*ws_client._tasks) assert len(ws_client._tasks) == 0 + + +@pytest.mark.asyncio +async def test_websocket_close_cancels_pending_tasks(ws_client): + """Test close() cancels pending callback tasks.""" + # Trigger a task creation + ws_client.state = STATE_CONNECTED + assert len(ws_client._tasks) == 1 + + # Close should cancel and drain tasks + await ws_client.close() + assert len(ws_client._tasks) == 0