feat: persist per-printer label settings (tape size, colors) — closes #20#39
Merged
Conversation
Remember the tape size and foreground/background colors loaded in each printer across visits, so they aren't re-selected every page load — especially painful in kiosk / shared-tablet setups (issue #20). Storage reuses the existing state file (LABELLE_STATE_FILE), but the prior usb_power saver did an atomic *full-file* overwrite, which would clobber a second writer's keys. Introduce server/state_store.py: the single owner of that file, doing atomic read-modify-write of the whole document under a shared lock so independent features each persist their own slice safely. usb_power now routes through it (same on-disk shape and path arg; ~20 fewer lines; all its tests still pass). - server/printer_settings.py: get/save the persisted subset (tapeSizeMm, foregroundColor, backgroundColor) keyed by PrinterInfo.id under a "printers" namespace, validated server-side. - API: GET/PUT /api/printers/<path:printer_id>/settings (ids carry spaces/colons, taken as a path segment). - Client: usePrinterSettings applies a printer's saved subset on selection and writes the full subset back on change (apply and save are separated so they can't loop). effectivePrinterId falls back to the sole connected printer on Auto-select, so single-printer setups persist even though the selector is hidden. Margins/justify/cutMark are deliberately out of scope (label preferences, not physical printer state). v2 follow-ups (aliases, presets, remember-last) extend this same per-printer map; the runtime capability/status cluster is tracked in #38. Tests: backend 247 pass, frontend 69 pass. Version 1.7.0 -> 1.8.0 (MINOR). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
There was a problem hiding this comment.
Pull request overview
Adds per-printer persistence for label “physical” settings (tape width + foreground/background colors) across visits by introducing a shared, lock-guarded JSON state store on the backend and wiring new GET/PUT endpoints into the client’s settings UI.
Changes:
- Introduces
server/state_store.pyas the single read/modify/write owner ofLABELLE_STATE_FILE, and routes existing USB power persistence through it. - Adds
server/printer_settings.pyplusGET/PUT /api/printers/<path:printer_id>/settingsfor per-printer tape/color persistence. - Adds client persistence plumbing (
usePrinterSettings, API helpers, and SettingsBar integration) with tests.
Reviewed changes
Copilot reviewed 20 out of 20 changed files in this pull request and generated 4 comments.
Show a summary per file
| File | Description |
|---|---|
| server/usb_power.py | Moves hub/port persistence to the shared state store to avoid clobbering. |
| server/state_store.py | Adds atomic read-modify-write store with a process lock around updates. |
| server/printer_settings.py | Adds per-printer persisted subset with server-side validation. |
| server/app.py | Adds printer settings GET/PUT endpoints using <path:printer_id>. |
| server/tests/test_state_store.py | Tests state store behavior, including concurrent updates. |
| server/tests/test_printer_settings.py | Tests per-printer persistence and coexistence with USB power keys. |
| server/tests/test_app.py | Tests new endpoints (including ids with spaces/colons). |
| server/tests/test_usb_power.py | Updates tests to monkeypatch state_store.STATE_FILE. |
| server/tests/test_smoke.py | Adds smoke imports for new modules. |
| client/src/types/label.ts | Defines PersistedPrinterSettings subset type. |
| client/src/lib/api.ts (+ test) | Adds fetch/save printer settings helpers with URL encoding. |
| client/src/lib/printerSettings.ts (+ test) | Adds effectivePrinterId + persisted-subset picking. |
| client/src/state/usePrinterSettings.ts | Applies saved settings on select and saves on user changes. |
| client/src/components/SettingsBar.tsx (+ test) | Hooks persistence into tape/color selectors and tests behavior. |
| README.md / docs/ARCHITECTURE.md | Documents the new persistence feature and state store. |
| package.json | Bumps version to 1.8.0. |
💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.
A hand-edited or corrupt state file could carry a non-dict "printers" value (e.g. "printers": []). get_settings then raised AttributeError on .get() (500 on GET), and save_settings' setdefault returned the non-dict so __setitem__ raised (500 on PUT). Treat a non-dict "printers" as empty on read, and replace it on write. Adds regression tests. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
persist() built the full per-printer subset from the settings captured in the current render. Consecutive user edits can fire before a re-render, so the other fields could be sent stale and overwrite a just-changed value server-side. Read useLabelStore.getState() at call time instead. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
read_all logs on every read of a corrupt file, not once — drop the "once" wording and note the file is overwritten cleanly on next write. Also trim the per-printer-settings README line per review. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Source-introspection check (same idiom as the module-manifest guard): LABELLE_STATE_FILE may be referenced only by state_store.py, so every feature persists via read_all/update and writes stay atomic and non-clobbering. Fails loudly if direct state-file access is reintroduced (as the pre-refactor usb_power saver had). Also registers the new /api/printers/<id>/settings route in the route smoke-check. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
PUT /api/printers/<id>/settings used get_json(silent=True) or {}, so a
malformed or non-JSON body coerced to {} — which passed validation and
overwrote the printer's saved settings with an empty dict. Reject a
non-dict body with 400 so an accidental bad request can't silently wipe
persisted state.
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
A corrupt or hand-edited state file shouldn't crash or serve junk: - get_settings sanitizes each entry to known keys with allowed values (the read-side mirror of the write-time _validate), so out-of-range or unknown values are dropped rather than handed to the client. - state_store.read_all now also catches UnicodeDecodeError (a ValueError, not OSError), so a non-UTF8 file is treated as empty per the docstring instead of raising. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
update() documented writes as best-effort but only caught OSError, so a non-JSON-serializable value (a feature accidentally writing e.g. a set) would raise TypeError from json.dumps and crash the caller's actual operation. Catch serialization errors (TypeError/ValueError) too — logged and swallowed like I/O errors — so persisting state never breaks the request. Prior on-disk state is left intact (the bad write never lands). Also drops a stray issue-#20 mention from the docstring. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The pre-refactor _load_state warned when the state file had hub/port with the wrong types; routing through state_store.read_all made it silent (read_all only logs missing/corrupt-JSON files). Restore the warning for a present-but-invalid shape, staying quiet on the normal "no saved port yet" case (e.g. a file holding only printer settings). Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The PR sprinkled "(issue #20)" across many files where it added no context once the feature exists. Keep a single canonical reference in the printer_settings.py module docstring (where the why/scope lives) and drop the rest from comments, type defs, tests, and ARCHITECTURE.md. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 13, 2026
szrudi
added a commit
that referenced
this pull request
Jun 13, 2026
Virtual ids were derived from the display name with a minimal sanitizer (spaces/parens only) and no uniqueness check, so: distinct names could collide onto one id (shared settings, second printer shadowed), and path-hostile chars like "/" survived into the /api/printers/<id>/settings route. The id also changed whenever the name was edited. - Add an optional explicit `id` to VIRTUAL_PRINTERS entries — a stable, URL-safe identity decoupled from the display name (the virtual analog of #40's serial). Renaming the name no longer orphans saved settings. - When absent, slug the name to a path-safe id (collapse non-word runs to "_"; keep Unicode letters, which %-encode fine; fall back to "printer"). - Validate at config load: reject a non-URL-safe explicit id, and skip an entry whose id collides with an earlier one (warn on both). - Funnel all VirtualPrinter construction through VirtualPrinter.from_config so the resolved id is consistent across listing, lookup, and printing. No version bump — stacked under the per-printer-settings work (#39), ships in the single v1.8.0 that PR cuts. Closes #42. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
Closes the apply/save races surfaced in review of the per-printer settings flow: - Block the persisted controls (tape/fg/bg) until the effective printer's settings resolve — before the printer list loads and while the fetch is in flight. A late-arriving fetch can no longer revert a concurrent edit, and edits made before the list loads aren't lost. A fetch error (or a failed /api/printers) unlocks gracefully with defaults rather than locking forever. Adds availablePrintersLoaded to the store and a `loading` flag from usePrinterSettings; SettingsBar disables the three controls and shows a brief hint while loading. - Strip printerId from exported label files (it's a local serial/virtual id, not portable content, and leaked the local printer identity); loadLabel keeps the printer you're currently on rather than adopting the file's. Fixes a loaded label clobbering its own tape/colors, the privacy leak, and the absent-printer selector state. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
The apply effect only applied fetched settings when non-empty (`Object.keys(saved).length > 0`), so selecting a printer with no saved settings left the *previous* printer's tape/colors in place instead of resetting — breaking per-printer isolation (found in manual testing on the Pi: a printer inherited another's 19mm white/blue). Always set the persisted fields to saved-where-present, defaults-otherwise. Extract DEFAULT_SETTINGS in the store as the single source for those defaults. Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
This was referenced Jun 13, 2026
Brings in #41 (serial ids), #42 (virtual ids), #44 (default printer selection), #45 (shared state_store + UTF-8). Conflict resolution: - state_store.py / usb_power.py / test_state_store.py: took main's (the #45 versions with UTF-8 read/write + corrected comment + non-ASCII test) — they supersede #39's older extracted copies. - test_smoke.py: kept #39's (superset — adds printer_settings module and the /api/printers/<id>/settings route on top of #45's ownership guard). - useLabelStore setAvailablePrinters: combined main's default-printer selection with #39's availablePrintersLoaded flag. - SettingsBar + its tests, useLabelStore tests, ARCHITECTURE.md: unioned #44's selector changes with #39's persist/load-gate. Frontend 81, backend 276, build green.
- Unhashable values (e.g. a client sends {"tapeSizeMm": [12]}, or a list
is in a hand-edited state file) made `value in <set>` raise TypeError —
a 500 on PUT and a crash in get_settings on read. Treat unhashable as
invalid via a hashable-safe membership helper used by both _validate
(raise) and _sanitize (drop).
- save_settings only checked printer_id non-empty. Require a string and
cap its length (256), so the unauthenticated API can't bloat the shared
state file with absurdly long keys. (Broader spam-many-ids abuse is
inherent to the LAN-trusted, unauthenticated API — out of scope here.)
Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
pickPersisted hard-coded the three fields while PERSISTED_PRINTER_KEYS right above it is documented as the single source of truth. Build the subset from that list so the two can't drift. (Per Copilot review.) Co-Authored-By: Claude Opus 4.8 (1M context) <noreply@anthropic.com>
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
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.
Closes #20.
Remembers the tape size and foreground/background colors loaded in each printer across visits, so they aren't re-selected on every page load — especially painful in kiosk / shared-tablet setups.
Architecture decision (worth a look in review)
The issue says "reuse the existing state file." The prior
usb_powersaver did an atomic full-file overwrite — atomic (tmp +os.replace, no torn reads) but it rewrote the whole document, so a second writer's keys would be clobbered. Rather than spawn a second state file, I addedserver/state_store.py: the single owner ofLABELLE_STATE_FILE, doing atomic read-modify-write of the whole doc under a shared lock so independent features each persist their own slice safely.usb_powernow routes throughstate_store— same on-disk shape, samepatharg, ~20 fewer lines, all its existing tests still pass.templates_store.pyis specced to "mirror the shape ofprinter_settings.py". SQLite stays reserved for the template collection (v2), not this tiny keyed config — see discussion in the issue.What's in here
Backend
state_store.py— atomic RMW state file, shared lock.printer_settings.py— get/save the persisted subset (tapeSizeMm,foregroundColor,backgroundColor) keyed byPrinterInfo.idunder a"printers"namespace, validated server-side.GET/PUT /api/printers/<path:printer_id>/settings(ids carry spaces/colons → taken as a path segment).Frontend
usePrinterSettingsapplies a printer's saved subset on selection and writes the full subset back on change (apply and save are separated so they can't loop; full-subset write means the server's wholesale per-printer write never drops a field).effectivePrinterIdfalls back to the sole connected printer on Auto-select, so single-printer setups persist even though the selector is hidden.Scope
Margins / justify / cutMark are deliberately out of scope — they're label preferences, not physical printer state (matches the issue's "to discuss" list). The runtime capability/status cluster is tracked in #38.
Tests
state_store,printer_settings, app endpoints incl. space/colon ids, usb_power no-clobber + concurrency).effectivePrinterId/pickPersisted, SettingsBar apply-on-select / save-on-change / Auto-select behavior).Versioning
1.7.0 → 1.8.0(MINOR — new backwards-compatible feature). README + ARCHITECTURE.md updated.🤖 Generated with Claude Code