Skip to content

Commit 871e18e

Browse files
author
Maksym Shtembuliak
committed
fix: stop optional polling timeout cascades
1 parent 106b1c5 commit 871e18e

5 files changed

Lines changed: 207 additions & 11 deletions

File tree

‎CHANGELOG.md‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -42,6 +42,9 @@ the GitHub release body should be rendered from the matching version section her
4242
- Fixed stale callback-session inventory that could outlive its physical TCP
4343
socket, suppress a new callback request, and leave Home Assistant waiting for
4444
a connection that no longer existed.
45+
- Fixed Protocol 3/4 optional-register polling so one silent collector session
46+
fails fast instead of expanding into minutes of per-register retries. Support
47+
archives also refresh their runtime snapshot after a capture reconnect.
4548
- Fixed fully silent callback collectors that open a TCP session without an
4649
initial payload. Home Assistant now performs one bounded FC=1 identity
4750
challenge on the exact observed session, accepts only a correlated full-PN

‎custom_components/eybond_local/drivers/smg.py‎

Lines changed: 19 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1041,13 +1041,13 @@ async def _read_optional_specs(
10411041
try:
10421042
raw_values = await session.read_holding(start_register, register_count)
10431043
except Exception as exc:
1044-
if not _is_optional_spec_error(exc):
1044+
if not _is_explicit_optional_register_rejection(exc):
10451045
raise
10461046
for spec in grouped_specs:
10471047
try:
10481048
raw_values = await session.read_holding(spec.register, spec.word_count)
10491049
except Exception as fallback_exc:
1050-
if not _is_optional_spec_error(fallback_exc):
1050+
if not _is_explicit_optional_register_rejection(fallback_exc):
10511051
raise
10521052
continue
10531053
decoded.update(_decode_block(spec.register, raw_values, (spec,)))
@@ -1067,7 +1067,7 @@ async def _read_optional_ascii_ranges(
10671067
try:
10681068
raw_values = await session.read_holding(register, word_count)
10691069
except Exception as exc:
1070-
if not _is_optional_spec_error(exc):
1070+
if not _is_explicit_optional_register_rejection(exc):
10711071
raise
10721072
continue
10731073
text = _decode_ascii_words(raw_values).strip()
@@ -1100,11 +1100,24 @@ async def _read_missing_optional_probe_details(
11001100
return decoded
11011101

11021102

1103-
def _is_optional_spec_error(exc: Exception) -> bool:
1104-
"""Return whether one optional-register read failure should be ignored."""
1103+
def _is_explicit_optional_register_rejection(exc: Exception) -> bool:
1104+
"""Return whether the inverter explicitly rejected an optional register.
1105+
1106+
A Modbus exception response (normally illegal function/address/value) is
1107+
positive wire evidence that the exact optional range is unavailable. It
1108+
is therefore safe to split a rejected group into individual reads, or to
1109+
skip one individually rejected field.
1110+
1111+
``request_timeout`` is deliberately NOT accepted here. Silence says
1112+
nothing about register support: the collector session may have stopped
1113+
responding. Treating it as an optional-field rejection used to fan one
1114+
lost session out into every grouped and individual optional read (hundreds
1115+
of seconds on the Protocol 3/4 maps), preventing runtime recovery from
1116+
acquiring the operation boundary.
1117+
"""
11051118

11061119
if isinstance(exc, ModbusError):
1107-
return True
1120+
return str(exc).startswith("exception_code:")
11081121
return str(exc).startswith("missing_register:")
11091122

11101123

‎custom_components/eybond_local/runtime/coordinator/support.py‎

Lines changed: 31 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -461,6 +461,7 @@ async def _async_build_support_package_payloads(
461461
"""Build support bundle payload and raw capture under one runtime operation lock."""
462462

463463
async with self._runtime_operation_lock:
464+
refreshed_connected = bool(getattr(self.data, "connected", False))
464465
try:
465466
snapshot = await self._async_update_data_with_runtime_lock()
466467
except Exception as exc: # noqa: BLE001 - support export must remain available
@@ -472,12 +473,12 @@ async def _async_build_support_package_payloads(
472473
else:
473474
if snapshot is not None:
474475
self.data = snapshot
476+
refreshed_connected = bool(
477+
getattr(snapshot, "connected", False)
478+
)
475479

476480
collector_registry_lookup = await self._async_collector_registry_lookup()
477-
support_bundle_payload = self._build_support_bundle_payload(
478-
integration_build_values=integration_build_values,
479-
collector_registry_lookup=collector_registry_lookup,
480-
)
481+
capture_completed = False
481482
try:
482483
raw_capture = await self._runtime.async_capture_support_evidence()
483484
except Exception as exc:
@@ -487,6 +488,32 @@ async def _async_build_support_package_payloads(
487488
"captured_ranges": [],
488489
"range_failures": [],
489490
}
491+
else:
492+
capture_completed = True
493+
494+
# Evidence capture owns its own bounded reconnect path. When the
495+
# pre-capture refresh was offline but the capture then completed,
496+
# rebuild one runtime snapshot on the now-live session before
497+
# serializing the bundle. Previously the bundle was frozen before
498+
# capture, so one archive could claim "offline" while its raw
499+
# capture had just read every planned range successfully.
500+
if capture_completed and not refreshed_connected:
501+
try:
502+
snapshot = await self._async_update_data_with_runtime_lock()
503+
except Exception as exc: # noqa: BLE001 - preserve captured evidence
504+
logger.warning(
505+
"Support archive post-capture refresh failed for entry %s: %s",
506+
self.config_entry.entry_id,
507+
exc,
508+
)
509+
else:
510+
if snapshot is not None:
511+
self.data = snapshot
512+
513+
support_bundle_payload = self._build_support_bundle_payload(
514+
integration_build_values=integration_build_values,
515+
collector_registry_lookup=collector_registry_lookup,
516+
)
490517

491518
return support_bundle_payload, raw_capture
492519

‎tests/test_coordinator_device_hierarchy.py‎

Lines changed: 70 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -10772,6 +10772,76 @@ async def _run() -> None:
1077210772
self.assertEqual(len(notifications), 1)
1077310773
self.assertIn("polling cycle is using", notifications[0]["body"])
1077410774

10775+
def test_support_package_refreshes_snapshot_after_capture_reconnect(self) -> None:
10776+
async def _run() -> None:
10777+
coordinator = object.__new__(self.coordinator_module.EybondLocalCoordinator)
10778+
coordinator.config_entry = types.SimpleNamespace(entry_id="entry-support")
10779+
coordinator._runtime_operation_lock = asyncio.Lock()
10780+
coordinator.data = self.RuntimeSnapshot(connected=False)
10781+
events: list[str] = []
10782+
snapshots = iter(
10783+
(
10784+
self.RuntimeSnapshot(
10785+
connected=False,
10786+
values={"runtime_session_state": "offline"},
10787+
),
10788+
self.RuntimeSnapshot(
10789+
connected=True,
10790+
values={"runtime_session_state": "online"},
10791+
),
10792+
)
10793+
)
10794+
10795+
async def _refresh():
10796+
snapshot = next(snapshots)
10797+
events.append(f"refresh:{snapshot.connected}")
10798+
return snapshot
10799+
10800+
async def _registry_lookup():
10801+
events.append("registry")
10802+
return ("found", object())
10803+
10804+
async def _capture():
10805+
events.append("capture")
10806+
return {
10807+
"capture_kind": "modbus_register_dump",
10808+
"captured_ranges": [{"start": 100, "count": 10}],
10809+
"range_failures": [],
10810+
}
10811+
10812+
def _build_payload(**_kwargs):
10813+
events.append("build")
10814+
return {
10815+
"runtime": {
10816+
"connected": coordinator.data.connected,
10817+
"values": dict(coordinator.data.values),
10818+
}
10819+
}
10820+
10821+
coordinator._async_update_data_with_runtime_lock = _refresh
10822+
coordinator._async_collector_registry_lookup = _registry_lookup
10823+
coordinator._runtime = types.SimpleNamespace(
10824+
async_capture_support_evidence=_capture
10825+
)
10826+
coordinator._build_support_bundle_payload = _build_payload
10827+
10828+
payload, raw_capture = await coordinator._async_build_support_package_payloads(
10829+
integration_build_values={}
10830+
)
10831+
10832+
self.assertEqual(
10833+
events,
10834+
["refresh:False", "registry", "capture", "refresh:True", "build"],
10835+
)
10836+
self.assertTrue(payload["runtime"]["connected"])
10837+
self.assertEqual(
10838+
payload["runtime"]["values"]["runtime_session_state"],
10839+
"online",
10840+
)
10841+
self.assertEqual(raw_capture["capture_kind"], "modbus_register_dump")
10842+
10843+
asyncio.run(_run())
10844+
1077510845

1077610846
if __name__ == "__main__":
1077710847
unittest.main()

‎tests/test_smg_driver.py‎

Lines changed: 84 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -18,6 +18,7 @@
1818
SmgModbusDriver,
1919
_apply_capability_read_back,
2020
_decode_block,
21+
_read_optional_specs,
2122
_support_capture_ranges,
2223
)
2324
from custom_components.eybond_local.drivers.read_result import ( # noqa: E402
@@ -36,7 +37,10 @@
3637
set_external_register_schema_roots,
3738
)
3839
from custom_components.eybond_local.models import DetectedInverter, ProbeTarget # noqa: E402
39-
from custom_components.eybond_local.payload.modbus import crc16_modbus # noqa: E402
40+
from custom_components.eybond_local.payload.modbus import ( # noqa: E402
41+
ModbusError,
42+
crc16_modbus,
43+
)
4044
from custom_components.eybond_local.telemetry import ( # noqa: E402
4145
TelemetryFreshness,
4246
TypedTelemetryFrame,
@@ -528,6 +532,85 @@ async def test_issue_13_sandisolar_fingerprint_uses_protocol_4_schema(self) -> N
528532
self.assertEqual(values["inverter_temperature"], 35)
529533
self.assertNotEqual(values["inverter_temperature"], 5001)
530534

535+
async def test_issue_13_sandisolar_timeout_does_not_fan_out_optional_reads(
536+
self,
537+
) -> None:
538+
"""One silent session must not become a 55-register timeout cascade."""
539+
540+
driver = SmgModbusDriver()
541+
target = ProbeTarget(devcode=0x0001, collector_addr=0xFF, device_addr=0x01)
542+
inverter = DetectedInverter(
543+
driver_key="modbus_smg",
544+
protocol_family="modbus_smg",
545+
model_name="Sandisolar SD 11KP48V WIFI",
546+
serial_number="92B32511113147",
547+
probe_target=target,
548+
variant_key="sandisolar_sd_11kp48v_wifi",
549+
profile_name="modbus_smg/models/sandisolar_sd_11kp48v_wifi.json",
550+
register_schema_name=(
551+
"modbus_smg/models/sandisolar_sd_11kp48v_wifi.json"
552+
),
553+
capabilities=(),
554+
)
555+
schema = load_register_schema(inverter.register_schema_name)
556+
self.assertEqual(len(schema.spec_set("aux_config")), 55)
557+
558+
class _Session:
559+
def __init__(self) -> None:
560+
self.calls: list[tuple[int, int]] = []
561+
562+
async def read_holding(self, register: int, count: int) -> list[int]:
563+
self.calls.append((register, count))
564+
if (register, count) in {(100, 10), (198, 34), (600, 57)}:
565+
return [0] * count
566+
raise ModbusError("request_timeout")
567+
568+
session = _Session()
569+
with patch.object(
570+
SmgModbusDriver,
571+
"_session",
572+
staticmethod(lambda *_args, **_kwargs: session),
573+
):
574+
with self.assertRaisesRegex(ModbusError, "^request_timeout$"):
575+
await driver.async_read_values(object(), inverter)
576+
577+
self.assertEqual(
578+
session.calls,
579+
[(100, 10), (198, 34), (600, 57), (171, 1)],
580+
)
581+
582+
async def test_explicit_optional_group_rejection_keeps_single_read_fallback(
583+
self,
584+
) -> None:
585+
"""An explicit illegal-range reply may still isolate supported fields."""
586+
587+
schema = load_register_schema(
588+
"modbus_smg/models/sandisolar_sd_11kp48v_wifi.json"
589+
)
590+
specs = tuple(
591+
spec
592+
for spec in schema.spec_set("aux_config")
593+
if 252 <= spec.register <= 256
594+
)
595+
self.assertEqual(len(specs), 5)
596+
597+
class _Session:
598+
def __init__(self) -> None:
599+
self.calls: list[tuple[int, int]] = []
600+
601+
async def read_holding(self, register: int, count: int) -> list[int]:
602+
self.calls.append((register, count))
603+
if count > 1:
604+
raise ModbusError("exception_code:2")
605+
return [register]
606+
607+
session = _Session()
608+
values = await _read_optional_specs(session, specs)
609+
610+
self.assertEqual(session.calls[0], (252, 5))
611+
self.assertEqual(session.calls[1:], [(register, 1) for register in range(252, 257)])
612+
self.assertEqual(set(values), {spec.key for spec in specs})
613+
531614
async def test_probe_selects_hhs_11kw_telemetry_without_anj_controls(self) -> None:
532615
driver = SmgModbusDriver()
533616
target = ProbeTarget(devcode=0x0001, collector_addr=0xFF, device_addr=0x01)

0 commit comments

Comments
 (0)