Skip to content

firmware: give the update handshake a deadline - #402

Open
zunda-pixel wants to merge 2 commits into
coredevices:masterfrom
zunda-pixel:zunda/firmware-update-start-timeout
Open

zunda-pixel wants to merge 2 commits into
coredevices:masterfrom
zunda-pixel:zunda/firmware-update-start-timeout

Conversation

@zunda-pixel

@zunda-pixel zunda-pixel commented Sep 3, 2026 •

Copy link
Copy Markdown
Contributor

beginFirmwareUpdate waits for the watch to answer FirmwareUpdateStart with no deadline. If the answer never comes, it sits on the await: _firmwareUpdateState keeps whatever value it held, nothing is reported as failed, and interruptedUpdates is never told about the attempt. It only unblocks when the connection scope is cancelled from somewhere else.

Everything else in this path already has a deadline:

wait deadline where
requestWatchVersion → requestWatchColor 20s Negotiator.negotiate, which catches TimeoutCancellationException and returns a null WatchInfo
requestFirmwareUpdateStatus 15s its own withTimeoutOrNull, returning null so the transfer starts from scratch
sendFirmwareUpdateStart — nothing

So this gives it the same 15 seconds, and an unanswered start now takes the path a refused one already took — error(...), caught by the IllegalStateException branch below, Idle(e).

Why withTimeoutOrNull and not withTimeout

The try this sits in has

} catch (e: CancellationException) {
   _firmwareUpdateState.value =
      FirmwareUpdateStatus.NotInProgress.ErrorStarting(FirmwareUpdateErrorStarting.ErrorDownloading)
   throw e
}

TimeoutCancellationException is a CancellationException, so withTimeout here would be reported as a download failure — neither what happened nor recoverable the same way. There is a comment on the call saying so, since the next person to touch it will reach for withTimeout first.

Everything else that waits for one of the watch's answers has one. The
negotiator bounds the version request at 20s and reports a null WatchInfo;
requestFirmwareUpdateStatus bounds itself at 15s and returns null so the
transfer starts from scratch. The start handshake was the exception: if the
watch never answered FirmwareUpdateStart, beginFirmwareUpdate sat on the
await, leaving _firmwareUpdateState on whatever it held and the interrupted
update unrecorded, until the connection scope was cancelled from somewhere
else.

Bounded the same way, and an unanswered start now takes the path a refused
one already took.

Deliberately withTimeoutOrNull rather than withTimeout: the catch below this
takes CancellationException to mean the download failed and rethrows, so a
TimeoutCancellationException would be reported as ErrorDownloading, which is
neither what happened nor recoverable in the same way.

Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

1 participant