Raise the update hotspot, and always put the network back #54

Merged
Claude merged 6 commits from feat/networkmanager-hotspot into feat/firmware-updates 2026-09-19 08:25:31 +00:00
Collaborator

updater/radio.py drives the host's NetworkManager through nmcli over the
system bus compose already mounts, so nodes can join a campervan-ota access
point to pull firmware. With Ethernet up, or with nothing up, the hotspot goes
alongside whatever is there; only when wlan0 is the uplink does it come down
first, and Home Assistant is then unreachable from the network until the update
finishes.

The failure that matters is a Pi stranded as an access point, so all four
guards are here: a stale campervan-ota is deleted at startup, a restore
deadline is armed when the hotspot goes up, the uplink profile keeps its
autoconnect so a power cut recovers without the updater, and SIGTERM restores
within the stop_grace_period compose sets. The address is read back from
NetworkManager rather than assumed to be 10.42.0.1, since a later ticket puts
it on the bus.

The hotspot passphrase is generated per run rather than configured — a fixed
one in the repository would be a shared secret, and the node learns this one
the same way it learns the address.

Tested through a fake nmcli at the seam: which case we are in, the startup
tear-down, the deadline firing, and restore on every failure path. nmcli
itself is not exercised, and the image now installs it.

CCS-UHA-2.

`updater/radio.py` drives the host's NetworkManager through `nmcli` over the system bus compose already mounts, so nodes can join a `campervan-ota` access point to pull firmware. With Ethernet up, or with nothing up, the hotspot goes alongside whatever is there; only when `wlan0` is the uplink does it come down first, and Home Assistant is then unreachable from the network until the update finishes. The failure that matters is a Pi stranded as an access point, so all four guards are here: a stale `campervan-ota` is deleted at startup, a restore deadline is armed when the hotspot goes up, the uplink profile keeps its `autoconnect` so a power cut recovers without the updater, and SIGTERM restores within the `stop_grace_period` compose sets. The address is read back from NetworkManager rather than assumed to be 10.42.0.1, since a later ticket puts it on the bus. The hotspot passphrase is generated per run rather than configured — a fixed one in the repository would be a shared secret, and the node learns this one the same way it learns the address. Tested through a fake `nmcli` at the seam: which case we are in, the startup tear-down, the deadline firing, and restore on every failure path. `nmcli` itself is not exercised, and the image now installs it. CCS-UHA-2.
Raise the update hotspot, and always put the network back
Some checks failed
Lint, type check and test / hassfest (pull_request) Successful in 9s
Lint, type check and test / quality (pull_request) Failing after 20s
Lint, type check and test / release (pull_request) Has been skipped
d224f60bcb
Claude left a comment

Three of these break the guarantee the ticket is built around — a restore that half-fails leaves no record of what to put back, and a crashed container leaves wlan0 down for good. Detail inline.

Three of these break the guarantee the ticket is built around — a restore that half-fails leaves no record of what to put back, and a crashed container leaves `wlan0` down for good. Detail inline.
@ -0,0 +270,4 @@
async def test_a_line_with_fewer_fields_than_asked_for_is_skipped() -> None:
"""nmcli prints the odd warning of its own, and it is not a device."""
Author
Collaborator

CI fails here: ruff D403 (pinned 0.16.6) wants the docstring's first word capitalised.

CI fails here: ruff D403 (pinned 0.16.6) wants the docstring's first word capitalised.
@ -0,0 +66,4 @@
passphrase: str
async def run_nmcli(*arguments: str) -> str:
Author
Collaborator

No timeout. nmcli defaults to 90 s for connection up and device wifi hotspot (nmcli(1)), so a restore on SIGTERM can outrun stop_grace_period: 30s and get SIGKILLed mid-way, and a slow _address() means the deadline at line 190 is never armed while the hotspot is already up. Pass --wait bounded under the grace period.

No timeout. nmcli defaults to 90 s for `connection up` and `device wifi hotspot` (nmcli(1)), so a restore on SIGTERM can outrun `stop_grace_period: 30s` and get SIGKILLed mid-way, and a slow `_address()` means the deadline at line 190 is never armed while the hotspot is already up. Pass `--wait` bounded under the grace period.
@ -0,0 +136,4 @@
)
wireless: Uplink | None = None
for line in listing.splitlines():
fields = _fields(line)
Author
Collaborator

Nothing checks the wireless uplink is on self._device, so an uplink on a second radio would be taken down for a hotspot that never touches it.

Nothing checks the wireless uplink is on `self._device`, so an uplink on a second radio would be taken down for a hotspot that never touches it.
updater/radio.py Outdated
@ -0,0 +166,4 @@
# Never `connection modify ... autoconnect no` to go with this: the
# profile's autoconnect is what recovers the Pi after a power cut,
# with the updater out of the picture entirely.
await self._nmcli("connection", "down", uplink.connection)
Author
Collaborator

nmcli connection down internally blocks that profile from autoconnecting "until reboot or until the user performs an action that unblocks autoconnect" (nmcli(1), connection down). So if the container is killed rather than stopped, the next start's clear_stale_hotspot frees wlan0 but NetworkManager will not reactivate the uplink, and _suspended died with the process. Leaving connection.autoconnect set only covers the power-cut case. Have the startup path run nmcli device connect <device> after clearing a stale hotspot, and drop the README claim (line 170) that a crash cannot leave it stranded.

`nmcli connection down` internally blocks that profile from autoconnecting "until reboot or until the user performs an action that unblocks autoconnect" (nmcli(1), `connection down`). So if the container is killed rather than stopped, the next start's `clear_stale_hotspot` frees `wlan0` but NetworkManager will not reactivate the uplink, and `_suspended` died with the process. Leaving `connection.autoconnect` set only covers the power-cut case. Have the startup path run `nmcli device connect <device>` after clearing a stale hotspot, and drop the README claim (line 170) that a crash cannot leave it stranded.
@ -0,0 +184,4 @@
passphrase,
)
hotspot = Hotspot(await self._address(), HOTSPOT_SSID, passphrase)
except RadioError:
Author
Collaborator

except RadioError misses OSError/FileNotFoundError from create_subprocess_exec — nmcli absent from the image, say — which leaves the uplink down with no local restore. Catch Exception and re-raise.

`except RadioError` misses `OSError`/`FileNotFoundError` from `create_subprocess_exec` — nmcli absent from the image, say — which leaves the uplink down with no local restore. Catch `Exception` and re-raise.
updater/radio.py Outdated
@ -0,0 +199,4 @@
service stopping, and more than one of them can arrive.
"""
self._disarm()
await self._remove_hotspot()
Author
Collaborator

If _remove_hotspot() raises, the uplink is never brought back up — the connection up below is unreachable. Put the teardown in a try/finally so the uplink comes back regardless of what the delete did.

If `_remove_hotspot()` raises, the uplink is never brought back up — the `connection up` below is unreachable. Put the teardown in a `try`/`finally` so the uplink comes back regardless of what the delete did.
updater/radio.py Outdated
@ -0,0 +200,4 @@
"""
self._disarm()
await self._remove_hotspot()
suspended, self._suspended = self._suspended, None
Author
Collaborator

_suspended is cleared before the connection up has succeeded, so a transient failure loses the only record of what to put back. Trace it: wireless uplink suspended, deadline fires, connection delete succeeds, connection up Van WiFi raises. The error propagates out of _restore_on_deadline into a task nobody awaits, _suspended is now None, and the SIGTERM restore later does nothing. Pi left with no hotspot and no uplink — the exact failure the ticket names. Clear _suspended only after the up returns.

`_suspended` is cleared before the `connection up` has succeeded, so a transient failure loses the only record of what to put back. Trace it: wireless uplink suspended, deadline fires, `connection delete` succeeds, `connection up Van WiFi` raises. The error propagates out of `_restore_on_deadline` into a task nobody awaits, `_suspended` is now `None`, and the SIGTERM restore later does nothing. Pi left with no hotspot and no uplink — the exact failure the ticket names. Clear `_suspended` only after the `up` returns.
@ -33,3 +35,4 @@
"""
loop = asyncio.get_running_loop()
stop = asyncio.Event()
radio = Radio() if radio is None else radio
Author
Collaborator

tests/test_updater.py::test_a_termination_signal_stops_the_service passes no radio, so it builds a real Radio and shells out to the host's nmcli — I confirmed two nmcli -g NAME connection show subprocesses against this machine's NetworkManager. On a host that has a campervan-ota profile the suite would delete it; on a host without nmcli the test fails. Pass a fake radio there.

`tests/test_updater.py::test_a_termination_signal_stops_the_service` passes no radio, so it builds a real `Radio` and shells out to the host's `nmcli` — I confirmed two `nmcli -g NAME connection show` subprocesses against this machine's NetworkManager. On a host that has a `campervan-ota` profile the suite would delete it; on a host without `nmcli` the test fails. Pass a fake radio there.
@ -39,2 +42,4 @@
await radio.clear_stale_hotspot()
await serve(settings, stop)
finally:
await radio.restore()
Author
Collaborator

If clear_stale_hotspot() fails — no /run/dbus, NetworkManager not running, AppArmor denying the bus — this restore() fails the same way, replaces the original exception and skips remove_signal_handler. The container then crash-loops on a secondary traceback that does not say what actually went wrong. Catch the startup failure, log it and carry on.

If `clear_stale_hotspot()` fails — no `/run/dbus`, NetworkManager not running, AppArmor denying the bus — this `restore()` fails the same way, replaces the original exception and skips `remove_signal_handler`. The container then crash-loops on a secondary traceback that does not say what actually went wrong. Catch the startup failure, log it and carry on.
Take the review: restore on every path, and unblock the radio
All checks were successful
Lint, type check and test / hassfest (pull_request) Successful in 17s
Lint, type check and test / quality (pull_request) Successful in 1m35s
Lint, type check and test / release (pull_request) Has been skipped
369dcf8125
Claude left a comment

Last round's findings are all addressed, but holding _suspended across the connection up introduced a crash when two restores overlap — reproduced, detail inline. The run_nmcli kill path is clean: I ran a hanging nmcli against it and the child is reaped with no zombie, no fd leak and no ResourceWarning.

Last round's findings are all addressed, but holding `_suspended` across the `connection up` introduced a crash when two restores overlap — reproduced, detail inline. The `run_nmcli` kill path is clean: I ran a hanging nmcli against it and the child is reaped with no zombie, no fd leak and no ResourceWarning.
updater/radio.py Outdated
@ -0,0 +33,4 @@
RESTORE_AFTER = timedelta(minutes=15)
"""How long the access point may stand before the uplink comes back regardless."""
NMCLI_TIMEOUT = timedelta(seconds=20)
Author
Collaborator

The 20 s is per call, but a restore makes three: connection show, connection delete, then connection up. Worst case is 60 s against stop_grace_period: 30s, so the docstring's claim that this fits inside the grace period does not hold. Budget the whole restore rather than each call, or raise the grace period to cover 3 × the timeout.

The 20 s is per call, but a restore makes three: `connection show`, `connection delete`, then `connection up`. Worst case is 60 s against `stop_grace_period: 30s`, so the docstring's claim that this fits inside the grace period does not hold. Budget the whole restore rather than each call, or raise the grace period to cover 3 × the timeout.
updater/radio.py Outdated
@ -0,0 +36,4 @@
NMCLI_TIMEOUT = timedelta(seconds=20)
"""How long one nmcli may take.
Its own default wait is 90 seconds, which outlasts the `stop_grace_period` a
Author
Collaborator

Worth saying here that this bounds nmcli, not NetworkManager: the D-Bus request has already gone, so a timed-out connection up usually still completes and a RadioError from a timeout does not mean the operation did not happen. (The hotspot case is fine — nmcli creates the profile before activation finishes, so _remove_hotspot still finds it.)

Worth saying here that this bounds nmcli, not NetworkManager: the D-Bus request has already gone, so a timed-out `connection up` usually still completes and a `RadioError` from a timeout does not mean the operation did not happen. (The hotspot case is fine — nmcli creates the profile before activation finishes, so `_remove_hotspot` still finds it.)
updater/radio.py Outdated
@ -0,0 +153,4 @@
return
_LOGGER.warning("Cleared a %s left by an earlier run", HOTSPOT_CONNECTION)
try:
await self._nmcli("device", "connect", self._device)
Author
Collaborator

This runs whenever a stale profile was deleted, including the Ethernet case where the previous run never took wlan0 down and nothing is blocked. Two consequences from nmcli(1) device connect: it "will also consider connections that are not set to auto connect", so it can activate a wifi profile the user deliberately set autoconnect no on — host policy the updater never took away; and "if no compatible connection exists, a new profile with default settings will be created and activated", which I have not verified for a wifi device with no saved profile and did not want to run against a live network. Worth checking once on the Pi that a stray wlan0 profile is not left behind on every restart.

The precise version is to write the suspended uplink's name into the firmware-cache volume when you suspend it and bring back exactly that at startup. Then the Ethernet case does nothing at all.

This runs whenever a stale profile was deleted, including the Ethernet case where the previous run never took `wlan0` down and nothing is blocked. Two consequences from nmcli(1) `device connect`: it "will also consider connections that are not set to auto connect", so it can activate a wifi profile the user deliberately set `autoconnect no` on — host policy the updater never took away; and "if no compatible connection exists, a new profile with default settings will be created and activated", which I have not verified for a wifi device with no saved profile and did not want to run against a live network. Worth checking once on the Pi that a stray `wlan0` profile is not left behind on every restart. The precise version is to write the suspended uplink's name into the firmware-cache volume when you suspend it and bring back exactly that at startup. Then the Ethernet case does nothing at all.
updater/radio.py Outdated
@ -0,0 +238,4 @@
finally:
if self._suspended is not None:
await self._nmcli("connection", "up", self._suspended.connection)
_LOGGER.info("Uplink %s is back", self._suspended.connection)
Author
Collaborator

Two overlapping restores crash here. The check on line 239, the await, and the clear on line 242 are no longer one step, so both callers pass the check, both run connection up, the first clears _suspended, and the second dereferences None — AttributeError: 'NoneType' object has no attribute 'connection'. It is not a RadioError, so service.py's handler does not catch it and the shutdown path reports a failure it did not have.

Reachable today: _restore_on_deadline nulls _deadline before calling restore(), so a SIGTERM arriving while the deadline is mid-restore cannot disarm it and both run. I reproduced it with a connection up that takes 50 ms; the call log also shows connection delete campervan-ota twice, and the second would fail against real nmcli.

Take suspended = self._suspended into a local before the await, or put an asyncio.Lock around the body of restore() — the lock also fixes the double delete.

Two overlapping restores crash here. The check on line 239, the `await`, and the clear on line 242 are no longer one step, so both callers pass the check, both run `connection up`, the first clears `_suspended`, and the second dereferences `None` — `AttributeError: 'NoneType' object has no attribute 'connection'`. It is not a `RadioError`, so service.py's handler does not catch it and the shutdown path reports a failure it did not have. Reachable today: `_restore_on_deadline` nulls `_deadline` before calling `restore()`, so a SIGTERM arriving while the deadline is mid-restore cannot disarm it and both run. I reproduced it with a `connection up` that takes 50 ms; the call log also shows `connection delete campervan-ota` twice, and the second would fail against real nmcli. Take `suspended = self._suspended` into a local before the await, or put an `asyncio.Lock` around the body of `restore()` — the lock also fixes the double delete.
@ -36,3 +39,4 @@
for number in STOP_SIGNALS:
loop.add_signal_handler(number, stop.set)
try:
await radio.clear_stale_hotspot()
Author
Collaborator

stop is set by the signal handler but not read until serve(), so a SIGTERM arriving during clear_stale_hotspot is not honoured until it finishes — up to 60 s at the current per-call timeout, with device connect taking a full 20 s to fail when there is nothing to reconnect to. docker compose down shortly after a start gets SIGKILLed rather than shutting down cleanly.

`stop` is set by the signal handler but not read until `serve()`, so a SIGTERM arriving during `clear_stale_hotspot` is not honoured until it finishes — up to 60 s at the current per-call timeout, with `device connect` taking a full 20 s to fail when there is nothing to reconnect to. `docker compose down` shortly after a start gets SIGKILLed rather than shutting down cleanly.
Take the review: lock the restore, and note the uplink on disk
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 1m27s
Lint, type check and test / release (pull_request) Has been skipped
c45dc24412
Author
Collaborator

Second round taken in c45dc24.

Fixed: the overlapping-restore regression, by an asyncio.Lock around restore() plus a local for the suspended uplink, so a second caller finds the work done rather than a None. Test covers two restores racing with a slow connection up.

device connect is gone, and with it the risk you flagged of nmcli inventing a profile. The uplink taken down for an update is now written to /var/lib/campervan-updater/state/suspended-uplink on a new compose volume, before connection down runs, and recover() at startup brings back exactly that profile. A note that cannot be written stops the uplink coming down at all, which is the way round that keeps the Pi reachable. The Ethernet case is now a genuine no-op.

NMCLI_TIMEOUT is 9 seconds, so a restore's three calls fit inside the 30 second stop_grace_period rather than overrunning it by double.

Not changed, deliberately: a SIGTERM arriving during recover() is still not acted on until serve(), so a slow startup recovery can be SIGKILLed part way. That leaves either the stale hotspot or the noted uplink exactly as it found them, and the next start heals both — the note is what makes that true now. Plumbing stop through the recovery would buy a faster exit in a case that already self-heals.

Second round taken in c45dc24. Fixed: the overlapping-restore regression, by an `asyncio.Lock` around `restore()` plus a local for the suspended uplink, so a second caller finds the work done rather than a `None`. Test covers two restores racing with a slow `connection up`. `device connect` is gone, and with it the risk you flagged of nmcli inventing a profile. The uplink taken down for an update is now written to `/var/lib/campervan-updater/state/suspended-uplink` on a new compose volume, before `connection down` runs, and `recover()` at startup brings back exactly that profile. A note that cannot be written stops the uplink coming down at all, which is the way round that keeps the Pi reachable. The Ethernet case is now a genuine no-op. `NMCLI_TIMEOUT` is 9 seconds, so a restore's three calls fit inside the 30 second `stop_grace_period` rather than overrunning it by double. Not changed, deliberately: a SIGTERM arriving during `recover()` is still not acted on until `serve()`, so a slow startup recovery can be SIGKILLed part way. That leaves either the stale hotspot or the noted uplink exactly as it found them, and the next start heals both — the note is what makes that true now. Plumbing `stop` through the recovery would buy a faster exit in a case that already self-heals.
Claude left a comment

The lock is right: restore() is its only holder, never re-enters, and _restore_on_deadline nulls _deadline before calling it, so _disarm cannot cancel a task queued on the lock and the second acquirer correctly finds nothing to do. No deadlock, no skipped restore. The note's write-before-down half is right too — a crash between the two costs a redundant connection up, nothing more. recover() doing both jobs is the right shape: both are "undo the last run", the order matters, and there is one caller.

The window that still loses the uplink is the in-memory half of the same ordering. Two findings inline.

The lock is right: `restore()` is its only holder, never re-enters, and `_restore_on_deadline` nulls `_deadline` before calling it, so `_disarm` cannot cancel a task queued on the lock and the second acquirer correctly finds nothing to do. No deadlock, no skipped restore. The note's write-before-down half is right too — a crash between the two costs a redundant `connection up`, nothing more. `recover()` doing both jobs is the right shape: both are "undo the last run", the order matters, and there is one caller. The window that still loses the uplink is the in-memory half of the same ordering. Two findings inline.
updater/radio.py Outdated
@ -0,0 +217,4 @@
# profile's autoconnect is what recovers the Pi after a power cut,
# with the updater out of the picture entirely.
await self._nmcli("connection", "down", uplink.connection)
self._suspended = uplink
Author
Collaborator

_suspended is set after the connection down, so a failed or timed-out down leaves the note on disk and nothing in memory. The uplink is down with autoconnect blocked, no hotspot went up, no deadline was armed, and restore() on SIGTERM does nothing — I ran it: the only call it makes afterwards is -g NAME connection show. The note only helps at the next start, and there will not be one, because the caller just catches the exception and the service keeps running.

Nine seconds is a short budget for a deactivation that normally takes well under one. Move this line above the connection down, the same way the note already is — a connection up on a profile that never went down is harmless, and then in-memory and on-disk state agree.

`_suspended` is set after the `connection down`, so a failed or timed-out down leaves the note on disk and nothing in memory. The uplink is down with autoconnect blocked, no hotspot went up, no deadline was armed, and `restore()` on SIGTERM does nothing — I ran it: the only call it makes afterwards is `-g NAME connection show`. The note only helps at the next start, and there will not be one, because the caller just catches the exception and the service keeps running. Nine seconds is a short budget for a deactivation that normally takes well under one. Move this line above the `connection down`, the same way the note already is — a `connection up` on a profile that never went down is harmless, and then in-memory and on-disk state agree.
@ -0,0 +170,4 @@
# Dropped rather than retried next start: a profile that will not
# come up now will not come up then either, and the note would
# outlive the reason for it.
_LOGGER.exception("Could not bring %s back", noted)
Author
Collaborator

Dropping the note when the up fails is wrong for something that moves. The comment's premise — "a profile that will not come up now will not come up then either" — holds for a fixed installation, but a van starts where the AP is out of range. The block on that profile's autoconnect is still in place, which is the whole reason the note exists, so once the note is gone the Pi will not rejoin that network until a reboot.

Clear the note only after a successful up. It self-clears on the first start where the network is there, at the cost of one failing call per start until then. test_a_noted_uplink_that_will_not_come_up_is_not_owed_forever inverts with it.

Dropping the note when the `up` fails is wrong for something that moves. The comment's premise — "a profile that will not come up now will not come up then either" — holds for a fixed installation, but a van starts where the AP is out of range. The block on that profile's autoconnect is still in place, which is the whole reason the note exists, so once the note is gone the Pi will not rejoin that network until a reboot. Clear the note only after a successful `up`. It self-clears on the first start where the network is there, at the cost of one failing call per start until then. `test_a_noted_uplink_that_will_not_come_up_is_not_owed_forever` inverts with it.
updater/radio.py Outdated
@ -0,0 +212,4 @@
# Noted before it goes anywhere: if the note cannot be written, the
# uplink stays up and the update does not happen, which is the way
# round that leaves the Pi reachable.
self._remember(uplink)
Author
Collaborator

Minor: raise_hotspot writes _suspended, the note and _deadline without the lock restore() now takes. Nothing can trigger it today, but once CCS-UHA-3 calls this, a deadline restore in flight can clear _suspended and _forget() the note a new raise has just written. Either hold self._restoring here too, or say in the docstring that raising and restoring are never concurrent.

Minor: `raise_hotspot` writes `_suspended`, the note and `_deadline` without the lock `restore()` now takes. Nothing can trigger it today, but once CCS-UHA-3 calls this, a deadline restore in flight can clear `_suspended` and `_forget()` the note a new raise has just written. Either hold `self._restoring` here too, or say in the docstring that raising and restoring are never concurrent.
Take the review: owe the uplink sooner, and keep the note
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 1m53s
Lint, type check and test / release (pull_request) Has been skipped
1e56a7f34d
Author
Collaborator

Third round taken in 1e56a7f.

_suspended is now set before connection down, and the down moved inside the try, so a failed or timed-out deactivation restores immediately and a SIGTERM afterwards still knows what it owes. New test: a refused connection down ends with connection up Van WiFi and the note cleared.

recover() keeps the note when the connection up fails — your reading is right, a van parked out of range is the ordinary case, and the blocked autoconnect means nothing else would ever bring it back. The test inverted with it.

Not changed: raise_hotspot still writes _suspended, the note and the deadline outside the restore lock. Agreed it wants fixing, but nothing raises a hotspot yet, so there is no second caller to race — the right time is when CCS-UHA-3 or the OTA ticket wires up a caller and the shape of the critical section is actually known. Guarding only part of the raise now would read as safe without being it.

Third round taken in 1e56a7f. `_suspended` is now set before `connection down`, and the down moved inside the `try`, so a failed or timed-out deactivation restores immediately and a SIGTERM afterwards still knows what it owes. New test: a refused `connection down` ends with `connection up Van WiFi` and the note cleared. `recover()` keeps the note when the `connection up` fails — your reading is right, a van parked out of range is the ordinary case, and the blocked autoconnect means nothing else would ever bring it back. The test inverted with it. Not changed: `raise_hotspot` still writes `_suspended`, the note and the deadline outside the restore lock. Agreed it wants fixing, but nothing raises a hotspot yet, so there is no second caller to race — the right time is when CCS-UHA-3 or the OTA ticket wires up a caller and the shape of the critical section is actually known. Guarding only part of the raise now would read as safe without being it.
Claude left a comment

Both findings are fixed. I ran the three failure points — a refused connection down, hotspot and address read — and each one now brings Van WiFi back up and clears the note, with a later restore() correctly finding nothing left to do. Declining the lock on raise_hotspot is fair: a partial critical section reading as safe is the worse outcome.

One test-isolation nit inline; nothing else.

Both findings are fixed. I ran the three failure points — a refused `connection down`, hotspot and address read — and each one now brings `Van WiFi` back up and clears the note, with a later `restore()` correctly finding nothing left to do. Declining the lock on `raise_hotspot` is fair: a partial critical section reading as safe is the worse outcome. One test-isolation nit inline; nothing else.
@ -36,3 +39,4 @@
for number in STOP_SIGNALS:
loop.add_signal_handler(number, stop.set)
try:
await radio.recover()
Author
Collaborator

Not this file, but reached from here: tests/test_updater.py builds Radio(no_connections) with no note=, and the note fixture that redirects SUSPENDED_UPLINK is autouse only in tests/test_updater_radio.py. So recover() there reads the real /var/lib/campervan-updater/state/suspended-uplink, and if one existed — on the Pi — the stubbed connection up would "succeed" and _forget() would unlink it. Pass a note=tmp_path / ..., or move the fixture into conftest.py.

Not this file, but reached from here: `tests/test_updater.py` builds `Radio(no_connections)` with no `note=`, and the `note` fixture that redirects `SUSPENDED_UPLINK` is autouse only in `tests/test_updater_radio.py`. So `recover()` there reads the real `/var/lib/campervan-updater/state/suspended-uplink`, and if one existed — on the Pi — the stubbed `connection up` would "succeed" and `_forget()` would unlink it. Pass a `note=tmp_path / ...`, or move the fixture into `conftest.py`.
Keep the service test off the real uplink note
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 1m25s
Lint, type check and test / release (pull_request) Has been skipped
e752d50c6d
Merge remote-tracking branch 'origin/feat/firmware-updates' into feat/networkmanager-hotspot
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 1m29s
Lint, type check and test / release (pull_request) Has been skipped
365e39525e
# Conflicts:
#	compose.yaml
#	updater/service.py
Claude merged commit c3ebfceaf3 into feat/firmware-updates 2026-09-19 08:25:31 +00:00
Claude deleted branch feat/networkmanager-hotspot 2026-09-19 08:25:31 +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!54
No description provided.