Skip to content

fix(serial): recover ATX/DC extension workers from transient read errors - #1525

Open
ilyongnoh-ux wants to merge 1 commit into
jetkvm:devfrom
ilyongnoh-ux:fix/atx-dc-serial-worker-recovery
Open

fix(serial): recover ATX/DC extension workers from transient read errors#1525
ilyongnoh-ux wants to merge 1 commit into
jetkvm:devfrom
ilyongnoh-ux:fix/atx-dc-serial-worker-recovery

Conversation

@ilyongnoh-ux

Copy link
Copy Markdown

What

runATXControl and runDCControl exit permanently on the first serial read error. After any transient UART hiccup the extension stays silent until the app restarts, with only a single Warn line as evidence. To users this looks like a dead ATX/DC board.

How

Both workers now take the port they were started with as a parameter. On a read error they distinguish two cases:

  • the global port was swapped (unmount / extension switch via reopenSerialPort) — the worker stops quietly, matching the existing implicit-shutdown design;
  • otherwise the error is treated as transient: warn, back off (1s doubling, capped at 30s), rebuild the reader and keep reading. The backoff resets after any successful read.

No new synchronization is introduced: the global port is read the same way the existing press/command paths already access it.

Testing

  • Built and deployed on a JetKVM (v2, system 0.2.8) on a 0.5.8-based build; both workers run stably.
  • Full E2E of the recovery path (unplug/replug the extension mid-session) is pending on my unit — its ATX board is currently not powering up (that investigation is what surfaced this bug). Happy to re-test once the board is replaced.

🤖 Generated with Claude Code

runATXControl and runDCControl returned on the first serial read error,
permanently terminating the goroutine. A single transient read failure
left the ATX/DC extension silent until the whole app was restarted.

Intentional teardown (unmount, extension switch) works by having
reopenSerialPort replace the global port, which makes the existing
reader fail - an implicit stop signal. To keep that behaviour while
surviving transient errors, pass the port each worker was started with
and compare port generations on error:

- If the global port differs from the worker's port, it was replaced on
  purpose: log Debug and return.
- Otherwise the error is transient: log Warn, sleep with an exponential
  backoff (1s doubling up to 30s), rebuild the reader and keep looping.
  The backoff resets after any successfully read line.

Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
@CLAassistant

Copy link
Copy Markdown

CLA assistant check
Thank you for your submission! We really appreciate it. Like many open source projects, we ask that you sign our Contributor License Agreement before we can accept your contribution.
You have signed the CLA already but the status is still pending? Let us recheck it.

@chatgpt-codex-connector chatgpt-codex-connector Bot left a comment

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

💡 Codex Review

Here are some automated review suggestions for this pull request.

Reviewed commit: e2fd043df8

ℹ️ About Codex in GitHub

Your team has set up Codex to review pull requests in this repo. Reviews are triggered when you

  • Open a pull request for review
  • Mark a draft as ready
  • Comment "@codex review".

If Codex has suggestions, it will comment; otherwise it will react with 👍.

Codex can also answer questions or update the PR. Try commenting "@codex address that feedback".

Comment thread serial.go
// If the global port has been swapped out (unmount or extension
// switch replaces it via reopenSerialPort), the read error is an
// intentional shutdown signal for this worker: stop quietly.
if port != p {

Copy link
Copy Markdown

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

P2 Badge Synchronize port replacement detection

When an extension is unmounted or switched, reopenSerialPort writes the package-level port while the control goroutine can reach this comparison immediately after Close unblocks ReadString; concurrently reading and writing an interface value is a Go data race and can produce race-detector failures or, in rare cases, observe a torn interface value. The same pattern appears in runDCControl, so the replacement signal should use synchronized state or an explicit shutdown channel rather than reading port directly from the worker.

Useful? React with 👍 / 👎.

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.

2 participants