Skip to content

Commit

Permalink
switch to new websockets asyncio (#1439)
Browse files Browse the repository at this point in the history
* switch to new websockets asyncio

* add errors raised

* fix error raise

* extra detail on nosub test

* add test for autoconfig

* remove old tests

* update deprecated enums in v16 tests

* extra test for reboot required

* update pre-commit

* fix key

---------

Co-authored-by: lbbrhzn <[email protected]>
  • Loading branch information
drc38 and lbbrhzn authored Dec 25, 2024
1 parent 9bddf55 commit 85b794d
Show file tree
Hide file tree
Showing 10 changed files with 124 additions and 145 deletions.
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
Expand Up @@ -12,7 +12,7 @@ repos:
# Run the formatter.
- id: ruff-format
- repo: https://github.com/pre-commit/pre-commit-hooks
rev: v4.5.0
rev: v5.0.0
hooks:
- id: check-executables-have-shebangs
stages: [manual]
Expand Down
56 changes: 33 additions & 23 deletions custom_components/ocpp/api.py
Original file line number Diff line number Diff line change
Expand Up @@ -8,9 +8,9 @@
from homeassistant.config_entries import ConfigEntry
from homeassistant.const import STATE_OK
from homeassistant.core import HomeAssistant
from websockets import Subprotocol
import websockets.protocol
from websockets import Subprotocol, NegotiationError
import websockets.server
from websockets.asyncio.server import ServerConnection

from .chargepoint import CentralSystemSettings
from .ocppv16 import ChargePoint as ChargePointv16
Expand All @@ -21,7 +21,6 @@
CONF_CSID,
CONF_HOST,
CONF_PORT,
CONF_SKIP_SCHEMA_VALIDATION,
CONF_SSL,
CONF_SSL_CERTFILE_PATH,
CONF_SSL_KEYFILE_PATH,
Expand All @@ -34,7 +33,6 @@
DEFAULT_CSID,
DEFAULT_HOST,
DEFAULT_PORT,
DEFAULT_SKIP_SCHEMA_VALIDATION,
DEFAULT_SSL,
DEFAULT_SSL_CERTFILE_PATH,
DEFAULT_SSL_KEYFILE_PATH,
Expand Down Expand Up @@ -112,10 +110,11 @@ async def create(hass: HomeAssistant, entry: ConfigEntry):
"""Create instance and start listening for OCPP connections on given port."""
self = CentralSystem(hass, entry)

server = await websockets.server.serve(
server = await websockets.serve(
self.on_connect,
self.host,
self.port,
select_subprotocol=self.select_subprotocol,
subprotocols=self.subprotocols,
ping_interval=None, # ping interval is not used here, because we send pings mamually in ChargePoint.monitor_connection()
ping_timeout=None,
Expand All @@ -125,27 +124,38 @@ async def create(hass: HomeAssistant, entry: ConfigEntry):
self._server = server
return self

async def on_connect(self, websocket: websockets.server.WebSocketServerProtocol):
def select_subprotocol(
self, connection: ServerConnection, subprotocols
) -> Subprotocol | None:
"""Override default subprotocol selection."""

# Server offers at least one subprotocol but client doesn't offer any.
# Default to None
if not subprotocols:
return None

# Server and client both offer subprotocols. Look for a shared one.
proposed_subprotocols = set(subprotocols)
for subprotocol in proposed_subprotocols:
if subprotocol in self.subprotocols:
return subprotocol

# No common subprotocol was found.
raise NegotiationError(
"invalid subprotocol; expected one of " + ", ".join(self.subprotocols)
)

async def on_connect(self, websocket: ServerConnection):
"""Request handler executed for every new OCPP connection."""
if self.config.get(CONF_SKIP_SCHEMA_VALIDATION, DEFAULT_SKIP_SCHEMA_VALIDATION):
_LOGGER.warning("Skipping websocket subprotocol validation")
if websocket.subprotocol is not None:
_LOGGER.info("Websocket Subprotocol matched: %s", websocket.subprotocol)
else:
if websocket.subprotocol is not None:
_LOGGER.info("Websocket Subprotocol matched: %s", websocket.subprotocol)
else:
# In the websockets lib if no subprotocols are supported by the
# client and the server, it proceeds without a subprotocol,
# so we have to manually close the connection.
_LOGGER.warning(
"Protocols mismatched | expected Subprotocols: %s,"
" but client supports %s | Closing connection",
websocket.available_subprotocols,
websocket.request_headers.get("Sec-WebSocket-Protocol", ""),
)
return await websocket.close()
_LOGGER.info(
"Websocket Subprotocol not provided by charger: default to ocpp1.6"
)

_LOGGER.info(f"Charger websocket path={websocket.path}")
cp_id = websocket.path.strip("/")
_LOGGER.info(f"Charger websocket path={websocket.request.path}")
cp_id = websocket.request.path.strip("/")
cp_id = cp_id[cp_id.rfind("/") + 1 :]
if self.settings.cpid not in self.charge_points:
_LOGGER.info(f"Charger {cp_id} connected to {self.host}:{self.port}.")
Expand Down
12 changes: 7 additions & 5 deletions custom_components/ocpp/chargepoint.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,9 @@
from homeassistant.helpers import device_registry, entity_component, entity_registry
import homeassistant.helpers.config_validation as cv
import voluptuous as vol
import websockets.server
from websockets.asyncio.server import ServerConnection
from websockets.exceptions import WebSocketException
from websockets.protocol import State

from ocpp.charge_point import ChargePoint as cp
from ocpp.v16 import call as callv16
Expand Down Expand Up @@ -471,7 +473,7 @@ async def monitor_connection(self):
self._metrics[cstat.latency_pong.value].unit = "ms"
connection = self._connection
timeout_counter = 0
while connection.open:
while connection.state is State.OPEN:
try:
await asyncio.sleep(self.central.websocket_ping_interval)
time0 = time.perf_counter()
Expand Down Expand Up @@ -529,7 +531,7 @@ async def run(self, tasks):
await asyncio.gather(*self.tasks)
except TimeoutError:
pass
except websockets.exceptions.WebSocketException as websocket_exception:
except WebSocketException as websocket_exception:
_LOGGER.debug(f"Connection closed to '{self.id}': {websocket_exception}")
except Exception as other_exception:
_LOGGER.error(
Expand All @@ -542,13 +544,13 @@ async def run(self, tasks):
async def stop(self):
"""Close connection and cancel ongoing tasks."""
self.status = STATE_UNAVAILABLE
if self._connection.open:
if self._connection.state is State.OPEN:
_LOGGER.debug(f"Closing websocket to '{self.id}'")
await self._connection.close()
for task in self.tasks:
task.cancel()

async def reconnect(self, connection: websockets.server.WebSocketServerProtocol):
async def reconnect(self, connection: ServerConnection):
"""Reconnect charge point."""
_LOGGER.debug(f"Reconnect websocket to {self.id}")

Expand Down
2 changes: 1 addition & 1 deletion custom_components/ocpp/manifest.json
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
"issue_tracker": "https://github.com/lbbrhzn/ocpp/issues",
"requirements": [
"ocpp>=1.0.0",
"websockets>=12.0"
"websockets>=13.1"
],
"version": "0.6.1"
}
4 changes: 2 additions & 2 deletions custom_components/ocpp/ocppv16.py
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@
from homeassistant.config_entries import ConfigEntry
from homeassistant.core import HomeAssistant
import voluptuous as vol
import websockets.server
from websockets.asyncio.server import ServerConnection

from ocpp.routing import on
from ocpp.v16 import call, call_result
Expand Down Expand Up @@ -74,7 +74,7 @@ class ChargePoint(cp):
def __init__(
self,
id: str,
connection: websockets.server.WebSocketServerProtocol,
connection: ServerConnection,
hass: HomeAssistant,
entry: ConfigEntry,
central: CentralSystemSettings,
Expand Down
4 changes: 2 additions & 2 deletions custom_components/ocpp/ocppv201.py
Original file line number Diff line number Diff line change
Expand Up @@ -11,7 +11,7 @@
from homeassistant.const import UnitOfTime
from homeassistant.core import HomeAssistant, SupportsResponse, ServiceResponse
from homeassistant.exceptions import ServiceValidationError, HomeAssistantError
import websockets.server
from websockets.asyncio.server import ServerConnection

from ocpp.routing import on
from ocpp.v201 import call, call_result
Expand Down Expand Up @@ -85,7 +85,7 @@ class ChargePoint(cp):
def __init__(
self,
id: str,
connection: websockets.server.WebSocketServerProtocol,
connection: ServerConnection,
hass: HomeAssistant,
entry: ConfigEntry,
central: CentralSystemSettings,
Expand Down
7 changes: 4 additions & 3 deletions tests/charge_point_test.py
Original file line number Diff line number Diff line change
Expand Up @@ -23,7 +23,8 @@
from pytest_homeassistant_custom_component.common import MockConfigEntry
from typing import Any
from collections.abc import Callable, Awaitable
import websockets
from websockets import connect
from websockets.asyncio.client import ClientConnection


async def set_switch(hass: HomeAssistant, cs: CentralSystem, key: str, on: bool):
Expand Down Expand Up @@ -108,12 +109,12 @@ async def run_charge_point_test(
config_entry: MockConfigEntry,
identity: str,
subprotocols: list[str] | None,
charge_point: Callable[[websockets.WebSocketClientProtocol], ChargePoint],
charge_point: Callable[[ClientConnection], ChargePoint],
parallel_tests: list[Callable[[ChargePoint], Awaitable]],
) -> Any:
"""Connect web socket client to the CSMS and run a number of tests in parallel."""
completed: list[list[bool]] = [[] for _ in parallel_tests]
async with websockets.connect(
async with connect(
f"ws://127.0.0.1:{config_entry.data[CONF_PORT]}/{identity}",
subprotocols=[Subprotocol(s) for s in subprotocols]
if subprotocols is not None
Expand Down
8 changes: 4 additions & 4 deletions tests/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -35,11 +35,11 @@ def skip_notifications_fixture():
def bypass_get_data_fixture():
"""Skip calls to get data from API."""
future = asyncio.Future()
future.set_result(websockets.WebSocketServer)
future.set_result(websockets.asyncio.server.Server)
with (
patch("websockets.server.serve", return_value=future),
patch("websockets.server.WebSocketServer.close"),
patch("websockets.server.WebSocketServer.wait_closed"),
patch("websockets.asyncio.server.serve", return_value=future),
patch("websockets.asyncio.server.Server.close"),
patch("websockets.asyncio.server.Server.wait_closed"),
):
yield

Expand Down
1 change: 1 addition & 0 deletions tests/const.py
Original file line number Diff line number Diff line change
Expand Up @@ -76,6 +76,7 @@
CONF_PORT: 9002,
CONF_CPID: "test_cpid_2",
CONF_SKIP_SCHEMA_VALIDATION: True,
CONF_MONITORED_VARIABLES_AUTOCONFIG: False,
}

# separate entry for switch so tests can run concurrently
Expand Down
Loading

0 comments on commit 85b794d

Please sign in to comment.