From dc1fb95adac09a9241c422e79d5147025842bdbf Mon Sep 17 00:00:00 2001 From: Timothy Allen Date: Sat, 10 Oct 2026 11:35:43 +0200 Subject: [PATCH] Log each failed Bluetooth read with its reason collect_data printed a failure only with -v, and gave no reason for a failed connection, a failed authentication, or a read with no telemetry. A device that was out of reach showed only as missing data. Each failed read now gives one log line with the reason, at every debug level. The device object keeps the reason for a failed connection in last_error. --- bmspy/ecoflow_bt.py | 22 +++++++++++++----- tests/test_ecoflow_bt.py | 49 +++++++++++++++++++++++++++++++++++++++- 2 files changed, 64 insertions(+), 7 deletions(-) diff --git a/bmspy/ecoflow_bt.py b/bmspy/ecoflow_bt.py index af56bf0..86511d9 100644 --- a/bmspy/ecoflow_bt.py +++ b/bmspy/ecoflow_bt.py @@ -413,6 +413,8 @@ class EcoFlowBTDevice: self._auth_fallback_task: asyncio.Task | None = None self._pkt_count = 0 + # Reason for the last failed connect(), for callers that log failures. + self.last_error: str | None = None self._authenticated = asyncio.Event() self._on_update: list[Callable[[], None]] = [] self._on_packet: list[Callable[[EcoFlowPacket], None]] = [] @@ -466,6 +468,7 @@ class EcoFlowBTDevice: self._priv_key = None self._set_state(_State.CONNECTING) self._authenticated.clear() + self.last_error = None try: self._client = BleakClient( @@ -474,6 +477,7 @@ class EcoFlowBTDevice: ) await self._client.connect() except Exception as exc: + self.last_error = f"connect failed: {type(exc).__name__}: {exc}" self._log(1, "connect failed: %s", exc) await self.disconnect() self._set_state(_State.ERROR) @@ -492,6 +496,7 @@ class EcoFlowBTDevice: else: await self._auth_type7_init() except Exception as exc: + self.last_error = f"auth error: {type(exc).__name__}: {exc}" self._log(1, "auth error: %s", exc) await self.disconnect() self._set_state(_State.ERROR) @@ -1057,9 +1062,16 @@ def collect_data(cfg: EcoFlowBTConfig, debug: int = 0) -> EcoFlowSnapshot | bool *listen_timeout* seconds in total. Then it disconnects. Returns the populated :class:`EcoFlowSnapshot`, or ``False`` on failure. + Each failure is logged with its reason, at every debug level. A snapshot from a collection that reached the limit is partial, and its ``ef_missing_measurements`` field names the reports that did not arrive. """ + def failed(reason: str) -> bool: + # Always logged: without this line, a device that is out of reach + # only shows as missing data. + debugger(f"ecoflow_bt [{cfg.address}]: read failed: {reason}") + return False + async def _run() -> EcoFlowSnapshot | bool: device = EcoFlowBTDevice( address=cfg.address, @@ -1071,13 +1083,13 @@ def collect_data(cfg: EcoFlowBTConfig, debug: int = 0) -> EcoFlowSnapshot | bool ) try: if not await device.connect(): - return False + return failed(device.last_error or "connect failed") if not await device.wait_authenticated(timeout=30.0): - return False + return failed("authentication did not complete in 30 seconds") missing = await device.wait_for_measurements(cfg.listen_duration, cfg.listen_timeout) # Metadata alone is not a successful telemetry collection. if not device._pkt_count: - return False + return failed("no telemetry packets received") _record_missing(cfg.address, device.snapshot, missing) return device.snapshot finally: @@ -1086,6 +1098,4 @@ def collect_data(cfg: EcoFlowBTConfig, debug: int = 0) -> EcoFlowSnapshot | bool try: return asyncio.run(_run()) except Exception as exc: - if debug > 0: - debugger(f"ecoflow_bt [{cfg.address}]: collect_data error: {exc}") - return False + return failed(f"{type(exc).__name__}: {exc}") diff --git a/tests/test_ecoflow_bt.py b/tests/test_ecoflow_bt.py index 20641f5..d93e0f6 100644 --- a/tests/test_ecoflow_bt.py +++ b/tests/test_ecoflow_bt.py @@ -495,7 +495,7 @@ def test_collection_always_disconnects(monkeypatch, connect_ok, authenticated, p connect=AsyncMock(return_value=connect_ok), wait_authenticated=AsyncMock(return_value=authenticated), wait_for_measurements=AsyncMock(return_value=set()), - disconnect=AsyncMock(), _pkt_count=packets, + disconnect=AsyncMock(), _pkt_count=packets, last_error=None, ) monkeypatch.setattr("bmspy.ecoflow_bt.EcoFlowBTDevice", Mock(return_value=fake)) result = collect_data(initialise_device(ADDRESS, SERIAL, "test-user", listen_duration=0)) @@ -503,6 +503,53 @@ def test_collection_always_disconnects(monkeypatch, connect_ok, authenticated, p fake.disconnect.assert_awaited_once() +@pytest.mark.parametrize("connect,authenticated,packets,reason", [ + (AsyncMock(return_value=False), False, 0, "connect failed: BleakError: device not found"), + (AsyncMock(return_value=True), False, 0, "authentication did not complete in 30 seconds"), + (AsyncMock(return_value=True), True, 0, "no telemetry packets received"), + (AsyncMock(side_effect=RuntimeError("adapter gone")), False, 0, "RuntimeError: adapter gone"), +], ids=["connect", "authentication", "no-packets", "exception"]) +def test_failed_collection_is_logged_without_a_debug_option(monkeypatch, capsys, connect, authenticated, packets, reason): + """A device that is out of reach must show in the server log, not only as missing data.""" + fake = Mock( + connect=connect, wait_authenticated=AsyncMock(return_value=authenticated), + wait_for_measurements=AsyncMock(return_value=set()), + disconnect=AsyncMock(), _pkt_count=packets, + last_error="connect failed: BleakError: device not found", + ) + monkeypatch.setattr("bmspy.ecoflow_bt.EcoFlowBTDevice", Mock(return_value=fake)) + config = initialise_device(ADDRESS, SERIAL, "test-user", listen_duration=0) + assert collect_data(config, debug=0) is False + assert f"ecoflow_bt [{ADDRESS}]: read failed: {reason}" in capsys.readouterr().out + + +def test_successful_collection_logs_no_failure(monkeypatch, capsys): + from bmspy.ecoflow_snapshot import EcoFlowSnapshot + + monkeypatch.setattr("bmspy.ecoflow_bt._last_missing", {}) + fake = Mock( + connect=AsyncMock(return_value=True), wait_authenticated=AsyncMock(return_value=True), + wait_for_measurements=AsyncMock(return_value=set()), + disconnect=AsyncMock(), _pkt_count=3, last_error=None, snapshot=EcoFlowSnapshot(), + ) + monkeypatch.setattr("bmspy.ecoflow_bt.EcoFlowBTDevice", Mock(return_value=fake)) + config = initialise_device(ADDRESS, SERIAL, "test-user", listen_duration=0) + assert collect_data(config) is fake.snapshot + assert capsys.readouterr().out == "" + + +def test_connect_records_the_reason_for_a_failure(monkeypatch): + client = Mock(connect=AsyncMock(side_effect=TimeoutError("no answer")), disconnect=AsyncMock(), is_connected=False) + monkeypatch.setitem(sys.modules, "bleak", SimpleNamespace(BleakClient=Mock(return_value=client))) + + async def run(): + dev = device() + assert not await dev.connect() + return dev.last_error + + assert asyncio.run(run()) == "connect failed: TimeoutError: no answer" + + def test_wait_ends_when_the_required_heartbeats_arrive(monkeypatch): monkeypatch.setattr("bmspy.ecoflow_bt._COMPLETION_POLL_SECONDS", 0.01)