Skip to content

Commit 12bf1fb

Browse files
committed
Merge branch 'fix/scan-performance'
2 parents 77d1019 + 26ba332 commit 12bf1fb

18 files changed

Lines changed: 421 additions & 167 deletions

File tree

packages/api/src/api/controllers/devices.py

Lines changed: 0 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -66,7 +66,6 @@ async def scan_devices(
6666

6767
scan_request = ScanRequest(
6868
targets=targets or [],
69-
use_predefined=use_predefined,
7069
use_mdns=use_mdns,
7170
timeout=timeout,
7271
max_workers=max_workers,

packages/api/src/api/dependencies/container.py

Lines changed: 8 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -107,7 +107,8 @@ def get_authentication_service(self) -> AuthenticationService:
107107
def get_rpc_client(self) -> AsyncShellyRPCClient:
108108
if self._rpc_client is None:
109109
self._rpc_client = AsyncShellyRPCClient(
110-
timeout=int(core_settings.network.timeout),
110+
timeout=core_settings.network.timeout,
111+
connect_timeout=core_settings.network.connect_timeout,
111112
verify=core_settings.network.verify_ssl,
112113
authentication_service=self.get_authentication_service(),
113114
auth_state_cache=self.get_auth_state_cache(),
@@ -141,6 +142,12 @@ async def close(self) -> None:
141142
except Exception:
142143
pass
143144

145+
await self._aclose_legacy_http_client()
146+
147+
self._rpc_client = None
148+
self._credentials_use_case = None
149+
self._reset_device_caches()
150+
144151

145152
def get_dependencies(container: APIContainer) -> dict:
146153
return {

packages/cli/src/cli/commands/common.py

Lines changed: 30 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -3,6 +3,7 @@
33
"""
44

55
import asyncio
6+
import inspect
67
import ipaddress
78
from collections.abc import Callable
89
from functools import wraps
@@ -45,12 +46,40 @@ def wrapper(*args: Any, **kwargs: Any) -> Any:
4546
return wrapper
4647

4748

49+
async def _close_container(container: Any) -> None:
50+
"""Close a CLI container's async resources, skipping mock containers.
51+
52+
The container's HTTP clients (httpx pools) are created inside the running
53+
event loop, so they must be closed from within it. Guarded on a real async
54+
``close`` so unit tests using mock containers are unaffected.
55+
"""
56+
close = getattr(container, "close", None)
57+
if inspect.iscoroutinefunction(close):
58+
try:
59+
await close()
60+
except Exception:
61+
pass
62+
63+
64+
def _container_from_args(args: Any) -> Any:
65+
if not args:
66+
return None
67+
return getattr(getattr(args[0], "obj", None), "container", None)
68+
69+
70+
async def _run_then_close(func: Callable, args: Any, kwargs: Any) -> Any:
71+
try:
72+
return await func(*args, **kwargs)
73+
finally:
74+
await _close_container(_container_from_args(args))
75+
76+
4877
def async_command(func: Callable) -> Callable:
4978
"""Decorator to run async functions in Click commands."""
5079

5180
@wraps(func)
5281
def wrapper(*args: Any, **kwargs: Any) -> Any:
53-
return asyncio.run(func(*args, **kwargs))
82+
return asyncio.run(_run_then_close(func, args, kwargs))
5483

5584
return wrapper
5685

packages/cli/src/cli/dependencies/container.py

Lines changed: 24 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -3,7 +3,6 @@
33
from collections.abc import AsyncGenerator
44
from contextlib import asynccontextmanager
55

6-
import httpx
76
from core.dependencies.container_base import BaseContainer
87
from core.gateways.network.async_shelly_rpc_client import AsyncShellyRPCClient
98
from core.repositories.db import async_session_factory
@@ -21,6 +20,7 @@
2120
)
2221
from core.services.authentication_service import AuthenticationService
2322
from core.services.encryption_service import EncryptionService
23+
from core.settings import settings as core_settings
2424
from core.use_cases.scan_devices import ScanDevicesUseCase
2525

2626

@@ -89,14 +89,34 @@ def get_authentication_service(self) -> AuthenticationService:
8989

9090
def get_rpc_client(self) -> AsyncShellyRPCClient:
9191
if self._rpc_client is None:
92-
# Shared session for connection pooling
93-
http_session = httpx.AsyncClient(timeout=3.0)
9492
self._rpc_client = AsyncShellyRPCClient(
95-
session=http_session,
93+
timeout=core_settings.network.timeout,
94+
connect_timeout=core_settings.network.connect_timeout,
95+
verify=core_settings.network.verify_ssl,
9696
authentication_service=self.get_authentication_service(),
97+
auth_state_cache=self.get_auth_state_cache(),
9798
)
9899
return self._rpc_client
99100

101+
async def close(self) -> None:
102+
"""Gracefully close async resources (HTTP connection pools)."""
103+
if self._rpc_client is not None:
104+
try:
105+
await self._rpc_client.close()
106+
except Exception:
107+
pass
108+
109+
if self._mdns_client is not None:
110+
try:
111+
await self._mdns_client.close()
112+
except Exception:
113+
pass
114+
115+
await self._aclose_legacy_http_client()
116+
117+
self._rpc_client = None
118+
self._reset_device_caches()
119+
100120
# Backwards compatibility helpers (optional convenience wrappers)
101121
def get_device_scan_interactor(self) -> ScanDevicesUseCase:
102122
return self.get_scan_interactor()

packages/core/src/core/dependencies/container_base.py

Lines changed: 37 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
class BaseContainer:
3131
def __init__(self) -> None:
3232
self._device_gateway: ShellyDeviceGateway | None = None
33+
self._legacy_http_client: LegacyHttpClient | None = None
3334
self._mdns_client: MDNSGateway | None = None
3435
self._scan_interactor: ScanDevicesUseCase | None = None
3536
self._execute_component_action_interactor: (
@@ -55,7 +56,12 @@ def get_rpc_client(self) -> Any:
5556

5657
def get_device_gateway(self) -> ShellyDeviceGateway:
5758
if self._device_gateway is None:
58-
legacy_http_client = LegacyHttpClient()
59+
from core.settings import settings as core_settings
60+
61+
legacy_http_client = LegacyHttpClient(
62+
connect_timeout=core_settings.network.connect_timeout,
63+
)
64+
self._legacy_http_client = legacy_http_client
5965
legacy_component_mapper = LegacyComponentMapper()
6066
legacy_gateway = LegacyDeviceGateway(
6167
http_client=legacy_http_client,
@@ -70,6 +76,36 @@ def get_device_gateway(self) -> ShellyDeviceGateway:
7076
)
7177
return self._device_gateway
7278

79+
async def _aclose_legacy_http_client(self) -> None:
80+
"""Close the legacy HTTP client's connection pool, if one was created."""
81+
if self._legacy_http_client is not None:
82+
try:
83+
await self._legacy_http_client.close()
84+
except Exception:
85+
pass
86+
87+
def _reset_device_caches(self) -> None:
88+
"""Drop cached gateways/interactors that hold now-closed clients.
89+
90+
Called after close() so that a reused container rebuilds live
91+
resources on next access instead of handing back closed ones.
92+
"""
93+
self._device_gateway = None
94+
self._legacy_http_client = None
95+
self._mdns_client = None
96+
self._scan_interactor = None
97+
self._execute_component_action_interactor = None
98+
self._component_actions_interactor = None
99+
self._status_interactor = None
100+
self._bulk_operations_interactor = None
101+
self._manage_profiles_interactor = None
102+
self._provision_device_interactor = None
103+
self._ap_device_detector = None
104+
self._backup_device_config_interactor = None
105+
self._restore_device_config_interactor = None
106+
self._manage_backup_schedules_interactor = None
107+
self._run_due_backups_interactor = None
108+
73109
def _get_authentication_service_optional(self) -> AuthenticationService | None:
74110
"""Return AuthenticationService if available, None otherwise."""
75111
if hasattr(self, "get_authentication_service"):

packages/core/src/core/domain/entities/exceptions.py

Lines changed: 9 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -34,6 +34,15 @@ def __init__(self, ip: str, error: str, message: str | None = None):
3434
super().__init__(msg, {"ip": ip, "error": error})
3535

3636

37+
class DeviceUnreachableError(DeviceCommunicationError):
38+
"""Raised when a host does not accept a TCP connection.
39+
40+
Signals that nothing is listening at the address (connect refused/timed
41+
out), as opposed to a device that responded but failed at the HTTP/RPC
42+
layer. Used to skip the legacy fallback probe for dead IPs during scans.
43+
"""
44+
45+
3746
class ConfigurationError(ShellyManagerError):
3847

3948
def __init__(self, operation: str, message: str | None = None):

packages/core/src/core/gateways/device/device.py

Lines changed: 3 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -14,7 +14,9 @@ class DeviceGateway(ABC):
1414
timeout: float = 10.0
1515

1616
@abstractmethod
17-
async def discover_device(self, ip: str) -> DiscoveredDevice | None:
17+
async def discover_device(
18+
self, ip: str, timeout: float | None = None
19+
) -> DiscoveredDevice | None:
1820
pass
1921

2022
@abstractmethod

packages/core/src/core/gateways/device/legacy_device_gateway.py

Lines changed: 40 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -4,6 +4,7 @@
44

55
from __future__ import annotations
66

7+
import asyncio
78
import logging
89
import time
910
from datetime import datetime
@@ -42,13 +43,15 @@ def __init__(
4243
self._ip_to_mac: dict[str, str] = {}
4344
self._basic_auth_cache: dict[str, tuple[str, str]] = {}
4445

45-
async def _ensure_mac(self, ip: str) -> str | None:
46+
async def _ensure_mac(self, ip: str, timeout: float | None = None) -> str | None:
4647
"""Get MAC address for an IP, fetching from /shelly if not cached."""
4748
normalized_ip = normalize_mac(ip)
4849
if normalized_ip in self._ip_to_mac:
4950
return self._ip_to_mac[normalized_ip]
5051
try:
51-
shelly_data = await self._http_client.fetch_json(ip, "shelly")
52+
shelly_data = await self._http_client.fetch_json(
53+
ip, "shelly", timeout=timeout
54+
)
5255
mac = shelly_data.get("mac")
5356
if mac:
5457
normalized_mac = normalize_mac(mac)
@@ -58,12 +61,14 @@ async def _ensure_mac(self, ip: str) -> str | None:
5861
pass
5962
return None
6063

61-
async def _resolve_auth(self, ip: str) -> tuple[str, str] | None:
64+
async def _resolve_auth(
65+
self, ip: str, timeout: float | None = None
66+
) -> tuple[str, str] | None:
6267
"""Resolve Basic Auth credentials for a device by IP."""
6368
if not self._authentication_service:
6469
return None
6570

66-
mac = await self._ensure_mac(ip)
71+
mac = await self._ensure_mac(ip, timeout)
6772
if not mac:
6873
return None
6974

@@ -82,18 +87,24 @@ def invalidate_credential_cache(self, mac: str) -> None:
8287
normalized_mac = normalize_mac(mac)
8388
self._basic_auth_cache.pop(normalized_mac, None)
8489

85-
async def discover_device(self, ip: str) -> DiscoveredDevice | None:
90+
async def discover_device(
91+
self, ip: str, timeout: float | None = None
92+
) -> DiscoveredDevice | None:
8693
"""Discover a legacy Gen1 Shelly device.
8794
8895
Args:
8996
ip: Device IP address
97+
timeout: Per-request timeout in seconds; falls back to the HTTP
98+
client default when not provided.
9099
91100
Returns:
92101
DiscoveredDevice or None if discovery fails
93102
"""
94103
try:
95104
start_time = time.perf_counter()
96-
device_info = await self._http_client.fetch_json(ip, "shelly")
105+
device_info = await self._http_client.fetch_json(
106+
ip, "shelly", timeout=timeout
107+
)
97108
response_time = time.perf_counter() - start_time
98109

99110
# Detect auth requirement from /shelly response
@@ -105,13 +116,16 @@ async def discover_device(self, ip: str) -> DiscoveredDevice | None:
105116
normalized_mac = normalize_mac(mac)
106117
self._ip_to_mac[normalize_mac(ip)] = normalized_mac
107118
self._auth_state_cache.mark_auth_required(normalized_mac)
108-
auth = await self._resolve_auth(ip)
109-
110-
status_data = await self._http_client.fetch_json_optional(
111-
ip, "status", auth=auth
112-
)
113-
settings_data = await self._http_client.fetch_json_optional(
114-
ip, "settings", auth=auth
119+
auth = await self._resolve_auth(ip, timeout)
120+
121+
# /status and /settings are independent; fetch them concurrently.
122+
status_data, settings_data = await asyncio.gather(
123+
self._http_client.fetch_json_optional(
124+
ip, "status", auth=auth, timeout=timeout
125+
),
126+
self._http_client.fetch_json_optional(
127+
ip, "settings", auth=auth, timeout=timeout
128+
),
115129
)
116130

117131
device_name = self._derive_device_name(device_info, settings_data)
@@ -154,29 +168,37 @@ async def discover_device(self, ip: str) -> DiscoveredDevice | None:
154168
)
155169
return None
156170

157-
async def get_device_status(self, ip: str) -> DeviceStatus | None:
171+
async def get_device_status(
172+
self, ip: str, timeout: float | None = None
173+
) -> DeviceStatus | None:
158174
"""Get device status for a legacy Gen1 device.
159175
160176
Args:
161177
ip: Device IP address
178+
timeout: Per-request timeout in seconds; falls back to the HTTP
179+
client default when not provided.
162180
163181
Returns:
164182
DeviceStatus or None if retrieval fails
165183
"""
166184
try:
167-
device_info = await self._http_client.fetch_json(ip, "shelly")
185+
device_info = await self._http_client.fetch_json(
186+
ip, "shelly", timeout=timeout
187+
)
168188

169189
auth: tuple[str, str] | None = None
170190
if device_info.get("auth", False):
171191
mac = device_info.get("mac")
172192
if mac:
173193
normalized_mac = normalize_mac(mac)
174194
self._ip_to_mac[normalize_mac(ip)] = normalized_mac
175-
auth = await self._resolve_auth(ip)
195+
auth = await self._resolve_auth(ip, timeout)
176196

177-
status_data = await self._http_client.fetch_json(ip, "status", auth=auth)
197+
status_data = await self._http_client.fetch_json(
198+
ip, "status", auth=auth, timeout=timeout
199+
)
178200
settings_data = await self._http_client.fetch_json_optional(
179-
ip, "settings", auth=auth
201+
ip, "settings", auth=auth, timeout=timeout
180202
)
181203
except DeviceAuthenticationError:
182204
raise

0 commit comments

Comments
 (0)