Skip to content

Refactor raw print delivery#278

Open
kilbot wants to merge 1 commit into
nextfrom
codex/raw-print-unification-electron
Open

Refactor raw print delivery#278
kilbot wants to merge 1 commit into
nextfrom
codex/raw-print-unification-electron

Conversation

@kilbot

@kilbot kilbot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Summary

  • Add a shared main-process raw-print module for byte validation, Buffer conversion, timeout/cleanup handling, ghost-print cleanup, and raw-print logging.
  • Refactor serial, TCP, native USB, and Windows spooler RAW printing into small Delivery seams while preserving existing IPC channels and transport-specific guards.
  • Add test:raw-print to cover the shared timeout/cleanup state machine and wire it into the Electron test chain.

Design decisions (preserve through rebases)

  • Main-side timeouts remain transport-specific — serial/USB/winspool use 20s and TCP uses 10s so the renderer's 30s timeout still sees a main-process error first.
  • Native USB now routes through the shared timeout path — this intentionally adds the missing 20s timeout while keeping printer-class interface selection, Linux usblp detach, and release+close cleanup.
  • printConfig.timeoutMs stays mutable and is read when creating the serial delivery — existing tests can still shrink the serial timeout for the ghost-print branch.
  • Winspool printer name and payload path still flow through WCPOS_RAW_PRINT_PRINTER and WCPOS_RAW_PRINT_FILE env vars — no user-controlled values are interpolated into the PowerShell script.

Companion PRs / cross-repo

Test plan

  • pnpm ts:check
  • pnpm lint (passes with pre-existing warnings only)
  • pnpm run test:serial-printer
  • pnpm run test:winspool-printer
  • pnpm run test:printer-discovery
  • pnpm run test:raw-print
  • pnpm test

🤖 Generated with Claude Code

@coderabbitai

coderabbitai Bot commented Jun 17, 2026

Copy link
Copy Markdown
Contributor

Review Change Stack

📝 Walkthrough

Walkthrough

Introduces a new raw-print.ts module exporting Delivery interface, RawPrintContext, validateRawPrintData, rawPrintBufferFromData, sendRawPrint, sendRawBytes, and withTimeout. All four printer backends (TCP, Serial, USB, Windows Spooler) are refactored to construct a Delivery and delegate to this shared dispatch path. A new test file covers the core module and a test:raw-print script is added to package.json.

Changes

Raw-print Delivery abstraction and backend adapters

Layer / File(s) Summary
raw-print core contracts and implementation
src/main/raw-print.ts
Defines RawPrintContext and Delivery interfaces, input validation, buffer conversion, sendRawBytes with logging, and withTimeout implementing single-settlement semantics with late-completion cleanup.
raw-print tests and package.json script
src/main/raw-print.test.ts, package.json
New test file patches ./log and global timers to assert successful delivery, invalid-data rejection, withTimeout cleanup on success/failure/timeout, and the ghost-print guard. package.json adds test:raw-print wired into the main test command.
TCP Delivery adapter
src/main/print-raw-tcp.ts
Adds createTcpDelivery(host, port) wrapping net.Socket lifecycle into a Delivery, and replaces the IPC handler's inline send promise with sendRawPrint({ data }, createTcpDelivery(host, port)).
Serial Delivery adapter
src/main/serial-printer.ts
Introduces createSerialDelivery(portPath) encapsulating SerialPort open/write/drain and cleanup; removes inline data validation from the handler; delegates to sendRawPrint(args, createSerialDelivery(portPath)).
USB Delivery adapter
src/main/usb-printer.ts
Adds createUsbDelivery(deviceKey, device) for open/claim/bulk-transfer/release; centralizes args.data conversion via rawPrintBufferFromData; routes winspool branch through printRawToSpooler(printerName, bytes); delegates direct USB to sendRawBytes(bytes, createUsbDelivery(...)).
Winspool Delivery adapter
src/main/winspool-printer.ts
Removes runRawPrintScript and logger import; adds createWinspoolDelivery(printerName, file) spawning PowerShell with stderr capture and ctx.settled() guard; printRawToSpooler now calls sendRawBytes(data, createWinspoolDelivery(...)).

Sequence Diagram(s)

sequenceDiagram
  participant IPC as IPC Handler
  participant sendRawPrint
  participant withTimeout
  participant Delivery as Delivery (TCP/Serial/USB/Winspool)

  IPC->>sendRawPrint: sendRawPrint({ data }, delivery)
  sendRawPrint->>sendRawPrint: validateRawPrintData → rawPrintBufferFromData
  sendRawPrint->>withTimeout: run=(ctx => delivery.send(bytes, ctx)), cleanup, timeoutMs

  rect rgba(70, 130, 180, 0.5)
    note over withTimeout,Delivery: Normal path
    withTimeout->>Delivery: send(bytes, ctx)
    Delivery-->>withTimeout: resolved
    withTimeout->>Delivery: cleanup()
    withTimeout-->>sendRawPrint: resolved
    sendRawPrint->>sendRawPrint: log successMessage
    sendRawPrint-->>IPC: resolved
  end

  rect rgba(200, 80, 80, 0.5)
    note over withTimeout,Delivery: Timeout path
    withTimeout->>withTimeout: timer fires → reject(timeoutMessage)
    withTimeout->>Delivery: cleanup()
    withTimeout-->>sendRawPrint: rejected
    sendRawPrint->>sendRawPrint: log failureMessage, rethrow
    sendRawPrint-->>IPC: rejected
  end
Loading

Estimated code review effort

🎯 4 (Complex) | ⏱️ ~60 minutes

Possibly related PRs

  • wcpos/electron#239: Introduced the print-raw-usb IPC handler in usb-printer.ts whose inline open/claim/transfer logic is now replaced by the createUsbDelivery/sendRawBytes delegation in this PR.

Poem

🖨️ Four printers once had their own tangled ways,
Each socket and port lost in timeout-filled haze.
Now Delivery arrives, a contract so clean,
With withTimeout guarding each ghost-print unseen.
sendRawBytes dispatches the bytes down the wire —
One abstraction to rule every printer's desire! 🎉

🚥 Pre-merge checks | ✅ 4 | ❌ 1

❌ Failed checks (1 warning)

Check name Status Explanation Resolution
Docstring Coverage ⚠️ Warning Docstring coverage is 21.43% which is insufficient. The required threshold is 80.00%. Write docstrings for the functions missing them to satisfy the coverage threshold.
✅ Passed checks (4 passed)
Check name Status Explanation
Title check ✅ Passed The title 'Refactor raw print delivery' accurately reflects the main objective of centralizing raw print delivery logic across multiple transport implementations.
Linked Issues check ✅ Passed Check skipped because no linked issues were found for this pull request.
Out of Scope Changes check ✅ Passed Check skipped because no linked issues were found for this pull request.
Description Check ✅ Passed Check skipped - CodeRabbit’s high-level summary is enabled.

✏️ Tip: You can configure your own custom pre-merge checks in the settings.

✨ Finishing Touches
📝 Generate docstrings
  • Create stacked PR
  • Commit on current branch
🧪 Generate unit tests (beta)
  • Create PR with unit tests
  • Commit unit tests in branch codex/raw-print-unification-electron

Comment @coderabbitai help to get the list of available commands and usage tips.

@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: 8b1dd7e201

ℹ️ 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 src/main/raw-print.ts
},
(err) => {
if (settled) {
if (timedOut) void runCleanup().catch(() => {});

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 Avoid re-entering cleanup after timeout

When a timeout fires during an async operation that cleanup itself aborts, such as USB out.transfer while createUsbDelivery.cleanup() is still awaiting iface.release(), the operation can reject before the first cleanup has finished and this branch starts a second runCleanup() immediately. Because the USB cleanup only flips ifaceClaimed/deviceOpened after release/close completes, this can call release/close concurrently on the same device after a timeout, making retries unreliable; serialize the cleanup promise or make the delivery cleanup idempotent before rerunning it.

Useful? React with 👍 / 👎.

@kilbot kilbot changed the base branch from main to next June 17, 2026 22:02
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