Skip to content

Clean up timeout-related, pass _request-function as an argument#637

New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Merged
bouwew merged 10 commits into from
Oct 19, 2024
Merged

Clean up timeout-related, pass _request-function as an argument #637

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
10 commits
Select commit Hold shift + click to select a range
2014a72
Initial timeout cleaning
bouwew Oct 19, 2024
9f321d6
Pass _request() as an argument
bouwew Oct 19, 2024
2752d4b
Add typing
bouwew Oct 19, 2024
454cfe2
Update CHANGELOG
bouwew Oct 19, 2024
3701bcb
Bump to v1.4.3a0 test-version
bouwew Oct 19, 2024
9ba772b
Implement typing-suggestions
bouwew Oct 19, 2024
b98f66d
[pre-commit.ci] auto fixes from pre-commit.com hooks
pre-commit-ci[bot] Oct 19, 2024
9f2e81e
Bump to a1
bouwew Oct 19, 2024
df1aa98
Bump to v1.4.3 release-version
bouwew Oct 19, 2024
d893a1b
Final timeout cleanup
bouwew Oct 19, 2024
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
4 changes: 4 additions & 0 deletions CHANGELOG.md
View file
Open in desktop
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,9 @@
# Changelog

## v1.4.3

- Clean up timeout-related, pass _request-function as an argument.

## v1.4.2

- Bugfix: implement solution for issue reported in [#739](https://github.com/plugwise/plugwise-beta/issues/739)
Expand Down
9 changes: 2 additions & 7 deletions plugwise/__init__.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -6,7 +6,6 @@

from plugwise.constants import (
DEFAULT_PORT,
DEFAULT_TIMEOUT,
DEFAULT_USERNAME,
DOMAIN_OBJECTS,
LOGGER,
Expand DownExpand Up@@ -61,7 +60,6 @@ def __init__(
self._host = host
self._passwd = password
self._port = port
self._timeout = timeout
self._user = username
self._websession = websession

Expand DownExpand Up@@ -128,7 +126,7 @@ async def connect(self) -> bool:
self._smile_api = SmileAPI(
self._host,
self._passwd,
self._timeout,
self._request,
self._websession,
self._cooling_present,
self._elga,
Expand All@@ -152,7 +150,7 @@ async def connect(self) -> bool:
) if not self.smile_legacy else SmileLegacyAPI(
self._host,
self._passwd,
self._timeout,
self._request,
self._websession,
self._is_thermostat,
self._on_off_device,
Expand DownExpand Up@@ -194,9 +192,6 @@ async def _smile_detect(self, result: etree, dsmrmain: etree) -> None:
else:
model = await self._smile_detect_legacy(result, dsmrmain, model)

if not self.smile_legacy:
self._timeout = DEFAULT_TIMEOUT

if model == "Unknown" or self.smile_fw_version is None: # pragma: no cover
# Corner case check
LOGGER.error(
Expand Down
34 changes: 13 additions & 21 deletions plugwise/legacy/smile.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,6 +4,7 @@
"""
from __future__ import annotations

from collections.abc import Awaitable, Callable
import datetime as dt
from typing import Any

Expand All@@ -24,14 +25,13 @@
ThermoLoc,
)
from plugwise.exceptions import ConnectionFailedError, PlugwiseError
from plugwise.helper import SmileComm
from plugwise.legacy.data import SmileLegacyData

import aiohttp
from munch import Munch


class SmileLegacyAPI(SmileComm, SmileLegacyData):
class SmileLegacyAPI(SmileLegacyData):
"""The Plugwise SmileLegacyAPI class."""

# pylint: disable=too-many-instance-attributes, too-many-public-methods
Expand All@@ -40,7 +40,7 @@ def __init__(
self,
host: str,
password: str,
timeout: int,
request: Callable[..., Awaitable[Any]],
websession: aiohttp.ClientSession,
_is_thermostat: bool,
_on_off_device: bool,
Expand All@@ -60,14 +60,6 @@ def __init__(
username: str = DEFAULT_USERNAME,
) -> None:
"""Set the constructor for this class."""
super().__init__(
host,
password,
port,
timeout,
username,
websession,
)
SmileLegacyData.__init__(self)

self._cooling_present = False
Expand All@@ -76,8 +68,8 @@ def __init__(
self._opentherm_device = _opentherm_device
self._stretch_v2 = _stretch_v2
self._target_smile = _target_smile
self._timeout = timeout
self.loc_data = loc_data
self.request = request
self.smile_fw_version = smile_fw_version
self.smile_hostname = smile_hostname
self.smile_hw_version = smile_hw_version
Expand All@@ -91,12 +83,12 @@ def __init__(

async def full_update_device(self) -> None:
"""Perform a first fetch of all XML data, needed for initialization."""
self._domain_objects = await self._request(DOMAIN_OBJECTS)
self._locations = await self._request(LOCATIONS)
self._modules = await self._request(MODULES)
self._domain_objects = await self.request(DOMAIN_OBJECTS)
self._locations = await self.request(LOCATIONS)
self._modules = await self.request(MODULES)
# P1 legacy has no appliances
if self.smile_type != "power":
self._appliances = await self._request(APPLIANCES)
self._appliances = await self.request(APPLIANCES)

def get_all_devices(self) -> None:
"""Determine the evices present from the obtained XML-data.
Expand DownExpand Up@@ -131,12 +123,12 @@ async def async_update(self) -> PlugwiseData:
self.get_all_devices()
# Otherwise perform an incremental update
else:
self._domain_objects = await self._request(DOMAIN_OBJECTS)
self._domain_objects = await self.request(DOMAIN_OBJECTS)
match self._target_smile:
case "smile_v2":
self._modules = await self._request(MODULES)
self._modules = await self.request(MODULES)
case self._target_smile if self._target_smile in REQUIRE_APPLIANCES:
self._appliances = await self._request(APPLIANCES)
self._appliances = await self.request(APPLIANCES)
Comment thread
bouwew marked this conversation as resolved.

self._update_gw_devices()

Expand DownExpand Up@@ -291,10 +283,10 @@ async def set_temperature(self, _: str, items: dict[str, float]) -> None:
await self.call_request(uri, method="put", data=data)

async def call_request(self, uri: str, **kwargs: Any) -> None:
"""ConnectionFailedError wrapper for calling _request()."""
"""ConnectionFailedError wrapper for calling request()."""
method: str = kwargs["method"]
data: str | None = kwargs.get("data")
try:
await self._request(uri, method=method, data=data)
await self.request(uri, method=method, data=data)
except ConnectionFailedError as exc:
raise ConnectionFailedError from exc
22 changes: 7 additions & 15 deletions plugwise/smile.py
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,6 +4,7 @@
"""
from __future__ import annotations

from collections.abc import Awaitable, Callable

@coderabbitaicoderabbitaiBotOct 19, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

🛠️ Refactor suggestion

Use typing module for type hints to ensure compatibility

To maintain compatibility with Python versions earlier than 3.9 and for consistency with other type hints in the code, consider importing Awaitable and Callable from the typing module instead of collections.abc.

Apply this diff to modify the import statements:

-from collections.abc import Awaitable, Callable+from typing import Any, Awaitable, Callable

Committable suggestion was skipped due to low confidence.

import datetime as dt
from typing import Any

Expand All@@ -28,7 +29,6 @@
)
from plugwise.data import SmileData
from plugwise.exceptions import ConnectionFailedError, DataMissingError, PlugwiseError
from plugwise.helper import SmileComm

import aiohttp
from defusedxml import ElementTree as etree
Expand All@@ -37,7 +37,7 @@
from munch import Munch


class SmileAPI(SmileComm, SmileData):
class SmileAPI(SmileData):
"""The Plugwise SmileAPI helper class for actual Plugwise devices."""

# pylint: disable=too-many-instance-attributes, too-many-public-methods
Expand All@@ -46,7 +46,7 @@ def __init__(
self,
host: str,
password: str,
timeout: int,
request: Callable[..., Awaitable[Any]],
websession: aiohttp.ClientSession,
_cooling_present: bool,
_elga: bool,
Expand All@@ -69,14 +69,6 @@ def __init__(
username: str = DEFAULT_USERNAME,
) -> None:
"""Set the constructor for this class."""
super().__init__(
host,
password,
port,
timeout,
username,
websession,
)
SmileData.__init__(self)

self._cooling_present = _cooling_present
Expand All@@ -86,9 +78,9 @@ def __init__(
self._on_off_device = _on_off_device
self._opentherm_device = _opentherm_device
self._schedule_old_states = _schedule_old_states
self._timeout = timeout
self.gateway_id = gateway_id
self.loc_data = loc_data
self.request = request
self.smile_fw_version = smile_fw_version
self.smile_hostname = smile_hostname
self.smile_hw_version = smile_hw_version
Expand All@@ -103,7 +95,7 @@ def __init__(

async def full_update_device(self) -> None:
"""Perform a first fetch of all XML data, needed for initialization."""
self._domain_objects = await self._request(DOMAIN_OBJECTS)
self._domain_objects = await self.request(DOMAIN_OBJECTS)
self._get_plugwise_notifications()

def get_all_devices(self) -> None:
Expand DownExpand Up@@ -461,10 +453,10 @@ async def set_temperature(self, loc_id: str, items: dict[str, float]) -> None:
await self.call_request(uri, method="put", data=data)

async def call_request(self, uri: str, **kwargs: Any) -> None:
"""ConnectionFailedError wrapper for calling _request()."""
"""ConnectionFailedError wrapper for calling request()."""
method: str = kwargs["method"]
data: str | None = kwargs.get("data")
Comment on lines +456 to 458

@coderabbitaicoderabbitaiBotOct 19, 2024

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

⚠️ Potential issue

Prevent KeyError by validating the presence of 'method' in kwargs

Accessing kwargs["method"] without checking if the key exists can raise a KeyError if 'method' is not provided. To improve robustness, add a validation to ensure 'method' is present.

Apply this diff to handle the missing 'method' key:

 async def call_request(self, uri: str, **kwargs: Any) -> None:
"""ConnectionFailedError wrapper for calling request()."""
- method: str = kwargs["method"]+ method: str | None = kwargs.get("method")+ if method is None:+ raise ValueError("The 'method' argument is required.")
data: str | None = kwargs.get("data")
try:
await self.request(uri, method=method, data=data)
📝 Committable suggestion

‼️IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.

Suggested change
"""ConnectionFailedError wrapper for calling request()."""
method: str=kwargs["method"]
data: str|None=kwargs.get("data")
"""ConnectionFailedError wrapper for calling request()."""
method: str|None=kwargs.get("method")
ifmethodisNone:
raiseValueError("The 'method' argument is required.")
data: str|None=kwargs.get("data")

try:
await self._request(uri, method=method, data=data)
await self.request(uri, method=method, data=data)
except ConnectionFailedError as exc:
raise ConnectionFailedError from exc
2 changes: 1 addition & 1 deletion pyproject.toml
View file
Open in desktop
Original file line numberDiff line numberDiff line change
Expand Up@@ -4,7 +4,7 @@ build-backend = "setuptools.build_meta"

[project]
name = "plugwise"
version = "1.4.2"
version = "1.4.3"
license = {file = "LICENSE"}
description = "Plugwise Smile (Adam/Anna/P1) and Stretch module for Python 3."
readme = "README.md"
Expand Down