From 90f3a6daaccc764fbbf18ca08b8c05d2ec6f6fc3 Mon Sep 17 00:00:00 2001 From: Andreas Schneider Date: Tue, 25 Aug 2026 10:03:44 +0200 Subject: [PATCH] fix: refresh device config before upload and skip stale-encoded frames A device's config (color scheme, panel type, etc.) was only ever re-read on an advertised reboot edge and cached. It could happend that this cache was never refreshed. So a change in color mode was never seend by the HA component. The E1003 reTerminal hit this and displayed a garbled, 4x-tiled frame after its color_scheme was reconfigured. Worse, delivery.py drained an already-stale-encoded queued upload before running a pending resync, so even the reboot-triggered fix arrived one wake too late. Every connection (live or queued-wake) now re-interrogates the device first and compares a display fingerprint (width/height/color_scheme/panel_ic_type/ rotation) against what the prepared image was built for. A live upload that finds a mismatch re-renders against the fresh config and sends the corrected frame transparently. A queued upload for a sleeping device, which only has the already-encoded bytes and not the source image, is dropped instead of sent, firing a new content_config_mismatch event so it isn't silently lost. --- custom_components/opendisplay/__init__.py | 31 ++++++ custom_components/opendisplay/const.py | 3 + custom_components/opendisplay/delivery.py | 112 +++++++++++++++++----- custom_components/opendisplay/services.py | 47 ++++++++- tests/test_binary_sensor.py | 2 + tests/test_delivery.py | 88 ++++++++++++++++- tests/test_image.py | 1 + tests/test_services.py | 95 ++++++++++++++++++ 8 files changed, 351 insertions(+), 28 deletions(-) diff --git a/custom_components/opendisplay/__init__.py b/custom_components/opendisplay/__init__.py index 070df978..627bb4b5 100644 --- a/custom_components/opendisplay/__init__.py +++ b/custom_components/opendisplay/__init__.py @@ -159,6 +159,37 @@ def _write_cache( ) +async def _refresh_config_from_device( + hass: HomeAssistant, + entry: OpenDisplayConfigEntry, + device: OpenDisplayDevice, +) -> GlobalConfig | None: + """Interrogate an already-connected device and refresh runtime + cache. + + ``OpenDisplayDevice`` skips live interrogation whenever it was constructed + with a pre-supplied ``config=`` which every connection in this + integration does. This avoids a redundant read when the cached config is + still accurate. ``interrogate()`` is cheap and idempotent, so calling it + here on every connection is the only way to catch config drift that + happened without a device reboot (the only other trigger for a refresh). + Returns the fresh config, or None if the device didn't return one. + """ + await device.interrogate() + live_config = device.config + if live_config is None: + return None + fw = await device.read_firmware_version() + is_flex = device.is_flex + landing_url = device.landing_url() + runtime = entry.runtime_data + runtime.firmware = fw + runtime.device_config = live_config + runtime.is_flex = is_flex + runtime.config_resync_pending = False + _write_cache(hass, entry, live_config, fw, is_flex, landing_url) + return live_config + + def _cache_setup_if_sleepy( entry: OpenDisplayConfigEntry, ) -> _CachedState | None: diff --git a/custom_components/opendisplay/const.py b/custom_components/opendisplay/const.py index b14f9959..648eac46 100644 --- a/custom_components/opendisplay/const.py +++ b/custom_components/opendisplay/const.py @@ -90,3 +90,6 @@ # --- Bus events ------------------------------------------------------------- EVENT_CONTENT_DELIVERED = f"{DOMAIN}_content_delivered" EVENT_CONTENT_EXPIRED = f"{DOMAIN}_content_expired" +# Fired when a queued upload is dropped because the device's live display +# config no longer matches the config the image was prepared against. +EVENT_CONTENT_CONFIG_MISMATCH = f"{DOMAIN}_content_config_mismatch" diff --git a/custom_components/opendisplay/delivery.py b/custom_components/opendisplay/delivery.py index 56c8fbe7..2a8b0415 100644 --- a/custom_components/opendisplay/delivery.py +++ b/custom_components/opendisplay/delivery.py @@ -37,6 +37,7 @@ AuthenticationRequiredError, BLEConnectionError, BLETimeoutError, + GlobalConfig, OpenDisplayDevice, OpenDisplayError, PartialState, @@ -50,6 +51,7 @@ CONF_MAX_QUEUE_SIZE, DEFAULT_BLOCKS_PER_ACK, DEFAULT_MAX_QUEUE_SIZE, + EVENT_CONTENT_CONFIG_MISMATCH, EVENT_CONTENT_DELIVERED, EVENT_CONTENT_EXPIRED, SIGNAL_IMAGE_UPDATED, @@ -80,6 +82,36 @@ _KEY_INVALID = object() +@dataclass(frozen=True) +class DisplayFingerprint: + """The subset of a display's config that determines image encoding. + + Used to detect drift between the config an image was prepared against and + the device's current live config, e.g. a `color_scheme` change made + without a reboot. A reboot-triggered resync would otherwise miss this. + Deliberately narrow: comparing the whole `GlobalConfig` would false-positive + on unrelated field changes (LEDs, WiFi, sensors, ...) and drop good uploads. + """ + + pixel_width: int + pixel_height: int + color_scheme: int + panel_ic_type: int + rotation: int + + +def display_fingerprint(config: GlobalConfig) -> DisplayFingerprint: + """Extract the encoding-relevant fingerprint from a device's config.""" + display = config.displays[0] + return DisplayFingerprint( + pixel_width=display.pixel_width, + pixel_height=display.pixel_height, + color_scheme=display.color_scheme, + panel_ic_type=display.panel_ic_type, + rotation=display.rotation, + ) + + @dataclass class PendingUpload: """A prepared image queued for delivery at the next wake.""" @@ -94,6 +126,9 @@ class PendingUpload: device_id: str | None queued_at: float expires_at: float + # The display config `prepared` was built against — compared against the + # live device at drain time so a stale-encoded frame is never sent. + fingerprint: DisplayFingerprint attempts: int = 0 cancel_deadline: CALLBACK_TYPE | None = None @@ -202,6 +237,7 @@ def submit_upload( use_measured_palettes: bool, preview_jpeg: bytes, device_id: str | None, + fingerprint: DisplayFingerprint, ) -> DeliveryReceipt: """Queue a prepared image for delivery at the next wake (latest-wins).""" now = time.time() @@ -218,6 +254,7 @@ def submit_upload( device_id=device_id, queued_at=now, expires_at=expires_at, + fingerprint=fingerprint, ) self._schedule_expiry(slot) self._pending_upload = slot @@ -347,14 +384,32 @@ async def _drain_once(self) -> None: } async def _run(device: OpenDisplayDevice) -> None: + # Always refresh first, unconditionally: a config change made + # without a device reboot is otherwise never picked up (the only + # other trigger is the reboot-advertised resync flag), and + # draining a stale-encoded upload before a pending resync ran was + # the original ordering bug this replaces. Local import avoids an + # import cycle (__init__ imports this module). + from . import _refresh_config_from_device + + live_config = await _refresh_config_from_device( + self._hass, self._entry, device + ) + if live_config is not None: + self._pending_config_resync = False + # Re-read pending state each invocation: if a WiFi attempt uploaded # then failed on resync, the BLE fallback re-runs this and must not # re-upload an already-delivered frame (``_drain_upload`` clears it). pending = self._pending_upload - if pending is not None: - await self._drain_upload(device, pending) - if self._pending_config_resync: - await self._drain_resync(device) + if pending is None: + return + if live_config is not None: + live_fingerprint = display_fingerprint(live_config) + if live_fingerprint != pending.fingerprint: + self._drop_mismatched_upload(pending, live_fingerprint) + return + await self._drain_upload(device, pending) # Prefer WiFi when the entry has a fresh mDNS host; fall back to BLE on any # WiFi failure. All inside the per-MAC lock (MAC-keyed, transport-neutral). @@ -394,26 +449,6 @@ async def _drain_upload( self._notify_state() _LOGGER.info("%s: queued content delivered", self._address) - async def _drain_resync(self, device: OpenDisplayDevice) -> None: - """Re-read firmware/config over the open link and refresh the cache.""" - # Local import avoids an import cycle (__init__ imports this module). - from . import _write_cache - - fw = await device.read_firmware_version() - is_flex = device.is_flex - landing_url = device.landing_url() - device_config = device.config - if device_config is None: - return - runtime = self._entry.runtime_data - runtime.firmware = fw - runtime.device_config = device_config - runtime.is_flex = is_flex - runtime.config_resync_pending = False - self._pending_config_resync = False - _write_cache(self._hass, self._entry, device_config, fw, is_flex, landing_url) - _LOGGER.debug("%s: config resync complete", self._address) - # -- expiry ------------------------------------------------------------- @callback @@ -466,6 +501,35 @@ def _give_up_upload(self, slot: PendingUpload, reason: str) -> None: self._fire_content_event(EVENT_CONTENT_EXPIRED, slot) self._notify_state() + @callback + def _drop_mismatched_upload( + self, slot: PendingUpload, live_fingerprint: DisplayFingerprint + ) -> None: + """Drop a queued upload whose fingerprint no longer matches the live device. + + The device's display config changed (e.g. `color_scheme` edited + without a reboot) since this frame was prepared, so its encoded bytes + no longer match what the panel expects — sending it would corrupt the + display. Unlike a live/immediate upload, there is no source image + available here to re-render from (``PendingUpload`` only retains the + already-encoded/dithered result), so the safest option is to drop it + and let the next scheduled push re-prepare against the current config. + """ + if slot.cancel_deadline: + slot.cancel_deadline() + slot.cancel_deadline = None + self._pending_upload = None + self._last_error = "config_mismatch" + _LOGGER.warning( + "%s: dropping queued upload - device config changed since the image " + "was prepared (prepared for %s, device is now %s)", + self._address, + slot.fingerprint, + live_fingerprint, + ) + self._fire_content_event(EVENT_CONTENT_CONFIG_MISMATCH, slot) + self._notify_state() + # -- helpers ------------------------------------------------------------ @callback diff --git a/custom_components/opendisplay/services.py b/custom_components/opendisplay/services.py index 2f220d78..524e32dc 100644 --- a/custom_components/opendisplay/services.py +++ b/custom_components/opendisplay/services.py @@ -74,7 +74,7 @@ DOMAIN, SIGNAL_IMAGE_UPDATED, ) -from .delivery import DELIVERY_DEADLINE_S, DeliveryReceipt +from .delivery import DELIVERY_DEADLINE_S, DeliveryReceipt, display_fingerprint from .transport import async_run_with_fallback ATTR_IMAGE = "image" @@ -624,6 +624,10 @@ async def _async_send_image( use_measured_palettes=use_measured_palettes, ) ) + # The config `prepared` was built against — compared against the live + # device right before sending, since it can drift without a reboot (the + # only event that otherwise triggers a resync). + fingerprint = display_fingerprint(config) # Partial refreshes diff against the entry's tracked frame; full/fast # refreshes re-baseline the panel, so start a fresh state that this upload @@ -653,11 +657,50 @@ def _queue() -> DeliveryReceipt: use_measured_palettes=use_measured_palettes, preview_jpeg=jpeg, device_id=device_id, + fingerprint=fingerprint, ) async def _upload(device: OpenDisplayDevice) -> None: + # Refresh from this live connection (config drift is otherwise only + # caught on a device reboot) and, if the device's config no longer + # matches what `prepared` was encoded for, re-render against the + # live config rather than send a corrupted frame. Unlike the queued + # path, the original `img` is still in scope here, so this can be + # corrected transparently instead of failing the upload. + from . import _refresh_config_from_device + + live_config = await _refresh_config_from_device(hass, entry, device) + to_send = prepared + if live_config is not None: + live_fingerprint = display_fingerprint(live_config) + if live_fingerprint != fingerprint: + live_display_cfg = live_config.displays[0] + live_supports_compression = ( + live_display_cfg.supports_zip + or live_display_cfg.supports_streaming_decompression + ) + _LOGGER.warning( + "%s: device config changed since the image was prepared " + "(prepared for %s, now %s); re-rendering before upload", + entry.unique_id, + fingerprint, + live_fingerprint, + ) + to_send = await hass.async_add_executor_job( + functools.partial( + prepare_image, + img, + config=live_config, + dither_mode=dither_mode, + compress=live_supports_compression, + tone=tone, + fit=fit, + rotate=rotate, + use_measured_palettes=use_measured_palettes, + ) + ) await device.upload_prepared_image( - prepared, refresh_mode=refresh_mode, state=state + to_send, refresh_mode=refresh_mode, state=state ) # Freshness gate: a probably-asleep tag will not usually answer a live diff --git a/tests/test_binary_sensor.py b/tests/test_binary_sensor.py index 92e3fc93..9ae93462 100644 --- a/tests/test_binary_sensor.py +++ b/tests/test_binary_sensor.py @@ -58,6 +58,7 @@ async def test_update_pending_turns_on_when_content_is_queued( use_measured_palettes=False, preview_jpeg=b"jpeg", device_id=None, + fingerprint=MagicMock(), ) await hass.async_block_till_done() @@ -87,6 +88,7 @@ async def test_update_pending_survives_a_dark_device( use_measured_palettes=False, preview_jpeg=b"jpeg", device_id=None, + fingerprint=MagicMock(), ) await hass.async_block_till_done() diff --git a/tests/test_delivery.py b/tests/test_delivery.py index e35d7ec1..c35ccbeb 100644 --- a/tests/test_delivery.py +++ b/tests/test_delivery.py @@ -7,6 +7,7 @@ """ import asyncio +from dataclasses import replace from datetime import timedelta import logging from unittest.mock import AsyncMock, MagicMock, patch @@ -37,10 +38,11 @@ CONF_MAX_QUEUE_SIZE, DEFAULT_BLOCKS_PER_ACK, DEFAULT_MAX_QUEUE_SIZE, + EVENT_CONTENT_CONFIG_MISMATCH, EVENT_CONTENT_DELIVERED, EVENT_CONTENT_EXPIRED, ) -from custom_components.opendisplay.delivery import DeliveryManager +from custom_components.opendisplay.delivery import DeliveryManager, display_fingerprint from . import ( TEST_ADDRESS as ADDRESS, @@ -87,6 +89,7 @@ def _submit(mgr: DeliveryManager, device_id: str = "dev1", **overrides): "use_measured_palettes": False, "preview_jpeg": b"jpeg", "device_id": device_id, + "fingerprint": display_fingerprint(make_sleepy_device_config()), } kwargs.update(overrides) return mgr.submit_upload(**kwargs) @@ -102,9 +105,18 @@ def _reauth_flows(hass: HomeAssistant) -> list: def _uploading_device() -> MagicMock: - """Return a device mock that accepts a prepared image.""" + """Return a device mock that accepts a prepared image. + + ``config`` defaults to None so the unconditional pre-upload refresh + (``_refresh_config_from_device``) short-circuits right after + ``interrogate()`` without touching firmware/cache — tests that care about + the refresh itself (or a fingerprint mismatch) override ``config`` + explicitly. + """ device = MagicMock() device.upload_prepared_image = AsyncMock() + device.interrogate = AsyncMock() + device.config = None return device @@ -202,6 +214,78 @@ async def test_drain_delivers_the_queued_upload( assert len(delivered) == 1 +async def test_drain_interrogates_before_uploading( + entry: MockConfigEntry, manager: DeliveryManager +) -> None: + """The device is always re-interrogated before a queued frame is sent. + + This is what fixes the original ordering bug: a config resync must never + be able to run *after* an already-stale-encoded upload went out. + """ + call_order: list[str] = [] + device = _uploading_device() + device.interrogate.side_effect = lambda: call_order.append("interrogate") + device.config = make_sleepy_device_config() + device.read_firmware_version = AsyncMock( + side_effect=lambda: ( + call_order.append("read_firmware_version") + or {"major": 1, "minor": 0, "sha": "abc"} + ) + ) + device.is_flex = True + device.landing_url = MagicMock(return_value="http://landing") + device.upload_prepared_image.side_effect = lambda *a, **k: call_order.append( + "upload_prepared_image" + ) + + with connects_to(device): + _submit(manager, fingerprint=display_fingerprint(device.config)) + await manager._deliver() + + assert call_order == [ + "interrogate", + "read_firmware_version", + "upload_prepared_image", + ] + assert entry.runtime_data.device_config is device.config + + +async def test_drain_config_mismatch_drops_the_upload( + hass: HomeAssistant, entry: MockConfigEntry, manager: DeliveryManager +) -> None: + """A frame prepared against a since-changed config is dropped, not sent. + + Sending it anyway would encode the image for the wrong panel format (the + exact bug this whole mechanism exists to prevent) rather than a merely + stale-but-still-valid frame. + """ + mismatched = async_capture_events(hass, EVENT_CONTENT_CONFIG_MISMATCH) + prepared_config = make_sleepy_device_config() + live_config = replace( + prepared_config, + displays=[replace(prepared_config.displays[0], color_scheme=99)], + ) + device = _uploading_device() + device.config = live_config + device.read_firmware_version = AsyncMock( + return_value={"major": 1, "minor": 0, "sha": "abc"} + ) + device.is_flex = True + device.landing_url = MagicMock(return_value="http://landing") + + with connects_to(device): + _submit(manager, fingerprint=display_fingerprint(prepared_config)) + await manager._deliver() + await hass.async_block_till_done() + + device.upload_prepared_image.assert_not_awaited() + assert manager.state.pending is False + assert manager.state.last_error == "config_mismatch" + assert len(mismatched) == 1 + # The live config was still adopted even though the stale upload was dropped. + assert entry.runtime_data.device_config is live_config + + async def test_drain_ble_failure_keeps_the_slot_and_counts_an_attempt( manager: DeliveryManager, ) -> None: diff --git a/tests/test_image.py b/tests/test_image.py index 3c372dff..e05db713 100644 --- a/tests/test_image.py +++ b/tests/test_image.py @@ -102,6 +102,7 @@ async def test_queued_frame_is_marked_pending( use_measured_palettes=False, preview_jpeg=JPEG, device_id=None, + fingerprint=MagicMock(), ) await hass.async_block_till_done() diff --git a/tests/test_services.py b/tests/test_services.py index ef08ab1a..b2797260 100644 --- a/tests/test_services.py +++ b/tests/test_services.py @@ -2,8 +2,10 @@ import asyncio from collections.abc import Generator +from dataclasses import replace import io from pathlib import Path +from typing import Any from unittest.mock import AsyncMock, MagicMock, patch import aiohttp @@ -16,6 +18,7 @@ AuthenticationFailedError, AuthenticationRequiredError, BLEConnectionError, + ColorScheme, NfcNotSupportedError, NfcWriteError, ) @@ -93,6 +96,98 @@ async def test_upload_image_local_file( mock_upload_device.upload_prepared_image.assert_called_once() +async def test_upload_image_refreshes_runtime_config( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_upload_device: MagicMock, + mock_resolve_media: MagicMock, +) -> None: + """A live upload re-interrogates the device and refreshes the cached config. + + This is what catches a config change made without a device reboot — the + only other event that would otherwise trigger a refresh. + """ + device_id = _device_id(hass, mock_config_entry) + + await hass.services.async_call( + DOMAIN, + "upload_image", + { + "device_id": device_id, + "image": { + "media_content_id": "media-source://local/test.png", + "media_content_type": "image/png", + }, + }, + blocking=True, + ) + + mock_upload_device.interrogate.assert_awaited() + assert mock_config_entry.runtime_data.device_config is mock_upload_device.config + + +async def test_upload_image_config_mismatch_rerenders_and_uploads( + hass: HomeAssistant, + mock_config_entry: MockConfigEntry, + mock_upload_device: MagicMock, + mock_resolve_media: MagicMock, +) -> None: + """A config drift detected mid-connection re-renders instead of sending stale bytes. + + Unlike the queued/sleepy path, the original source image is still in + scope for a live upload, so the corrected frame can be sent transparently + -- the service call must still succeed, not raise. + """ + device_id = _device_id(hass, mock_config_entry) + original_config = mock_upload_device.config + mismatched_config = replace( + original_config, + displays=[ + replace(original_config.displays[0], color_scheme=ColorScheme.MONO.value) + ], + ) + + def _drift_on_interrogate() -> None: + mock_upload_device.config = mismatched_config + + mock_upload_device.interrogate.side_effect = _drift_on_interrogate + + from custom_components.opendisplay import services as services_mod + + real_prepare_image = services_mod.prepare_image + prepared_results: list[Any] = [] + + def _tracking_prepare_image(*args: Any, **kwargs: Any) -> Any: + result = real_prepare_image(*args, **kwargs) + prepared_results.append(result) + return result + + with patch.object( + services_mod, "prepare_image", side_effect=_tracking_prepare_image + ) as spy_prepare: + await hass.services.async_call( + DOMAIN, + "upload_image", + { + "device_id": device_id, + "image": { + "media_content_id": "media-source://local/test.png", + "media_content_type": "image/png", + }, + }, + blocking=True, + ) + + assert spy_prepare.call_count == 2 + assert spy_prepare.call_args_list[0].kwargs["config"] is original_config + assert spy_prepare.call_args_list[1].kwargs["config"] is mismatched_config + # Different color schemes must not encode to the same bytes. + assert prepared_results[0] != prepared_results[1] + mock_upload_device.upload_prepared_image.assert_called_once() + sent_data = mock_upload_device.upload_prepared_image.call_args[0][0] + assert sent_data == prepared_results[1] + + async def test_upload_image_remote_url( hass: HomeAssistant, mock_config_entry: MockConfigEntry,