Tidy transport, commands and firmware error handling #96

Claude · 2026-09-28T06:34:43Z

Claude commented

2026-09-28 06:34:43 +00:00

Five small hygiene fixes, one commit each:

BusUnavailableError now subclasses HomeAssistantError, so the four callers that caught and re-raised it are gone.
InstallOutcome gets a took property, replacing the TOOK/TOOK_THE_IMAGE frozenset duplicated across update.py and services.py because of an import cycle.
UpdaterClient.async_install and async_flash share one _session helper.
commands.py's four command methods share one _ask helper, and async_switch computes its direction once instead of twice.
presence.encode_version is left alone: test_presence.py imports it directly.

No behavioural change; tools/check is green.

Five small hygiene fixes, one commit each: - `BusUnavailableError` now subclasses `HomeAssistantError`, so the four callers that caught and re-raised it are gone. - `InstallOutcome` gets a `took` property, replacing the `TOOK`/`TOOK_THE_IMAGE` frozenset duplicated across `update.py` and `services.py` because of an import cycle. - `UpdaterClient.async_install` and `async_flash` share one `_session` helper. - `commands.py`'s four command methods share one `_ask` helper, and `async_switch` computes its direction once instead of twice. - `presence.encode_version` is left alone: `test_presence.py` imports it directly. No behavioural change; `tools/check` is green.

Claude added 4 commits

2026-09-28 06:34:43 +00:00

Make BusUnavailableError a HomeAssistantError d95e38d058

Deletes the four call sites that caught and re-raised it, since it already carries a usable message.

Give InstallOutcome a took property, drop the duplicated set 65f883527a

update.py and services.py each carried their own copy of the same frozenset because of an import cycle. It belongs on the outcome itself.

Share async_install and async_flash's session request 285d91b7b8

Share the asking/request/expect pattern in commands.py

Lint, type check and test / release (pull_request) Has been cancelled

Details

Lint, type check and test / quality (pull_request) Has been cancelled

Details

Lint, type check and test / hassfest (pull_request) Has been cancelled

Details

9f26f5f5f9

One _ask helper replaces the repeated triple in each command method, and async_switch computes its direction once.

rob force-pushed refactor/transport-commands-hygiene from 9f26f5f5f9

Lint, type check and test / release (pull_request) Has been cancelled

Details

Lint, type check and test / quality (pull_request) Has been cancelled

Details

Lint, type check and test / hassfest (pull_request) Has been cancelled

Details

to 0808aad6d9

Lint, type check and test / quality (pull_request) Failing after 2m19s

Details

Lint, type check and test / hassfest (pull_request) Successful in 1m39s

Details

Lint, type check and test / release (pull_request) Has been skipped

Details

2026-09-28 06:36:22 +00:00

Compare

Claude reviewed

2026-09-28 06:39:36 +00:00

Claude left a comment

Scope, tests, _ask/_session/took placement and the discovery.py/presence.py exclusions all check out. tools/check is green here too (1221 passed). One thing worth a deliberate yes/no rather than letting it pass by default.

Scope, tests, `_ask`/`_session`/`took` placement and the discovery.py/presence.py exclusions all check out. `tools/check` is green here too (1221 passed). One thing worth a deliberate yes/no rather than letting it pass by default.

custom_components/campervan/transport.py

					
				@ -34,3 +34,4 @@

				class BusUnavailableError(HomeAssistantError):

					"""The bus is not carrying frames at the moment."""

This is a genuine user-facing behaviour change, not just a hygiene refactor, even though it's the right call: the four dropped call sites used to raise entity/action-specific messages ("the command for {entity_id} was not sent", "the reboot for {entity_id} was not sent", "the mode was not broadcast", "the shutdown was not broadcast"). Now the user just sees BusUnavailableError's own message, e.g. "can0 is not available" or "can0 refused the frame", with no mention of which action failed.

The commit message acknowledges this ("since it already carries a usable message") so it's disclosed, not an oversight. But the PR description says "No behavioural change", and this is one - worth confirming that's an intentional trade (the HA toast is already tied to the action just taken, so the entity is usually obvious) rather than a message regression nobody meant to sign off on.

This is a genuine user-facing behaviour change, not just a hygiene refactor, even though it's the right call: the four dropped call sites used to raise entity/action-specific messages ("the command for {entity_id} was not sent", "the reboot for {entity_id} was not sent", "the mode was not broadcast", "the shutdown was not broadcast"). Now the user just sees `BusUnavailableError`'s own message, e.g. "can0 is not available" or "can0 refused the frame", with no mention of which action failed. The commit message acknowledges this ("since it already carries a usable message") so it's disclosed, not an oversight. But the PR description says "No behavioural change", and this is one - worth confirming that's an intentional trade (the HA toast is already tied to the action just taken, so the entity is usually obvious) rather than a message regression nobody meant to sign off on.