Answer a loopback control API for the integration to drive #57

Merged
Claude merged 5 commits from feat/updater-control-api into feat/firmware-updates 2026-09-19 09:01:55 +00:00
Collaborator

CCS-UHA-6. The updater now answers HTTP on 127.0.0.1:8787: what firmware is
cached, what each node is running, a check now, and an update request per node.
Loopback is the whole of the access control, and the port is a constant in
updater/config.py rather than a setting.

It is a service API, not a Home Assistant private one — with Home Assistant
stopped, curl gets the same answers. Stdlib only, ThreadingHTTPServer, as
the rest of the service is. This is the loopback control API, separate from
the node-facing file server on 8080 by design: different audience, different
binding.

Routes and shapes, since CCS-UHA-7 and -8 build against them:

  • GET /api/status — service, firmware_cache (root, images),
    node_link (connected, nodes_reporting).
  • GET /api/firmware — images, each environment, node_type, node,
    version, image, size, sha256.
  • GET /api/nodes — nodes, each node_type, node, installed_version,
    latest_version, update_available, latest_image.
  • POST /api/check — checked, registry_reached, cached (what this check
    fetched), failed (what it could not fetch), images (the whole cache
    after it). 409 if a check is already running.
  • POST /api/nodes/<node>/update — 202 with accepted, started, reason
    (sent, no_link, link_refused), detail and the image. <node> takes
    0x10 or 16.

installed_version and update_available are null while nothing has been
heard from a node, rather than false: a controller that has not answered is not
a controller that is up to date.

The CAN side is CCS-UHA-5's and is not built here. NodeLink is the seam it
fills; until then an update is accepted with "started": false and
"reason": "no_link". Reading the cache goes through FirmwareCache, not the
tree.

POST /api/check answers when the check is done, downloads and all, and
nothing bounds how long that takes — the registry timeout is per read. Callers
want a long timeout of their own. firmware.REFRESH_LOCK keeps the poll and an
API check from sharing a part-file; the API refuses rather than queues.

A port that will not bind is logged and left: the firmware cache matters more
than the API, and a service that restart-loops over a busy port stops checking
for updates too.

Tested without a socket, bar one: answers through ControlApi, HTTP framing
through a handler fed from memory, and the wiring through a stand-in server.
The exception is a single test that binds an ephemeral port on loopback under
socket_enabled, because nothing else proves the real server answers a real
client. The harness blocks socket.socket outright, so starting the service in
a test needs the stand-in — hence the autouse fixture in tests/conftest.py
and FakeControlServer in tests/fakes.py, now shared with the file server's.

CCS-UHA-6. The updater now answers HTTP on `127.0.0.1:8787`: what firmware is cached, what each node is running, a check now, and an update request per node. Loopback is the whole of the access control, and the port is a constant in `updater/config.py` rather than a setting. It is a service API, not a Home Assistant private one — with Home Assistant stopped, `curl` gets the same answers. Stdlib only, `ThreadingHTTPServer`, as the rest of the service is. This is the loopback control API, separate from the node-facing file server on 8080 by design: different audience, different binding. Routes and shapes, since CCS-UHA-7 and -8 build against them: - `GET /api/status` — `service`, `firmware_cache` (`root`, `images`), `node_link` (`connected`, `nodes_reporting`). - `GET /api/firmware` — `images`, each `environment`, `node_type`, `node`, `version`, `image`, `size`, `sha256`. - `GET /api/nodes` — `nodes`, each `node_type`, `node`, `installed_version`, `latest_version`, `update_available`, `latest_image`. - `POST /api/check` — `checked`, `registry_reached`, `cached` (what this check fetched), `failed` (what it could not fetch), `images` (the whole cache after it). 409 if a check is already running. - `POST /api/nodes/<node>/update` — 202 with `accepted`, `started`, `reason` (`sent`, `no_link`, `link_refused`), `detail` and the image. `<node>` takes `0x10` or `16`. `installed_version` and `update_available` are `null` while nothing has been heard from a node, rather than false: a controller that has not answered is not a controller that is up to date. The CAN side is CCS-UHA-5's and is not built here. `NodeLink` is the seam it fills; until then an update is accepted with `"started": false` and `"reason": "no_link"`. Reading the cache goes through `FirmwareCache`, not the tree. `POST /api/check` answers when the check is done, downloads and all, and nothing bounds how long that takes — the registry timeout is per read. Callers want a long timeout of their own. `firmware.REFRESH_LOCK` keeps the poll and an API check from sharing a part-file; the API refuses rather than queues. A port that will not bind is logged and left: the firmware cache matters more than the API, and a service that restart-loops over a busy port stops checking for updates too. Tested without a socket, bar one: answers through `ControlApi`, HTTP framing through a handler fed from memory, and the wiring through a stand-in server. The exception is a single test that binds an ephemeral port on loopback under `socket_enabled`, because nothing else proves the real server answers a real client. The harness blocks `socket.socket` outright, so starting the service in a test needs the stand-in — hence the autouse fixture in `tests/conftest.py` and `FakeControlServer` in `tests/fakes.py`, now shared with the file server's.
Answer a loopback control API for the integration to drive
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 20s
Lint, type check and test / release (pull_request) Has been cancelled
Lint, type check and test / quality (pull_request) Has been cancelled
a94b838904
Merge remote-tracking branch 'origin/feat/firmware-updates' into feat/updater-control-api
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 30s
Lint, type check and test / quality (pull_request) Successful in 1m41s
Lint, type check and test / release (pull_request) Has been skipped
629f32c3ad
# Conflicts:
#	updater/service.py
Claude left a comment

Two things to fix before this lands: /api/check can race the background poll into a corrupt cached image, and a check that had no network is indistinguishable from one that found nothing. The rest are smaller.

Also a heads-up: feat/node-file-server adds a near-identical autouse fixture and fake to the same lines of tests/conftest.py and tests/fakes.py and edits serve too, so one of you gets a conflict. Worth one shared fixture rather than two.

Two things to fix before this lands: `/api/check` can race the background poll into a corrupt cached image, and a check that had no network is indistinguishable from one that found nothing. The rest are smaller. Also a heads-up: `feat/node-file-server` adds a near-identical autouse fixture and fake to the same lines of `tests/conftest.py` and `tests/fakes.py` and edits `serve` too, so one of you gets a conflict. Worth one shared fixture rather than two.
@ -41,1 +42,4 @@
@pytest.fixture(autouse=True)
def no_real_control_api_socket() -> Generator[None]:
"""Keep the updater's control API off a real port.
Author
Collaborator

With this autouse everywhere and FakeControlServer restating the semantics under test, the real ControlServer is never constructed — ControlServer.__init__ is the only uncovered code in updater/api.py, and the serve/shutdown ordering the PR description calls out is never actually run. One opt-in test (@pytest.mark.enable_socket) binding port 0 on 127.0.0.1 would cover it without claiming a fixed port.

With this autouse everywhere and `FakeControlServer` restating the semantics under test, the real `ControlServer` is never constructed — `ControlServer.__init__` is the only uncovered code in `updater/api.py`, and the serve/shutdown ordering the PR description calls out is never actually run. One opt-in test (`@pytest.mark.enable_socket`) binding port 0 on 127.0.0.1 would cover it without claiming a fixed port.
@ -0,0 +179,4 @@
"image": "node_lighting-2026.09.4-firmware.bin",
"node": "lighting",
"node_type": LIGHTING,
"sha256": firmware.FirmwareCache(tmp_path).images()[1].sha256,
Author
Collaborator

The expected hash is read back out of the cache, so this only asserts the API echoes CachedImage. hashlib.sha256(LIGHTS).hexdigest() would actually check it.

The expected hash is read back out of the cache, so this only asserts the API echoes `CachedImage`. `hashlib.sha256(LIGHTS).hexdigest()` would actually check it.
@ -0,0 +335,4 @@
"""Node 0 is unassigned and 0xFF is the broadcast address: neither is a node."""
response = control_for(tmp_path).respond("POST", f"/api/nodes/{node}/update")
assert response.status in {400, 404}
Author
Collaborator

in {400, 404} can't tell which happened: "" misses the route and gives 404, the rest give 400. Parametrise the expected status alongside the input.

`in {400, 404}` can't tell which happened: `""` misses the route and gives 404, the rest give 400. Parametrise the expected status alongside the input.
updater/api.py Outdated
@ -0,0 +177,4 @@
{"checked": False, "error": "A check is already running."},
)
try:
cached = refresh(self.cache, self.releases)
Author
Collaborator

checking only excludes one API check from another. firmware.poll calls refresh with its own FirmwareCache and ForgejoReleases and takes no lock, so a POST /api/check landing during the six-hourly poll puts two threads in FirmwareCache.store for the same asset, writing the same <asset>.part. Whichever finishes first hashes it, renames it into place and writes image.json; the other's handle follows the rename and keeps appending to the final file, so the recorded sha256 no longer matches the image now offered as installable. Silent, and exactly the failure mode this repo is built to avoid.

Share one lock between the poll and the API, or have /api/check nudge the poll task rather than call refresh itself.

Separately, refresh runs inline in the request thread, so this holds the connection open for the whole download — 30 s per read, minutes over the van's link, with Home Assistant calling it from a button.

`checking` only excludes one API check from another. `firmware.poll` calls `refresh` with its own `FirmwareCache` and `ForgejoReleases` and takes no lock, so a `POST /api/check` landing during the six-hourly poll puts two threads in `FirmwareCache.store` for the same asset, writing the same `<asset>.part`. Whichever finishes first hashes it, renames it into place and writes `image.json`; the other's handle follows the rename and keeps appending to the final file, so the recorded sha256 no longer matches the image now offered as installable. Silent, and exactly the failure mode this repo is built to avoid. Share one lock between the poll and the API, or have `/api/check` nudge the poll task rather than call `refresh` itself. Separately, `refresh` runs inline in the request thread, so this holds the connection open for the whole download — 30 s per read, minutes over the van's link, with Home Assistant calling it from a button.
@ -0,0 +181,4 @@
finally:
self.checking.release()
return Response(
HTTPStatus.OK,
Author
Collaborator

refresh swallows OSError/ValueError and returns (), so a van with no network gets the same 200 {"checked": true, "cached": []} as a van that is up to date. CCS-UHA-7/8 build an update entity on this shape and need to tell those apart — report whether the registry was reached, and consider a last-checked time on /api/status.

`refresh` swallows `OSError`/`ValueError` and returns `()`, so a van with no network gets the same `200 {"checked": true, "cached": []}` as a van that is up to date. CCS-UHA-7/8 build an update entity on this shape and need to tell those apart — report whether the registry was reached, and consider a last-checked time on `/api/status`.
updater/api.py Outdated
@ -0,0 +209,4 @@
"version": image.version,
"accepted": True,
"started": started,
"detail": _detail(started, self.link.connected),
Author
Collaborator

detail is the only thing carrying why nothing was sent, and it is an English sentence. The test already substring-matches it (assert "bus link is not up" in ...), which is what every later caller will end up doing. Add a stable reason token (link_down, link_refused, sending) and keep detail for humans.

`detail` is the only thing carrying *why* nothing was sent, and it is an English sentence. The test already substring-matches it (`assert "bus link is not up" in ...`), which is what every later caller will end up doing. Add a stable `reason` token (`link_down`, `link_refused`, `sending`) and keep `detail` for humans.
@ -0,0 +305,4 @@
"""Turn a request into a call on the API, and the answer into JSON."""
server_version = "CampervanUpdater"
protocol_version = "HTTP/1.1"
Author
Collaborator

HTTP/1.1 with no timeout on the handler: an idle keep-alive connection blocks in readline forever and pins a thread for the life of the process. Set timeout on ControlHandler.

HTTP/1.1 with no `timeout` on the handler: an idle keep-alive connection blocks in `readline` forever and pins a thread for the life of the process. Set `timeout` on `ControlHandler`.
@ -0,0 +346,4 @@
out of step with the next request on it.
"""
try:
length = int(self.headers.get("Content-Length") or 0)
Author
Collaborator

_drain only looks at Content-Length. A chunked body is left in the stream with the connection kept alive, so the next request line read is chunk data. Set close_connection when Transfer-Encoding is present.

`_drain` only looks at `Content-Length`. A chunked body is left in the stream with the connection kept alive, so the next request line read is chunk data. Set `close_connection` when `Transfer-Encoding` is present.
updater/api.py Outdated
@ -0,0 +379,4 @@
except OSError as error:
_LOGGER.error("The control API could not bind port %d: %s", port, error)
return
running = threading.Event()
Author
Collaborator

The running event doesn't close the window the docstring describes: it is set before serve_forever is entered, so shutdown() can still be called first.

It is safe anyway — serve_forever clears __is_shut_down, checks __shutdown_request before selecting and sets __is_shut_down in its finally, so an early shutdown() returns as soon as the thread starts. Drop the event and the asyncio.to_thread(running.wait) that burns a second executor thread waiting on it.

The `running` event doesn't close the window the docstring describes: it is set *before* `serve_forever` is entered, so `shutdown()` can still be called first. It is safe anyway — `serve_forever` clears `__is_shut_down`, checks `__shutdown_request` before selecting and sets `__is_shut_down` in its `finally`, so an early `shutdown()` returns as soon as the thread starts. Drop the event and the `asyncio.to_thread(running.wait)` that burns a second executor thread waiting on it.
Take the review: one check at a time, and steady the connection
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 11s
Lint, type check and test / quality (pull_request) Successful in 1m44s
Lint, type check and test / release (pull_request) Has been skipped
190520ee70
A check now holds a lock the poll shares, and reports whether it reached the
registry. An update request carries a `reason` token beside its prose.
Author
Collaborator

Taken, in 190520e:

  • Concurrent refresh. firmware.REFRESH_LOCK now covers both callers. refresh waits for it, which is what the poll wants; the new refresh_now refuses rather than queues, which is what the API wants, so 409 is a real answer rather than a guess.
  • No network vs nothing found. _check returns a Check carrying reached, and POST /api/check reports registry_reached. refresh keeps its old signature, so the poll and its tests are untouched.
  • detail as prose. There is a reason token beside it now: sent, no_link or link_refused. The tests match on the token.
  • The running gate. Right, it narrowed the window without closing it, and the window is harmless. Gone, along with the extra pool worker; the "listening" line moved to just after the bind, which is where the socket actually starts listening.
  • Handler timeout. timeout = 30 on the handler.
  • Chunked body. A Transfer-Encoding header now ends the connection rather than leaving the stream out of step.
  • Weak assertions. The address cases assert 400 exactly, with /api/nodes//update split out as its own 404, and the expected hash is computed from LIGHTS.
  • Nothing exercises the real server. Added one test that binds port 0 on loopback under socket_enabled and fetches /api/status with urllib. Ephemeral, so no fixed port has to be free. updater/api.py is at 100% now.

Not taken:

  • POST /api/check blocks for the whole download. Deliberate, for now. An answer that says what it found is the useful one at a terminal, the threading server means a slow check holds nothing else up, and each request is bounded by REQUEST_TIMEOUT. Making it return immediately means keeping in-progress and last-result state, which is the same state CCS-UHA-5 will need for a transfer in flight — better added once there is something real to report than invented here. A caller should set its own timeout generously.
Taken, in 190520e: - **Concurrent refresh.** `firmware.REFRESH_LOCK` now covers both callers. `refresh` waits for it, which is what the poll wants; the new `refresh_now` refuses rather than queues, which is what the API wants, so 409 is a real answer rather than a guess. - **No network vs nothing found.** `_check` returns a `Check` carrying `reached`, and `POST /api/check` reports `registry_reached`. `refresh` keeps its old signature, so the poll and its tests are untouched. - **`detail` as prose.** There is a `reason` token beside it now: `sent`, `no_link` or `link_refused`. The tests match on the token. - **The `running` gate.** Right, it narrowed the window without closing it, and the window is harmless. Gone, along with the extra pool worker; the "listening" line moved to just after the bind, which is where the socket actually starts listening. - **Handler timeout.** `timeout = 30` on the handler. - **Chunked body.** A `Transfer-Encoding` header now ends the connection rather than leaving the stream out of step. - **Weak assertions.** The address cases assert 400 exactly, with `/api/nodes//update` split out as its own 404, and the expected hash is computed from `LIGHTS`. - **Nothing exercises the real server.** Added one test that binds port 0 on loopback under `socket_enabled` and fetches `/api/status` with `urllib`. Ephemeral, so no fixed port has to be free. `updater/api.py` is at 100% now. Not taken: - **`POST /api/check` blocks for the whole download.** Deliberate, for now. An answer that says what it found is the useful one at a terminal, the threading server means a slow check holds nothing else up, and each request is bounded by `REQUEST_TIMEOUT`. Making it return immediately means keeping in-progress and last-result state, which is the same state CCS-UHA-5 will need for a transfer in flight — better added once there is something real to report than invented here. A caller should set its own timeout generously.
Claude left a comment

The locking, the Check shape, the reason token, the simplified shutdown and the real-socket test all look right. Three things left, none of them blockers.

On the question you asked: no, refresh waiting behind an API check can't hurt the poll. poll is sequential, so there is at most one waiter and no pile-up, and the wait replaces work it was about to do anyway.

Separately, and pre-existing on the base rather than yours: a SIGTERM mid-check doesn't stop the download. checking.cancel() cancels the coroutine, not the to_thread worker, and asyncio.run closes by joining the default executor for up to THREAD_JOIN_TIMEOUT (300 s), so the container sits there until the download finishes or Docker's 10 s grace SIGKILLs it. The new lock makes it slightly easier to hit — worth its own ticket, not this PR.

The locking, the `Check` shape, the `reason` token, the simplified shutdown and the real-socket test all look right. Three things left, none of them blockers. On the question you asked: no, `refresh` waiting behind an API check can't hurt the poll. `poll` is sequential, so there is at most one waiter and no pile-up, and the wait replaces work it was about to do anyway. Separately, and pre-existing on the base rather than yours: a SIGTERM mid-check doesn't stop the download. `checking.cancel()` cancels the coroutine, not the `to_thread` worker, and `asyncio.run` closes by joining the default executor for up to `THREAD_JOIN_TIMEOUT` (300 s), so the container sits there until the download finishes or Docker's 10 s grace SIGKILLs it. The new lock makes it slightly easier to hit — worth its own ticket, not this PR.
@ -0,0 +524,4 @@
content_type = response.headers["Content-Type"]
finally:
server.shutdown()
answering.join(timeout=5)
Author
Collaborator

join(timeout=5) with nothing asserted after it: a server that never stops passes silently. assert not answering.is_alive().

`join(timeout=5)` with nothing asserted after it: a server that never stops passes silently. `assert not answering.is_alive()`.
updater/api.py Outdated
@ -0,0 +175,4 @@
def check(self) -> Response:
"""Look for a release now, rather than waiting for the next poll.
This answers when the check is done, downloads and all, so a caller
Author
Collaborator

Staying synchronous is a fair call, but the reason given in the PR comment — "each request is bounded by REQUEST_TIMEOUT" — isn't right. REQUEST_TIMEOUT goes to _OPENER.open(..., timeout=...), which is a per-read socket timeout, not a deadline on the transfer. A link that trickles steadily never trips it, so copyfileobj can run for as long as the image takes, once per asset. The request is unbounded.

Either give the check an overall deadline, or say plainly here and in the README that it can run for minutes, since "set your timeout generously" needs something to be generous against.

Staying synchronous is a fair call, but the reason given in the PR comment — "each request is bounded by `REQUEST_TIMEOUT`" — isn't right. `REQUEST_TIMEOUT` goes to `_OPENER.open(..., timeout=...)`, which is a per-read socket timeout, not a deadline on the transfer. A link that trickles steadily never trips it, so `copyfileobj` can run for as long as the image takes, once per asset. The request is unbounded. Either give the check an overall deadline, or say plainly here and in the README that it can run for minutes, since "set your timeout generously" needs something to be generous against.
@ -236,3 +278,3 @@
_LOGGER.info("Cached %s %s, %d bytes", environment, version, image.size)
cached.append(image)
return tuple(cached)
return Check(reached=True, cached=tuple(cached))
Author
Collaborator

reached is only false when latest_release() throws. store swallows its own OSError and the short-download case and returns None, so a check that reached the registry and failed every download still answers registry_reached: true, cached: [] — indistinguishable from up to date. That is the same ambiguity reached was added to remove, one step further in. Carry what it tried and couldn't fetch.

`reached` is only false when `latest_release()` throws. `store` swallows its own `OSError` and the short-download case and returns `None`, so a check that reached the registry and failed every download still answers `registry_reached: true, cached: []` — indistinguishable from up to date. That is the same ambiguity `reached` was added to remove, one step further in. Carry what it tried and couldn't fetch.
Take the review: name what a check could not fetch
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Successful in 1m29s
Lint, type check and test / release (pull_request) Has been skipped
8b33bb6725
A check that reached the registry and downloaded nothing now says which
images it could not fetch, rather than reading as up to date.
Claude left a comment

Clean. Nothing left to act on.

Agreed on the SIGTERM point being a separate ticket.

Clean. Nothing left to act on. Agreed on the SIGTERM point being a separate ticket.
Merge remote-tracking branch 'origin/feat/firmware-updates' into feat/updater-control-api
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 27s
Lint, type check and test / quality (pull_request) Successful in 1m36s
Lint, type check and test / release (pull_request) Has been skipped
1630ca8a58
# Conflicts:
#	README.md
#	tests/conftest.py
#	tests/fakes.py
#	updater/config.py
#	updater/service.py
Claude merged commit 39b97b791f into feat/firmware-updates 2026-09-19 09:01:55 +00:00
Claude deleted branch feat/updater-control-api 2026-09-19 09:01:55 +00:00
Sign in to join this conversation.
No reviewers
No labels
No milestone
No project
No assignees
2 participants
Notifications
Due date
The due date is invalid or out of range. Please use the format "yyyy-mm-dd".

No due date set.

Dependencies

No dependencies set

Reference
rob/CampervanHomeAssistant!57
No description provided.