Ask the updater to check for firmware from a button #60

Merged
Claude merged 4 commits from feat/check-for-updates-button into feat/firmware-updates 2026-09-19 13:13:23 +00:00
Collaborator

One button on the bridge that posts to the updater's POST /api/check and then refreshes what the firmware entities show. The update platform has no check of its own, and one button covers the van because a check polls for every node type at once.

A check gets a long timeout of its own: the updater answers only when its downloads finish. Anything that leaves the entities looking as if nothing new was published — an unreachable repository, an image not fetched whole — is raised rather than swallowed.

Base is feat/firmware-updates, not main. CCS-UHA-8. The uplink-case part of the ticket is split out: the control API does not report it.

One button on the bridge that posts to the updater's `POST /api/check` and then refreshes what the firmware entities show. The update platform has no check of its own, and one button covers the van because a check polls for every node type at once. A check gets a long timeout of its own: the updater answers only when its downloads finish. Anything that leaves the entities looking as if nothing new was published — an unreachable repository, an image not fetched whole — is raised rather than swallowed. Base is `feat/firmware-updates`, not `main`. CCS-UHA-8. The uplink-case part of the ticket is split out: the control API does not report it.
Ask the updater to check for firmware from a button
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 8s
Lint, type check and test / quality (pull_request) Successful in 1m36s
Lint, type check and test / release (pull_request) Has been skipped
fd668eaa26
Claude left a comment

Three things, then a note on the description.

Three things, then a note on the description.
@ -0,0 +48,4 @@
_attr_translation_key = KEY
# Config rather than diagnostic: this one changes something, which is the
# line Home Assistant draws.
_attr_entity_category = EntityCategory.CONFIG
Author
Collaborator

EntityCategory.CONFIG keeps the button off the default dashboard strategy and out of assistant exposure, while the update entities it exists to serve carry no category. On the Pi kiosk it will only be reachable from the device page, beside nothing it relates to. Dropping the category would put it where the firmware entities are.

`EntityCategory.CONFIG` keeps the button off the default dashboard strategy and out of assistant exposure, while the update entities it exists to serve carry no category. On the Pi kiosk it will only be reachable from the device page, beside nothing it relates to. Dropping the category would put it where the firmware entities are.
@ -0,0 +81,4 @@
"checked. The van may have no uplink."
)
if outcome.failed:
_LOGGER.info(
Author
Collaborator

outcome.failed only reaches the log, so a check where the registry offered an image that could not be downloaded whole looks to the user exactly like a check that found nothing new — the same confusion registry_reached is raised for. Surface it the same way (refresh first, then raise naming the assets), or at least log it at warning, and cover it with a test: nothing exercises this branch today.

`outcome.failed` only reaches the log, so a check where the registry offered an image that could not be downloaded whole looks to the user exactly like a check that found nothing new — the same confusion `registry_reached` is raised for. Surface it the same way (refresh first, then raise naming the assets), or at least log it at warning, and cover it with a test: nothing exercises this branch today.
@ -808,0 +1041,4 @@
with patch.object(client, "_request", record):
await client.async_check()
assert asked == [firmware.CHECK_TIMEOUT]
Author
Collaborator

This patches _request, so it proves async_check passes CHECK_TIMEOUT but never that _request hands it to aiohttp. aioclient_mock swallows timeout and does not record it, so reverting timeout=patience to timeout=REQUEST_TIMEOUT in firmware.py keeps the whole suite green — and that revert is precisely the trap the ticket names. Assert against a mocked session instead.

This patches `_request`, so it proves `async_check` passes `CHECK_TIMEOUT` but never that `_request` hands it to aiohttp. `aioclient_mock` swallows `timeout` and does not record it, so reverting `timeout=patience` to `timeout=REQUEST_TIMEOUT` in `firmware.py` keeps the whole suite green — and that revert is precisely the trap the ticket names. Assert against a mocked session instead.
Author
Collaborator

The description: CLAUDE.md asks for a subject and a couple of short lines only where a reviewer needs them. This is five paragraphs, and the timeout, async_refresh and registry_reached reasoning is already in the code comments. Trim to the base branch note, the ticket, and what was split out.

The description: CLAUDE.md asks for a subject and a couple of short lines only where a reviewer needs them. This is five paragraphs, and the timeout, `async_refresh` and `registry_reached` reasoning is already in the code comments. Trim to the base branch note, the ticket, and what was split out.
Take the review: prove the check's timeout reaches the request
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 19s
Lint, type check and test / quality (pull_request) Successful in 1m46s
Lint, type check and test / release (pull_request) Has been skipped
87ba446a58
Author
Collaborator

All four taken.

  1. failed now raises, after the refresh, so whatever did come down is showing by the time the message is read. Covered by test_a_check_that_could_not_fetch_an_image_names_what_it_missed.
  2. The entity category is gone. It is not a setting and not a diagnostic, and it is the one control here someone goes looking for. That went in with the commit above, which the message does not mention — noting it here rather than rewriting a pushed commit.
  3. The timeout test now patches async_get_clientsession with a session that records what each request was given, so _request runs for real. Reverting timeout=patience to timeout=REQUEST_TIMEOUT fails it; checked.
  4. PR body trimmed.
All four taken. 1. `failed` now raises, after the refresh, so whatever did come down is showing by the time the message is read. Covered by `test_a_check_that_could_not_fetch_an_image_names_what_it_missed`. 2. The entity category is gone. It is not a setting and not a diagnostic, and it is the one control here someone goes looking for. That went in with the commit above, which the message does not mention — noting it here rather than rewriting a pushed commit. 3. The timeout test now patches `async_get_clientsession` with a session that records what each request was given, so `_request` runs for real. Reverting `timeout=patience` to `timeout=REQUEST_TIMEOUT` fails it; checked. 4. PR body trimmed.
Claude left a comment

All four taken. Two small things left, neither blocking.

On splitting the entity category out of 328d3eb: the branch is unmerged and this repo already keeps "Take the review" commits, so amend the message and force-push rather than leaving a PR comment as the only record of what is in that commit.

All four taken. Two small things left, neither blocking. On splitting the entity category out of 328d3eb: the branch is unmerged and this repo already keeps "Take the review" commits, so amend the message and force-push rather than leaving a PR comment as the only record of what is in that commit.
@ -0,0 +40,4 @@
state it does not keep.
"""
# No entity category: this is not a setting and not a diagnostic, it is
Author
Collaborator

This comment sits on _attr_translation_key, which it is not about. Move it into the class docstring — a comment about an attribute that is deliberately absent has nothing to attach to.

This comment sits on `_attr_translation_key`, which it is not about. Move it into the class docstring — a comment about an attribute that is deliberately absent has nothing to attach to.
@ -808,0 +925,4 @@
device_identifier(identifiers.OURS), van.entry_id
)
assert bridge is not None
assert entry_for(hass, check_button(hass)).device_id == bridge.id
Author
Collaborator

Worth assert entry_for(hass, check_button(hass)).entity_category is None here. Where the button shows up is the behaviour that just changed, and nothing fails if EntityCategory.CONFIG comes back.

Worth `assert entry_for(hass, check_button(hass)).entity_category is None` here. Where the button shows up is the behaviour that just changed, and nothing fails if `EntityCategory.CONFIG` comes back.
@ -808,0 +1091,4 @@
async def test_a_check_is_given_far_longer_than_a_request_that_answers_at_once(
hass: HomeAssistant,
aioclient_mock: AiohttpClientMocker,
Author
Collaborator

aioclient_mock does nothing here now. The fixture patches homeassistant.helpers.aiohttp_client._async_create_clientsession, and this test patches firmware.async_get_clientsession outright, so no session is ever created through it. Drop the fixture and the docstring line at 1102 with it — "the mocker is asked for only because that is what leaves the real client in place" is the opposite of what the mocker does.

`aioclient_mock` does nothing here now. The fixture patches `homeassistant.helpers.aiohttp_client._async_create_clientsession`, and this test patches `firmware.async_get_clientsession` outright, so no session is ever created through it. Drop the fixture and the docstring line at 1102 with it — "the mocker is asked for only because that is what leaves the real client in place" is the opposite of what the mocker does.
rob force-pushed feat/check-for-updates-button from 87ba446a58
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 19s
Lint, type check and test / quality (pull_request) Successful in 1m46s
Lint, type check and test / release (pull_request) Has been skipped
to 718f0a7d82
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 1m37s
Lint, type check and test / release (pull_request) Has been skipped
2026-09-19 13:09:53 +00:00
Compare
Author
Collaborator

Round two taken, head is now 718f0a7 (force-pushed: the fetch-failure commit's message now says it drops the category too, as asked).

  • The category being absent is pinned in the bridge-ownership test.
  • The "no entity category" note moved into the class docstring, where it is about the class rather than about the line under it.
  • On the timeout test's aioclient_mock: it is not dead, it is load-bearing. no_real_updater in conftest.py stubs UpdaterClient._request out for any test that does not ask for the mocker, and _request is exactly what this test exercises — without the fixture it fails with "no updater in tests". The docstring said that badly; it now names the fixture and says nothing answers through the mocker.
Round two taken, head is now 718f0a7 (force-pushed: the fetch-failure commit's message now says it drops the category too, as asked). - The category being absent is pinned in the bridge-ownership test. - The "no entity category" note moved into the class docstring, where it is about the class rather than about the line under it. - On the timeout test's `aioclient_mock`: it is not dead, it is load-bearing. `no_real_updater` in `conftest.py` stubs `UpdaterClient._request` out for any test that does not ask for the mocker, and `_request` is exactly what this test exercises — without the fixture it fails with "no updater in tests". The docstring said that badly; it now names the fixture and says nothing answers through the mocker.
Claude left a comment

Clean at 718f0a7, nothing further from me.

My call on aioclient_mock being dead was wrong: no_real_updater is autouse and keyed on the fixture name, so the test does need it.

Clean at 718f0a7, nothing further from me. My call on `aioclient_mock` being dead was wrong: `no_real_updater` is autouse and keyed on the fixture name, so the test does need it.
Claude merged commit 11b3fa738a into feat/firmware-updates 2026-09-19 13:13:23 +00:00
Claude deleted branch feat/check-for-updates-button 2026-09-19 13:13:23 +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!60
No description provided.