diff --git a/CLAUDE.md b/CLAUDE.md index e9f2a52..8437adf 100644 --- a/CLAUDE.md +++ b/CLAUDE.md @@ -8,7 +8,8 @@ Read `docs/STEERING.md` before non-trivial work (new operations, transport or fr ## Conventions - Dev setup: `uv venv && uv pip install -e ".[dev]"` (or plain venv + pip, see contributing.md). Run tests with `.venv/bin/python -m pytest`. The suite passes offline - no hardware, no `hid` import, no `DYLD_LIBRARY_PATH`. -- Two namespaces, one package (ADR-0006): `pyquadcortex` is the model of the unit, `pyquadcortex.protocol` is the message-level API. The model imports the protocol layer; nothing under `pyquadcortex/protocol/` may import from `pyquadcortex/model/`. +- Two namespaces, one package (ADR-0006): `pyquadcortex` is the model of the unit, `pyquadcortex.protocol` is the message-level API. The model's code lives in `pyquadcortex/device/` - not `model/`, because in this codebase the identifier `model` means an amp or pedal block (`protocol/models.py`, `catalog.Model`, `ModelCatalog`, `set_block(model=...)`). The model imports the protocol layer; nothing under `pyquadcortex/protocol/` may import from `pyquadcortex/device/`. +- Every conversion between a screen value and a wire value lives in `pyquadcortex/device/translate.py` and nowhere else in the package outside `pyquadcortex/protocol/` - the whole package, not just `device/`, because a rule scoped to a directory is satisfied by moving the code one directory up. It covers rows 1-4, slots 1-8, scene and footswitch letters, preset addresses and display units. Outside the boundary that means no `+1`/`-1` on a coordinate AND none of the other spellings of the same conversion (`ord`/`chr`, a letter table in any container, `divmod` on a position, a one-based `enumerate`, `ROWS.index(...)`), and no module reaching past the boundary for a protocol-layer name that carries a coordinate or a raw scale - the converters and also the readers that hand back wire indexes, such as `protocol.stomp_assignments`. `tests/test_translation.py` reads the source and proves both, and pins where each check stops seeing rather than implying it sees everything. A model API takes `FootswitchLetter`, never a bare footswitch integer, because a footswitch index and a block's column are different numbers that usually agree. A new conversion goes in that module with its own test, however small it is, and its protocol-layer name joins that file's allowlist in the same commit. - The model represents what the unit shows, in the unit's own words, and never guesses. A control we understand but cannot yet drive is modelled and REFUSES the operation (ADR-0007); a control we do not understand is omitted, with the reason recorded in `docs/domain-model.md`'s appendix. Nothing ships with a "this might be stale or wrong" caveat. - A model property that reads a device field checks the field is PRESENT (`protocol.field_present`) before reporting it. Most of this schema sits in synthetic `oneof`s, so protobuf returns `""` or `0` for a field the unit never sent, and reporting that as the answer is the guess the rule above forbids. Never cache a reply that came back incomplete - a retry has to be able to recover. - Anything the model caches is valid only while its connection is. A closed `Device` refuses reads rather than answering from cache, because a model that reports the unit's state through an object with no unit behind it is the failure the whole layer exists to avoid. diff --git a/changelog.md b/changelog.md index 3f7da17..33c90e6 100644 --- a/changelog.md +++ b/changelog.md @@ -131,6 +131,34 @@ with protocol.connect(before_handshake=lambda t: t.add_listener(watch)) as qc: The decision behind the two rules is ADR-0009. This is the groundwork for the model keeping itself current without asking twice. +### Groundwork: the model will talk in the numbers on your screen + +Rows will be 1 to 4, slots 1 to 8, scenes and footswitches letters, levels the dB +the unit displays, and the tempo the bpm it displays. The wire counts from zero +and stores raw scales, and the model now converts in exactly one place so nothing +else has to remember to. + +**Nothing that reads a row or a level exists yet** - the preset and grid surfaces +are still being built - so what you can use today is the three value types this +groundwork brought with it, exported from `pyquadcortex`: + +```python +from pyquadcortex import PresetAddress, FootswitchLetter, SceneLetter + +PresetAddress.parse("28C") # bank 28, position C +PresetAddress.parse("28X") # ValueError, here rather than at write time +FootswitchLetter.E # a footswitch is a letter, never a number +``` + +The footswitch rule is worth the sentence it costs. A footswitch index and a +block's column are different numbers that agree most of the time, which is how a +bug hid for months: a block in column 3 assigned to footswitch E is stored under +key 4. No model API takes a bare footswitch number, so a column cannot be passed +where a footswitch belongs. + +Until the Directory arrives, `PresetAddress` is most useful for checking an +address before you hand it to the protocol layer. + ### Withdrawn: the Tempo menu's MODE is "not on the wire" The 0.23.0 entry below records, under **Settled**, that the Tempo menu's MODE diff --git a/docs/STEERING.md b/docs/STEERING.md index 71eebcc..93460b8 100644 --- a/docs/STEERING.md +++ b/docs/STEERING.md @@ -2,7 +2,7 @@ > **What this is:** durable technical context for the pyquadcortex library - what the system is and why it is shaped this way. > **What this is not:** coding rules (see the repo-root `CLAUDE.md`) or decision rationale (see [`ADR.md`](ADR.md)). -> **Last reviewed:** 2026-08-03 by Stokes +> **Last reviewed:** 2026-08-13 by Stokes > **Owners:** Stokes ## 1. Purpose @@ -41,7 +41,7 @@ The protocol layer is stateless between calls: every read is a live exchange, an ## 4. Owned Paths -- `pyquadcortex/` - the package. Two namespaces: `pyquadcortex/model/` (the model of the unit) and `pyquadcortex/protocol/` (the message-level API, including the committed generated bindings in `pyquadcortex/protocol/proto/`) +- `pyquadcortex/` - the package. Two namespaces: `pyquadcortex/device/` (the model of the unit) and `pyquadcortex/protocol/` (the message-level API, including the committed generated bindings in `pyquadcortex/protocol/proto/`) - `protocol/` - the recovered `.proto` schema and its tooling - `tests/` - the fully offline suite and its fixtures - `examples/` - runnable scripts, also used as hardware-verification shapes @@ -57,6 +57,7 @@ The protocol layer is stateless between calls: every read is a live exchange, an | Fake-per-layer offline tests | Each layer has a purpose-built double: golden captured frames for `framing`, `FakeHid` for `transport`, `FakeTransport` for `client` | see ADR-0002 | `FakeTransport` in `tests/test_client.py` | Hardware verification happens manually via `examples/`, outside the suite | | Evidence-bearing docstrings | Each operation's docstring states what is confirmed on hardware vs inferred from the schema | The device gives no errors for wrong writes, so recorded evidence is the only trail | `QuadCortex.read_preset` in `pyquadcortex/protocol/client.py` | Non-protocol helpers (pure functions) carry ordinary docstrings | | Keyed grid edits | Mutations are row/column-keyed `Grid` UPDATEs | The device applies grid updates by key; wholesale preset writes are silently ignored (see [`architecture.md`](architecture.md), "write_preset is a trap") | `QuadCortex.set_bypass` in `pyquadcortex/protocol/client.py` | Read paths, and non-grid operations | +| One translation boundary | Screen values become wire values in exactly one module, and a source-reading test proves no other module in the package does it - the whole package outside `protocol/`, not just `device/` | An off-by-one row is silent - the write lands on a real row and reads back perfectly - so a convention cannot be trusted to hold (design principle 5 in [`domain-model.md`](domain-model.md)) | `pyquadcortex/device/translate.py` | The protocol layer, which keeps its zero-based indexes and raw scales | ## 6. Constraints @@ -122,6 +123,108 @@ Single-device, single-connection USB HID at interactive rates (129-byte reports) ## Change Log +### 2026-08-13 - One translation boundary, and the model package is `device/` + +**What changed:** +- `pyquadcortex/device/translate.py`: the one module where a screen value becomes a wire + value and back - rows 1-4, slots 1-8, scene and footswitch letters, preset addresses, + and five display-unit mappings (input gain dB, lane and mixer dB, tempo bpm, tuner + reference Hz, hold timing ms). The two level scales and the tempo call the protocol + helper that carries the measurement; the other two have no helper to call, so they are + pinned against what the protocol write method expects, and the tuner's docstring says + how thin its evidence is - one observed pair. `PresetAddress`, `FootswitchLetter` and + `SceneLetter` are its public value types, re-exported from `pyquadcortex` +- Section 5 gained the pattern row; section 4's owned-paths line and CLAUDE.md name the + new rule. `architecture.md` carries the module in its layer map and a section on it; + `domain-model.md` marks principle 5, `PresetAddress` and `FootswitchLetter` as built +- **The model package directory is `pyquadcortex/device/`, renamed from `model/`.** Done + as its own commit so the story's diff stays readable + +**Why:** +- M1 Epic (stokes-audio/pyquadcortex#8), Story #10. It ships before the surfaces that use + it so no later story invents its own conversion. The Intent Brief names off-by-one as a + silent failure mode, and the protocol layer's own header agrees: an edit to the wrong + row still succeeds and still reads back correctly, so nothing tells you. A centralized, + exhaustively tested boundary is the whole mitigation, which is why two of its tests read + the model package's source instead of calling it +- The rename is an owner decision. In this codebase the identifier `model` means an amp or + pedal block - `protocol/models.py`, `catalog.Model`, `ModelCatalog`, + `set_block(model=...)`. `domain-model.md` §5 renamed that concept to *virtual device* in + the model's vocabulary, because that is what the screen calls it, but the protocol layer + still spells it `model` and will keep doing so. A directory named `model/` therefore + collides with real code a reader is looking at, whatever the design doc calls the + concept + +**Scope of impact:** +- **Updated:** STEERING.md, CLAUDE.md, architecture.md, domain-model.md, roadmap.md, + changelog.md, `pyquadcortex/device/`, `pyquadcortex/__init__.py`, + `scripts/check_artifacts.py`, `tests/test_translation.py` (new), + `tests/test_namespace.py`, `tests/test_import_cleanliness.py`, `tests/test_docs.py` +- **Not updated (intentionally):** ADR.md - neither change reverses or refines a recorded + decision. The boundary IS design principle 5, already written and reviewed in + `domain-model.md`; the rename is a directory name, chosen to stop colliding with an + identifier the protocol layer uses. README.md and api.md - the new value types have + no surface handing them out yet (the Directory is story #12), and the readme tour should + show what a caller can do, not what exists. The protocol layer - it keeps its zero-based + indexes, its `Footswitch` enum and its measured scales, and nothing below the seam + changed +- **No deprecation shim for the rename.** `pyquadcortex.__all__` lists `protocol` and never + listed `model`, and the model namespace has not been released - 0.40.0 predates the flip + - so nothing published points at the old path + +**Downstream to consider:** +- Stories #11 through #16 convert through this module rather than doing their own + arithmetic, and the source-reading tests will fail them if they do not +- The conversions M1 does not need yet land here too, with the surface that needs them. + A parameter whose display mapping is unverified stays out of the model entirely + (principle 3), so no mapping is ever invented in this module +- The arithmetic check is deliberately blunt and deliberately wide: a literal one in any + spelling (`1`, `1.0`, `True`, `-1`), `ord`/`chr`, the literal 65, a letter table as a + string, tuple, list or dict, `string.ascii_uppercase`, `divmod`, a one-based + `enumerate`, and `.index()` on `ROWS` or `SLOTS` all fail it, anywhere in the package + outside the boundary and the protocol layer. If a future module has a genuine counter, + narrowing the check is a deliberate edit with a reason, not a quiet one. What it cannot + see - a one behind a name, a table built at run time, arithmetic inside somebody else's + helper - is pinned as a failing-if-it-changes list in the same file, because a sample + table where every case passes reads like a completeness proof and is not one +- The scan is scoped to the whole package rather than to `pyquadcortex/device/`, because + a rule scoped to a directory is satisfiable by moving the code one directory up - which + is precisely what a failure message naming a directory invites +- A protocol conversion can be delegated to and still not be movable. `bpm_to_tempo` + (PR #22) is called by `QuadCortex.set_tempo_param` from inside the protocol layer, so + the helper stays there and the boundary wraps it, the same way it wraps the level + scales. Adding the name to the boundary's allowlist is the half that matters: without + it, a model module reaching for `protocol.tempo_bpm` passes the check +- The allowlist is judgement, and its criterion is what a name HANDS OVER rather than + whether it reads like a conversion. `protocol.stomp_assignments` returns three raw wire + indexes including the footswitch one, so a model module could key a mapping by it and + reintroduce the exact bug `FootswitchLetter` exists to prevent, without writing a `- 1` + anywhere. The readers are therefore listed next to the converters + +**Also in this branch:** +- Merged main (PRs #19, #20, #22) up. The three change logs conflicted in the same place + and both sides were kept in the order they landed +- The review found the boundary's own front door open: `translate.slot_to_position` + accepted non-ASCII digits, so `"٢٨C"` returned preset 218. Only `PresetAddress.parse` + carried the ASCII pattern, while a comment and a test both read as though the module + was covered. The two doors now share one pattern, and one list of malformed names is + run through both +- Both source-reading checks were narrower than they read, again. The arithmetic check + now sees a letter table in a tuple, list or dict, `string.ascii_uppercase`, the literal + 65, and `ROWS.index(row)` - a coordinate conversion written with the boundary's own + exported table and no arithmetic in it at all. The allowlist gained the protocol + readers that hand back wire coordinates. A file doing all of that at once passed both + checks before and fails both now +- The backstop that proves the boundary still converts is anchored to the four converters + by name. It had been satisfied by an error-message formatter elsewhere in the file, so + the converters could have gone arithmetic-free with nothing failing +- Each check now pins its KNOWN blind spots as blind spots. A sample table where every + "should be caught" case is caught reads like a completeness proof; these fail if a + listed gap ever closes, which is the edit where the prose gets corrected too +- The layering check could not see `from pyquadcortex import PresetAddress`, a hole this + story opened by re-exporting the value types at top level. It reads `device.__all__` + now, so it follows the code + ### 2026-08-12 - TEMPO MODE closes, and ADR-0010 **What changed:** diff --git a/docs/architecture.md b/docs/architecture.md index f9a0d53..7ca8e9e 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -36,10 +36,14 @@ only about the layer directly below it. ``` pyquadcortex/ THE MODEL - what import pyquadcortex hands back - model/device.py connect(): opens the unit, returns a Device. + device/device.py connect(): opens the unit, returns a Device. | Speaks the unit's vocabulary, never the wire. | (Directory, cache and grid land in later stories.) | + device/translate.py Screen values <-> wire values, and the ONLY place + | either becomes the other: rows, slots, scene and + | footswitch letters, preset addresses, display units + | | -- the model/protocol seam -- | pyquadcortex/protocol/ THE PROTOCOL LAYER - one call per protocol message @@ -75,7 +79,7 @@ only about the layer directly below it. ``` The model calls the protocol layer and never the other way round: nothing under -`pyquadcortex/protocol/` may import from `pyquadcortex/model/`. A caller can use +`pyquadcortex/protocol/` may import from `pyquadcortex/device/`. A caller can use either namespace, or both - `Device.from_client(qc)` puts a model on a protocol connection that is already open. @@ -188,7 +192,7 @@ gate, and the handshake's own version announce would race that READ's reply. `pyproject.toml` declares the console script as `pyquadcortex.protocol.cli:main`; `qcctl` itself is unchanged. -### model/device.py +### device/device.py `pyquadcortex.connect()` opens the unit through `protocol.connect()` and returns a `Device`, which carries the unit's identity and owns the connection. @@ -200,6 +204,43 @@ The rest of the model - the Directory, the write-through cache, the loaded prese and the grid - is designed in [domain-model.md](domain-model.md) and is being built story by story. Nothing is stubbed out to look finished. +### device/translate.py + +The model speaks what the touchscreen shows - rows 1 to 4, slots 1 to 8, scenes +and footswitches as letters, dB, Hz, bpm, ms - and the wire speaks zero-based indexes +and raw scales. Every conversion between the two lives here and nowhere else in +`pyquadcortex/` outside `protocol/` - the whole package, not just the model +directory (design principle 5 in [domain-model.md](domain-model.md)). + +One module rather than a convention, because the mistake it prevents is silent. +This document's own layer map sits above a protocol layer whose header says it: a +write to the wrong row lands on a real row and reads back perfectly, so nothing +tells the caller. Collecting the arithmetic in one place makes it reviewable in +one place, and `tests/test_translation.py` proves the rest of the package does +none of it by reading the source, rather than by trusting anyone to remember. +Two things it also proves, because neither is obvious: the four converters +themselves still do the arithmetic (otherwise "nowhere else" passes because +nowhere converts), and each check's known blind spots are pinned as blind spots, +so the sample tables cannot read as completeness proofs. + +Where a protocol-layer helper already performs the conversion - input gain dB, +lane and mixer dB, tempo bpm, the slot-name/position pair - this module calls it +instead of restating the arithmetic. Two copies of a measured scale drift apart, +and both copies go on returning a plausible number. The tuner and hold-timing +mappings have no helper to call, only a documented rule and a shared constant, so +their tests pin them against what the protocol write method expects. + +Delegating is not always a choice between two homes. `bpm_to_tempo` is called by +`QuadCortex.set_tempo_param` from inside the protocol layer, so it has to stay +there: moving it to the boundary would make the protocol layer import the model, +which `tests/test_namespace.py` refuses. The wrapper here is what gives the model +one way in without a second copy of the span. + +Public value types: `PresetAddress`, `FootswitchLetter`, `SceneLetter`, +re-exported from `pyquadcortex`. The conversion functions are the module's own +surface and are not re-exported: a caller never needs them, and the model reaches +them as `translate.row_to_wire(...)`. + ## What flows through the layers A host command, top to bottom: @@ -326,8 +367,8 @@ record the result: update your docstring and the coverage table in An operation whose shape comes only from the schema should say so. Useful shapes for hardware work live in `examples/` (`switch_scenes.py`, -`list_presets.py`, `reroute_and_save.py`). `scripts/compile_protos.sh` is the -only script in the repo. +`list_presets.py`, `reroute_and_save.py`). `scripts/` holds +`compile_protos.sh`, `check_artifacts.py` and `generate_models.py`. ## Capturing the device's traffic @@ -434,6 +475,7 @@ How each layer is faked: | `model` | `FakeClient`: answers the calls the model makes on a `QuadCortex`, plus the same monkeypatched device+transport as `session` | `tests/test_device.py` | | schema | asserts the enum integers the code relies on and that core messages instantiate | `tests/test_schema_compiles.py` | | namespaces | the pre-flip `__all__`, read verbatim from git, must all resolve under `pyquadcortex.protocol` | `tests/test_namespace.py` | +| the translation boundary | none: it is pure functions, so it is called directly. Two of its tests take the package's SOURCE as their input instead, and read it with `ast` | `tests/test_translation.py` | ### The import-safety contract diff --git a/docs/domain-model.md b/docs/domain-model.md index 567bdfb..391d2a6 100644 --- a/docs/domain-model.md +++ b/docs/domain-model.md @@ -39,6 +39,9 @@ 5. **One translation boundary.** The model speaks touchscreen coordinates and display units everywhere. Conversion to protocol values (0-based indexes, raw scales) happens in exactly one module at the model-to-protocol seam. No `-1`/`+1` anywhere else. + **Built:** `pyquadcortex/device/translate.py`, with the rule enforced by a test that + reads the source of the whole package outside `protocol/` - not just the model + directory - rather than trusting a convention. ## Namespaces: the model becomes the front door @@ -150,6 +153,14 @@ class PresetAddress: > with the mode - linear position 5 reads "1F" normally and "2B" under the hybrid - so an > address is only unambiguous alongside the mode it was read in. +> **`PresetAddress` is built**, in `pyquadcortex/device/translate.py` and exported from +> `pyquadcortex`. It speaks the non-hybrid naming, "A".."H". `PresetAddress.parse("28C")` +> refuses a malformed address there and then, rather than at write time, and `.to_wire()` +> / `.from_wire()` convert through the protocol layer's own `slot_to_position` pair so the +> two layers cannot drift on what "28C" means. The mode caveat above is on the converting +> function's docstring, where someone converting will read it. The Directory that hands +> addresses out is still ahead of the code. + ### Directory items are a type family Everything a Directory list can hold shares an `Item` base - the Directory's own word, @@ -676,6 +687,11 @@ class Stomps: # preset.stomps > eventually passes a column to it and gets a write that silently does nothing, which is > precisely the bug that cost a hardware session to find. > +> **`FootswitchLetter` is built**, in `pyquadcortex/device/translate.py` and exported from +> `pyquadcortex`. It is a `StrEnum`, so `stomps["E"]` and `stomps[FootswitchLetter.E]` are +> the same key and it prints as the screen labels it. Passing the number 4 raises, with a +> message naming the column trap. `SceneLetter` is the same type for scenes. +> > **A device-level footswitch object is deferred, deliberately.** There are now two > footswitch-keyed collections at different scopes - `preset.stomps` per preset and > `settings.looper_actions` global - plus the mode that decides which is live, so nothing @@ -1308,3 +1324,11 @@ the n/a rows below where they intersect the API at all. capture every field of every message the device answers in each switch position and diff, rather than looking for a field you expect. Still an M3 surface, so still not a behaviour change at M1. +- **2026-08-13** - Design principle 5 is built (M1 story #10): `pyquadcortex/device/translate.py` + owns every conversion between a screen value and a wire value - including the tempo's + bpm, which arrived from the TEMPO MODE work above - with `PresetAddress`, + `FootswitchLetter` and `SceneLetter` landing as part of it. The model package directory + is `device/` rather than `model/`, because the protocol layer spells an amp or pedal + block `model` in code (`models.py`, `Model`, `ModelCatalog`) and will keep doing so, + whatever §5 renamed the concept to in this document. No design changed here; this + records what is now code. diff --git a/docs/roadmap.md b/docs/roadmap.md index a8be7d4..c6ab4e3 100644 --- a/docs/roadmap.md +++ b/docs/roadmap.md @@ -63,8 +63,8 @@ with pyquadcortex.connect() as qc: for block in scene.blocks: print(block.position, block.enabled) # effective state, sceneMode absorbed - scene.blocks[1, 3].enabled = False # grid coordinates - preset.rows[0].input = Input.RETURN_1 + scene.blocks[1, 3].enabled = False # grid coordinates, as on screen + preset.rows[1].input = Input.RETURN_1 # rows are 1 to 4, like the unit preset.save() # the recall/edit/save dance is internal preset.scenes["D"].copy_from(scene, keep_label=True) diff --git a/pyquadcortex/__init__.py b/pyquadcortex/__init__.py index 383e468..967fbc9 100644 --- a/pyquadcortex/__init__.py +++ b/pyquadcortex/__init__.py @@ -33,7 +33,8 @@ logging.getLogger(__name__).addHandler(logging.NullHandler()) from pyquadcortex import protocol # noqa: E402 -from pyquadcortex.model import Device, connect # noqa: E402 +from pyquadcortex.device import (Device, FootswitchLetter, # noqa: E402 + PresetAddress, SceneLetter, connect) from pyquadcortex.protocol import (DeviceLostError, # noqa: E402 DeviceNotFoundError) @@ -41,6 +42,9 @@ "__version__", "connect", "Device", + "FootswitchLetter", + "SceneLetter", + "PresetAddress", "protocol", "DeviceNotFoundError", "DeviceLostError", diff --git a/pyquadcortex/device/__init__.py b/pyquadcortex/device/__init__.py new file mode 100644 index 0000000..9843273 --- /dev/null +++ b/pyquadcortex/device/__init__.py @@ -0,0 +1,24 @@ +"""The model of the unit: objects that look and behave the way the Quad Cortex does. + +This package is what ``import pyquadcortex`` hands a caller. Nothing here speaks +the wire; it sits on :mod:`pyquadcortex.protocol` and turns the messages into the +unit's own vocabulary - presets, scenes, rows, slots, blocks. + +The directory is named ``device`` rather than ``model`` because in this codebase +the identifier ``model`` already means an amp or a pedal block: the protocol +layer's ``models.py``, ``Model``, ``ModelCatalog`` and ``set_block(model=...)`` +are all that sense of the word. ``docs/domain-model.md`` section 5 renamed that +concept to *virtual device* in the model's own vocabulary, which is what the +screen calls it - but the protocol layer still spells it ``model``, so a +directory named ``model/`` collides with real code a reader is looking at. + +Its public names are re-exported from :mod:`pyquadcortex`, which is where callers +should import them from. The design is in ``docs/domain-model.md``. +""" + +from pyquadcortex.device.device import Device, connect +from pyquadcortex.device.translate import (FootswitchLetter, PresetAddress, + SceneLetter) + +__all__ = ["Device", "connect", "FootswitchLetter", "SceneLetter", + "PresetAddress"] diff --git a/pyquadcortex/model/device.py b/pyquadcortex/device/device.py similarity index 100% rename from pyquadcortex/model/device.py rename to pyquadcortex/device/device.py diff --git a/pyquadcortex/device/translate.py b/pyquadcortex/device/translate.py new file mode 100644 index 0000000..670c313 --- /dev/null +++ b/pyquadcortex/device/translate.py @@ -0,0 +1,583 @@ +"""The one place a screen value becomes a wire value, and back. + +The model speaks what the touchscreen shows: rows 1 to 4, slots 1 to 8, scenes +and footswitches as letters, levels in dB, the tuner in Hz, the tempo in bpm. The +wire speaks zero-based indexes and raw scales. Every conversion between the two +lives here, and nowhere else in :mod:`pyquadcortex.device` - design principle 5 +in ``docs/domain-model.md``. + +**Why one module rather than a convention.** The protocol layer's own header says +it plainly: rows are zero-based, "getting this wrong is quiet rather than loud - +an edit lands on a real row, just not the one intended, and it reads back +perfectly". There is no error, no wrong-looking value, and no complaint from the +unit. A ``- 1`` written in the wrong place is therefore invisible until someone +plays the preset. Collecting the arithmetic in one module makes it reviewable in +one place, and ``tests/test_translation.py`` proves the rest of the model package +contains none of it. + +Nothing here talks to a device. These are pure functions and value types, so they +are cheap to test exhaustively, which is the point. + +Two words carry two meanings in this file, both of them the unit's own: + +* a **slot** is one of the eight cells in a grid row (``row.slots[3]``), and it + is also a preset's place in a setlist ("28C"). The design doc uses both. The + grid sense converts with :func:`slot_to_wire`; the setlist sense with + :func:`slot_to_position` and :class:`PresetAddress`. +* a **position** is the letter part of a preset address ("C") to the model, and + the linear index of that address (218) to the wire. +""" + +import enum +import re +from dataclasses import dataclass + +from pyquadcortex import protocol + +#: Rows on the touchscreen, top to bottom. The wire numbers the same four 0 to 3. +ROWS = (1, 2, 3, 4) + +#: The eight cells in a row, as the manual counts them ("four rows, each +#: containing eight device block slots"). The wire calls a cell a ``column`` and +#: numbers them 0 to 7. +SLOTS = (1, 2, 3, 4, 5, 6, 7, 8) + +#: The tuner's reference pitch when the wire offset is zero. The wire stores an +#: OFFSET from this, not the pitch itself - see :func:`tuner_reference_hz`. +CONCERT_A_HZ = 440.0 + +# What the wire may carry for a row and a slot, derived from the screen values +# above so the two accounts cannot disagree about how many there are. Scene and +# footswitch indexes are NOT validated against these: they have their own enums +# at the protocol layer, and borrowing a range named for grid columns to check a +# footswitch is the confusion this module exists to end. +_WIRE_ROWS = tuple(range(len(ROWS))) +_WIRE_COLUMNS = tuple(range(len(SLOTS))) + +#: Only the three value types are re-exported from :mod:`pyquadcortex`; a caller +#: holds those. The conversions are the seam's own business and are reached as +#: ``translate.row_to_wire(...)`` from inside the model. +__all__ = [ + "ROWS", "SLOTS", "CONCERT_A_HZ", + "FootswitchLetter", "SceneLetter", "PresetAddress", + "row_to_wire", "row_from_wire", "slot_to_wire", "slot_from_wire", + "footswitch_to_wire", "footswitch_from_wire", + "scene_to_wire", "scene_from_wire", + "slot_to_position", "position_to_slot", + "input_level_db", "db_to_input_level", + "lane_level_db", "db_to_lane_level", + "tempo_bpm", "bpm_to_tempo", + "tuner_reference_hz", "hz_to_tuner_reference", + "hold_timing_ms", "ms_to_hold_timing", +] + + +def _a_whole_number(value, what: str) -> int: + """A plain ``int``, and nothing that is merely spelled like one. + + Three things are refused here, and each one is a silent wrong answer rather + than a crash if it gets through: + + * ``bool``, because it subclasses ``int`` and ``True == 1``, so an unguarded + check converts ``True`` to the first row or slot and edits it. + * a ``float``, because ``1.0`` means the caller is computing coordinates in + a type that rounds, and 218.9 becoming preset 218 recalls a real preset. + * any :class:`enum.Enum`, because the protocol layer's coordinate enums are + ``IntEnum``: :class:`~pyquadcortex.protocol.enums.Scene` ``B`` is 1, and + handing it to a row converter otherwise produces row 2 without complaint. + That is the footswitch-versus-column confusion in another costume. The two + wire-index converters that legitimately take one of those enums unwrap it + themselves, so only the RIGHT enum gets through. + """ + if isinstance(value, enum.Enum): + raise TypeError( + f"{what} must be a plain int; {value!r} is a {type(value).__name__}, " + f"which numbers something else" + ) + if isinstance(value, bool) or not isinstance(value, int): + raise TypeError( + f"{what} must be an int, not {type(value).__name__} ({value!r})" + ) + return value + + +def _screen_number(value, what: str, allowed: tuple) -> int: + """One screen coordinate, checked against the values the unit shows.""" + _a_whole_number(value, what) + if value not in allowed: + contiguous = allowed[-1] - allowed[0] + 1 == len(allowed) + span = (f"{allowed[0]} to {allowed[-1]}" if contiguous + else f"one of {list(allowed)}") + raise ValueError(f"{what} must be {span} - the unit has " + f"{len(allowed)} of them; got {value}") + return value + + +def _a_number(value, what: str) -> float: + """A real number, and not a bool wearing one's clothes.""" + if isinstance(value, bool) or not isinstance(value, (int, float)): + raise TypeError( + f"{what} must be a number, not {type(value).__name__} ({value!r})") + return float(value) + + +# -- coordinates: rows and slots -------------------------------------------- + + +def row_to_wire(row: int) -> int: + """The screen's row number (1-4) as the wire's row index (0-3).""" + return _screen_number(row, "a row", ROWS) - 1 + + +def row_from_wire(index: int) -> int: + """The wire's row index (0-3) as the row number the screen shows (1-4).""" + return _screen_number(index, "a wire row index", _WIRE_ROWS) + 1 + + +def slot_to_wire(slot: int) -> int: + """A row's slot number (1-8) as the wire's column index (0-7). + + The manual calls the eight cells in a row slots; the wire calls the same + thing a column. Same cell, two vocabularies, and this is the seam. + """ + return _screen_number(slot, "a slot", SLOTS) - 1 + + +def slot_from_wire(column: int) -> int: + """The wire's column index (0-7) as the slot number the screen shows (1-8).""" + return _screen_number(column, "a wire column index", _WIRE_COLUMNS) + 1 + + +# -- letters: scenes and footswitches --------------------------------------- + + +class FootswitchLetter(enum.StrEnum): + """A footswitch, as the unit labels it: A to H. + + **The model's only public footswitch key.** A footswitch index is not a + column, and the two are equal often enough to look like the same number: + ``stomp_is_momentary`` is keyed by footswitch index, and that stayed hidden + for months because every sample happened to have the two agree - until a + block at column 3 assigned to footswitch E came back keyed 4 + (``docs/domain-model.md`` section 7). Documenting the difference was not + enough, so the model takes a letter and the zero-based index stays inside the + protocol layer, where :class:`~pyquadcortex.protocol.enums.Footswitch` + already lives. + + It is a ``str``, so it prints as the screen shows it and keys an ordinary + mapping:: + + preset.stomps[FootswitchLetter.E] + preset.stomps["E"] # the same key + """ + + A = "A" + B = "B" + C = "C" + D = "D" + E = "E" + F = "F" + G = "G" + H = "H" + + +class SceneLetter(enum.StrEnum): + """A scene, as the unit labels it: A to H. A ``str``, like + :class:`FootswitchLetter`.""" + + A = "A" + B = "B" + C = "C" + D = "D" + E = "E" + F = "F" + G = "G" + H = "H" + + +def _letter(value, kind: type, what: str, trap: str): + """Coerce a caller's letter into `kind`, refusing a number outright. + + A number is refused rather than converted even though the wire is numeric. + ``trap`` names what that number would more likely have been - the mistake the + letter types exist to make impossible. + + The OTHER letter type is refused too. :class:`SceneLetter` and + :class:`FootswitchLetter` are both ``StrEnum`` over A to H, so each is a + plain string as far as any check goes, and scene E reaching a footswitch API + is the same wrong-thing-right-shape mistake as passing the number 4. + """ + if isinstance(value, kind): + return value + if isinstance(value, enum.Enum): + raise TypeError( + f"{what} is a {kind.__name__}; {value!r} is a " + f"{type(value).__name__}, which labels something else" + ) + if isinstance(value, bool) or isinstance(value, (int, float)): + raise TypeError( + f"{what} is a letter A to H, not the number {value!r} - the model " + f"never takes a bare index here, because {trap}" + ) + if not isinstance(value, str): + raise TypeError( + f"{what} is a letter A to H, not {type(value).__name__} ({value!r})") + try: + return kind(value.strip().upper()) + except ValueError: + raise ValueError( + f"{what} is a letter A to H - the unit shows eight; got {value!r}" + ) from None + + +def footswitch_to_wire(footswitch) -> protocol.Footswitch: + """A footswitch letter as the zero-based index the wire carries. + + Takes a :class:`FootswitchLetter` or the plain letter. An ``int`` is refused: + see :class:`FootswitchLetter` for the block-at-column-3 case that makes a + number here a write that silently lands on the wrong switch. + """ + letter = _letter(footswitch, FootswitchLetter, "a footswitch", + "a footswitch index and a block's column are different " + "numbers that are equal often enough to look alike") + return protocol.Footswitch[letter.value] + + +def footswitch_from_wire(index) -> FootswitchLetter: + """The wire's footswitch index (0-7) as the letter the unit labels it with. + + Takes a plain int or a :class:`~pyquadcortex.protocol.enums.Footswitch`. A + :class:`~pyquadcortex.protocol.enums.Scene` is refused even though it is an + ``IntEnum`` over the same eight numbers, because a scene index arriving here + means something upstream mixed up two things the unit keeps apart. + + The letter comes from the protocol enum's own member name rather than a + second copy of the alphabet, so the two layers cannot disagree about which + index is which switch. + """ + if not isinstance(index, protocol.Footswitch): + _a_whole_number(index, "a wire footswitch index") + return FootswitchLetter(protocol.Footswitch(index).name) + + +def scene_to_wire(scene) -> protocol.Scene: + """A scene letter as the zero-based index the wire carries. + + Takes a :class:`SceneLetter` or the plain letter, as ``scenes["B"]`` does. + """ + letter = _letter(scene, SceneLetter, "a scene", + "scene B is wire index 1, and a number here reads as " + "either one") + return protocol.Scene[letter.value] + + +def scene_from_wire(index) -> SceneLetter: + """The wire's scene index (0-7) as the letter the unit labels it with. + + Takes a plain int or a :class:`~pyquadcortex.protocol.enums.Scene`, and + refuses a footswitch index for the reason in :func:`footswitch_from_wire`. + """ + if not isinstance(index, protocol.Scene): + _a_whole_number(index, "a wire scene index") + return SceneLetter(protocol.Scene(index).name) + + +# -- preset addresses: "28C" on screen, a linear position on the wire ------- + +#: What a slot name may look like to the model: ASCII digits, then one letter, +#: with nothing between them. +#: +#: The protocol helper is looser. It checks the bank with ``str.isdigit()``, +#: which is true for every Unicode digit, and ``int()`` reads those too - so +#: ``protocol.slot_to_position("٢٨C")`` returns 218. That is a real position for +#: a name no screen ever shows, which is the shape of mistake this module exists +#: to stop, so the model checks the name before handing it down. Both of the +#: boundary's doors use this one pattern; they disagreed when only +#: :meth:`PresetAddress.parse` had it. +_SLOT_NAME = re.compile(r"\s*([0-9]+)([A-Za-z])\s*") + + +def slot_to_position(name: str) -> int: + """A preset's slot name ("28C") as the linear position the wire carries (218). + + The letters run **A to H, eight to a bank** - the non-hybrid naming, which is + what the unit shows in every mode except one. A PRESET-containing HYBRID mode + halves the bank to four, so the SAME preset is named differently: linear + position 5 reads "1F" normally and "2B" under that hybrid. So a slot name - + and therefore a :class:`PresetAddress` - is only unambiguous alongside the + mode it was read in. The linear position is not: it means one preset whatever + the footswitches are doing, which is why it is what goes on the wire and why + two addresses are best compared as positions. + + How big a setlist is, and the arithmetic, are + :func:`pyquadcortex.protocol.slot_to_position`'s, so the model and the + protocol layer cannot drift apart on what "28C" means. The SHAPE of the name + is checked here first, against :data:`_SLOT_NAME`, because the protocol + helper accepts non-ASCII digits and the model should not. A zero-padded bank + ("01A") is accepted; :func:`position_to_slot` renders unpadded by default, + because that is what the unit displays. + """ + if not isinstance(name, str): + raise TypeError( + f"a slot name is text like '28C', not {type(name).__name__} " + f"({name!r})") + if not _SLOT_NAME.fullmatch(name): + raise ValueError( + f"a slot name is a bank number and a letter A to H, like '28C': " + f"{name!r}") + return protocol.slot_to_position(name) + + +def position_to_slot(position: int, pad: bool = False) -> str: + """The wire's linear position (218) as the slot name the unit shows ("28C"). + + The inverse of :func:`slot_to_position`, and it carries the same caveat: the + name it returns is the non-hybrid one, so it is only unambiguous alongside + the mode the address was read in. + + Unpadded by default ("1A"), which is what the unit displays; ``pad=True`` + gives "01A". + + A whole number only. The protocol helper takes ``int(position)``, so 218.9 + would quietly become 218 and ``True`` would become 1 - and unlike a bad row, + a bad position names a real preset that recalls without complaint. + """ + return protocol.position_to_slot( + _a_whole_number(position, "a wire preset position"), pad=pad) + + +@dataclass(frozen=True) +class PresetAddress: + """Where a preset lives, as the Directory shows it: a bank and a position. + + ``PresetAddress(28, "C")`` renders as ``"28C"`` and + :meth:`parse` reads the same form back. Malformed input is refused here, + when the address is built, rather than later when something writes it: a bad + address that survives parsing turns into a wire position anyway, and the + device recalls whatever preset is at that position without complaint. + + ``position`` is the letter, "A" to "H" - the non-hybrid naming. See + :func:`slot_to_position` for why an address needs the mode beside it to be + unambiguous, and why comparing positions beats comparing names. + """ + + bank: int + position: str + + def __post_init__(self): + _a_whole_number(self.bank, "a bank") + if not isinstance(self.position, str): + raise TypeError( + f"a position is a letter A to H, not " + f"{type(self.position).__name__} ({self.position!r})") + object.__setattr__(self, "position", self.position.strip().upper()) + # Validation is the protocol helper's, so there is one account of how big + # a setlist is and what a slot name may look like. + slot_to_position(f"{self.bank}{self.position}") + + def __str__(self) -> str: + return f"{self.bank}{self.position}" + + def __repr__(self) -> str: + return f"PresetAddress({str(self)!r})" + + @classmethod + def parse(cls, text: str) -> "PresetAddress": + """Read an address a person wrote: ``"28C"``, ``"01a"``, ``" 32H "``. + + Raises ``ValueError`` for anything that is not a bank number followed by + a letter A to H, and ``TypeError`` for anything that is not text. + """ + if not isinstance(text, str): + raise TypeError( + f"a preset address is text like '28C', not " + f"{type(text).__name__} ({text!r})") + # The same pattern :func:`slot_to_position` uses. Python's `\d` spans + # every Unicode digit, and so does `str.isdigit()`, so an unrestricted + # check reads "٢٨C" as bank 28 - see :data:`_SLOT_NAME`. + match = _SLOT_NAME.fullmatch(text) + if not match: + raise ValueError( + f"a preset address is a bank number and a letter A to H, like " + f"'28C': {text!r}") + return cls(int(match.group(1)), match.group(2)) + + @classmethod + def from_wire(cls, position: int) -> "PresetAddress": + """The address at a linear wire position: ``218`` gives ``"28C"``.""" + return cls.parse(position_to_slot(position)) + + def to_wire(self) -> int: + """This address as the linear position the wire carries.""" + return slot_to_position(str(self)) + + +# -- display units ---------------------------------------------------------- +# +# Every mapping below was measured on hardware and is written up at the protocol +# layer. Three of the five - the two level scales and the tempo - have a protocol +# helper that performs the conversion, and this module calls it rather than +# restating the arithmetic: two copies of a measured scale drift, and both copies +# go on returning a plausible number. The other two have no helper to call. The +# tuner has only a documented rule, and hold timing has the protocol layer's +# constant tuple, which is the part worth sharing. Both are pinned in +# tests/test_translation.py against what the protocol WRITE method expects. +# +# What this module adds either way is the type guard, because the protocol +# helpers are arithmetic and will happily multiply a bool. + + +def input_level_db(level: float) -> float: + """An input port's wire level (0..1) as the dB the unit displays. + + Input gain spans -12 to +60 dB. Delegates to + :func:`pyquadcortex.protocol.input_level_db`, which carries the measurement. + + An input port and a lane are both a 0..1 wire value and they are NOT the + same scale - see :func:`lane_level_db`. + + A level outside 0..1 is converted rather than refused, unlike + :func:`hold_timing_ms`, which refuses an index outside its six. The + difference is that an out-of-span level still has a meaning under a linear + scale - it is off the end of the knob - while an index outside its list + names nothing at all. Neither has been seen from a unit. + """ + return protocol.input_level_db(_a_number(level, "an input level")) + + +def db_to_input_level(db: float) -> float: + """Displayed input-gain dB as the wire level an input port takes. + + Refuses anything outside -12..+60 dB rather than clamping, because a clamped + write lands and reads back as a value the caller never asked for. + """ + return protocol.db_to_input_level(_a_number(db, "an input gain in dB")) + + +def lane_level_db(value: float) -> float: + """A lane, mixer or splitter LEVEL wire value (0..1) as displayed dB. + + These span -40 to +12 dB, with 0 dB at :data:`pyquadcortex.protocol.UNITY_LEVEL` + (10/13). Delegates to :func:`pyquadcortex.protocol.lane_level_db`. + + The bottom of the knob is a detent, not a dB value: wire 0.0 reads "Off" on + screen and -39.5 dB (wire 0.01) is the lowest numeric step. This converts the + scale; it does not model the Off position. + """ + return protocol.lane_level_db(_a_number(value, "a lane level")) + + +def db_to_lane_level(db: float) -> float: + """Displayed dB as the wire value a lane, mixer or splitter LEVEL takes. + + Refuses anything outside -40..+12 dB. + + **-40.0 dB is silence, not the bottom of the knob.** It converts to wire + 0.0, which is the Off detent: the lowest NUMERIC step on the unit is -39.5 + dB, and the screen reads "Off" below it. So asking for -40 dB mutes the + lane, and anything between -40.0 and -39.5 is a reading the screen has no + way to show. For silence, write the wire's 0.0 directly and mean it. + """ + return protocol.db_to_lane_level(_a_number(db, "a lane level in dB")) + + +def tempo_bpm(value: float) -> float: + """A ``TEMPO`` wire value (0..1) as the bpm the unit displays. + + Tempo spans 40 to 240 bpm. Delegates to + :func:`pyquadcortex.protocol.tempo_bpm`, which carries the measurement and its + limits: three screen-vs-wire points, with the two endpoints coming from the + fit rather than from a driven extreme. + + A wire value outside 0..1 is REFUSED, where :func:`input_level_db` and + :func:`lane_level_db` convert one. That difference is the protocol helpers' + rather than a rule added at this seam - the tempo one refuses because the bpm + a caller would read back does not exist on the unit - and this wrapper + neither widens it nor narrows it. + """ + return protocol.tempo_bpm(_a_number(value, "a tempo wire value")) + + +def bpm_to_tempo(bpm: float) -> float: + """A displayed tempo in bpm as the wire value ``TEMPO`` takes. + + Refuses anything outside 40..240 bpm rather than clamping, because a clamped + write lands and reads back as a tempo the caller never asked for. + + This pair is a wrapper and not a home. The protocol layer calls + :func:`pyquadcortex.protocol.bpm_to_tempo` itself, inside + :meth:`~pyquadcortex.protocol.QuadCortex.set_tempo_param`, so the helper has + to stay down there: moving it up here would make the protocol layer import + the model, which is the one direction the layering forbids. + """ + return protocol.bpm_to_tempo(_a_number(bpm, "a tempo in bpm")) + + +def tuner_reference_hz(offset: float) -> float: + """The tuner's wire ``frequency`` as the absolute reference pitch on screen. + + The wire stores an OFFSET from 440 Hz, not the pitch: setting FREQ to 442 on + the unit broadcast ``frequency: 1.99999809``. The screen shows 442, so the + model does too. + + **Evidence:** that single observed pair (442 -> 2.0) is the whole of it. It + fixes the zero point and the direction; that the unit is one Hz per unit + rather than something that merely agrees at 2.0 has not been checked against + a second value on screen, and this function says so rather than implying more + (see :meth:`pyquadcortex.protocol.QuadCortex.set_tuner_reference`). No range + is enforced for the same reason: the unit's FREQ limits have not been read, + and a limit invented here would refuse a setting the unit allows. + + Nothing is rounded either, so the wire's 1.99999809 reads back as + 441.99999809 rather than the 442 on the screen. How many digits the unit's + FREQ field shows has not been read off it, and rounding to a precision + nobody has checked would be the same guess in the other direction. + """ + return CONCERT_A_HZ + _a_number(offset, "a tuner reference offset") + + +def hz_to_tuner_reference(hz: float) -> float: + """A reference pitch in Hz as the offset from 440 the wire carries. + + Inverse of :func:`tuner_reference_hz`; see it for the evidence and for why + no range is enforced. + """ + return _a_number(hz, "a tuner reference pitch") - CONCERT_A_HZ + + +def hold_timing_ms(index: int) -> int: + """The wire's ``hold_timing`` index as the milliseconds the screen shows. + + Six settings, 500 to 1000 ms in 100 ms steps. The device accepts and stores + any integer in that field without validating it, so an index outside the six + means something wrote a value no screen can show - reported rather than + rounded to the nearest real setting. + """ + choices = protocol.QuadCortex.HOLD_TIMING_MS + _a_whole_number(index, "a wire hold-timing index") + if not 0 <= index < len(choices): + raise ValueError( + f"hold timing reads {index!r}, which is outside the " + f"{len(choices)} values the unit offers - something wrote an " + f"unvalidated value into it" + ) + return choices[index] + + +def ms_to_hold_timing(milliseconds: int) -> int: + """Milliseconds as the ``hold_timing`` index the wire carries. + + Only the six values the unit offers convert. Anything else is refused rather + than rounded, and that is meant literally: 500.9 ms is not 500 ms, and + ``"500"`` is not a number. The protocol layer's setter takes + ``int(milliseconds)`` and so accepts both, which is the behaviour this + docstring would otherwise be describing wrongly. + """ + choices = protocol.QuadCortex.HOLD_TIMING_MS + _a_whole_number(milliseconds, "hold timing in ms") + if milliseconds not in choices: + raise ValueError( + f"hold timing must be one of {list(choices)} ms, " + f"not {milliseconds!r}" + ) + return choices.index(milliseconds) diff --git a/pyquadcortex/model/__init__.py b/pyquadcortex/model/__init__.py deleted file mode 100644 index bb20e52..0000000 --- a/pyquadcortex/model/__init__.py +++ /dev/null @@ -1,13 +0,0 @@ -"""The model of the unit: objects that look and behave the way the Quad Cortex does. - -This package is what ``import pyquadcortex`` hands a caller. Nothing here speaks -the wire; it sits on :mod:`pyquadcortex.protocol` and turns the messages into the -unit's own vocabulary - presets, scenes, rows, slots, blocks. - -Its public names are re-exported from :mod:`pyquadcortex`, which is where callers -should import them from. The design is in ``docs/domain-model.md``. -""" - -from pyquadcortex.model.device import Device, connect - -__all__ = ["Device", "connect"] diff --git a/scripts/check_artifacts.py b/scripts/check_artifacts.py index 61600ff..41cecd7 100755 --- a/scripts/check_artifacts.py +++ b/scripts/check_artifacts.py @@ -25,7 +25,13 @@ "pyquadcortex/protocol/proto/ProductionAutomation_pb2.py", "pyquadcortex/protocol/cli.py", "pyquadcortex/protocol/client.py", - "pyquadcortex/model/device.py", + "pyquadcortex/device/device.py", + "pyquadcortex/device/translate.py", + # The two files that DECIDE what `import pyquadcortex` hands back. Ship a + # wheel without either and every module above is still present and correct, + # while the package exports nothing. + "pyquadcortex/device/__init__.py", + "pyquadcortex/__init__.py", "pyquadcortex/_version.py", ) diff --git a/tests/test_docs.py b/tests/test_docs.py index f53ebe2..a279d49 100644 --- a/tests/test_docs.py +++ b/tests/test_docs.py @@ -62,8 +62,14 @@ def test_the_api_table_carries_no_pre_flip_import_paths(): it wants a `(`. Before the move that row was the case being checked. A rule that silently passes over exactly the rows this change could leave behind is worth nothing here, so stale spellings are named instead. + + The two exempt prefixes are the two namespaces that exist: `protocol` and + `device`. The exemption said `model` until the model package was renamed, + which had it backwards - waving through the dead path and flagging the live + one. Nothing failed, because api.md documents the protocol layer and carries + no model rows yet; the first one would have tripped it. """ - stale = re.findall(r"`pyquadcortex\.(?!protocol\b|model\b)[a-z_][a-z0-9_.]*\(", + stale = re.findall(r"`pyquadcortex\.(?!protocol\b|device\b)[a-z_][a-z0-9_.]*\(", _api_table_rows()) assert not stale, ( f"docs/api.md still shows {sorted(set(stale))} - the message-level API " diff --git a/tests/test_import_cleanliness.py b/tests/test_import_cleanliness.py index 5c764a2..0cef51a 100644 --- a/tests/test_import_cleanliness.py +++ b/tests/test_import_cleanliness.py @@ -41,13 +41,13 @@ def test_the_walk_found_both_namespaces(): """Guards the parametrisation: an empty walk would pass vacuously. Both sentinels take the trailing dot, so each one needs a real module - INSIDE its namespace. Without it, `pyquadcortex.model` - the package entry + INSIDE its namespace. Without it, `pyquadcortex.device` - the package entry itself - satisfies the check, and a walk that quietly stopped descending would still look complete. The dot is also what keeps a future top-level - `pyquadcortex/models.py` from standing in for the model namespace. + `pyquadcortex/devices.py` from standing in for the model namespace. """ assert any(m.startswith("pyquadcortex.protocol.") for m in MODULES) - assert any(m.startswith("pyquadcortex.model.") for m in MODULES) + assert any(m.startswith("pyquadcortex.device.") for m in MODULES) @pytest.mark.parametrize("module", ["pyquadcortex"] + MODULES) diff --git a/tests/test_namespace.py b/tests/test_namespace.py index 97b4b0d..e6c7323 100644 --- a/tests/test_namespace.py +++ b/tests/test_namespace.py @@ -29,7 +29,7 @@ import pytest import pyquadcortex -from pyquadcortex import protocol +from pyquadcortex import device, protocol PRE_FLIP_INIT = (pathlib.Path(__file__).resolve().parent / "fixtures" / "surface" / "pre_flip_init.py.txt") @@ -130,6 +130,7 @@ def test_the_model_is_what_the_top_level_offers(): assert pyquadcortex.connect is not protocol.connect assert set(pyquadcortex.__all__) == { "__version__", "connect", "Device", "protocol", + "FootswitchLetter", "SceneLetter", "PresetAddress", "DeviceNotFoundError", "DeviceLostError", } for name in pyquadcortex.__all__: @@ -146,17 +147,32 @@ def test_the_protocol_sources_were_actually_found(): assert len(PROTOCOL_SOURCES) > 5 -MODEL_PACKAGE = "pyquadcortex.model" +#: Read from the package rather than typed here. A hardcoded string survives the +#: package being renamed or moved, and the check below then looks for imports of +#: a package that no longer exists - passing for every source file, for ever, +#: with its own guard test still green. +MODEL_PACKAGE = device.__name__ + +#: The model's names as `pyquadcortex` re-exports them, taken from the model +#: package itself so the set follows the code. +#: +#: `from pyquadcortex import PresetAddress` reaches the model without naming the +#: model package at all - the AST sees `pyquadcortex.PresetAddress`, which is not +#: a module and does not start with `pyquadcortex.device`. That spelling became +#: reachable when the boundary re-exported its value types at top level, so the +#: check has to know which top-level names ARE the model. +RE_EXPORTED = {f"pyquadcortex.{name}" for name in device.__all__} def _is_the_model(dotted: str) -> bool: - """True for the model package and anything inside it, and nothing else. + """True for the model package, anything inside it, and its re-exported names. - The dot boundary matters: a top-level `pyquadcortex/models.py` is a - different module, and a prefix test with no boundary would report importing - it as a layering violation. + The dot boundary matters: a hypothetical top-level `pyquadcortex/devices.py` + is a different module, and a prefix test with no boundary would report + importing it as a layering violation. """ - return dotted == MODEL_PACKAGE or dotted.startswith(MODEL_PACKAGE + ".") + return (dotted == MODEL_PACKAGE or dotted.startswith(MODEL_PACKAGE + ".") + or dotted in RE_EXPORTED) def _package_of(source: pathlib.Path) -> str: @@ -181,9 +197,9 @@ def _imported_modules(tree: ast.AST, package: str) -> list[str]: relative imports are resolved against. Covering all the spellings matters because the house style here is the - package-attribute form - `pyquadcortex/model/device.py` opens with - `from pyquadcortex import protocol` - so `from pyquadcortex import model` is - the spelling a future author is most likely to reach for, and it names no + package-attribute form - `pyquadcortex/device/device.py` opens with + `from pyquadcortex import protocol` - so `from pyquadcortex import device` + is the spelling a future author is most likely to reach for, and it names no module at all in the AST. The relative forms need `node.level` resolved for the same reason. """ @@ -222,14 +238,22 @@ def test_the_protocol_layer_never_imports_the_model(source): IMPORT_SPELLINGS = [ - ("absolute from", "from pyquadcortex.model import Device", True), - ("absolute plain", "import pyquadcortex.model", True), - ("package attribute", "from pyquadcortex import model", True), - ("relative from", "from ..model import Device", True), - ("relative attribute", "from .. import model", True), + ("absolute from", "from pyquadcortex.device import Device", True), + ("absolute plain", "import pyquadcortex.device", True), + ("package attribute", "from pyquadcortex import device", True), + ("relative from", "from ..device import Device", True), + ("relative attribute", "from .. import device", True), + ("the module inside it", "from pyquadcortex.device import device", True), + ("a re-exported value type", "from pyquadcortex import PresetAddress", True), + ("a re-exported type, renamed", + "from pyquadcortex import FootswitchLetter as FS", True), + ("two of them at once", + "from pyquadcortex import SceneLetter, PresetAddress", True), ("a sibling module", "from pyquadcortex.protocol import client", False), - ("a top-level models.py", "from pyquadcortex import models", False), - ("the model's own package", "from pyquadcortex.models import X", False), + ("a hypothetical devices.py", "from pyquadcortex import devices", False), + ("a name, not a module", "from pyquadcortex.protocol import open_device", + False), + ("the package's own version", "from pyquadcortex import __version__", False), ] @@ -240,7 +264,32 @@ def test_the_layering_check_reads_every_import_spelling(label, source, """Guards the check above against the imports it cannot see. A layering rule enforced by a check with blind spots is enforced only for - the spellings someone happened to think of. + the spellings someone happened to think of. The re-export cases are the ones + this story added: `pyquadcortex` now hands out three model types at top + level, and `from pyquadcortex import PresetAddress` names no module. """ found = _imported_modules(ast.parse(source), "pyquadcortex.protocol") assert any(_is_the_model(m) for m in found) is is_a_violation + + +IMPORTS_THE_CHECK_CANNOT_SEE = [ + ("a star import", "from pyquadcortex import *"), + ("the package, then an attribute reach", + "import pyquadcortex\nx = pyquadcortex.device.translate.row_to_wire(row)"), +] + + +@pytest.mark.parametrize("label,source", IMPORTS_THE_CHECK_CANNOT_SEE, + ids=[s[0] for s in IMPORTS_THE_CHECK_CANNOT_SEE]) +def test_the_layering_check_says_where_it_stops(label, source): + """Written down rather than left to be discovered. + + This reads only import statements, so a name that arrives through `*`, or a + module reached by attribute after importing the package, is invisible to it. + Neither is house style and neither appears in the tree, but a sample table + where every case passes reads like a completeness proof. If one of these + starts being caught, move it up into `IMPORT_SPELLINGS`. + """ + found = _imported_modules(ast.parse(source), "pyquadcortex.protocol") + assert not any(_is_the_model(m) for m in found), ( + f"the check now sees {label!r} - move it into IMPORT_SPELLINGS") diff --git a/tests/test_translation.py b/tests/test_translation.py new file mode 100644 index 0000000..3c1391d --- /dev/null +++ b/tests/test_translation.py @@ -0,0 +1,1119 @@ +"""The one place screen coordinates and display units become wire values. + +Design principle 5 in ``docs/domain-model.md``: the model speaks touchscreen +coordinates and display units everywhere, and the conversion happens in exactly +one module. These tests are deliberately exhaustive, because the bug they exist +to stop is silent - the protocol layer's own header says an edit to the wrong row +"lands on a real row, just not the one intended, and it reads back perfectly". +Nothing on the unit and nothing in the reply tells you. So a test is the only +thing that can. + +Two of the checks below are structural rather than behavioural: they read the +source of every file in the package that is not the protocol layer, and prove no +other module does the arithmetic or reaches past the boundary for a protocol +helper that does. Not just the model directory - a rule scoped to a directory is +satisfiable by moving the code to a different one. +""" +import ast +import importlib +import pathlib +import pkgutil + +import pytest + +import pyquadcortex +from pyquadcortex import protocol +from pyquadcortex.device import translate + + +# -- rows: 1-4 on screen, 0-3 on the wire ------------------------------------ + + +@pytest.mark.parametrize("row,index", [(1, 0), (2, 1), (3, 2), (4, 3)]) +def test_every_row_converts_both_ways(row, index): + assert translate.row_to_wire(row) == index + assert translate.row_from_wire(index) == row + + +@pytest.mark.parametrize("row", [0, 5, -1, 100]) +def test_a_row_the_screen_does_not_show_is_refused(row): + with pytest.raises(ValueError, match="1 to 4"): + translate.row_to_wire(row) + + +@pytest.mark.parametrize("index", [-1, 4, 99]) +def test_a_wire_row_outside_the_grid_is_refused(index): + with pytest.raises(ValueError, match="0 to 3"): + translate.row_from_wire(index) + + +def test_a_bool_is_not_a_row(): + # True == 1, so an unguarded check converts True to wire row 0 and edits the + # top row. bool is a subclass of int, which is why this needs saying. + with pytest.raises(TypeError): + translate.row_to_wire(True) + + +def test_a_float_is_not_a_row(): + with pytest.raises(TypeError): + translate.row_to_wire(1.0) + + +# -- slots: 1-8 on screen, columns 0-7 on the wire --------------------------- + + +@pytest.mark.parametrize("slot,column", list(zip(range(1, 9), range(0, 8)))) +def test_every_slot_converts_both_ways(slot, column): + assert translate.slot_to_wire(slot) == column + assert translate.slot_from_wire(column) == slot + + +@pytest.mark.parametrize("slot", [0, 9, -1]) +def test_a_slot_the_screen_does_not_show_is_refused(slot): + with pytest.raises(ValueError, match="1 to 8"): + translate.slot_to_wire(slot) + + +@pytest.mark.parametrize("column", [-1, 8]) +def test_a_wire_column_outside_the_row_is_refused(column): + with pytest.raises(ValueError, match="0 to 7"): + translate.slot_from_wire(column) + + +def test_a_bool_is_not_a_slot(): + with pytest.raises(TypeError): + translate.slot_to_wire(True) + + +# -- one index is never another index ---------------------------------------- +# +# The protocol layer's coordinate enums are IntEnums, so each one is an int and +# passes any `isinstance(x, int)` check. Scene B is 1 and row 2 is wire 1, which +# means a Scene handed to a row converter produces a real row rather than a +# complaint. That is the same class of mistake as the footswitch-versus-column +# confusion this module exists to prevent, so it is refused the same way. + + +@pytest.mark.parametrize("converter,wrong", [ + ("row_to_wire", protocol.Scene.B), + ("slot_to_wire", protocol.Footswitch.C), + ("row_from_wire", protocol.Footswitch.A), + ("slot_from_wire", protocol.Scene.H), +]) +def test_a_coordinate_from_somewhere_else_is_refused(converter, wrong): + with pytest.raises(TypeError): + getattr(translate, converter)(wrong) + + +def test_a_scene_index_is_not_a_footswitch_index(): + with pytest.raises(TypeError): + translate.footswitch_from_wire(protocol.Scene.E) + + +def test_a_footswitch_index_is_not_a_scene_index(): + with pytest.raises(TypeError): + translate.scene_from_wire(protocol.Footswitch.E) + + +def test_a_scene_letter_is_not_a_footswitch(): + """Both are letters A to H and both are strings, so nothing but the type + itself keeps `scenes["E"]`'s key out of a footswitch API.""" + with pytest.raises(TypeError): + translate.footswitch_to_wire(translate.SceneLetter.E) + + +def test_a_footswitch_letter_is_not_a_scene(): + with pytest.raises(TypeError): + translate.scene_to_wire(translate.FootswitchLetter.B) + + +# -- footswitches: letters in the model, indexes on the wire ----------------- +# +# The protocol layer's `Footswitch` enum is the reference. It is also the reason +# this type exists: `stomp_is_momentary` is keyed by footswitch index, and a +# block at column 3 assigned to footswitch E comes back keyed 4 (domain-model.md +# section 7). Where a bare int can reach a model API, someone eventually passes +# a column to it. + +LETTERS = ("A", "B", "C", "D", "E", "F", "G", "H") + + +@pytest.mark.parametrize("letter", LETTERS) +def test_every_footswitch_converts_both_ways(letter): + reference = getattr(protocol.Footswitch, letter) + switch = translate.FootswitchLetter(letter) + assert translate.footswitch_to_wire(switch) == reference + assert translate.footswitch_from_wire(reference) is switch + + +def test_a_footswitch_letter_is_a_string(): + """So it prints as the screen shows it and keys an ordinary dict.""" + assert str(translate.FootswitchLetter.E) == "E" + assert translate.FootswitchLetter.E == "E" + assert {translate.FootswitchLetter.E: "vibe"}["E"] == "vibe" + + +def test_a_plain_letter_is_accepted_where_a_footswitch_is_wanted(): + assert translate.footswitch_to_wire("E") == protocol.Footswitch.E + assert translate.footswitch_to_wire("e") == protocol.Footswitch.E + + +def test_a_bare_integer_is_never_a_footswitch(): + """The whole reason FootswitchLetter exists. 4 is E, and it is also column 5.""" + with pytest.raises(TypeError, match="column"): + translate.footswitch_to_wire(4) + + +def test_a_letter_no_footswitch_carries_is_refused(): + with pytest.raises(ValueError, match="A to H"): + translate.footswitch_to_wire("J") + + +@pytest.mark.parametrize("index", [-1, 8]) +def test_a_wire_footswitch_index_off_the_pedalboard_is_refused(index): + with pytest.raises(ValueError): + translate.footswitch_from_wire(index) + + +# -- scenes: letters in the model, indexes on the wire ----------------------- + + +@pytest.mark.parametrize("letter", LETTERS) +def test_every_scene_converts_both_ways(letter): + reference = getattr(protocol.Scene, letter) + scene = translate.SceneLetter(letter) + assert translate.scene_to_wire(scene) == reference + assert translate.scene_from_wire(reference) is scene + + +def test_a_plain_letter_is_accepted_where_a_scene_is_wanted(): + assert translate.scene_to_wire("b") == protocol.Scene.B + + +def test_a_bare_integer_is_never_a_scene(): + with pytest.raises(TypeError): + translate.scene_to_wire(1) + + +def test_a_letter_no_scene_carries_is_refused(): + with pytest.raises(ValueError, match="A to H"): + translate.scene_to_wire("I") + + +# -- preset addresses: "28C" on screen, 218 on the wire ---------------------- + + +def test_an_address_renders_the_way_the_directory_shows_it(): + assert str(translate.PresetAddress(28, "C")) == "28C" + assert str(translate.PresetAddress(1, "A")) == "1A" + + +def test_an_address_parses_the_same_form_it_renders(): + assert translate.PresetAddress.parse("28C") == translate.PresetAddress(28, "C") + + +def test_a_padded_bank_parses_and_renders_unpadded(): + """The unit displays "1A". `slot_to_position` takes "01A" too, so parsing + accepts it and rendering does not produce it.""" + assert translate.PresetAddress.parse("01A") == translate.PresetAddress(1, "A") + assert str(translate.PresetAddress.parse("01A")) == "1A" + + +@pytest.mark.parametrize("written,bank,position", [ + (" 28c ", 28, "C"), + ("32H", 32, "H"), + ("1a", 1, "A"), +]) +def test_parsing_normalises_what_a_person_types(written, bank, position): + address = translate.PresetAddress.parse(written) + assert (address.bank, address.position) == (bank, position) + + +#: Names the unit never shows. One list, because the boundary has TWO public +#: doors onto this conversion and a name that only one of them refuses is a hole +#: with a test in front of it - see the two tests below. +#: +#: "٢٨C" and "28²C" are the ones that were getting through. Python's `\d` spans +#: every Unicode digit and so does `str.isdigit()`, which is what the protocol +#: helper checks the bank with, and `int()` reads those digits too. +MALFORMED_NAMES = ["", " ", "C", "28", "28I", "0A", "33A", "28CC", "-1A", + "2.5C", "A28", "28 C", "2 8C", "٢٨C", "28²C"] + + +@pytest.mark.parametrize("malformed", MALFORMED_NAMES) +def test_a_malformed_address_is_refused_when_it_is_parsed(malformed): + """Not when it is written. A bad address that survives parsing becomes a + wire position, and a wrong position is a preset that recalls fine and is the + wrong preset.""" + with pytest.raises(ValueError): + translate.PresetAddress.parse(malformed) + + +@pytest.mark.parametrize("malformed", MALFORMED_NAMES) +def test_the_other_door_onto_a_slot_name_refuses_them_too(malformed): + """`slot_to_position` is public, converts the same names, and was refusing a + different set. + + Only `PresetAddress.parse` had the ASCII-digit pattern, so + `translate.slot_to_position("٢٨C")` returned 218 - a real preset, from a name + no screen shows, through the front door of the module whose whole job is to + stop exactly that. Delegation could not fix it: the protocol helper accepts + those digits by design, so the shape check has to be at the boundary. + """ + with pytest.raises(ValueError): + translate.slot_to_position(malformed) + + +@pytest.mark.parametrize("wrong_type", [None, 218, ["28C"]]) +def test_an_address_that_is_not_text_is_refused(wrong_type): + with pytest.raises(TypeError): + translate.PresetAddress.parse(wrong_type) + + +@pytest.mark.parametrize("bank,position", [(0, "A"), (33, "A"), (28, "I"), + (28, ""), (28, "CC")]) +def test_an_impossible_address_is_refused_at_construction(bank, position): + with pytest.raises(ValueError): + translate.PresetAddress(bank, position) + + +def test_an_address_is_hashable_and_compares_by_value(): + a = translate.PresetAddress(28, "C") + assert a == translate.PresetAddress.parse("28C") + assert len({a, translate.PresetAddress(28, "C")}) == 1 + + +ALL_POSITIONS = list(range(32 * 8)) + + +@pytest.mark.parametrize("position", ALL_POSITIONS) +def test_every_address_in_a_setlist_round_trips_against_the_protocol_layer(position): + """All 256 of them, against the protocol layer's own pair as the reference.""" + name = protocol.position_to_slot(position) + address = translate.PresetAddress.from_wire(position) + assert str(address) == name + assert address.to_wire() == position + assert translate.slot_to_position(name) == protocol.slot_to_position(name) + assert translate.position_to_slot(position) == name + + +def test_a_padded_render_is_available_for_the_wire_facing_form(): + assert translate.position_to_slot(0, pad=True) == "01A" + assert protocol.position_to_slot(0, pad=True) == "01A" + + +@pytest.mark.parametrize("position", [-1, 256, 1000]) +def test_a_wire_position_outside_a_setlist_is_refused(position): + with pytest.raises(ValueError): + translate.position_to_slot(position) + with pytest.raises(ValueError): + translate.PresetAddress.from_wire(position) + + +@pytest.mark.parametrize("position", [218.9, True, "218", None]) +def test_a_wire_position_that_is_not_a_whole_number_is_refused(position): + """The protocol helper takes `int(position)`, so 218.9 quietly becomes 218 + and True becomes 1. Every other coordinate path here refuses a float and a + bool; this is the path where the wrong answer recalls a real preset.""" + with pytest.raises(TypeError): + translate.position_to_slot(position) + with pytest.raises(TypeError): + translate.PresetAddress.from_wire(position) + + +@pytest.mark.parametrize("name", [218, None, ["28C"]]) +def test_a_slot_name_that_is_not_text_is_refused(name): + with pytest.raises(TypeError): + translate.slot_to_position(name) + + +def test_the_protocol_helper_really_is_looser_about_digits(): + """The reason the shape check is at the boundary and not left to delegation. + + If this ever starts raising, the protocol layer has tightened and the + boundary's own pattern is no longer the thing holding the line - which is + worth knowing, because the comment on `_SLOT_NAME` says it is. + """ + assert protocol.slot_to_position("٢٨C") == 218 + with pytest.raises(ValueError): + translate.slot_to_position("٢٨C") + + +def test_the_address_conversion_says_the_naming_depends_on_the_mode(): + """An address is only unambiguous alongside the mode it was read in: linear + position 5 reads "1F" normally and "2B" under a PRESET-containing HYBRID. + A caller who does not know that will mis-address a preset, so the function + that converts has to say it.""" + doc = translate.slot_to_position.__doc__ + if doc is None: # python -OO strips docstrings + pytest.skip("docstrings are stripped under -OO") + assert "mode" in doc and "unambiguous" in doc + assert "HYBRID" in doc or "hybrid" in doc + + +# -- display units ----------------------------------------------------------- +# +# **What the equality assertions below do and do not prove.** Three of these five +# mappings delegate to a protocol-layer helper, so `translate.input_level_db(v) +# == protocol.input_level_db(v)` cannot fail today - it is one function calling +# the other. It is not a check on the arithmetic, and it is not a substitute for +# one. What it pins is that the boundary goes on DELEGATING: the day someone +# copies the formula in here to add a clamp or a rounding rule, this is what +# fails. +# +# The measured numbers themselves are pinned where the measurement lives, in +# tests/test_client.py: `test_input_level_db_matches_the_four_measured_points`, +# `test_lane_level_db_matches_the_three_measured_points` and +# `test_tempo_bpm_matches_every_measured_point` check the screen readings taken +# against simultaneous wire reads. Nothing here restates them, because a second +# copy of a measured constant drifts and both copies keep returning a plausible +# number. +# +# The other two mappings - the tuner and hold timing - have no protocol helper +# to call, only a documented rule and a shared constant, so they are pinned +# below against the protocol WRITE path through a fake transport. That is a real +# check: it fails if the model's idea of 442 Hz stops matching what the method +# that sends it expects. + + +class Recorder: + """The smallest transport a `QuadCortex` needs to record what it would send.""" + + def __init__(self): + self.sent = [] + + def send(self, message): + self.sent.append(message) + + +WIRE_LEVELS = [0.0, 0.01, 0.16667, 0.4, 0.5, protocol.UNITY_LEVEL, 0.9, 1.0] + + +@pytest.mark.parametrize("level", WIRE_LEVELS) +def test_input_level_matches_the_protocol_layers_own_conversion(level): + assert translate.input_level_db(level) == protocol.input_level_db(level) + + +@pytest.mark.parametrize("db", [-12.0, -6.0, 0.0, 17.2, 24.0, 60.0]) +def test_input_level_round_trips_through_the_wire_scale(db): + assert translate.input_level_db(translate.db_to_input_level(db)) == \ + pytest.approx(db) + assert translate.db_to_input_level(db) == protocol.db_to_input_level(db) + + +@pytest.mark.parametrize("db", [-12.1, 60.1, -100.0]) +def test_an_input_gain_the_unit_has_no_setting_for_is_refused(db): + with pytest.raises(ValueError, match="-12"): + translate.db_to_input_level(db) + + +@pytest.mark.parametrize("value", WIRE_LEVELS) +def test_lane_level_matches_the_protocol_layers_own_conversion(value): + assert translate.lane_level_db(value) == protocol.lane_level_db(value) + + +@pytest.mark.parametrize("db", [-40.0, -39.5, -3.1, 0.0, 6.0, 12.0]) +def test_lane_level_round_trips_through_the_wire_scale(db): + assert translate.lane_level_db(translate.db_to_lane_level(db)) == \ + pytest.approx(db) + assert translate.db_to_lane_level(db) == protocol.db_to_lane_level(db) + + +def test_unity_on_a_lane_level_is_zero_db(): + """10/13, measured on every row carrying one across 17 factory presets. + + The tolerance is absolute because the target is zero, and loose because + `UNITY_LEVEL` is 10/13 rounded to eight decimals - well inside what the + screen can show. + """ + assert translate.lane_level_db(protocol.UNITY_LEVEL) == \ + pytest.approx(0.0, abs=1e-6) + + +@pytest.mark.parametrize("db", [-40.1, 12.1, 60.0]) +def test_a_lane_level_the_unit_has_no_setting_for_is_refused(db): + with pytest.raises(ValueError, match="-40"): + translate.db_to_lane_level(db) + + +@pytest.mark.parametrize("converter", ["input_level_db", "db_to_input_level", + "lane_level_db", "db_to_lane_level", + "tempo_bpm", "bpm_to_tempo"]) +@pytest.mark.parametrize("wrong", [True, "0.5", None]) +def test_a_display_value_that_is_not_a_number_is_refused(converter, wrong): + """`True` is an int, so an unguarded lane level read it as full scale and + returned +12 dB. The tempo has the same shape - `protocol.tempo_bpm(True)` + is 240.0, the top of the span - and `bpm_to_tempo(True)` would be a bpm of + 1, refused for being off the bottom rather than for being a bool. A string + reached the protocol layer and came back as a `TypeError` about multiplying + a sequence.""" + with pytest.raises(TypeError): + getattr(translate, converter)(wrong) + + +def test_the_two_level_scales_are_not_interchangeable(): + """Both are a 0..1 wire value and they mean different dB. Reading a lane + level with the input mapping is a wrong answer that looks plausible.""" + assert translate.input_level_db(0.5) != translate.lane_level_db(0.5) + + +# -- the tempo: bpm on screen, a 0..1 value on the wire ----------------------- +# +# The wrapper is here rather than the helper itself. `set_tempo_param(real=)` +# calls `protocol.bpm_to_tempo` from inside the protocol layer, so moving the +# helper up to the boundary would make the protocol layer import the model - +# which `tests/test_namespace.py` refuses. Delegating gets one copy of the +# measured span either way. + + +TEMPO_WIRE_VALUES = [0.0, 0.095, 0.355, 0.4, 0.5, 1.0] + + +@pytest.mark.parametrize("value", TEMPO_WIRE_VALUES) +def test_the_tempo_matches_the_protocol_layers_own_conversion(value): + assert translate.tempo_bpm(value) == protocol.tempo_bpm(value) + + +@pytest.mark.parametrize("bpm", [40.0, 59.0, 111.0, 120.0, 240.0]) +def test_the_tempo_round_trips_through_the_wire_scale(bpm): + assert translate.tempo_bpm(translate.bpm_to_tempo(bpm)) == pytest.approx(bpm) + assert translate.bpm_to_tempo(bpm) == protocol.bpm_to_tempo(bpm) + + +@pytest.mark.parametrize("bpm", [39.9, 240.1, 0.0, 1000.0]) +def test_a_tempo_the_unit_has_no_setting_for_is_refused(bpm): + with pytest.raises(ValueError, match=r"40\.\.240"): + translate.bpm_to_tempo(bpm) + + +@pytest.mark.parametrize("value", [-0.01, 1.01, 2.0]) +def test_a_tempo_wire_value_off_the_scale_is_refused(value): + """Unlike a level, which converts off the end of its knob. An out-of-span + tempo names a bpm the unit has no setting for, so the protocol helper + refuses it and this one inherits that.""" + with pytest.raises(ValueError): + translate.tempo_bpm(value) + + +def test_the_tempo_is_what_the_protocol_write_expects(): + """The same shape as the tuner and hold-timing checks: the model's idea of + 111 bpm has to be the number `set_tempo_param(real=)` puts on the wire. + + Weaker than those two, and worth saying so. Both sides of this equality run + through `protocol.bpm_to_tempo`, so it does not check the arithmetic. What it + fails on is `set_tempo_param` routing `real=` somewhere else - the catalog, + whose published range for TEMPO is a placeholder and would send a different + number.""" + recorder = Recorder() + qc = protocol.QuadCortex(recorder) + qc.set_tempo_param("TEMPO", real=111.0) + sent = recorder.sent[-1].preset.tempoProgramData[0].params[0] + assert sent.param_values[0].float_value == \ + pytest.approx(translate.bpm_to_tempo(111.0)) + + +# -- the tuner's reference pitch: absolute Hz on screen, an offset on the wire + + +@pytest.mark.parametrize("hz,offset", [(440.0, 0.0), (442.0, 2.0), (445.0, 5.0), + (436.5, -3.5)]) +def test_the_tuner_reference_converts_both_ways(hz, offset): + assert translate.hz_to_tuner_reference(hz) == pytest.approx(offset) + assert translate.tuner_reference_hz(offset) == pytest.approx(hz) + + +def test_the_tuner_reference_is_what_the_protocol_write_expects(): + """442 Hz on screen broadcast `frequency: 1.99999809`, so the wire carries + the offset from 440 and the model has to hand the method that offset. + + `set_tuner_reference` does no arithmetic - it passes its argument through as + `frequency=` - so the protocol METHOD is not an independent reference for + the number, and this does not check the 440. What it catches is the `+ 440` + moving across the seam: if the model started sending absolute Hz, or the + method started adding the offset itself, one of the two would double up and + this fails. + """ + recorder = Recorder() + qc = protocol.QuadCortex(recorder) + qc.set_tuner_reference(translate.hz_to_tuner_reference(442.0)) + assert recorder.sent[-1].frequency == pytest.approx(2.0) + + +def test_the_tuner_reference_refuses_something_that_is_not_a_pitch(): + with pytest.raises(TypeError): + translate.hz_to_tuner_reference("442") + + +def test_the_tuner_reference_is_not_rounded_to_a_precision_nobody_has_read(): + """The wire value the unit sent for a screen reading of 442 was + 1.99999809, so this returns 441.99999809. Rounding it to 442 would mean + knowing how many digits the FREQ field shows, and nobody has read that off + the unit.""" + assert translate.tuner_reference_hz(1.99999809) == pytest.approx(441.99999809) + + +# -- hold timing: milliseconds on screen, one of six indexes on the wire ------ + + +@pytest.mark.parametrize("index", range(6)) +def test_every_hold_timing_index_reads_the_same_way_the_protocol_layer_does(index): + """The shared constant is the reference, so this does NOT check the six + numbers - `ms_to_hold_timing(reference[index]) == index` reduces to + `tuple.index(tuple[i]) == i`, true for any six distinct values. Replace + `HOLD_TIMING_MS` with nonsense and this stays green. + + The numbers are pinned as literals where they were read off the unit, in + `tests/test_client.py::test_set_hold_timing_writes_the_index_not_the_milliseconds` + (500 ms is index 0, 800 is 3, 1000 is 5). What this pins is the pair being + inverses of each other over that constant, whatever it holds. + """ + reference = protocol.QuadCortex.HOLD_TIMING_MS + assert translate.hold_timing_ms(index) == reference[index] + assert translate.ms_to_hold_timing(reference[index]) == index + + +def test_hold_timing_is_what_the_protocol_write_expects(): + recorder = Recorder() + qc = protocol.QuadCortex(recorder) + qc.set_hold_timing(800) + assert recorder.sent[-1].hold_timing == translate.ms_to_hold_timing(800) + + +@pytest.mark.parametrize("ms", [499, 550, 1100, 0]) +def test_a_hold_timing_the_unit_does_not_offer_is_refused(ms): + """The device stores 0 or 5000 as happily as a real index and validates + nothing, so a value that is not one of the six is a setting no screen can + show and no gesture will match.""" + with pytest.raises(ValueError): + translate.ms_to_hold_timing(ms) + + +@pytest.mark.parametrize("index", [-1, 6, 5000]) +def test_a_hold_timing_index_the_unit_cannot_have_written_is_refused(index): + with pytest.raises(ValueError): + translate.hold_timing_ms(index) + + +@pytest.mark.parametrize("ms", [500.9, 800.0, "500", True]) +def test_a_hold_timing_that_is_not_a_whole_number_of_ms_is_refused(ms): + """`int(milliseconds)` rounded 500.9 down to a valid setting and read "500" + as a number, which is not what "refused rather than rounded" means.""" + with pytest.raises(TypeError): + translate.ms_to_hold_timing(ms) + + +@pytest.mark.parametrize("index", ["3", 3.0, True]) +def test_a_hold_timing_index_that_is_not_a_whole_number_is_refused(index): + """A wrong TYPE raises TypeError here, as it does everywhere else in this + module; a wrong VALUE raises ValueError. This one used to raise ValueError + for both.""" + with pytest.raises(TypeError): + translate.hold_timing_ms(index) + + +# -- the boundary is the ONLY place ----------------------------------------- +# +# Everything above proves the conversions are right. These two prove they are +# the only ones, which is the half a reviewer cannot check by reading a diff: +# a stray `- 1` in a future module is one character, looks deliberate, and +# produces an edit that lands on a real row and reads back perfectly. +# +# The scan covers EVERY source file in the package that is not the protocol +# layer, not just the model directory. Scoping it to `device/` would leave the +# rule satisfiable by putting the arithmetic in `pyquadcortex/coords.py`, one +# directory up - which is where somebody would put it after reading a failure +# message that named a directory. + +BOUNDARY = pathlib.Path(translate.__file__).resolve() +PACKAGE_ROOT = pathlib.Path(pyquadcortex.__file__).resolve().parent +PROTOCOL_ROOT = pathlib.Path(protocol.__file__).resolve().parent +MODEL_SOURCES = sorted(p for p in PACKAGE_ROOT.rglob("*.py") + if not p.is_relative_to(PROTOCOL_ROOT)) +OTHER_MODEL_SOURCES = [p for p in MODEL_SOURCES if p != BOUNDARY] + + +#: The boundary's own coordinate tables. It publishes them, so a model module +#: can convert a coordinate with `translate.ROWS.index(row)` and never write a +#: `- 1` at all. That spelling used the boundary's own names to get around the +#: boundary, which is why looking one up is treated as arithmetic. +COORDINATE_TABLES = ("ROWS", "SLOTS") + +#: Letters the unit labels a scene or a footswitch with. A table of these, in +#: any container, is a conversion whether or not any number appears near it. +UNIT_LETTERS = "ABCDEFGH" + + +def _index_arithmetic(tree: ast.AST) -> list[str]: + """Every spelling of index arithmetic in `tree`, as "line N: what". + + Blunt on purpose, and wider than `+ 1`. A rule that only fired on operands + spelled `row` or `slot` would miss `n - 1`, and `n` is what the arithmetic + is called by the time somebody has extracted a helper for it. The calls + listed here are how a person actually writes the conversions this module + owns: `ord`/`chr` or a letter table for a scene or footswitch letter, + `divmod` for a preset address, and `ROWS.index(...)` for a coordinate, + which does the job with no arithmetic in it anywhere. + + **What it cannot see**, so that nobody reads this as a proof. A constant + with a name (`OFFSET = 1` then `row - OFFSET`) needs the value followed + across statements, and a table built at run time (`tuple(range(1, 5))`, + a comprehension over `ascii_uppercase`) needs it evaluated. Both are past + what an AST pass does, and a check that claimed otherwise would be worse + than one that says where it stops. `ARITHMETIC_BLIND_SPOTS` below pins the + known ones so the gap is written down rather than discovered. + """ + def one(node) -> bool: + """A literal one, however spelled: 1, 1.0, or True - which equals 1.""" + return (isinstance(node, ast.Constant) + and isinstance(node.value, (int, float)) + and not isinstance(node.value, str) + and node.value == 1) + + def offset(node) -> bool: + return one(node) or (isinstance(node, ast.UnaryOp) + and isinstance(node.op, ast.USub) + and one(node.operand)) + + def letters(values) -> bool: + """A run of the unit's letters, as a sequence of one-character strings.""" + if len(values) < 3 or not all(isinstance(v, str) and len(v) == 1 + for v in values): + return False + return "".join(values) in UNIT_LETTERS + + def letter_table(node) -> bool: + """A literal table of the letters the unit labels with. + + A string ("ABCDEFGH"), a tuple or list of them, or the keys of a dict - + all four are one lookup away from an index, and only the string spelling + was caught before. A run that does not start at "A" counts: "BCDEFGH" + with an offset is the same conversion with a bug in it. + """ + if isinstance(node, ast.Constant) and isinstance(node.value, str): + return len(node.value) >= 3 and node.value in UNIT_LETTERS + if isinstance(node, (ast.Tuple, ast.List, ast.Set)): + return letters([e.value for e in node.elts + if isinstance(e, ast.Constant)] + if all(isinstance(e, ast.Constant) for e in node.elts) + else []) + if isinstance(node, ast.Dict): + return letters([k.value for k in node.keys + if isinstance(k, ast.Constant)] + if all(isinstance(k, ast.Constant) + for k in node.keys) else []) + return False + + def called(node): + if isinstance(node.func, ast.Name): + return node.func.id + return node.func.attr if isinstance(node.func, ast.Attribute) else None + + def table_lookup(node) -> bool: + """`.index()` on one of the boundary's coordinate tables.""" + return (isinstance(node.func, ast.Attribute) and node.func.attr == "index" + and isinstance(node.func.value, (ast.Name, ast.Attribute)) + and _rightmost_name(node.func.value) in COORDINATE_TABLES) + + found = [] + for node in ast.walk(tree): + where = f"line {getattr(node, 'lineno', 0)}" + if isinstance(node, ast.BinOp) and isinstance(node.op, (ast.Add, ast.Sub)): + if offset(node.left) or offset(node.right): + found.append(f"{where}: adds or subtracts one") + elif _ord_of_a(node.left) or _ord_of_a(node.right): + found.append(f"{where}: 65, which is ord('A') - letter arithmetic") + elif isinstance(node, ast.AugAssign) \ + and isinstance(node.op, (ast.Add, ast.Sub)) and offset(node.value): + found.append(f"{where}: adds or subtracts one") + elif isinstance(node, ast.Call): + name = called(node) + if name in ("ord", "chr"): + found.append(f"{where}: {name}() - letter arithmetic") + elif name == "divmod": + found.append(f"{where}: divmod() - splitting a linear position") + elif name == "enumerate" and len(node.args) > 1: + found.append(f"{where}: enumerate() with a start offset") + elif table_lookup(node): + found.append(f"{where}: .index() on a coordinate table - a " + f"conversion with no arithmetic in it") + elif isinstance(node, ast.Attribute) \ + and node.attr in ("ascii_uppercase", "ascii_letters"): + found.append(f"{where}: string.{node.attr} - a letter table") + elif letter_table(node): + found.append(f"{where}: a letter table") + return sorted(set(found)) + + +def _rightmost_name(node) -> str | None: + """The last name in an attribute chain: `translate.ROWS` gives "ROWS".""" + if isinstance(node, ast.Attribute): + return node.attr + return node.id if isinstance(node, ast.Name) else None + + +def _ord_of_a(node) -> bool: + """The literal 65, which is `ord("A")` written without saying so.""" + return (isinstance(node, ast.Constant) and isinstance(node.value, int) + and not isinstance(node.value, bool) and node.value == 65) + + +#: Protocol-layer names that carry a coordinate or a raw scale. Reaching for one +#: of these outside the boundary is how a second conversion gets written. +#: +#: **The criterion is what a name HANDS OVER, not whether it looks like a +#: conversion.** `protocol.stomp_assignments` reads as a plain query and returns +#: `StompAssignment(row, column, footswitch)` - three raw wire indexes, one of +#: them the footswitch index whose confusion with a column is the reason +#: `FootswitchLetter` exists at all. A model module that calls it and keys a +#: mapping by that footswitch reintroduces the original bug without writing a +#: single `- 1`, so the readers below are on the list next to the converters. +#: +#: Two tests keep this honest, in the two directions it can rot: every name here +#: must resolve in the protocol layer, and everything the boundary itself +#: delegates to must be here. Neither can tell whether a name the boundary does +#: NOT use is missing - that half is judgement, applied when a protocol-layer +#: name starts handing out a coordinate or a raw scale. +PROTOCOL_CONVERSIONS = { + # conversions + "Footswitch", "Scene", "slot_to_position", "position_to_slot", + "input_level_db", "db_to_input_level", "lane_level_db", "db_to_lane_level", + "tempo_bpm", "bpm_to_tempo", "option_at", "option_value", + "UNITY_LEVEL", "HOLD_TIMING_MS", + # readers that hand back raw wire coordinates + "blocks", "Block", "stomp_assignments", "StompAssignment", + "free_rows", "row_status", "RowStatus", "input_chain_rows", + "splits", "Split", +} +#: Deliberately NOT here, having been considered: `beats` (already keyed by the +#: 1-based BEAT the screen shows, so nothing is left to convert), `param_options` +#: (option NAMES, no coordinate and no scale) and `describe_mode` (names a mode +#: for a log line). Listing those would make the check fire on model code that +#: has no conversion to do, which teaches people to work around it. + + +def _protocol_aliases(tree: ast.AST) -> set: + """The local names that MEAN the protocol layer in `tree`. + + Worked out first, because the spellings that reach it are not all + `protocol.`: a module can alias the package, import a submodule of it, or + reach through two attributes at once + (`protocol.QuadCortex.HOLD_TIMING_MS`). Each of those was a hole in the + first version of this check, and each is one a person would write without + any idea they were evading anything. + """ + # `protocol` is seeded because the house style is + # `from pyquadcortex import protocol`, and a module using that name without + # the import in the same snippet is the ordinary case in a sample below. + aliases = {"pyquadcortex", "protocol"} + for node in ast.walk(tree): + if isinstance(node, ast.Import): + for alias in node.names: + if alias.name.startswith("pyquadcortex"): + aliases.add(alias.asname or alias.name.split(".")[0]) + elif isinstance(node, ast.ImportFrom): + module = node.module or "" + if module == "pyquadcortex" or module.startswith("pyquadcortex.protocol"): + for alias in node.names: + if module != "pyquadcortex" or alias.name == "protocol": + aliases.add(alias.asname or alias.name) + return aliases + + +def _protocol_names_reached(tree: ast.AST) -> set: + """Every protocol-layer name `tree` reaches for, conversion or not. + + The END of each attribute chain, not the pieces of it: + `protocol.QuadCortex.HOLD_TIMING_MS` reaches for `HOLD_TIMING_MS`, and + counting `QuadCortex` as well would make the allowlist test below demand a + class be listed as a conversion. + """ + aliases = _protocol_aliases(tree) + stepped_through = {id(node.value) for node in ast.walk(tree) + if isinstance(node, ast.Attribute)} + + def root_of(node): + """The leftmost name in an attribute chain, or None.""" + while isinstance(node, ast.Attribute): + node = node.value + return node.id if isinstance(node, ast.Name) else None + + found = set() + for node in ast.walk(tree): + if isinstance(node, ast.Attribute) and id(node) not in stepped_through \ + and root_of(node.value) in aliases: + found.add(node.attr) + elif isinstance(node, ast.ImportFrom) \ + and (node.module or "").startswith("pyquadcortex.protocol"): + found |= {a.name for a in node.names} + return found + + +def _protocol_conversions_used(tree: ast.AST) -> list[str]: + """Every protocol-layer CONVERSION name `tree` reaches for. + + Only through the protocol layer: `translate.slot_to_position(...)` is the + boundary doing its job and must not be reported, so a bare attribute name is + not enough to accuse on. + """ + aliases = _protocol_aliases(tree) + + def root_of(node): + while isinstance(node, ast.Attribute): + node = node.value + return node.id if isinstance(node, ast.Name) else None + + found = [] + for node in ast.walk(tree): + if isinstance(node, ast.Attribute) and node.attr in PROTOCOL_CONVERSIONS \ + and root_of(node.value) in aliases: + found.append(node.attr) + elif isinstance(node, ast.ImportFrom) \ + and (node.module or "").startswith("pyquadcortex.protocol"): + found += [a.name for a in node.names if a.name in PROTOCOL_CONVERSIONS] + return sorted(set(found)) + + +def test_the_scan_covers_every_module_that_is_not_the_protocol_layer(): + """Guards both checks below. An empty list passes them vacuously, and a + scan that skipped a module enforces nothing in it. + + Checked against the import machinery's own walk, so a module added tomorrow + is covered the day it is created rather than the day somebody remembers to + add it here. + """ + assert BOUNDARY in MODEL_SOURCES + assert len(OTHER_MODEL_SOURCES) >= 2 + walked = { + pathlib.Path(importlib.import_module(info.name).__file__).resolve() + for info in pkgutil.walk_packages(pyquadcortex.__path__, "pyquadcortex.") + if not info.name.startswith("pyquadcortex.protocol") + } + missed = sorted(str(p) for p in walked - set(MODEL_SOURCES)) + assert not missed, f"the scan does not cover {missed}" + + +def test_every_name_on_the_allowlist_is_a_real_protocol_name(): + """A misspelled entry protects nothing and says nothing about it. + + The check below only ever compares an attribute name against this set, so + `tempo_bmp` in it would sit there looking like a rule while `protocol.tempo_bpm` + went unwatched. Two lookups because `HOLD_TIMING_MS` hangs off `QuadCortex` + and the rest are module-level. + """ + missing = object() + unresolved = sorted( + name for name in PROTOCOL_CONVERSIONS + if getattr(protocol, name, missing) is missing + and getattr(protocol.QuadCortex, name, missing) is missing + ) + assert not unresolved, ( + f"{unresolved} is on the allowlist but is not a name the protocol layer " + f"publishes, so nothing is being kept out of the model by it") + + +def test_the_allowlist_covers_everything_the_boundary_delegates_to(): + """The direction the list actually rots in. + + A conversion arrives at the protocol layer, the boundary starts calling it, + and the allowlist is not updated - so every OTHER module may call it too and + nothing says a word. That is not hypothetical: PR #22 added `tempo_bpm` and + `bpm_to_tempo`, and they sat unlisted until this branch put them in by hand. + + Derived rather than listed, so it cannot rot the same way: whatever + `translate.py` reaches into the protocol layer for is a conversion by + definition, because converting is all that module does. + """ + reached = _protocol_names_reached(ast.parse(BOUNDARY.read_text())) + assert reached, "the boundary reaches for nothing - this check is vacuous" + unlisted = sorted(reached - PROTOCOL_CONVERSIONS) + assert not unlisted, ( + f"{BOUNDARY.name} delegates to the protocol layer's {unlisted}, which " + f"is not in PROTOCOL_CONVERSIONS - so every other module in the package " + f"may call it directly and this suite will not notice") + + +#: The four functions the exclusion exists for. Named, because "somewhere in +#: translate.py" is not the thing being protected. +COORDINATE_CONVERTERS = ("row_to_wire", "row_from_wire", + "slot_to_wire", "slot_from_wire") + + +def test_the_boundary_itself_still_does_the_arithmetic(): + """The exclusion below has to be load-bearing. If the boundary stopped + converting, the check would pass because nothing anywhere converts - which + is the one failure a "no arithmetic elsewhere" test cannot see. + + Anchored to the four converters BY NAME rather than to the file. The + file-wide version was satisfied by `_screen_number`'s error-message + formatter (`allowed[-1] - allowed[0] + 1 == len(allowed)`), which converts + nothing - so all four converters could have been rewritten + `ROWS.index(row)`, arithmetic-free, and this backstop would have stayed + green on that one line while the "nowhere else" check stayed silent too. + """ + functions = {node.name: node + for node in ast.walk(ast.parse(BOUNDARY.read_text())) + if isinstance(node, ast.FunctionDef)} + gone = [name for name in COORDINATE_CONVERTERS if name not in functions] + assert not gone, ( + f"{BOUNDARY.name} no longer defines {gone} - this test names the " + f"converters it is protecting, so a rename has to come through here") + silent = [name for name in COORDINATE_CONVERTERS + if not _index_arithmetic(functions[name])] + assert not silent, ( + f"{silent} in {BOUNDARY.name} do no index arithmetic. If the conversion " + f"moved somewhere else, the 'nowhere else' check below is now passing " + f"because nothing anywhere converts") + + +@pytest.mark.parametrize("source", OTHER_MODEL_SOURCES, ids=lambda p: p.name) +def test_no_index_arithmetic_outside_the_boundary(source): + found = _index_arithmetic(ast.parse(source.read_text())) + assert not found, ( + f"{source.name} does index arithmetic - {found}. If that is a screen " + f"value becoming a wire value it belongs in {BOUNDARY.name} with a " + f"test, wherever in the package the file sits - an off-by-one here " + f"edits a real row, just not the one intended, and reads back perfectly" + ) + + +@pytest.mark.parametrize("source", OTHER_MODEL_SOURCES, ids=lambda p: p.name) +def test_only_the_boundary_reaches_for_a_protocol_conversion(source): + found = _protocol_conversions_used(ast.parse(source.read_text())) + assert not found, ( + f"{source.name} uses the protocol layer's {found} directly. Convert " + f"through {BOUNDARY.name} instead, so there is one account of what a " + f"row, a slot, a letter or a dB means" + ) + + +ARITHMETIC_SAMPLES = [ + ("a bare decrement", "wire_row = row - 1", True), + ("a bare increment", "row = wire_row + 1", True), + ("an augmented one", "slot += 1", True), + ("one on the left", "column = 1 - offset", True), + ("hidden in a call", "qc.set_param(row=row - 1, column=slot - 1)", True), + ("a comprehension", "[s - 1 for s in slots]", True), + ("a negated one", "wire_row = row + -1", True), + ("a float one", "wire_row = row - 1.0", True), + ("a bool one", "wire_row = row - True", True), + ("a letter from an index", "letter = chr(ord('A') + index)", True), + ("an index from a letter", "index = ord(letter) - ord('A')", True), + ("a letter table lookup", "letter = 'ABCDEFGH'[index]", True), + ("a letter table search", "index = 'ABCDEFGH'.index(letter)", True), + ("a letter table under any name", "LETTERS = 'ABCDEFGH'", True), + ("a letter table as a tuple", + "LETTERS = ('A', 'B', 'C', 'D', 'E', 'F', 'G', 'H')", True), + ("a letter table as a list", "LETTERS = ['A', 'B', 'C']", True), + ("a letter table as dict keys", "INDEX = {'A': 0, 'B': 1, 'C': 2}", True), + ("a letter table that starts late", "letter = 'BCDEFGH'[index]", True), + ("the standard library's letter table", + "import string\nletter = string.ascii_uppercase[index]", True), + ("ord('A') without ord", "index = letter.encode()[0] - 65", True), + ("splitting a linear position", "bank, letter = divmod(position, 8)", True), + ("a one-based enumerate", "[(n, r) for n, r in enumerate(rows, 1)]", True), + ("the boundary's own table, looked up", + "from pyquadcortex.device import translate\n" + "wire_row = translate.ROWS.index(row)", True), + ("the same, imported bare", "wire_column = SLOTS.index(slot)", True), + ("the boundary doing it", "wire_row = translate.row_to_wire(row)", False), + ("arithmetic that is not off-by-one", "total = a + 2", False), + ("a plain enumerate", "[(i, r) for i, r in enumerate(rows)]", False), + ("a string that is not a letter table", "name = 'ABY Splitter'", False), + ("an ordinary attribute", "name = block.device.name", False), + ("a list of names, not letters", "MODELS = ['Amp', 'Bass', 'Cab']", False), + ("index() on something that is not a coordinate table", + "position = names.index(name)", False), +] + + +@pytest.mark.parametrize("label,source,detected", ARITHMETIC_SAMPLES, + ids=[s[0] for s in ARITHMETIC_SAMPLES]) +def test_the_arithmetic_check_sees_what_it_claims_to(label, source, detected): + """A check with blind spots enforces the rule only for the spellings + somebody happened to think of, while reading as though it enforced all of + them. + + The samples that earn their place are the ones a version of this check + missed: letter arithmetic, a letter table in any of four containers, + `divmod` on a preset position, the three ways of writing one that are not + the token `1`, and `ROWS.index(row)` - which converts a coordinate using + the boundary's own published table and contains no arithmetic at all. + None of those is exotic. They are how the conversions this module owns get + written by somebody writing them somewhere else. + """ + assert bool(_index_arithmetic(ast.parse(source))) is detected + + +#: Spellings this check CANNOT see, asserted as unseen. +#: +#: A table of samples where every "should be caught" case is caught reads like +#: proof of a complete check. It is not one, and the honest way to say so is to +#: pin the gap rather than describe it: each of these needs a value followed +#: across statements or a table evaluated, which is past what an AST pass does. +#: +#: This test fails if one of them starts being caught. That is the point - the +#: entry moves up into `ARITHMETIC_SAMPLES` and the docs saying where the check +#: stops get corrected in the same edit. +ARITHMETIC_BLIND_SPOTS = [ + ("a one with a name", "OFFSET = 1\nwire_row = row - OFFSET"), + ("a coordinate table under another name, built at run time", + "GRID_ROWS = tuple(range(1, 5))\nwire_row = GRID_ROWS.index(row)"), + ("a letter table under another name, sliced out of a longer one", + "TAGS = 'ABCDEFGHIJ'[:8]\nletter = TAGS[index]"), + ("arithmetic behind a helper somebody else wrote", + "from elsewhere import to_wire\nwire_row = to_wire(row)"), +] + + +@pytest.mark.parametrize("label,source", ARITHMETIC_BLIND_SPOTS, + ids=[s[0] for s in ARITHMETIC_BLIND_SPOTS]) +def test_the_arithmetic_check_says_where_it_stops(label, source): + """See `ARITHMETIC_BLIND_SPOTS`. These are the ones known to get through.""" + assert not _index_arithmetic(ast.parse(source)), ( + f"the check now catches {label!r} - move it into ARITHMETIC_SAMPLES and " + f"correct the 'what it cannot see' paragraph in _index_arithmetic") + + +CONVERSION_SAMPLES = [ + ("an attribute", "x = protocol.Footswitch.A", True), + ("through the package", "x = pyquadcortex.protocol.lane_level_db(v)", True), + ("an import", "from pyquadcortex.protocol import slot_to_position", True), + ("a module import", "from pyquadcortex.protocol.client import UNITY_LEVEL", True), + ("two attributes deep", "x = protocol.QuadCortex.HOLD_TIMING_MS", True), + ("an aliased package", + "from pyquadcortex import protocol as p\nx = p.Footswitch.A", True), + ("an imported submodule", + "from pyquadcortex.protocol import client\nx = client.lane_level_db(v)", True), + ("an aliased submodule", + "from pyquadcortex.protocol import client as c\nx = c.UNITY_LEVEL", True), + ("a plain import", + "import pyquadcortex.protocol\nx = pyquadcortex.protocol.Scene.A", True), + ("the boundary's own name", "x = translate.slot_to_position(name)", False), + ("a protocol name that is not a conversion", + "x = protocol.field_present(reply, 'serial')", False), + ("something else entirely, named the same", "x = self.grid.Scene.A", False), +] + + +@pytest.mark.parametrize("label,source,detected", CONVERSION_SAMPLES, + ids=[s[0] for s in CONVERSION_SAMPLES]) +def test_the_reach_past_the_boundary_check_sees_what_it_claims_to(label, source, + detected): + """Same standard as the arithmetic samples: the entries worth having are + the spellings the first version of this check could not see.""" + assert bool(_protocol_conversions_used(ast.parse(source))) is detected + + +# -- what a caller can import ------------------------------------------------ + + +def test_the_letter_types_and_the_address_are_public(): + """A caller holds these: `preset.stomps[FootswitchLetter.E]` and + `device.recall(PresetAddress.parse("28C"))`. The conversion functions are + not published - they are the seam's own business.""" + import pyquadcortex + + assert pyquadcortex.FootswitchLetter is translate.FootswitchLetter + assert pyquadcortex.SceneLetter is translate.SceneLetter + assert pyquadcortex.PresetAddress is translate.PresetAddress + assert not hasattr(pyquadcortex, "row_to_wire")