fix: repair the dead stable channel and clear the review findings (#2)

* fix: repoint the stable channel at the published OTA manifest

The stable manifest URL 404s: neither
Faikin-S3-MINI-N4-R2-manifest.json (what we shipped) nor the
Faikout- spelling exists on ota.faikout.uk. Every stable-channel
user has been getting UpdateFailed on every refresh since v0.1.0.
The published stable manifest is Faikout.manifest -- same
"id": "Faikout-S3-MINI-N4-R2", self-referential url, and an
app: true entry pointing at Faikout-S3-MINI-N4-R2.bin. Verified
end to end: manifest -> app image -> HTTP 206 -> version c763929d.

The bug hid because the only live check skipped on any
FirmwareFetchError, so a 404 looked identical to an offline OTA
server. Split FirmwareUnavailableError out of FirmwareFetchError
for the case where the host never answered, and skip only on that
-- a bad status now fails the run. Nothing in CI ran that check at
all, so add a nightly OTA job to the Validate workflow, off the PR
path so an upstream outage blocks nobody.

Also in the OTA client, since the exception split touched it:

- Bound the Range-ignored fallback to 512 bytes. A server that
  answers 200 instead of 206 was having its whole ~1.5 MB image
  buffered, defeating the point of the ranged request.
- Fold the duplicated except-wrapper into one _fetch helper.
- Replace a tautological `assert session.closed is False` (the
  fake never had a close()) with a real close-contract test.

* fix: collapse the channel onto entry.options and fix partial-failure logging

Remediation of the outstanding two-axis review findings.

Config entry (schema v2, with migration):

- The channel was written to entry.data at creation and to
  entry.options on change, so every reader consulted both -- three
  differently-defaulted spellings of the same lookup. It now lives
  in options alone; async_migrate_entry moves v1 entries across,
  preferring the options value as the one the user last chose, and
  refuses an entry from a newer release rather than guessing.
- The config flow never checked MQTT, though the design requires an
  abort. It does now, once per flow on the way to the form rather
  than again on submit -- async_wait_for_mqtt_client can block for
  up to 50s while MQTT is still setting up.

Coordinator logging:

- A wholly-failed refresh is now left entirely to
  DataUpdateCoordinator, which already logs it once down and once
  recovered; we were logging it a second time alongside.
- A *partial* failure counts as a successful refresh, so the base
  class says nothing and a missing target was invisible above
  debug. Those are reported here instead: unreachable-host warns
  once per outage with a matching recovery, while a 404 or bad
  metadata warns every refresh -- warn-once-then-silence is exactly
  how the dead stable manifest stayed hidden for two months.
- UpdateFailed could render "...: None" when a channel resolved to
  no URLs at all; that case gets its own translated message.

Smaller cleanups:

- binary_sensor reached into the config entry for a channel the
  coordinator already holds; it now asks the coordinator, and the
  four repeated tracker lookups collapse into one _device property.
- Guard the options read so an entry without a channel falls back
  to stable instead of raising KeyError out of setup.

The design doc is updated to match on the two points settled by
decision -- the Gold quality target (it said Silver) and the
coordinator's deliberate whole-table iteration -- plus the
FirmwareUnavailableError split and the corrected logging contract.

* chore: release 0.2.0

Config entries move to schema v2 (channel in options, with
migration) and the stable channel URL changes, so this is not a
patch release.

Author: steynovich

.github/workflows/validate.yml

@@ -2,6 +2,7 @@ name: Validate
 on:
   push:
   pull_request:
+  workflow_dispatch:
   schedule:
     - cron: "0 0 * * *"
 
@@ -11,6 +12,20 @@ jobs:
     steps:
       - uses: actions/checkout@v4
       - uses: home-assistant/actions/hassfest@master
+  ota:
+    # The offline suite cannot notice a dead OTA URL: a 404 on the stable manifest
+    # went unseen for two months because the only live check skipped on any fetch
+    # error. Run it nightly, off the PR path, so an upstream outage blocks nobody.
+    if: github.event_name == 'schedule' || github.event_name == 'workflow_dispatch'
+    runs-on: ubuntu-latest
+    steps:
+      - uses: actions/checkout@v4
+      - uses: astral-sh/setup-uv@v6
+        with:
+          python-version: "3.13"
+      - run: uv sync --dev
+      - run: uv run pytest -m network
+
   hacs:
     runs-on: ubuntu-latest
     steps:

custom_components/faikout/__init__.py

@@ -29,11 +29,28 @@ class FaikoutRuntimeData:
 type FaikoutConfigEntry = ConfigEntry[FaikoutRuntimeData]
 
 
+async def async_migrate_entry(hass: HomeAssistant, entry: FaikoutConfigEntry) -> bool:
+    if entry.version > 2:
+        # Written by a newer release of this integration; refuse rather than guess.
+        return False
+    if entry.version == 1:
+        # v1 wrote the channel to entry.data at creation and to entry.options on
+        # change, so every reader had to consult both. Collapse onto options,
+        # preferring the options value because it is the one the user last chose.
+        data = dict(entry.data)
+        stale_channel = data.pop(CONF_CHANNEL, Channel.STABLE.value)
+        channel = entry.options.get(CONF_CHANNEL, stale_channel)
+        hass.config_entries.async_update_entry(
+            entry, data=data, options={**entry.options, CONF_CHANNEL: channel}, version=2
+        )
+    return True
+
+
 async def async_setup_entry(hass: HomeAssistant, entry: FaikoutConfigEntry) -> bool:
     if not await mqtt.async_wait_for_mqtt_client(hass):
         raise ConfigEntryNotReady(translation_domain=DOMAIN, translation_key="mqtt_unavailable")
 
-    channel = Channel(entry.options.get(CONF_CHANNEL, entry.data[CONF_CHANNEL]))
+    channel = Channel(entry.options.get(CONF_CHANNEL, Channel.STABLE.value))
     client = FaikoutOtaClient(async_get_clientsession(hass))
     coordinator = FaikoutCoordinator(hass, client, channel)
     tracker = FaikoutDeviceTracker(hass)

custom_components/faikout/binary_sensor.py

@@ -17,9 +17,9 @@
 from homeassistant.helpers.update_coordinator import CoordinatorEntity
 
 from . import FaikoutConfigEntry
-from .const import CONF_CHANNEL, DOMAIN, MANUFACTURER, SIGNAL_DEVICE_UPDATE
+from .const import DOMAIN, MANUFACTURER, SIGNAL_DEVICE_UPDATE
 from .coordinator import FaikoutCoordinator
-from .device_tracker import FaikoutDeviceTracker
+from .device_tracker import FaikoutDevice, FaikoutDeviceTracker
 
 PARALLEL_UPDATES = 0
 
@@ -37,9 +37,7 @@ def _add(device_id: str) -> None:
         if device_id in known or device_id not in data.tracker.devices:
             return
         known.add(device_id)
-        async_add_entities(
-            [FirmwareUpdateBinarySensor(data.coordinator, data.tracker, entry, device_id)]
-        )
+        async_add_entities([FirmwareUpdateBinarySensor(data.coordinator, data.tracker, device_id)])
 
     for device_id in list(data.tracker.devices):
         _add(device_id)
@@ -58,12 +56,10 @@ def __init__(
         self,
         coordinator: FaikoutCoordinator,
         tracker: FaikoutDeviceTracker,
-        entry: FaikoutConfigEntry,
         device_id: str,
     ) -> None:
         super().__init__(coordinator)
         self._tracker = tracker
-        self._entry = entry
         self._device_id = device_id
         self._attr_unique_id = f"{device_id}_firmware_update"
         device = tracker.devices[device_id]
@@ -75,33 +71,38 @@ def __init__(
             model=device.target,
         )
 
+    @property
+    def _device(self) -> FaikoutDevice | None:
+        # The tracker drops a device when MQTT stops reporting it, so every read
+        # goes through here rather than caching the instance from __init__.
+        return self._tracker.devices.get(self._device_id)
+
     @property
     def _latest(self) -> str | None:
-        device = self._tracker.devices.get(self._device_id)
+        device = self._device
         if device is None:
             return None
         return self.coordinator.data.get(device.target)
 
     @property
     def available(self) -> bool:
-        device = self._tracker.devices.get(self._device_id)
-        return super().available and device is not None and self._latest is not None
+        return super().available and self._device is not None and self._latest is not None
 
     @property
     def is_on(self) -> bool | None:
-        device = self._tracker.devices.get(self._device_id)
+        device = self._device
         latest = self._latest
         if device is None or latest is None:
             return None
         return device.version != latest
 
     @property
     def extra_state_attributes(self) -> dict[str, str | None]:
-        device = self._tracker.devices.get(self._device_id)
+        device = self._device
         return {
             "installed_version": device.version if device else None,
             "latest_version": self._latest,
-            "channel": self._entry.options.get(CONF_CHANNEL, self._entry.data[CONF_CHANNEL]),
+            "channel": self.coordinator.channel.value,
             "target": device.target if device else None,
         }
 

custom_components/faikout/config_flow.py

@@ -5,6 +5,7 @@
 from typing import Any
 
 import voluptuous as vol
+from homeassistant.components import mqtt
 from homeassistant.config_entries import (
     ConfigEntry,
     ConfigFlow,
@@ -21,15 +22,26 @@
 class FaikoutConfigFlow(ConfigFlow, domain=DOMAIN):
     """Handle the initial configuration."""
 
+    # v2 moved the channel from entry.data to entry.options.
+    VERSION = 2
+
     async def async_step_user(self, user_input: dict[str, Any] | None = None) -> ConfigFlowResult:
         if self._async_current_entries():
             return self.async_abort(reason="single_instance_allowed")
-        if user_input is not None:
-            return self.async_create_entry(title="Faikout Firmware Update", data=user_input)
-        schema = vol.Schema(
-            {vol.Required(CONF_CHANNEL, default=Channel.STABLE.value): vol.In(_CHANNELS)}
-        )
-        return self.async_show_form(step_id="user", data_schema=schema)
+        if user_input is None:
+            # MQTT is a hard dependency: without it no device is ever discovered.
+            # Checked here, on the way to the form, so it runs once per flow rather
+            # than again on submit — it can wait up to 50s while MQTT is still
+            # setting up — and so the user is turned away before picking a channel.
+            if not await mqtt.async_wait_for_mqtt_client(self.hass):
+                return self.async_abort(reason="mqtt_unavailable")
+            schema = vol.Schema(
+                {vol.Required(CONF_CHANNEL, default=Channel.STABLE.value): vol.In(_CHANNELS)}
+            )
+            return self.async_show_form(step_id="user", data_schema=schema)
+        # The channel does not establish the connection, so it lives in options and
+        # stays a single source of truth for the options flow to rewrite.
+        return self.async_create_entry(title="Faikout Firmware Update", data={}, options=user_input)
 
     @staticmethod
     @callback
@@ -43,8 +55,6 @@ class FaikoutOptionsFlow(OptionsFlow):
     async def async_step_init(self, user_input: dict[str, Any] | None = None) -> ConfigFlowResult:
         if user_input is not None:
             return self.async_create_entry(title="", data=user_input)
-        current = self.config_entry.options.get(
-            CONF_CHANNEL, self.config_entry.data.get(CONF_CHANNEL, Channel.STABLE.value)
-        )
+        current = self.config_entry.options.get(CONF_CHANNEL, Channel.STABLE.value)
         schema = vol.Schema({vol.Required(CONF_CHANNEL, default=current): vol.In(_CHANNELS)})
         return self.async_show_form(step_id="init", data_schema=schema)

custom_components/faikout/const.py

@@ -19,9 +19,9 @@ class Channel(StrEnum):
 
 
 MANIFEST_URLS: dict[tuple[str, Channel], str] = {
-    ("Faikout-S3-MINI-N4-R2", Channel.STABLE): (
-        "https://ota.faikout.uk/Faikin-S3-MINI-N4-R2-manifest.json"
-    ),
+    # The stable channel is published as the unversioned "Faikout.manifest"; the
+    # per-target "Faikout-S3-MINI-N4-R2-manifest.json" path 404s on the OTA server.
+    ("Faikout-S3-MINI-N4-R2", Channel.STABLE): "https://ota.faikout.uk/Faikout.manifest",
     ("Faikout-S3-MINI-N4-R2", Channel.BETA): (
         "https://ota.faikout.uk/beta/Faikout-S3-MINI-N4-R2-beta-manifest.json"
     ),

custom_components/faikout/coordinator.py

@@ -9,7 +9,7 @@
 
 from .const import DOMAIN, MANIFEST_URLS, UPDATE_INTERVAL, Channel
 from .ota.client import FaikoutOtaClient
-from .ota.exceptions import FaikoutError
+from .ota.exceptions import FaikoutError, FirmwareUnavailableError
 
 _LOGGER = logging.getLogger(__name__)
 
@@ -21,6 +21,9 @@ def __init__(self, hass: HomeAssistant, client: FaikoutOtaClient, channel: Chann
         super().__init__(hass, _LOGGER, name=DOMAIN, update_interval=UPDATE_INTERVAL)
         self._client = client
         self.channel = channel
+        # Targets already warned about as an unreachable-server outage, so the
+        # warning lands once on the way down and a recovery lands once on the way up.
+        self._unreachable_targets: set[str] = set()
 
     async def _async_update_data(self) -> dict[str, str]:
         # Iterate ALL entries in MANIFEST_URLS for this channel, not just targets seen
@@ -30,19 +33,63 @@ async def _async_update_data(self) -> dict[str, str]:
         # transient network errors and permanent data faults are treated as retryable
         # here, since a server-side data fault may later be corrected server-side.
         result: dict[str, str] = {}
-        last_error: Exception | None = None
+        failures: dict[str, FaikoutError] = {}
         for (target, channel), url in MANIFEST_URLS.items():
             if channel != self.channel:
                 continue
             try:
                 result[target] = await self._client.async_get_latest_version(url)
-            except FaikoutError as err:
-                last_error = err
-                _LOGGER.debug("Failed to fetch latest version for %s: %s", target, err)
+            except FaikoutError as error:
+                # Safe to keep past the except block: only the name is unbound.
+                failures[target] = error
+
         if not result:
+            # Every target failed, which DataUpdateCoordinator already logs once on
+            # the way down and once on recovery. Stay quiet so the outage is not
+            # reported twice; just carry the state so a later partial failure is
+            # still judged against it.
+            self._unreachable_targets = {
+                target
+                for target, err in failures.items()
+                if isinstance(err, FirmwareUnavailableError)
+            }
+            last_error = next(iter(failures.values()), None)
+            if last_error is None:
+                # The loop never ran: no manifest URL is mapped for this channel, so
+                # there is no underlying error to report and retrying cannot help.
+                raise UpdateFailed(
+                    translation_domain=DOMAIN,
+                    translation_key="no_manifest_urls",
+                    translation_placeholders={"channel": self.channel.value},
+                )
             raise UpdateFailed(
                 translation_domain=DOMAIN,
                 translation_key="cannot_fetch_version",
                 translation_placeholders={"error": str(last_error)},
             )
+
+        # Some targets resolved, so the refresh counts as a success and the base
+        # class logs nothing. Report the stragglers here or they stay invisible.
+        for target, err in failures.items():
+            self._log_target_failed(target, err)
+        for target in result:
+            self._log_target_recovered(target)
         return result
+
+    def _log_target_failed(self, target: str, err: FaikoutError) -> None:
+        if not isinstance(err, FirmwareUnavailableError):
+            # A 404, a malformed manifest or a bad image will not clear itself on
+            # the next poll, and staying quiet is how a dead stable-channel URL
+            # went unnoticed for two months. Say so every time.
+            self._unreachable_targets.discard(target)
+            _LOGGER.warning("Firmware metadata for %s is unusable: %s", target, err)
+        elif target in self._unreachable_targets:
+            _LOGGER.debug("OTA server still unreachable for %s: %s", target, err)
+        else:
+            self._unreachable_targets.add(target)
+            _LOGGER.warning("Cannot reach the OTA server for %s: %s", target, err)
+
+    def _log_target_recovered(self, target: str) -> None:
+        if target in self._unreachable_targets:
+            self._unreachable_targets.discard(target)
+            _LOGGER.info("OTA server reachable again for %s", target)

custom_components/faikout/manifest.json

@@ -10,5 +10,5 @@
   "issue_tracker": "https://github.com/steynovich/ha-faikout-firmware/issues",
   "quality_scale": "gold",
   "requirements": [],
-  "version": "0.1.0"
+  "version": "0.2.0"
 }

custom_components/faikout/ota/client.py

@@ -2,15 +2,36 @@
 
 from __future__ import annotations
 
+from collections.abc import Awaitable, Callable
+from typing import TypeVar
+
 import aiohttp
 
-from .exceptions import FirmwareFetchError
+from .exceptions import FirmwareFetchError, FirmwareUnavailableError
 from .manifest import parse_manifest
 from .parser import parse_app_descriptor
 
 HEAD_BYTES = 512
 DEFAULT_TIMEOUT = 30.0
 
+_T = TypeVar("_T")
+
+
+async def _read_head(resp: aiohttp.ClientResponse) -> bytes:
+    """Read at most HEAD_BYTES from the response body.
+
+    A server that ignores the Range header answers 200 with the whole ~1.5 MB
+    image, so reading the full body would defeat the point of the ranged request.
+    StreamReader.read(n) may return fewer than n bytes, hence the loop.
+    """
+    buf = bytearray()
+    while len(buf) < HEAD_BYTES:
+        chunk = await resp.content.read(HEAD_BYTES - len(buf))
+        if not chunk:
+            break
+        buf.extend(chunk)
+    return bytes(buf)
+
 
 class FaikoutOtaClient:
     """Fetch and parse the latest Faikout firmware version.
@@ -33,22 +54,26 @@ async def async_get_latest_version(self, manifest_url: str) -> str:
         head = await self._get_head(app_url)
         return parse_app_descriptor(head)
 
-    async def _get_text(self, url: str) -> str:
+    async def _fetch(
+        self,
+        url: str,
+        reader: Callable[[aiohttp.ClientResponse], Awaitable[_T]],
+        *,
+        headers: dict[str, str] | None = None,
+    ) -> _T:
         try:
-            async with self._session.get(url, timeout=self._timeout) as resp:
+            async with self._session.get(url, headers=headers, timeout=self._timeout) as resp:
                 resp.raise_for_status()
-                return await resp.text()
-        except (aiohttp.ClientError, TimeoutError) as err:
+                return await reader(resp)
+        except (aiohttp.ClientConnectionError, TimeoutError) as err:
+            # The host never answered; retryable, and not a sign of a bad URL.
+            raise FirmwareUnavailableError(f"failed to fetch {url}: {err}") from err
+        except aiohttp.ClientError as err:
             raise FirmwareFetchError(f"failed to fetch {url}: {err}") from err
 
+    async def _get_text(self, url: str) -> str:
+        return await self._fetch(url, lambda resp: resp.text())
+
     async def _get_head(self, url: str) -> bytes:
         headers = {"Range": f"bytes=0-{HEAD_BYTES - 1}"}
-        try:
-            async with self._session.get(url, headers=headers, timeout=self._timeout) as resp:
-                resp.raise_for_status()
-                body = await resp.read()
-                if resp.status == 206:
-                    return body
-                return body[:HEAD_BYTES]
-        except (aiohttp.ClientError, TimeoutError) as err:
-            raise FirmwareFetchError(f"failed to fetch {url}: {err}") from err
+        return await self._fetch(url, _read_head, headers=headers)

custom_components/faikout/ota/exceptions.py

@@ -15,3 +15,12 @@ class FirmwareParseError(FaikoutError):
 
 class FirmwareFetchError(FaikoutError):
     """A network request to the OTA server failed."""
+
+
+class FirmwareUnavailableError(FirmwareFetchError):
+    """The OTA server could not be reached at all (connection refused, timeout).
+
+    Distinct from a plain FirmwareFetchError, which also covers a server that
+    answered with an error status: a 404 is a broken URL worth failing on, while
+    an unreachable host is an environment problem worth skipping or retrying.
+    """