feat(printer-models): PrinterModel Protocol + ModelRegistry for plugin discovery#48
Conversation
…n discovery Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
…test isolation - Fix base.py module docstring: @runtime_checkable does not mean the registry itself runs isinstance() checks; it is available for plugin authors and guard utilities if they choose to use it. - Rephrase register() docstring: clearer wording, no misleading "idempotent" language. - Add registration-order / first-match semantics note to both find_by_pjl() and find_by_snmp_oid_value() docstrings. - Extract the repeated monkeypatch.setattr(ModelRegistry, "_models", []) into an autouse fixture in a new conftest.py; remove it from all five test functions and drop the now-unused monkeypatch parameter from each signature. Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
Summary of ChangesHello, I'm Gemini Code Assist1! I'm currently reviewing this pull request and will post my feedback shortly. In the meantime, here's a summary to help you and other reviewers quickly get up to speed! This pull request establishes the foundational infrastructure for the printer-model plugin system. It introduces a standardized contract for printer drivers and a registry mechanism to resolve discovered hardware to their respective implementations, facilitating future integration of specific printer series. Highlights
Using Gemini Code AssistThe full guide for Gemini Code Assist can be found on our documentation page, here are some quick tips. Invoking Gemini You can request assistance from Gemini at any point by creating a comment using either
Customization To customize the Gemini Code Assist for GitHub experience, repository maintainers can create a configuration file and/or provide a custom code review style guide (such as PEP-8 for Python) by creating and adding files to a Limitations & Feedback Gemini Code Assist may make mistakes. Please leave feedback on any instances where its feedback is incorrect or counterproductive. You can react with 👍 and 👎 on @gemini-code-assist comments. If you're interested in giving your feedback about your experience with Gemini Code Assist for GitHub and other Google products, sign up here. Footnotes
|
There was a problem hiding this comment.
Pull request overview
Establishes the backend’s printer-model plugin foundation by introducing a PrinterModel protocol contract and a simple in-process registry to resolve model plugins via PJL/SNMP fingerprints, with unit tests ensuring basic registry behavior and test isolation.
Changes:
- Added
PrinterModel(@runtime_checkable Protocol) defining the model-plugin contract. - Added
ModelRegistry+ModelNotFoundErrorfor registration and fingerprint-based lookup (PJL / SNMP). - Added unit tests for registry behavior, plus an autouse fixture to reset registry state between tests.
Reviewed changes
Copilot reviewed 4 out of 4 changed files in this pull request and generated 2 comments.
| File | Description |
|---|---|
| backend/app/printer_models/base.py | Defines the PrinterModel protocol used as the plugin contract. |
| backend/app/printer_models/registry.py | Adds a class-level plugin registry with PJL/SNMP lookup helpers and a not-found exception. |
| backend/tests/unit/printer_models/conftest.py | Adds an autouse fixture to reset the registry for test isolation. |
| backend/tests/unit/printer_models/test_registry.py | Adds unit tests covering register/find behavior, not-found cases, and all() copy semantics. |
| model_id: str # canonical, e.g. "PT-P750W" | ||
| pjl_signatures: list[str] # PJL MDL substrings this plugin handles | ||
| snmp_model_oid_value_substr: str # substring of the SNMP OID 2435.2.3.9.1.1.7.0 value | ||
| dpi: tuple[int, int] # (width_dpi, height_dpi) | ||
| print_head_pins: int # physical pin count |
| def test_registry_unknown_pjl_raises() -> None: | ||
| with pytest.raises(ModelNotFoundError): | ||
| ModelRegistry.find_by_pjl("MDL:UnknownModel;") | ||
|
|
||
|
|
||
| def test_registry_unknown_snmp_raises() -> None: | ||
| with pytest.raises(ModelNotFoundError): | ||
| ModelRegistry.find_by_snmp_oid_value("Unknown printer") |
There was a problem hiding this comment.
Code Review
This pull request establishes a plugin-based architecture for printer models by introducing a PrinterModel protocol and a ModelRegistry for resolving models via PJL or SNMP signatures. Feedback focuses on hardening the registry lookup logic to prevent accidental matches with empty signature strings and ensuring that test mocks fully implement the protocol to comply with type-safety requirements.
| """ | ||
| for model in cls._models: | ||
| for sig in model.pjl_signatures: | ||
| if sig in pjl_string: |
There was a problem hiding this comment.
The substring match sig in pjl_string will return True for any input if sig is an empty string. This could lead to incorrect model resolution if a plugin is misconfigured with an empty signature. Adding a truthiness check ensures that only valid, non-empty signatures are matched.
| if sig in pjl_string: | |
| if sig and sig in pjl_string: |
| order determines priority if multiple plugins match. | ||
| """ | ||
| for model in cls._models: | ||
| if model.snmp_model_oid_value_substr in oid_value: |
There was a problem hiding this comment.
Similar to the PJL lookup, an empty snmp_model_oid_value_substr would cause this method to match any OID value. Adding a check for a non-empty substring prevents accidental matches on misconfigured plugins.
| if model.snmp_model_oid_value_substr in oid_value: | |
| if model.snmp_model_oid_value_substr and model.snmp_model_oid_value_substr in oid_value: |
| class FakePtModel: | ||
| model_id = "PT-P750W" | ||
| pjl_signatures: ClassVar[list[str]] = ["MDL:PT-P750W"] | ||
| snmp_model_oid_value_substr = "PT-P750W" | ||
| dpi = (180, 180) | ||
| print_head_pins = 128 |
There was a problem hiding this comment.
FakePtModel does not implement the PrinterModel protocol (missing query_status, width_to_pixels, and build_print_job). While the registry currently only uses attributes for lookup, this incomplete implementation will fail mypy --strict checks and isinstance(fake, PrinterModel) runtime checks, making the tests fragile and unrepresentative of real plugins.
| class FakePtModel: | |
| model_id = "PT-P750W" | |
| pjl_signatures: ClassVar[list[str]] = ["MDL:PT-P750W"] | |
| snmp_model_oid_value_substr = "PT-P750W" | |
| dpi = (180, 180) | |
| print_head_pins = 128 | |
| class FakePtModel: | |
| model_id = "PT-P750W" | |
| pjl_signatures: ClassVar[list[str]] = ["MDL:PT-P750W"] | |
| snmp_model_oid_value_substr = "PT-P750W" | |
| dpi = (180, 180) | |
| print_head_pins = 128 | |
| async def query_status(self, *args, **kwargs): ... | |
| def width_to_pixels(self, *args, **kwargs): ... | |
| def build_print_job(self, *args, **kwargs): ... |
…n test invariants - register() now raises ValueError at registration time when a plugin supplies an empty PJL signature or an empty SNMP OID substring; empty substrings match every input and would silently shadow other plugins - FakePtModel completed with all three Protocol methods (query_status, width_to_pixels, build_print_job) so isinstance() checks against @runtime_checkable PrinterModel are correct - Unknown-input error assertions tightened with match= to verify the offending string appears in the exception message - Two new tests: test_register_rejects_empty_pjl_signature, test_register_rejects_empty_snmp_substring Co-Authored-By: Claude Sonnet 4.6 <noreply@anthropic.com>
## 0.3.0 (2026-05-12) * feat(config): pydantic-settings module with env-driven runtime configuration (#45) ([878e9e0](878e9e0)), closes [#45](#45) * feat(integrations): AppLookupService aggregator — Phase 3 complete (#53) ([222bef4](222bef4)), closes [#53](#53) * feat(integrations): Grocy + Spoolman lookup clients with shared NotFoundError base (#52) ([b1c9c3c](b1c9c3c)), closes [#52](#52) * feat(integrations): LabelData schema + Snipe-IT lookup client (#51) ([3bc180f](3bc180f)), closes [#51](#51) * feat(label-renderer): Template schema + Pillow/qrcode renderer for 1-bit label bitmaps (#54) ([fb77028](fb77028)), closes [#54](#54) * feat(printer-models): Brother PT-Series TapeRegistry with TZe and heat-shrink specs (#47) ([7526019](7526019)), closes [#47](#47) * feat(printer-models): Job lifecycle FSM with explicit state machine (#49) ([1a8c40e](1a8c40e)), closes [#49](#49) * feat(printer-models): PrinterModel Protocol + ModelRegistry for plugin discovery (#48) ([2ae0e09](2ae0e09)), closes [#48](#48) * feat(printer-models): PrintQueue worker with pause/resume/cancel/retry (#50) ([dfdf6fe](dfdf6fe)), closes [#50](#50) [skip ci]
Summary
Phase 2 Task 2.7 from the plan. Establishes the plugin contract every Brother (and future) printer-model series must implement, plus a class-level registry that resolves a discovered printer to its driver via PJL or SNMP fingerprint.
This PR adds the shared infrastructure ONLY — no PT or QL series plugin yet. Those come in Tasks 2.1 / 2.2.
What's in this PR
backend/app/printer_models/base.py@runtime_checkable ProtocolPrinterModelwith the per-model contract:model_id,pjl_signatures,snmp_model_oid_value_substr,dpi,print_head_pinsasync query_status(host, port, timeout_s) -> StatusBlock,width_to_pixels(tape_spec) -> int,build_print_job(image, tape_spec, auto_cut, high_resolution) -> bytesTapeSpecfromapp.models.tapeandStatusBlockfromapp.services.status_block.backend/app/printer_models/registry.pyModelRegistrywith class-level_models: ClassVar[list[PrinterModel]] = []register(model)appends — duplicates not prevented (documented).all()returns a copy so callers can iterate / sort without mutating the registry.find_by_pjl(pjl_string)/find_by_snmp_oid_value(oid_value)substring-match; raiseModelNotFoundErrorwith the offending input on miss.backend/tests/unit/printer_models/test_registry.py+conftest.pyall()returns a copy.conftest.pyhas an autouse fixture_clear_model_registrythat resets_modelsbefore each test — replaces five manualmonkeypatch.setattrcalls and makes new tests automatically isolated.What's NOT in this PR
importregisters each plugin)isinstance(model, PrinterModel)enforcement inregister()(the docstring onbase.pynow correctly notes that callers/utilities may use the@runtime_checkabledecorator if they want a guard)Test plan
pytest -q→ 48 passed (43 from before + 5 new).pytest tests/unit/printer_models/ -v→ 5/5.ruff format --check .clean.ruff check .clean.mypy app/(strict) clean.Review history (subagent-driven)
7e6ca4e— initial commit.isinstance, repetitivemonkeypatch.setattr, and ordering-semantics ambiguity.33a4369— corrected docstrings, added autouse fixture, documented order semantics.Linked plan
docs/superpowers/plans/2026-05-11-label-printer-hub.mdTask 2.7 (Phase 2 — Plugin Foundation).