Skip to content

Commit 1659ca5

Browse files
ekutnerekutner
authored
Add retry and error logging if communication with the CoolMaster device fails (#148699)
1 parent 8ea16da commit 1659ca5

File tree

4 files changed

+253
-5
lines changed

4 files changed

+253
-5
lines changed

‎homeassistant/components/coolmaster/const.py‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,3 +6,5 @@
66

77
CONF_SUPPORTED_MODES = "supported_modes"
88
CONF_SWING_SUPPORT = "swing_support"
9+
MAX_RETRIES = 3
10+
BACKOFF_BASE_DELAY = 2

‎homeassistant/components/coolmaster/coordinator.py‎

Lines changed: 33 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -2,6 +2,7 @@
22

33
from __future__ import annotations
44

5+
import asyncio
56
import logging
67

78
from pycoolmasternet_async import CoolMasterNet
@@ -12,7 +13,7 @@
1213
from homeassistant.core import HomeAssistant
1314
from homeassistant.helpers.update_coordinator import DataUpdateCoordinator, UpdateFailed
1415

15-
from .const import DOMAIN
16+
from .const import BACKOFF_BASE_DELAY, DOMAIN, MAX_RETRIES
1617

1718
_LOGGER = logging.getLogger(__name__)
1819

@@ -46,7 +47,34 @@ def __init__(
4647

4748
async def _async_update_data(self) -> dict[str, CoolMasterNetUnit]:
4849
"""Fetch data from Coolmaster."""
49-
try:
50-
return await self._coolmaster.status()
51-
except OSError as error:
52-
raise UpdateFailed from error
50+
retries_left = MAX_RETRIES
51+
status: dict[str, CoolMasterNetUnit] = {}
52+
while retries_left > 0 and not status:
53+
retries_left -= 1
54+
try:
55+
status = await self._coolmaster.status()
56+
except OSError as error:
57+
if retries_left == 0:
58+
raise UpdateFailed(
59+
f"Error communicating with Coolmaster (aborting after {MAX_RETRIES} retries): {error}"
60+
) from error
61+
_LOGGER.debug(
62+
"Error communicating with coolmaster (%d retries left): %s",
63+
retries_left,
64+
str(error),
65+
)
66+
else:
67+
if status:
68+
return status
69+
70+
_LOGGER.debug(
71+
"Error communicating with coolmaster: empty status received (%d retries left)",
72+
retries_left,
73+
)
74+
75+
backoff = BACKOFF_BASE_DELAY ** (MAX_RETRIES - retries_left)
76+
await asyncio.sleep(backoff)
77+
78+
raise UpdateFailed(
79+
f"Error communicating with Coolmaster (aborting after {MAX_RETRIES} retries): empty status received"
80+
)

‎tests/components/coolmaster/conftest.py‎

Lines changed: 95 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -111,6 +111,53 @@ async def status(self) -> dict[str, CoolMasterNetUnitMock]:
111111
}
112112

113113

114+
class CoolMasterNetErrorMock:
115+
"""Mock for CoolMasterNet."""
116+
117+
def __init__(self, *_args: Any, **kwargs: Any) -> None:
118+
"""Initialize the CoolMasterNetMock."""
119+
self._units = copy.deepcopy(TEST_UNITS)
120+
self._fail_count = 0
121+
122+
async def info(self) -> dict[str, Any]:
123+
"""Return info about the bridge device."""
124+
return DEFAULT_INFO
125+
126+
async def status(self) -> dict[str, CoolMasterNetUnitMock]:
127+
"""Return the units."""
128+
if self._fail_count > 0:
129+
self._fail_count -= 1
130+
raise OSError("Simulated communication error")
131+
return {
132+
unit_id: CoolMasterNetUnitMock(unit_id, attributes)
133+
for unit_id, attributes in self._units.items()
134+
}
135+
136+
137+
class CoolMasterNetEmptyStatusMock:
138+
"""Mock for CoolMasterNet."""
139+
140+
def __init__(self, *_args: Any, **kwargs: Any) -> None:
141+
"""Initialize the CoolMasterNetMock."""
142+
self._units = copy.deepcopy(TEST_UNITS)
143+
self._call_count = 0
144+
145+
async def info(self) -> dict[str, Any]:
146+
"""Return info about the bridge device."""
147+
return DEFAULT_INFO
148+
149+
async def status(self) -> dict[str, CoolMasterNetUnitMock]:
150+
"""Return the units."""
151+
self._call_count += 1
152+
if self._call_count == 1:
153+
return {
154+
unit_id: CoolMasterNetUnitMock(unit_id, attributes)
155+
for unit_id, attributes in self._units.items()
156+
}
157+
158+
return {}
159+
160+
114161
@pytest.fixture
115162
async def load_int(hass: HomeAssistant) -> MockConfigEntry:
116163
"""Set up the Coolmaster integration in Home Assistant."""
@@ -133,3 +180,51 @@ async def load_int(hass: HomeAssistant) -> MockConfigEntry:
133180
await hass.async_block_till_done()
134181

135182
return config_entry
183+
184+
185+
@pytest.fixture
186+
async def config_entry_with_errors(hass: HomeAssistant) -> MockConfigEntry:
187+
"""Set up the Coolmaster integration in Home Assistant."""
188+
config_entry = MockConfigEntry(
189+
domain=DOMAIN,
190+
data={
191+
"host": "1.2.3.4",
192+
"port": 1234,
193+
"supported_modes": [HVACMode.OFF, HVACMode.COOL, HVACMode.HEAT],
194+
},
195+
)
196+
197+
config_entry.add_to_hass(hass)
198+
199+
with patch(
200+
"homeassistant.components.coolmaster.CoolMasterNet",
201+
new=CoolMasterNetErrorMock,
202+
):
203+
await hass.config_entries.async_setup(config_entry.entry_id)
204+
await hass.async_block_till_done()
205+
206+
return config_entry
207+
208+
209+
@pytest.fixture
210+
async def config_entry_with_empty_status(hass: HomeAssistant) -> MockConfigEntry:
211+
"""Set up the Coolmaster integration in Home Assistant."""
212+
config_entry = MockConfigEntry(
213+
domain=DOMAIN,
214+
data={
215+
"host": "1.2.3.4",
216+
"port": 1234,
217+
"supported_modes": [HVACMode.OFF, HVACMode.COOL, HVACMode.HEAT],
218+
},
219+
)
220+
221+
config_entry.add_to_hass(hass)
222+
223+
with patch(
224+
"homeassistant.components.coolmaster.CoolMasterNet",
225+
new=CoolMasterNetEmptyStatusMock,
226+
):
227+
await hass.config_entries.async_setup(config_entry.entry_id)
228+
await hass.async_block_till_done()
229+
230+
return config_entry

‎tests/components/coolmaster/test_sensor.py‎

Lines changed: 123 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -2,8 +2,15 @@
22

33
from __future__ import annotations
44

5+
import logging
6+
from unittest.mock import patch
7+
8+
import pytest
9+
10+
from homeassistant.components.coolmaster.const import MAX_RETRIES
511
from homeassistant.config_entries import ConfigEntry
612
from homeassistant.core import HomeAssistant
13+
from homeassistant.helpers.entity_component import async_update_entity
714

815

916
async def test_sensor(
@@ -13,3 +20,119 @@ async def test_sensor(
1320
"""Test the Coolmaster sensor."""
1421
assert hass.states.get("sensor.l1_100_error_code").state == "OK"
1522
assert hass.states.get("sensor.l1_101_error_code").state == "Err1"
23+
24+
25+
async def test_retry_with_no_error(
26+
hass: HomeAssistant,
27+
config_entry_with_errors: ConfigEntry,
28+
caplog: pytest.LogCaptureFixture,
29+
) -> None:
30+
"""Test without errors."""
31+
32+
caplog.set_level(logging.DEBUG, logger="homeassistant.components.coolmaster")
33+
34+
with patch(
35+
"tests.components.coolmaster.conftest.CoolMasterNetErrorMock.status",
36+
wraps=config_entry_with_errors.runtime_data._coolmaster.status,
37+
) as mock_status:
38+
config_entry_with_errors.runtime_data._coolmaster._fail_count = 0
39+
await async_update_entity(hass, "sensor.l1_101_error_code")
40+
await hass.async_block_till_done()
41+
42+
assert mock_status.call_count == 1
43+
debugs, errors = count_logs(caplog.records)
44+
assert debugs == 0
45+
assert errors == 0
46+
47+
48+
@patch("homeassistant.components.coolmaster.coordinator.BACKOFF_BASE_DELAY", new=0)
49+
async def test_retry_with_less_than_max_errors(
50+
hass: HomeAssistant,
51+
config_entry_with_errors: ConfigEntry,
52+
caplog: pytest.LogCaptureFixture,
53+
) -> None:
54+
"""Test MAX_RETRIES-1 errors."""
55+
56+
caplog.set_level(logging.DEBUG, logger="homeassistant.components.coolmaster")
57+
58+
with patch(
59+
"tests.components.coolmaster.conftest.CoolMasterNetErrorMock.status",
60+
wraps=config_entry_with_errors.runtime_data._coolmaster.status,
61+
) as mock_status:
62+
config_entry_with_errors.runtime_data._coolmaster._fail_count = MAX_RETRIES - 1
63+
await async_update_entity(hass, "sensor.l1_101_error_code")
64+
await hass.async_block_till_done()
65+
66+
assert mock_status.call_count == MAX_RETRIES # The last try succeeds
67+
debugs, errors = count_logs(caplog.records)
68+
assert errors == 0
69+
assert debugs == MAX_RETRIES - 1
70+
71+
72+
@patch("homeassistant.components.coolmaster.coordinator.BACKOFF_BASE_DELAY", new=0)
73+
async def test_retry_with_more_than_max_errors(
74+
hass: HomeAssistant,
75+
config_entry_with_errors: ConfigEntry,
76+
caplog: pytest.LogCaptureFixture,
77+
) -> None:
78+
"""Test MAX_RETRIES+1 errors."""
79+
80+
caplog.set_level(logging.DEBUG, logger="homeassistant.components.coolmaster")
81+
82+
with patch(
83+
"tests.components.coolmaster.conftest.CoolMasterNetErrorMock.status",
84+
wraps=config_entry_with_errors.runtime_data._coolmaster.status,
85+
) as mock_status:
86+
config_entry_with_errors.runtime_data._coolmaster._fail_count = MAX_RETRIES + 1
87+
await async_update_entity(hass, "sensor.l1_101_error_code")
88+
await hass.async_block_till_done()
89+
90+
assert (
91+
mock_status.call_count == MAX_RETRIES
92+
) # The retries are capped at MAX_RETRIES
93+
debugs, errors = count_logs(caplog.records)
94+
assert errors == 1
95+
assert debugs == MAX_RETRIES - 1
96+
97+
98+
@patch("homeassistant.components.coolmaster.coordinator.BACKOFF_BASE_DELAY", new=0)
99+
async def test_retry_with_empty_status(
100+
hass: HomeAssistant,
101+
config_entry_with_empty_status: ConfigEntry,
102+
caplog: pytest.LogCaptureFixture,
103+
) -> None:
104+
"""Test empty status response."""
105+
106+
caplog.set_level(logging.DEBUG, logger="homeassistant.components.coolmaster")
107+
108+
with patch(
109+
"tests.components.coolmaster.conftest.CoolMasterNetEmptyStatusMock.status",
110+
wraps=config_entry_with_empty_status.runtime_data._coolmaster.status,
111+
) as mock_status:
112+
await async_update_entity(hass, "sensor.l1_101_error_code")
113+
await hass.async_block_till_done()
114+
115+
assert (
116+
mock_status.call_count == MAX_RETRIES
117+
) # The retries are capped at MAX_RETRIES
118+
debugs, errors = count_logs(caplog.records)
119+
assert errors == 1
120+
assert debugs == MAX_RETRIES
121+
122+
123+
def count_logs(log_records: list[logging.LogRecord]) -> tuple[int, int]:
124+
"""Count the number of log records."""
125+
debug_logs = [
126+
rec
127+
for rec in log_records
128+
if rec.levelno == logging.DEBUG
129+
and "Error communicating with coolmaster" in rec.getMessage()
130+
]
131+
132+
error_logs = [
133+
rec
134+
for rec in log_records
135+
if rec.levelno == logging.ERROR
136+
and "Error fetching coolmaster data" in rec.getMessage()
137+
]
138+
return len(debug_logs), len(error_logs)

0 commit comments

Comments
 (0)