feat: Bluetooth printer setup overhaul — scan feedback, OS-paired Classic printers, typed discovery errors#590
Merged
Merged
Conversation
- Add generation counter; increment in start() and finish() to invalidate
stale timers and closures from ended sessions.
- Guard discovery timer body, connect timer body, and startChooser
onConnected closure with gen !== generation checks.
- Bail after startChooser call (Fix 2) if a synchronous onConnected already
ended the session, preventing a dangling discovery timer.
- Send sendSelection('') in onConnected when phase === 'discovering' (Fix 3)
to dismiss the still-pending main-process chooser on auto-reconnect.
- Add 7 new vitest tests: timeout overrides, cancel-during-connecting,
restart interval hygiene (×2), discovering auto-connect, stale-closure
ignore, and reentrancy/onError-restart.
…ed discovery errors - Rewrites use-printer-discovery.electron.ts: wires createBluetoothScanSession (per-session generation guards, discovery/connect timeouts), adds isUsbScanning busy state, migrates error type from string to DiscoveryError, exposes bluetoothCandidates/selectBluetoothCandidate/ cancelBluetoothScan, expands ElectronIpc to include send/on for session IPC - Migrates use-printer-discovery.web.ts: DiscoveryError error type, optional session members for interface congruence, structured error codes (network-none-found, discovery-failed) - Migrates use-printer-discovery.ts (base/native): same DiscoveryError migration, optional session members, maps SDK failure strings to structured codes - Adds JSDoc to DiscoveryError in types.ts (review feedback) - Adds 6 hook-level integration tests in vitest; adds webbluetooth ESM alias to vitest config so the browser-only package resolves in the node test environment # Conflicts: # pnpm-lock.yaml
…connect-timeout, ipc-unavailable, cancel paths
…egardless of connection type
…s on BT tab, typed error mapping
- add-printer.electron: wire isUsbScanning/isBluetoothScanning/bluetoothCandidates/selectBluetoothCandidate/cancelBluetoothScan from hook; USB scan button shows loading/disabled state; Bluetooth section replaced with scan+cancel button row, searching text, ElectronBtPicker (props-driven), and InstalledPrintersSection on Windows; error render uses formatDiscoveryError
- add-printer.tsx + add-printer.web.tsx: replace .replace('%s', error) with formatDiscoveryError(error, t)
- installed-printers-section: add useWatch selected-state highlight matching DeviceList idiom; test adds scanning branch and useWatch mock
…n owns its empty state)
…or electron Adds SerialElectronAdapter (mirrors UsbElectronAdapter, channel print-raw-serial), routes serial: addresses in device-adapter.electron.ts, and adds connectSerialDevice / isSerialScanning to the electron discovery hook (no error on empty — UI owns that state). Interface members added to web and native hook variants for congruence.
…paired serial devices Generalise InstalledPrintersSection → OsPrintersSection with configurable addressPrefix/heading/hint/emptyText/loadingText/testIdPrefix props, covering both the Windows winspool: path and the new macOS/Linux serial: path. Wire the Bluetooth tab of add-printer.electron.tsx to show the winspool variant on Windows and the serial variant elsewhere (guarded by connectSerialDevice existence). Exclude serial: entries from DeviceList to avoid duplication.
…ion; restore serialport lockfile entries
Contributor
|
No actionable comments were generated in the recent review. 🎉 ℹ️ Recent review info⚙️ Run configurationConfiguration used: Path: .coderabbit.yaml Review profile: CHILL Plan: Pro Plus Run ID: 📒 Files selected for processing (2)
🚧 Files skipped from review as they are similar to previous changes (1)
📝 WalkthroughWalkthroughThis PR implements structured Bluetooth printer discovery and error handling across the POS system. It introduces a state-machine-based Bluetooth session controller with timeout management, refactors printer discovery hooks to report typed errors, adds composable UI components for device selection, and extends the transport layer to support serial device printing via Electron. ChangesPrinter Discovery and Error Handling Refactoring
Sequence Diagram(s)sequenceDiagram
participant User as User
participant Dialog as Add-Printer Dialog
participant Hook as usePrinterDiscovery
participant Session as BluetoothScanSession
participant IPC as Electron IPC
participant Chooser as Web Bluetooth Chooser
User->>Dialog: Click "Scan for Bluetooth"
Dialog->>Hook: connectBluetoothDevice()
Hook->>Session: start()
Session->>IPC: invoke startChooser
IPC->>Chooser: show device picker
Chooser-->>IPC: onConnected(device)
IPC->>Hook: emit bluetooth-devices update with candidates
Dialog->>Dialog: render ElectronBtPicker with candidates
User->>Dialog: select device
Dialog->>Hook: selectBluetoothCandidate(id)
Hook->>Session: select(deviceId)
Session->>IPC: sendSelection(deviceId)
Session->>Session: schedule 30s connect timeout
Chooser-->>IPC: onConnected(selectedDevice)
Hook->>Hook: merge device into printers list
Dialog->>Dialog: show selected printer
User->>Dialog: confirm and save
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Possibly related PRs
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches📝 Generate docstrings
🧪 Generate unit tests (beta)
Comment |
Contributor
Dependency ReviewThe following issues were found:
|
![chatgpt-codex-connector[bot]](https://avatars.githubusercontent.com/in/1144995?s=60&v=4){kind=small}
There was a problem hiding this comment.
💡 Codex Review
Here are some automated review suggestions for this pull request.
Reviewed commit: a28be5196b
ℹ️ 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".
Contributor
|
PR fix round 3 triage (pre-fix):
Skipped threads: none at pre-fix inventory time; the fresh GraphQL query shows one unresolved inline thread and it is row #1. |
…mes Refresh usb-discovery is an instant enumeration (libusb on macOS/Linux, spooler queues on Windows), so trigger it on mount via UsbPrintersSection — consistent with OsPrintersSection's auto-load pattern. The button's remaining purpose is re-checking after hot-plug; relabelled to Refresh (settings.refresh_devices).
Contributor
🚀 Deployment Summary
🔗 Quick Links🤖 Updated by GitHub Actions |
concurrently@9.2.1 (apps/electron devDep) pins the vulnerable 1.8.3 exactly, so the resolver cannot pick the patched release without an override.
…issing translation source strings Codex review: a sync throw from the connector constructor (no navigator.bluetooth / policy-disabled) escaped start() after scanning state was set, wedging the scan button. The session machine now catches it and finishes with discovery-failed. Adds the 16 new settings.* keys to the English locale file (translations:check gate).
Contributor
Author
|
Fix round triage (complete — extends wcpos-bot's pre-fix inventory):
Validation on |
Contributor
Author
|
PR fix round triage (post-fix):
Excluded threads:
CI blockers observed during this round:
Remaining unresolved inline review threads: 0. |
Contributor
📊 Test Coverage Report
Coverage Legend
|
Contributor
Author
|
PR fix round 1 triage (pre-fix inventory on 2026-06-12):
Skipped threads: none. Fresh GraphQL review-thread inventory shows 0 unresolved inline threads. |
Contributor
|
PR fix round triage (post-push): Fresh unresolved inline review-thread inventory: 0 remaining.
Skipped threads:
Verification:
Note: full |
Contributor
Author
|
PR fix status update:
Excluded threads:
Current non-thread blockers:
Remaining unresolved inline review threads: 0. |
Contributor
|
Post-fix triage for PR #590:
Excluded threads:
CI handled separately:
|
Contributor
Author
|
PR fix round 2 triage (post-check, no-op on current head):
Excluded threads:
CI blockers checked this round:
No new commit was pushed in this round because the current PR head already contains the remaining dependency-review fix and the rerun has passed. |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
Contributor
There was a problem hiding this comment.
Actionable comments posted: 1
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In `@packages/printer/src/transport/serial-adapter.electron.ts`:
- Around line 26-37: The printRaw function leaves the timeout active because
Promise.race can't cancel losing branches; modify printRaw (in
serial-adapter.electron.ts) to create a cancelable timeout and clear it when
ipc.invoke('print-raw-serial', { device: this.deviceKey, data: Array.from(data)
}) resolves or rejects: e.g., store the setTimeout id and call
clearTimeout(timeoutId) after the IPC promise settles, or use an AbortController
to signal cancellation to the timeout branch, ensuring the timeout created with
PRINT_TIMEOUT_MS is cleaned up to avoid timer leaks.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Path: .coderabbit.yaml
Review profile: CHILL
Plan: Pro Plus
Run ID: d91f6e02-dd4b-4b58-a8e4-b5b4b76e325c
⛔ Files ignored due to path filters (1)
pnpm-lock.yamlis excluded by!**/pnpm-lock.yaml
📒 Files selected for processing (29)
apps/electronpackages/core/src/contexts/translations/locales/en/core.jsonpackages/core/src/screens/main/settings/printer/add-printer.electron.tsxpackages/core/src/screens/main/settings/printer/add-printer.tsxpackages/core/src/screens/main/settings/printer/add-printer.web.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/electron-bt-picker.test.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/electron-bt-picker.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/is-windows.tspackages/core/src/screens/main/settings/printer/dialog/connection/os-printers-section.test.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/os-printers-section.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/usb-printers-section.test.tsxpackages/core/src/screens/main/settings/printer/dialog/connection/usb-printers-section.tsxpackages/core/src/screens/main/settings/printer/dialog/discovery-error-message.test.tspackages/core/src/screens/main/settings/printer/dialog/discovery-error-message.tspackages/printer/package.jsonpackages/printer/src/discovery/bluetooth-scan-session.test.tspackages/printer/src/discovery/bluetooth-scan-session.tspackages/printer/src/hooks/use-printer-discovery.electron.test.tsxpackages/printer/src/hooks/use-printer-discovery.electron.tspackages/printer/src/hooks/use-printer-discovery.tspackages/printer/src/hooks/use-printer-discovery.web.tspackages/printer/src/index.tspackages/printer/src/transport/__tests__/device-adapter.electron.test.tspackages/printer/src/transport/__tests__/serial-adapter.electron.test.tspackages/printer/src/transport/device-adapter.electron.tspackages/printer/src/transport/serial-adapter.electron.tspackages/printer/src/types.tspackages/printer/vitest.config.tspnpm-workspace.yaml
Contributor
Author
|
PR fix round 1 triage update (fresh pre-commit inventory on 2026-06-12):
Skipped threads: none. Fresh GraphQL review-thread inventory shows 1 unresolved inline thread, row #2, and it will be fixed in this round. |
Contributor
Author
|
PR fix round 2 triage (post-fix):
Excluded threads:
CI blockers checked this round:
|
Contributor
Author
|
PR fix round 1 post-fix triage:
Excluded threads:
CI blockers handled this round:
Validation:
Remaining unresolved inline review threads: 0. Current GitHub checks on head |
Contributor
|
Final post-fix triage for PR #590:
Excluded threads:
CI handled separately:
|
Contributor
Author
|
PR fix round 2 no-op verification on current head
Excluded threads:
CI status checked after the rerun:
No code changes were pushed in this round because the live PR head already contains the requested timer cleanup and all current blockers are clear. |
Contributor
|
Post-fix triage for PR #590 at
Skipped threads: none. Fresh GraphQL unresolved inline review-thread inventory returned 0 unresolved threads. CI status checked on latest head
No additional push was made from this session because the remote branch advanced while I was working and now contains the required fixes; my push attempt was rejected as non-fast-forward before the current green head. |
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
Why
Windows 1.9.3 users report the Bluetooth "Scan for printers" button "does nothing". Diagnosis (instrumented Electron main + standalone CoreBluetooth scan): the Web Bluetooth chooser opened invisibly, found no filter-matching BLE device, and waited forever with zero UI feedback; a re-click cancelled the orphaned session with an error the connector library swallows. Deeper: Web Bluetooth is BLE-only — Bluetooth Classic printers (most receipt printers, e.g. Epson TM-m30 family) can never appear in that scan, on any platform.
Companion PR: wcpos/electron#268 (chooser listener-leak fix + serial printing path). Merge order: electron PR first, then bump the submodule pointer here to electron main's merge commit (currently pinned to the PR branch head
8ab4894so CI builds the right code).What
Make Classic printers work via OS pairing (the printers users actually have)
serial-discoveryIPC →/dev/cu.*, noise-filtered); printing routes via a newSerialElectronAdapter→print-raw-serial(serialport, open→write→drain→close, timeout + ghost-print guard).winspool:/serial:→ native adapters, before connectionType), so profiles created from the Bluetooth tab always print natively.Make the BLE scan honest and stateful
bluetooth-scan-session.ts): 20s discovery timeout, 30s connect timeout (GATT setup on slow printers; deliberate deviation from the spec's 10s), cancel, per-session generation guards against stale timers/closures from uncancellable in-flight connects.ElectronBtPickeris now a dumb list).Typed discovery errors
DiscoveryErrorcodes (bt-none-found,bt-connect-failed,usb-none-found,network-none-found,ipc-unavailable,discovery-failed+detail) instead of hardcoded English strings; dialogs map codes → translated strings viaformatDiscoveryError. Note: this migrates the error shape across web/native hook variants too (compatibility migration forced by shared types; all consumers updated in this PR).getPrintersAsynccan stall on Windows with offline queues — the button looked dead).Validation
pnpm typecheck --force6/6 · new CI gates pass locally (check-dep-duplicates,check-expo-alignment)packages/printervitest: 388 passed (incl. 15 session-machine, 13 hook, 7 adapter/routing tests)packages/corejest (printer settings): 75 passedtest:bluetooth-select,test:serial-printer, and the packaging test that now genuinely loads serialport from the packaged treeNeeds live verification on Windows: the Installed-printers section + winspool print from the BT tab (no Windows machine locally).
Known limitations / follow-ups
/dev/ttyS*/ttyUSB*appear alongside rfcomm) — deliberate, USB-serial printers are legitimate POS hardware.bt-none-foundmessage can remain visible after switching tabs (onlyusb-none-foundis suppressed cross-tab) — cosmetic.Spec + plan: local planning docs (not committed per repo policy).
Summary by CodeRabbit
New Features
Improvements
Tests