Skip to content

Commit c79da53

Browse files
committed
refactor: unify temperature ranges, remove deprecated methods, sync helpers
- Unify temperature range storage: replace legacy _min_temp/_max_temp with per-circuit _circuit_temp_ranges dict, enabling proper multi-circuit support - Make _extract_params_summary synchronous (no I/O, was unnecessarily async) - Remove deprecated _initialize_api_data method (unused since _copy_api_config) - Remove deprecated _initialize_api_validator method (replaced by _setup_api_validator) - Update all affected tests to match the refactored internals
1 parent 6d346ab commit c79da53

11 files changed

Lines changed: 71 additions & 305 deletions

src/bsblan/bsblan.py

Lines changed: 14 additions & 86 deletions
Original file line numberDiff line numberDiff line change
@@ -96,14 +96,11 @@ class BSBLAN:
9696
_close_session: bool = False
9797
_firmware_version: str | None = None
9898
_api_version: str | None = None
99-
_min_temp: float | None = None
100-
_max_temp: float | None = None
101-
_temperature_range_initialized: bool = False
10299
_api_data: APIConfig | None = None
103100
_initialized: bool = False
104101
_api_validator: APIValidator = field(init=False)
105102
_temperature_unit: str = "°C"
106-
# Per-circuit temperature ranges: circuit_number -> (min, max, initialized)
103+
# Per-circuit temperature ranges: circuit_number -> {min, max}
107104
_circuit_temp_ranges: dict[int, dict[str, float | None]] = field(
108105
default_factory=dict,
109106
)
@@ -345,7 +342,7 @@ async def _ensure_hot_water_group_validated(
345342
return
346343

347344
# Request only these specific parameters from the device
348-
params = await self._extract_params_summary(group_params)
345+
params = self._extract_params_summary(group_params)
349346
response_data = await self._request(
350347
params={"Parameter": params["string_par"]}
351348
)
@@ -386,39 +383,6 @@ async def _ensure_hot_water_group_validated(
386383
len(params_to_remove),
387384
)
388385

389-
async def _initialize_api_validator(self) -> None:
390-
"""Initialize and validate API data against device capabilities.
391-
392-
DEPRECATED: This method validates all sections upfront.
393-
Use _setup_api_validator() + _ensure_section_validated() for lazy loading.
394-
This method is kept for backwards compatibility.
395-
"""
396-
if self._api_version is None:
397-
raise BSBLANError(ErrorMsg.API_VERSION)
398-
399-
# Initialize API data if not already done
400-
if self._api_data is None:
401-
self._api_data = self._copy_api_config()
402-
403-
# Initialize the API validator
404-
self._api_validator = APIValidator(self._api_data)
405-
406-
# Perform initial validation of each section (eager loading)
407-
sections: list[SectionLiteral] = [
408-
"heating",
409-
"sensor",
410-
"staticValues",
411-
"device",
412-
"hot_water",
413-
]
414-
for section in sections:
415-
response_data = await self._validate_api_section(section)
416-
417-
# Extract temperature unit from heating section validation
418-
# (parameter 710 - target_temperature is always in heating section)
419-
if section == "heating" and response_data:
420-
self._extract_temperature_unit_from_response(response_data)
421-
422386
async def _validate_api_section(
423387
self, section: SectionLiteral, include: list[str] | None = None
424388
) -> dict[str, Any] | None:
@@ -466,7 +430,7 @@ async def _validate_api_section(
466430

467431
try:
468432
# Request data from device for validation
469-
params = await self._extract_params_summary(section_data)
433+
params = self._extract_params_summary(section_data)
470434
response_data = await self._request(
471435
params={"Parameter": params["string_par"]}
472436
)
@@ -635,22 +599,12 @@ async def _initialize_temperature_range(
635599
from the response (parameter 710), so no extra API call is needed here.
636600
637601
"""
638-
if circuit == 1 and self._temperature_range_initialized:
639-
return
640-
if circuit != 1 and circuit in self._circuit_temp_initialized:
602+
if circuit in self._circuit_temp_initialized:
641603
return
642604

643605
temp_range = await self._fetch_temperature_range(circuit)
644-
645-
if circuit == 1:
646-
# HC1 uses legacy fields for backwards compatibility
647-
self._min_temp = temp_range["min"]
648-
self._max_temp = temp_range["max"]
649-
self._temperature_range_initialized = True
650-
else:
651-
# HC2 uses per-circuit storage
652-
self._circuit_temp_ranges[circuit] = temp_range
653-
self._circuit_temp_initialized.add(circuit)
606+
self._circuit_temp_ranges[circuit] = temp_range
607+
self._circuit_temp_initialized.add(circuit)
654608

655609
def _validate_circuit(self, circuit: int) -> None:
656610
"""Validate the circuit number.
@@ -680,23 +634,6 @@ def get_temperature_unit(self) -> str:
680634
"""
681635
return self._temperature_unit
682636

683-
async def _initialize_api_data(self) -> APIConfig:
684-
"""Initialize and cache the API data.
685-
686-
Returns:
687-
APIConfig: The API configuration data.
688-
689-
Raises:
690-
BSBLANError: If the API version or data is not initialized.
691-
692-
"""
693-
if self._api_data is None:
694-
self._api_data = self._copy_api_config()
695-
logger.debug("API data initialized for version: %s", self._api_version)
696-
if self._api_data is None:
697-
raise BSBLANError(ErrorMsg.API_DATA_NOT_INITIALIZED)
698-
return self._api_data
699-
700637
def _copy_api_config(self) -> APIConfig:
701638
"""Create a copy of the API configuration for the current version.
702639
@@ -900,7 +837,7 @@ def _validate_single_parameter(self, *params: Any, error_msg: str) -> None:
900837
if sum(param is not None for param in params) != 1:
901838
raise BSBLANError(error_msg)
902839

903-
async def _extract_params_summary(self, params: dict[Any, Any]) -> dict[Any, Any]:
840+
def _extract_params_summary(self, params: dict[Any, Any]) -> dict[Any, Any]:
904841
"""Get the parameters info from BSBLAN device.
905842
906843
Args:
@@ -961,7 +898,7 @@ async def _fetch_section_data(
961898
if not section_params:
962899
raise BSBLANError(ErrorMsg.INVALID_INCLUDE_PARAMS)
963900

964-
params = await self._extract_params_summary(section_params)
901+
params = self._extract_params_summary(section_params)
965902
data = await self._request(params={"Parameter": params["string_par"]})
966903
data = dict(zip(params["list"], list(data.values()), strict=True))
967904
return model_class.model_validate(data)
@@ -1219,21 +1156,12 @@ async def _validate_target_temperature(
12191156
raise BSBLANInvalidParameterError(target_temperature) from err
12201157

12211158
# Try to load temperature range for bounds checking
1222-
if circuit == 1:
1223-
# HC1 uses legacy fields for backwards compatibility
1224-
if self._min_temp is None or self._max_temp is None:
1225-
await self._initialize_temperature_range(circuit)
1226-
1227-
min_temp = self._min_temp
1228-
max_temp = self._max_temp
1229-
else:
1230-
# HC2 uses per-circuit storage
1231-
if circuit not in self._circuit_temp_initialized:
1232-
await self._initialize_temperature_range(circuit)
1159+
if circuit not in self._circuit_temp_initialized:
1160+
await self._initialize_temperature_range(circuit)
12331161

1234-
temp_range = self._circuit_temp_ranges.get(circuit, {})
1235-
min_temp = temp_range.get("min")
1236-
max_temp = temp_range.get("max")
1162+
temp_range = self._circuit_temp_ranges.get(circuit, {})
1163+
min_temp = temp_range.get("min")
1164+
max_temp = temp_range.get("max")
12371165

12381166
# Skip range validation if device doesn't provide min/max
12391167
if min_temp is None or max_temp is None:
@@ -1337,7 +1265,7 @@ async def _fetch_hot_water_data(
13371265
if not filtered_params:
13381266
raise BSBLANError(error_msg)
13391267

1340-
params = await self._extract_params_summary(filtered_params)
1268+
params = self._extract_params_summary(filtered_params)
13411269
data = await self._request(params={"Parameter": params["string_par"]})
13421270
data = dict(zip(params["list"], list(data.values()), strict=True))
13431271
return model_class.model_validate(data)

tests/conftest.py

Lines changed: 0 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,6 @@ async def mock_bsblan(
3636
monkeypatch.setattr(bsblan, "_firmware_version", "1.0.38-20200730234859")
3737
monkeypatch.setattr(bsblan, "_api_version", "v3")
3838
monkeypatch.setattr(bsblan, "_api_data", API_V3)
39-
initialize_api_data_mock: AsyncMock = AsyncMock()
40-
# return the constant dictionary
41-
monkeypatch.setattr(bsblan, "_initialize_api_data", initialize_api_data_mock)
4239
request_mock: AsyncMock = AsyncMock(return_value={"status": "ok"})
4340
monkeypatch.setattr(bsblan, "_request", request_mock)
4441
yield bsblan

tests/test_api_initialization.py

Lines changed: 7 additions & 51 deletions
Original file line numberDiff line numberDiff line change
@@ -1,32 +1,17 @@
11
"""Tests for API data initialization error handling."""
22
# pylint: disable=protected-access
33

4-
from typing import Any
5-
from unittest.mock import AsyncMock
6-
74
import aiohttp
85
import pytest
96

107
from bsblan import BSBLAN
118
from bsblan.bsblan import BSBLANConfig
129
from bsblan.constants import (
1310
API_VERSIONS,
14-
ErrorMsg,
1511
)
1612
from bsblan.exceptions import BSBLANError
1713

1814

19-
@pytest.mark.asyncio
20-
async def test_initialize_api_data_no_api_version() -> None:
21-
"""Test initializing API data with no API version set."""
22-
config = BSBLANConfig(host="example.com")
23-
bsblan = BSBLAN(config)
24-
25-
# API version is None by default
26-
with pytest.raises(BSBLANError, match=ErrorMsg.API_VERSION):
27-
await bsblan._initialize_api_data()
28-
29-
3015
@pytest.mark.asyncio
3116
async def test_request_no_session() -> None:
3217
"""Test request method with no session initialized."""
@@ -39,8 +24,8 @@ async def test_request_no_session() -> None:
3924

4025

4126
@pytest.mark.asyncio
42-
async def test_api_data_initialized_from_versions(monkeypatch: Any) -> None:
43-
"""Test that API data is initialized from API_VERSIONS when None (line 141)."""
27+
async def test_api_data_initialized_from_versions() -> None:
28+
"""Test that API data is initialized via _setup_api_validator."""
4429
async with aiohttp.ClientSession() as session:
4530
config = BSBLANConfig(host="example.com")
4631
client = BSBLAN(config, session=session)
@@ -49,23 +34,18 @@ async def test_api_data_initialized_from_versions(monkeypatch: Any) -> None:
4934
client._api_version = "v1"
5035
client._api_data = None # This should be initialized
5136

52-
# Mock request to avoid real network calls
53-
request_mock: AsyncMock = AsyncMock(return_value={})
54-
monkeypatch.setattr(client, "_request", request_mock)
55-
56-
# Call _initialize_api_validator which should initialize _api_data
57-
await client._initialize_api_validator()
37+
# Call _setup_api_validator which should initialize _api_data
38+
await client._setup_api_validator()
5839

5940
# Verify API data was initialized (should be a copy, not the same object)
6041
assert client._api_data is not None
6142
# Verify it started with the same keys as API_VERSIONS["v1"]
6243
assert set(client._api_data.keys()) == set(API_VERSIONS["v1"].keys())
63-
# Note: Values will differ after validation since validator modifies the copy
6444

6545

6646
@pytest.mark.asyncio
67-
async def test_api_data_property_raises_without_version() -> None:
68-
"""Test _initialize_api_data raises error when API version is None (line 368)."""
47+
async def test_copy_api_config_raises_without_version() -> None:
48+
"""Test _copy_api_config raises error when API version is None."""
6949
async with aiohttp.ClientSession() as session:
7050
config = BSBLANConfig(host="example.com")
7151
client = BSBLAN(config, session=session)
@@ -76,28 +56,4 @@ async def test_api_data_property_raises_without_version() -> None:
7656

7757
# This should raise BSBLANError
7858
with pytest.raises(BSBLANError, match="API version not set"):
79-
await client._initialize_api_data()
80-
81-
82-
@pytest.mark.asyncio
83-
async def test_initialize_api_data_returns_existing() -> None:
84-
"""Test _initialize_api_data returns existing data when already initialized."""
85-
async with aiohttp.ClientSession() as session:
86-
config = BSBLANConfig(host="example.com")
87-
client = BSBLAN(config, session=session)
88-
89-
# Set up with pre-initialized data
90-
client._api_version = "v1"
91-
existing_data = {
92-
"heating": {"710": "Test"},
93-
"staticValues": {"714": "Test"},
94-
"device": {},
95-
"sensor": {},
96-
"hot_water": {},
97-
}
98-
client._api_data = existing_data # type: ignore[assignment]
99-
100-
# This should return the existing data without re-initializing
101-
result = await client._initialize_api_data()
102-
assert result is existing_data
103-
assert result == existing_data
59+
client._copy_api_config()

tests/test_api_validation.py

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -174,7 +174,7 @@ def mock_validate(
174174

175175
try:
176176
# _api_validator is already set on bsblan
177-
async def mock_extract_params(*_args: Any) -> dict[str, Any]:
177+
def mock_extract_params(*_args: Any) -> dict[str, Any]:
178178
# Not using the parameters
179179
return {"string_par": "5870", "list": ["Device Parameter"]}
180180

tests/test_bsblan_edge_cases.py

Lines changed: 0 additions & 48 deletions
Original file line numberDiff line numberDiff line change
@@ -9,25 +9,9 @@
99
import pytest
1010

1111
from bsblan import BSBLAN, BSBLANConfig
12-
from bsblan.constants import ErrorMsg
1312
from bsblan.exceptions import BSBLANConnectionError, BSBLANError
1413

1514

16-
@pytest.mark.asyncio
17-
async def test_initialize_api_data_edge_case() -> None:
18-
"""Test _initialize_api_data when API data is None after version setting."""
19-
config = BSBLANConfig(host="example.com")
20-
bsblan = BSBLAN(config)
21-
22-
# Force API version to be set but data to be None
23-
bsblan._api_version = "v3"
24-
bsblan._api_data = None
25-
26-
# This should trigger the defensive check in _initialize_api_data
27-
api_data = await bsblan._initialize_api_data()
28-
assert api_data is not None
29-
30-
3115
@pytest.mark.asyncio
3216
async def test_validate_api_section_key_error(monkeypatch: Any) -> None:
3317
"""Test validate_api_section when section is not found in API data."""
@@ -107,35 +91,3 @@ def test_bsblan_config_initialization_edge_cases() -> None:
10791
assert bsblan.session is None
10892
assert bsblan._initialized is False
10993
assert len(bsblan._hot_water_param_cache) == 0
110-
111-
112-
@pytest.mark.asyncio
113-
async def test_initialize_api_data_none_after_init(monkeypatch: Any) -> None:
114-
"""Test _initialize_api_data raises error when api_data remains None.
115-
116-
This covers the defensive check at line 391 in bsblan.py.
117-
"""
118-
config = BSBLANConfig(host="example.com")
119-
bsblan = BSBLAN(config)
120-
121-
# Set API version so we pass the first check
122-
bsblan._api_version = "v3"
123-
124-
# Monkeypatch dict comprehension to return None (simulating failure)
125-
original_items = dict.items
126-
127-
def mock_items(self: dict[str, Any]) -> Any:
128-
# Return empty to prevent assignment
129-
return original_items(self)
130-
131-
# Force _api_data to stay None by patching the assignment
132-
def mock_setattr(obj: Any, name: str, value: Any) -> None:
133-
if name == "_api_data":
134-
object.__setattr__(obj, name, None)
135-
else:
136-
object.__setattr__(obj, name, value)
137-
138-
monkeypatch.setattr(BSBLAN, "__setattr__", mock_setattr)
139-
140-
with pytest.raises(BSBLANError, match=ErrorMsg.API_DATA_NOT_INITIALIZED):
141-
await bsblan._initialize_api_data()

tests/test_circuit.py

Lines changed: 5 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -36,9 +36,8 @@ async def mock_bsblan_circuit() -> AsyncGenerator[BSBLAN, None]:
3636
bsblan._firmware_version = "1.0.38-20200730234859"
3737
bsblan._api_version = "v3"
3838
bsblan._api_data = build_api_config("v3")
39-
bsblan._min_temp = 8.0
40-
bsblan._max_temp = 30.0
41-
bsblan._temperature_range_initialized = True
39+
bsblan._circuit_temp_ranges[1] = {"min": 8.0, "max": 30.0}
40+
bsblan._circuit_temp_initialized.add(1)
4241

4342
api_validator = APIValidator(bsblan._api_data)
4443
api_validator.validated_sections.add("heating")
@@ -464,11 +463,9 @@ async def test_circuit1_temp_range_unchanged(
464463

465464
await bsblan._initialize_temperature_range(circuit=1)
466465

467-
assert bsblan._temperature_range_initialized
468-
assert bsblan._min_temp == 8.0
469-
assert bsblan._max_temp == 20.0
470-
# HC1 should NOT be in per-circuit storage
471-
assert 1 not in bsblan._circuit_temp_initialized
466+
assert 1 in bsblan._circuit_temp_initialized
467+
assert bsblan._circuit_temp_ranges[1]["min"] == 8.0
468+
assert bsblan._circuit_temp_ranges[1]["max"] == 20.0
472469

473470

474471
@pytest.mark.asyncio

0 commit comments

Comments
 (0)