From 78054d1451646d5eb12face8080fb8adf84bed9b Mon Sep 17 00:00:00 2001 From: Jonathan Stokes Date: Wed, 12 Aug 2026 22:57:31 -0500 Subject: [PATCH 1/3] feat: TEMPO MODE is the device tempo block's parameter 1 The Tempo menu's GLOBAL/PRESET switch is readable and writable: tempo_mode() / set_tempo_mode(), on GlobalTempo.params[1] with 0.0 PRESET and 1.0 GLOBAL. The unit keeps two tempo blocks at once - the preset's in tempoProgramData and the device's in GlobalTempo.params - and MODE picks which one plays, touching neither. Three tests had established that the unit never BROADCASTS the switch, which the docs carried for eight releases as "not on the wire". The silence is real; the inference was not. Every one of those tests listened, and none asked. Found by diffing rather than hunting: every set field of every message the device answers, captured in each switch position, including field numbers the recovered schema does not know. Exactly one field moved, and moved back. Earlier attempts looked in GeneralSettings and in the preset, and MODE was one index away inside a GlobalTempo shape a single earlier READ had written off after landing on its clock shape rather than its params shape. Confirmed on the wire, on the unit's own screen, and by the tempo in effect. A host write left the preset's own parameter 1 untouched, so the scope is known rather than assumed - the failure ADR-0007 names. Second finding from the same session: TEMPO's span is 40..240 bpm, three screen-vs-wire points each exact to the displayed integer (59 at 0.095, 111 at 0.355, 120 at 0.400). set_tempo_param("TEMPO", real=) now takes bpm via tempo_bpm() / bpm_to_tempo(), so protocol.md's list of unrecoverable placeholder spans is down to two of four. ADR-0008: a control gets a differential state capture before it is recorded as having no wire path. ADR-0007's rule is unchanged and now has no instance. tests/hardware/state_snapshot.py is the harness, with tests/test_state_snapshot.py proving offline that it can see an unknown field number, a presence-tracked zero, and a value present in only one of two message shapes. --- .gitignore | 3 + changelog.md | 45 ++++- docs/ADR.md | 17 ++ docs/STEERING.md | 15 ++ docs/api.md | 36 +++- docs/architecture.md | 8 +- docs/capture.md | 46 +++++ docs/domain-model.md | 73 ++++---- docs/manual-coverage.md | 25 +-- docs/protocol.md | 147 ++++++++++++---- pyquadcortex/protocol/__init__.py | 7 +- pyquadcortex/protocol/client.py | 144 ++++++++++++++-- pyquadcortex/protocol/enums.py | 19 +++ tests/hardware/state_snapshot.py | 275 ++++++++++++++++++++++++++++++ tests/hardware/test_tempo_mode.py | 195 +++++++++++++++++++++ tests/test_client.py | 118 ++++++++++++- tests/test_state_snapshot.py | 195 +++++++++++++++++++++ 17 files changed, 1256 insertions(+), 112 deletions(-) create mode 100644 tests/hardware/state_snapshot.py create mode 100644 tests/hardware/test_tempo_mode.py create mode 100644 tests/test_state_snapshot.py diff --git a/.gitignore b/.gitignore index 09084d9..a36d9ab 100644 --- a/.gitignore +++ b/.gitignore @@ -49,3 +49,6 @@ desktop.ini # NOTE: pyquadcortex/protocol/proto/*_pb2.py are generated but INTENTIONALLY committed, # so that installing the package needs no protoc toolchain. Do not ignore them. # See docs/architecture.md. + +# Hardware-suite working artifacts. The findings go in docs/, not here. +tests/hardware/captures/ diff --git a/changelog.md b/changelog.md index 1d894d6..94d05b4 100644 --- a/changelog.md +++ b/changelog.md @@ -99,14 +99,43 @@ types decoded, with a liveness heartbeat proving the link was up. But all three listened, and none of them asked. **A control the device never announces may still answer a READ.** Nothing has tried one. -So MODE is an open protocol investigation rather than a settled dead end. Nothing -in the library changes: there was no MODE surface before and there is none now. -The reason it is worth an entry is that the withdrawn claim is what the coverage -audit and the model design were both written against. The correction is in -`docs/protocol.md`, `docs/manual-coverage.md`, `docs/capture.md` and -`docs/domain-model.md`, and the decision it forced - that the model shows a -control it cannot yet drive and refuses it, rather than omitting it or guessing - -is ADR-0007. +So MODE is an open protocol investigation rather than a settled dead end. **It was +asked, and it answered** - see the next entry. + +### The Tempo menu's MODE switch is readable and writable + +`qc.tempo_mode()` returns a `TempoMode` - `PRESET` or `GLOBAL` - and +`qc.set_tempo_mode(TempoMode.GLOBAL)` moves the switch. It is the DEVICE tempo +block's parameter 1, carried in `GlobalTempo.params`. + +**This is a global setting**, despite riding a tempo message. It affects every +preset and there is nothing to save afterwards, so read it first if you intend to +put it back. It does not move either tempo block: the unit keeps the preset's +settings and the device's at the same time, and MODE only picks which one plays. + +The entry above withdrew the claim that this control was not on the wire. It was +on the wire the whole time, and one READ shows it. The three tests that found +nothing were sound - the unit genuinely never broadcasts the switch - and the +mistake was reading "does not announce" as "cannot be asked". Confirmed on the +wire, on the unit's own screen, and by the tempo actually in effect, which +switched between the two blocks' stored values. + +Watch out for one thing if you read `GlobalTempo` yourself: it alternates two +message shapes, one carrying the running clock and one carrying the 25 +parameters. Wait for a reply that actually has parameters. Taking the first +`GlobalTempo` to arrive is what produced the original dead end. + +### `TEMPO` takes bpm: the span is 40 to 240 + +`set_tempo_param("TEMPO", real=120)` now works, and `tempo_bpm()` / +`bpm_to_tempo()` convert if you want the numbers directly. Previously `real=` +was refused here, because the catalog publishes a placeholder range for this +parameter and converting against it gives a number that means something else. + +The span was measured off the screen instead: 59 bpm at `0.095`, 111 at `0.355`, +120 at `0.400`, each exact to the displayed integer. The endpoints are the fit's +rather than driven, and they land on the 40-240 range the unit's manual +documents. ## 0.40.0 - 2026-08-10 diff --git a/docs/ADR.md b/docs/ADR.md index 79f66d6..0743116 100644 --- a/docs/ADR.md +++ b/docs/ADR.md @@ -90,3 +90,20 @@ Records are append-only once `Decided` and built upon: a shipped decision is nev - Finding the MODE wire path becomes a prerequisite of M3's device-settings Epic, not of M1. **No tempo surface ships at M1**, so nothing in this record is user-visible yet. - Design principle 3 keeps its meaning and gains a boundary: omission is for behaviour we do not understand, refusal is for behaviour we understand and cannot yet drive. A record that says which one applies is now expected of anything the model leaves out. - This does not license modelling controls on a hunch. It applies where the unit's behaviour is confirmed and only the message is missing; a control we have not understood on the hardware is still omitted. + +## ADR-0008: A control with no known wire path gets a bounded search before it is modelled as refused + +- **Status:** Decided (2026-08-12) +- **Decision:** ADR-0007's rule stands and is unchanged: a control we understand but cannot drive is modelled and refuses. What changes is what has to happen first. Before a control is recorded as having no wire path, it gets a **differential state capture**: read everything the device will answer, in each position of the control, and diff the two - including field numbers the recovered schema does not know. "No broadcast was observed" is not a finding about a wire path and does not on its own justify the refusal. TEMPO MODE, the case that raised ADR-0007, is no longer an instance of it: `Tempo.mode` is an ordinary readable, writable property. +- **Context:** ADR-0007 recorded that three tests had watched for a broadcast when the MODE switch moves and seen nothing, that this had been over-read as "not on the wire at all", and that the honest state was an open investigation. It was right about all of that, and the investigation took one session. **MODE is the DEVICE tempo block's parameter 1**, carried in `GlobalTempo.params`: `0.0` PRESET, `1.0` GLOBAL, readable by `GlobalTempo{READ}` and writable by a `GlobalTempo{UPDATE, params{index: 1, param_values}}`. Confirmed three ways - the wire value moved and moved back with nothing else in the device's readable state moving either way; a host write moved the unit's own menu, watched at the unit; and the tempo in effect switched between the two blocks' stored values, 111 bpm from the preset's `0.355` and 120 from the device's `0.400`, both exact on a 40-240 range. `protocol.md`, "MODE is the DEVICE tempo block's parameter 1", is the record. +- **Options:** + - **(a) Require a differential capture before recording "no wire path" - chosen.** Costs one session per control. The cost is bounded and known, because it is now a harness (`tests/hardware/state_snapshot.py`) rather than a bespoke experiment. + - **(b) Leave ADR-0007 as it stands.** Its rule is sound, and the failure was not in the rule. But nothing in it required anyone to ASK before concluding, and the eight releases the wrong claim survived were the cost of that gap. + - **(c) Treat every unfound control as merely undiscovered and model nothing until found.** Collapses back into design principle 3's omission, which ADR-0007 rejected for good reasons that have not changed. +- **Open Questions:** ADR-0007's - how a refusal reads in practice - is now unforced, because the model has no refused control left. It stays open and gets settled by the first genuine instance rather than by TEMPO MODE. +- **Rationale:** The three tests were good instruments honestly reported. What went wrong is that a listener answers "does the device announce this?" and the conclusion drawn was "is this on the wire?" - a different question, never asked. Diffing rather than hunting is what makes the search bounded: the earlier work looked for the field it expected in the message it expected, so a field one index away in a message shape it had already written off was invisible to it. Requiring the capture also removes the incentive to reach for ADR-0007 as an easier answer than another hardware session. +- **Consequences:** + - `Tempo.mode` is readable and writable, and is a **device** setting - it affects every preset and there is nothing to save. It ships with the rest of `Tempo` at M3. `QuadCortex.tempo_mode()` / `set_tempo_mode()` and `TempoMode` exist at the protocol layer now. + - ADR-0007 keeps its status and its rule. It currently has no instance, which is the healthy state for it. + - Epic #8's dependency on the TEMPO MODE wire path is resolved. It never gated M1; it no longer gates M3. + - A negative result about device traffic now states which question the instrument answered. "The unit does not announce X" and "X is not on the wire" are separate claims and the second needs a READ. diff --git a/docs/STEERING.md b/docs/STEERING.md index ca4d799..552318a 100644 --- a/docs/STEERING.md +++ b/docs/STEERING.md @@ -80,6 +80,7 @@ Decisions for this area are recorded in [`ADR.md`](ADR.md): | ADR-0005 | A hardware-in-the-loop integration suite, state-neutral on success | | ADR-0006 | The domain model takes the top-level namespace; the protocol layer moves to `pyquadcortex.protocol` | | ADR-0007 | The model may represent a control whose wire path is still open | +| ADR-0008 | A control with no known wire path gets a bounded search before it is modelled as refused | ## 8. Open Questions @@ -119,6 +120,20 @@ Single-device, single-connection USB HID at interactive rates (129-byte reports) ## Change Log +### 2026-08-12 - TEMPO MODE closes, and ADR-0008 + +**What changed:** +- **The Tempo menu's MODE switch is readable and writable.** It is the DEVICE tempo block's parameter 1, carried in `GlobalTempo.params`: `0.0` PRESET, `1.0` GLOBAL. `QuadCortex.tempo_mode()` / `set_tempo_mode()` and the `TempoMode` enum ship at the protocol layer; `docs/protocol.md` gains "MODE is the DEVICE tempo block's parameter 1" and a coverage-table row +- ADR.md: ADR-0008 - a control with no known wire path gets a differential state capture before it is recorded as having none. ADR-0007's rule is unchanged and now has no instance, which is the healthy state for it +- `docs/domain-model.md`: `Tempo.mode` stops being refused and becomes an ordinary property; §13's *Genuinely open* loses its first entry and the *Closed* table records where the answer lives; both appendix tempo rows updated. `manual-coverage.md` gains a MODE row and its tally moves to 104 / 65 yes +- `docs/capture.md` gains "Diff the whole state, do not hunt for a field" - the method that found it, and the four things in the harness that are load-bearing. Its listener chapter, which used this claim as its exemplar, now carries the ending +- **`TEMPO`'s span is 40..240 bpm**, measured at three screen-vs-wire points during the same session and exact to the displayed integer at each. `real=` on that parameter now takes bpm, via `tempo_bpm()` / `bpm_to_tempo()`; `protocol.md`'s list of unrecoverable placeholder spans is down to two of four +- `tests/hardware/state_snapshot.py` is the harness, reusable for the next control of this kind; `tests/test_state_snapshot.py` proves offline that it can see an unknown field number, a presence-tracked zero, and a value in only one of two message shapes + +**Why:** +- The wire path was a named dependency of Epic #8 and a prerequisite of M3's device-settings work. Three earlier tests had established that the unit never BROADCASTS the switch, which had been over-read as "not on the wire"; a READ found it in one session +- The method is the durable part. Earlier attempts hunted for the field they expected, in the messages they expected; MODE was one index away inside a message shape the investigation had already written off. Diffing the whole answerable state finds a thing without knowing where to look + ### 2026-08-11 - The namespace flip lands, and ADR-0007 **What changed:** diff --git a/docs/api.md b/docs/api.md index 1391e1a..35ebd79 100644 --- a/docs/api.md +++ b/docs/api.md @@ -55,6 +55,7 @@ already read and need no connection; calling them as methods raises | **Footswitches** | `set_stomp_assignment(row, column, footswitch)`, `set_stomp_momentary()`, `set_stomp_label()`, `protocol.stomp_assignments(preset)` | | **Expression pedals** | `set_expression(row, column, param, pedal, minimum, maximum)` | | **Preset MIDI Out** | `set_midi_out(source, [MidiOut.cc(...)])`, `set_preset_load_midi_out([...])`, `protocol.midi_out(preset)` | +| **Tempo MODE** | `tempo_mode()`, `set_tempo_mode(TempoMode.GLOBAL)` - global, and it picks which tempo block plays | | **Per-preset tempo** | `set_tempo_param(name, ...)`, `set_tempo_option(name, n)`, `protocol.tempo_params(preset)`, `set_tempo_led(on)`, `set_metronome_volume(v)` | | **Metronome** | `set_tempo_subdivision()`, `set_metronome_sound()`, `set_metronome_routing()`, `set_time_signature()` - all taking full enums | | **Per-beat accents** | `set_beat(n, MetronomeBeat.ACCENT)`, `set_beats([...])`, `protocol.beats(preset)` | @@ -255,14 +256,43 @@ pv = param_state(preset, row=0, column=3, param_index=0) # .scene_mode, .value ## Per-preset tempo and the metronome -Each preset carries its own tempo block, separate from the global tempo: +Each preset carries its own tempo block, separate from the global tempo. **The unit +holds both at once, and the Tempo menu's MODE switch picks which one plays:** ```python -qc.set_tempo_led(False) # this preset's TEMPO LED off -qc.set_metronome_muted(True) # silence the click - the unit's own MUTE +from pyquadcortex.protocol import TempoMode + +qc.tempo_mode() # TempoMode.PRESET or TempoMode.GLOBAL +qc.set_tempo_mode(TempoMode.GLOBAL) # run every preset on the device's tempo +``` + +`set_tempo_mode` is **global**: it affects every preset and there is nothing to save, +so read it first if you mean to put it back. It moves neither tempo block. + +Which block you HEAR follows MODE - measured, on one unit minutes apart: 111 bpm under +PRESET from the preset's stored `0.355`, 120 under GLOBAL from the device's `0.400`. The +setters below address the preset's block by construction, since they write +`tempoProgramData`, so writing one while MODE is GLOBAL should store a value you will not +hear until you switch back. That last step is inferred from those two facts rather than +measured, so treat it as a caution and not as a verified behaviour. + +The device never broadcasts this switch, so a state tracker cannot learn it from +pushes - `tempo_mode()` has to ask, and it waits for a `GlobalTempo` reply carrying +parameters rather than the running clock, which can take a few seconds. + +The per-preset controls: + +```python +qc.set_tempo_param("TEMPO", real=120) # bpm - the span is 40..240, measured +qc.set_tempo_led(False) # this preset's TEMPO LED off +qc.set_metronome_muted(True) # silence the click - the unit's own MUTE qc.set_tempo_param("TIME SIGNATURE", value=0.1) ``` +`TEMPO` is the one tempo parameter whose `real=` comes from a measurement rather than +the catalog, which publishes a placeholder range for it. `tempo_bpm()` and +`bpm_to_tempo()` convert if you need the numbers directly. + Use `set_metronome_muted` and not the volume to silence a click: `set_metronome_volume(0.0)` is **-60 dB, quiet but still audible**, not silence. diff --git a/docs/architecture.md b/docs/architecture.md index fe1bbc0..5196d38 100644 --- a/docs/architecture.md +++ b/docs/architecture.md @@ -426,9 +426,11 @@ next, roughly in order of how well the ground is prepared: `RecentsFavorites`, `PresetDirty`, `Updater`, `ModelRepo` and others are decoded and pushed to us but have no API. These are the cheapest additions: the type already exists in the registry, so it is one client method plus - tests. (`GlobalTempo` is a special case: it is global rather than per preset and - only ever returned a running clock, so the useful per-preset tempo controls live - in `tempoProgramData` instead - see `set_tempo_param`.) + tests. (`GlobalTempo` is a special case: it is global rather than per preset, + and it alternates a clock shape with a 25-parameter shape, so a reader has to + match on a reply that actually carries parameters. Its parameter 1 is the Tempo + menu's MODE switch - see `tempo_mode`. The per-preset tempo controls live in + `tempoProgramData` instead - see `set_tempo_param`.) - **Types not in the registry at all.** The schema declares 71 message types. Whole feature areas are untouched: `Tuner` / `ShowTuner`, `Looper`, `MIDISettings`, `NeuralCapture` / `NeuralCapture2`, `Screenshot`, diff --git a/docs/capture.md b/docs/capture.md index f00adbe..c0b4235 100644 --- a/docs/capture.md +++ b/docs/capture.md @@ -101,6 +101,13 @@ eight releases - 0.33.0 through 0.40.0 - that measurement was written up as "MOD the wire at all", which is a claim about readability that no amount of listening can support. Say what the instrument measured, not what it implies. +And the ending, which is the point: **MODE was on the wire the whole time.** It is the +device tempo block's parameter 1, and one READ shows it. The three listener runs were +correct and correctly reported; the eight releases were lost to the gap between the +question they answered and the question everyone read them as answering. When a listener +comes back silent, the next move is to ASK - see "Diff the whole state, do not hunt for a +field" below. + **3. A match predicate that tests a field the reply never sets rejects every valid answer.** Reading the unit's Favorites list needs `RecentsFavorites{READ, is_favorites: true}`, and the reply comes back with `is_favorites` ABSENT - the flag selects which list you get, it is @@ -115,6 +122,45 @@ documentation, along with a method that quietly returned the wrong list. Correla `request_id`, which the device does echo, and when a match predicate times out, log what DID arrive before concluding nothing did. +## Diff the whole state, do not hunt for a field + +The listener above answers "what does the device SAY when I do this?". When the answer is +"nothing", the next instrument answers a different question: "what does the device's +ANSWER look like in each position?" Capture everything readable with the control one way, +have the operator move it, capture again, and diff. + +The discipline that makes it work is refusing to look for the field you expect. TEMPO +MODE had been hunted for in `GeneralSettings` and in the preset, and it was one index away +inside a message shape the investigation had already written off. A diff finds it without +knowing where to look; a search only finds it where someone guessed. + +`tests/hardware/state_snapshot.py` is the harness. Four things in it are load-bearing, and +each is there because of a specific way this kind of capture lies: + +- **Record every SET field, flattened to `path -> value`.** `ListFields()` is the + presence-correct reading of this schema - a synthetic-`oneof` field appears only if the + device sent it - so an absent field shows in the diff as a key appearing rather than as + a zero that could mean either thing. +- **Record field numbers the schema does not know.** The schema is recovered from one + Cortex Control build, so a field the firmware sends and that build never had decodes to + nothing at all. `GeneralSettingsMessage` uses numbers 1-39 with no gaps, so anything new + there would have been invisible. Use `google.protobuf.unknown_fields.UnknownFieldSet` - + the upb runtime raises `NotImplementedError` on `msg.UnknownFields()`. +- **Collect values as a SET per path over a window, not one sample.** `GlobalTempo` + alternates two shapes, one push each; sampling one message per type compares a clock + reply against a params reply and reports the difference as real. +- **Label noise, never filter it.** Clocks, meters and request ids move on their own and + are printed under their own heading. A filter is how the question got its previous wrong + answer, and the answer here turned out to sit in a message the noise list would have + been a natural home for. + +Two practical notes. Prove the instrument offline first - `tests/test_state_snapshot.py` +feeds it a message carrying each thing it must not miss and fails if the snapshot comes +back empty, which is the only cheap way to tell "the device said nothing" from "the +capture cannot see it". And expect a large, boring diff: the connect burst's `File` +enumeration arrives in a different order every run, so several thousand lines of it are +noise around the one line that matters. + ## Check a believed polarity against factory content When you think you know which way a boolean or toggle goes, ask what every factory preset diff --git a/docs/domain-model.md b/docs/domain-model.md index 6604074..3850650 100644 --- a/docs/domain-model.md +++ b/docs/domain-model.md @@ -572,9 +572,9 @@ class Tuner: class Tempo: # the Tempo & Metronome menu bpm: float # the tempo IN EFFECT - see the note below mode: TempoMode # GLOBAL or PRESET, as the menu shows it. - # The wire path is still open, so both - # reading and writing it REFUSE rather - # than guess - see ADR-0007 and section 13 + # Readable and writable: the wire path is + # the device tempo block's parameter 1 + # (found 2026-08-12). See section 13 led: bool metronome: Metronome class Metronome: @@ -620,19 +620,25 @@ class System: # the SYSTEM SETTINGS section of chapte master_volume_knob: MasterVolumeKnob # enum: global vs output-specific ``` -> **`Tempo.mode` is modelled and refused, on purpose.** The unit's Tempo menu has a -> GLOBAL / PRESET switch, and in PRESET mode the tempo and all seven metronome settings -> belong to the preset - which the wire confirms, since every preset carries a writable -> `TempoControl` block. What has not been found is the message that reads or moves the -> switch: three tests watched for a broadcast on commit and saw nothing. That is not the -> same as unreadable, so the question is open rather than closed. The model therefore -> shows the switch, because the player can see it, and refuses both reading and writing it -> until the wire path is found. It does not let a tempo write through and work out the -> scope afterwards - that would look like it worked. This is ADR-0007, and it gates M3, -> not M1; no tempo surface ships at M1. +> **`Tempo.mode` is an ordinary readable, writable property.** It was modelled and +> refused for one release under ADR-0007, on the strength of three tests that watched +> for a broadcast when the switch moves and saw nothing. The switch does not broadcast - +> that holds - but it answers a READ, and it takes a write. The wire path is the DEVICE +> tempo block's parameter 1, carried in `GlobalTempo.params`: `0.0` is PRESET, `1.0` is +> GLOBAL. Found 2026-08-12 and confirmed three ways - the wire value moved and moved +> back, the unit's own menu followed a host write, and the tempo in effect switched +> between the two blocks' stored values. `protocol.md`, "MODE is the DEVICE tempo block's +> parameter 1", has the method and the evidence. > -> `bpm` is the tempo in effect, which is the preset's own tempo in PRESET mode and the -> device's in GLOBAL mode. The unit resolves that, not the model. +> **`mode` is a DEVICE setting, not a preset one**, even though it rides a tempo +> message. Writing it affects every preset and there is nothing to save afterwards. It +> belongs to the M3 device-settings surface with the rest of `Tempo`; nothing here ships +> at M1. +> +> The unit keeps BOTH tempo blocks at all times and `mode` selects which one plays - +> writing it moves neither. So `bpm` is the tempo in effect, which is the preset's own +> tempo in PRESET mode and the device's in GLOBAL mode. The unit resolves that, not the +> model. > **Two sections, not one.** Chapter 10 has four named subsections - Account, System, > Device, Support. Brightness, device storage and the master-volume knob function live @@ -1017,23 +1023,6 @@ tried rather than just what is unknown. ### Genuinely open -- **The TEMPO MODE wire path.** The unit's Tempo menu has a GLOBAL / PRESET switch, and - in PRESET mode the tempo and all seven metronome settings belong to the preset. Three - independent tests watched for a broadcast when the switch is changed, and saw nothing. - The strong instrument is **the second**, the re-test: 70 of the device's 72 message - types decoded over a 420-second window with a liveness heartbeat. The third is the - device-wide sweep of 2026-08-06, a much longer run whose Tempo menu section falls - between roughly 765 s and 1395 s; its script toggles MODE and toggles it back, and - records no OK step, so the commit case rests on the first two. `protocol.md`'s - "Per-preset tempo, LED and metronome" has all three set out. The negative is - trustworthy, and it goes exactly this far: **the unit does not ANNOUNCE the switch.** It - does not follow that the switch cannot be read, and this list previously said it did. - Cortex Control offers the same control, so some route exists. What has not been tried is - a targeted READ of the candidate messages - `GlobalTempo` first, since it is the only - type seen carrying global tempo parameters - or decoding the two message types the - re-test did not cover. Until it is found, the model shows `Tempo.mode` and refuses it - rather than guessing which scope a tempo write landed in (ADR-0007). This gates M3's - device-settings work; it does not gate M1, because no tempo surface ships at M1. - **`RecallPreset.reason` UNDO.** The value exists in the schema and has never been observed. It may be unreachable on this firmware: there is **no grid-level undo on the unit** - the only UNDO is Looper X's, which is a Looper action and not a preset recall - @@ -1075,7 +1064,7 @@ tried rather than just what is unknown. | I/O device variant | `Version.is_ess`, subject to the caveat above | | Capture metadata | `ProductData.instrument` and `.device`, subject to the caveat above | | Master volume | writable; the recorded refusal was a stale read | -| Per-preset tempo MODE | **reopened.** Three tests confirm the unit never broadcasts it, which is not the same as never answering. Moved back to *Genuinely open* above; how the model behaves meanwhile is ADR-0007 | +| Per-preset tempo MODE | **closed 2026-08-12.** `GlobalTempo.params[1]`: `0.0` PRESET, `1.0` GLOBAL. Readable and writable - `tempo_mode()` / `set_tempo_mode()`. Reopened one release earlier on the argument that three tests proving the unit never BROADCASTS it had been over-read as "not on the wire"; asking found it. It is a DEVICE setting, so `Tempo.mode` is an ordinary property and ADR-0007's refusal no longer applies to it (ADR-0008) | ### Two method notes this round earned @@ -1096,8 +1085,8 @@ carries one permanently. Every feature the manual describes, mapped to the model or explicitly omitted. **Protocol** is the current reachability from [`manual-coverage.md`](manual-coverage.md) (*yes* / *partly* / *no* / *n/a*); *unaudited* marks features this design pass found -missing from that audit, and *open* marks one this project understands on the unit but -cannot yet drive, because the message that carries it has not been found (ADR-0007). An +missing from that audit. (*open* - understood on the unit but not yet drivable, ADR-0007 - +is defined and currently unused: its only holder, TEMPO MODE, closed on 2026-08-12.) An omission with a protocol path of *no* becomes reachable work only after the protocol layer grows the path - closing wire gaps is separate work. @@ -1122,9 +1111,9 @@ the n/a rows below where they intersect the API at all. | Tuner input source | `device.tuner.source` | yes | `RETURN_1_2` refused by the device itself | | Tuner mute | `device.tuner.muted` | yes | | | Live Tuner (streaming needle) | **omitted** | no | the device refuses `enable_meter` from a host; unsupported by decision | -| Tempo (BPM) | `device.tempo.bpm` | yes | the tempo in effect. Which scope it comes from is the unit's business, and depends on the MODE row below | -| Tempo MODE (Global vs Preset) | `device.tempo.mode` - **modelled, and refused until the wire path is found** | open | the switch is real and what it does is understood; the message that drives it is not. Three tests watched for a broadcast on commit and saw nothing, which rules out a broadcast and not a read. An **open protocol investigation**, not a permanent omission (ADR-0007), and a prerequisite of M3 rather than M1 | -| Tap tempo | **omitted** | no | `GlobalTempo` read returns only a running clock; MIDI CC#44 is the documented route | +| Tempo (BPM) | `device.tempo.bpm` | yes | the tempo in effect. Which scope it comes from is the unit's business, and depends on the MODE row below. Both blocks exist at once: measured 111 bpm from the preset's and 120 from the device's on the same unit, minutes apart | +| Tempo MODE (Global vs Preset) | `device.tempo.mode` | yes | `GlobalTempo.params[1]`, `0.0` PRESET and `1.0` GLOBAL, readable and writable (`tempo_mode()` / `set_tempo_mode()`). A **device** setting despite riding a tempo message: it affects every preset and there is nothing to save. Never broadcast, which is why three earlier tests found nothing and why only a READ finds it. M3 with the rest of `Tempo` | +| Tap tempo | **omitted** | no | a `GlobalTempo` READ carries the 25 tempo parameters, none of which is a tap; MIDI CC#44 is the documented route | | Tempo LED | `device.tempo.led` | yes | | | Metronome volume/playback/pan/T-sig/subdivisions/sound/routing | `device.tempo.metronome.*` | yes | full enums for all four option lists | | Per-scene tempo (Cortex Control's bottom bar claims it) | **omitted** | n/a | the unit has no per-scene tempo; `scene_tempo` is inert on the wire. On-unit presentation wins | @@ -1311,3 +1300,11 @@ the n/a rows below where they intersect the API at all. and refused rather than omitted or guessed (ADR-0007); the appendix row and §13 say the same. Nothing here ships at M1 - tempo is an M3 surface - so this is a design change, not a behaviour change. +- **2026-08-12** - TEMPO MODE is **closed**, one release after being reopened. It is the + DEVICE tempo block's parameter 1, carried in `GlobalTempo.params`: `0.0` PRESET, `1.0` + GLOBAL, readable and writable, confirmed on the wire, on the unit's own screen, and by + the tempo actually in effect. `Tempo.mode` becomes an ordinary property and ADR-0007 + loses its only instance (ADR-0008). The method that found it is the one worth keeping: + 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. diff --git a/docs/manual-coverage.md b/docs/manual-coverage.md index 2225da8..77a4782 100644 --- a/docs/manual-coverage.md +++ b/docs/manual-coverage.md @@ -20,10 +20,10 @@ or a field in `BinaryPreset`. A named candidate is a lead, not a claim that it w ## Summary -Of 103 features audited: **64 yes**, **8 partly**, **20 no**, **11 n/a**. +Of 104 features audited: **65 yes**, **8 partly**, **20 no**, **11 n/a**. -Of the 92 features a host could plausibly drive - everything above except the 11 marked -n/a - **64 are fully covered** and 8 more are partly covered, which here means the state +Of the 93 features a host could plausibly drive - everything above except the 11 marked +n/a - **65 are fully covered** and 8 more are partly covered, which here means the state is readable and at least one field of it is confirmed writable, with the neighbours the same shape but not individually exercised. Only 20 remain untouched. @@ -41,11 +41,13 @@ every option of the metronome's four lists, and the per-beat accent cells. What is left is of two kinds. A few writes are **confirmed no-ops** with no route found: preset tags, and duplicating a setlist as a device operation (the library does it by -recall-and-save instead). One feature has **no wire path found yet** - the Tempo menu's -MODE, which broadcasts nothing even on commit, though nothing has ever asked it directly -(see [`domain-model.md`](domain-model.md#13-still-open)). And two whole features remain -unexplored because they need the physical world: Neural Capture, and loading from the -factory Captures Library. +recall-and-save instead). And two whole features remain unexplored because they need the +physical world: Neural Capture, and loading from the factory Captures Library. + +The Tempo menu's MODE was the last feature with no wire path found. It closed on +2026-08-12: the device never broadcasts it, which three tests established correctly and +which this document had over-read as unreachable, and it answers a READ perfectly well +(`tempo_mode()` / `set_tempo_mode()`). --- @@ -62,10 +64,11 @@ factory Captures Library. | Tuner: open/close | partly | `show_tuner()` is accepted; that it opens on screen has not been eyeballed | | Tuner: reference pitch, input source, mute | yes | `set_tuner_input()`, `set_tuner_reference()` and `set_tuner_mute()` all confirmed. Reference is an OFFSET in Hz from 440. Input accepts both inputs, both returns, INPUT_1_2 and USB 5/6; `RETURN_1_2` is refused by the DEVICE, so combined-returns tuning does not exist | | Tuner: Live Tuner (the needle) | no | UNSUPPORTED by decision. `enable_meter` refuses a host write - it stays false and `meter` stays 0.0 - so the needle never streams. Not worth chasing for an instrument you have to be holding; see `docs/roadmap.md` | -| Tap tempo | no | candidate `GlobalTempo`. A READ of it returned only a running clock, never parameters | -| Tempo value (per preset) | yes | `set_tempo_param("TEMPO", value=...)`. Note the catalog range is a placeholder, so `value=` not `real=` | -| Metronome level, LED, time signature, note length | yes | `set_tempo_param()` by screen name, `set_tempo_option()` by option number, and typed setters with full enums: `set_tempo_subdivision()`, `set_metronome_sound()`, `set_metronome_routing()`, `set_time_signature()`. The menu's MODE has no wire path found yet - it is never broadcast, and nothing has tried reading it directly | +| Tap tempo | no | a `GlobalTempo` READ carries the 25 device tempo parameters, and none of them is a tap. MIDI CC#44 is the documented route | +| Tempo value (per preset) | yes | `set_tempo_param("TEMPO", real=120)` in bpm, or `value=` for the raw 0..1. The catalog range is a placeholder, so the 40..240 bpm span was measured off the screen instead - three points, exact to the displayed integer | +| Metronome level, LED, time signature, note length | yes | `set_tempo_param()` by screen name, `set_tempo_option()` by option number, and typed setters with full enums: `set_tempo_subdivision()`, `set_metronome_sound()`, `set_metronome_routing()`, `set_time_signature()`. The menu's MODE is `tempo_mode()` / `set_tempo_mode()` - see the row below | | Per-beat accents (customizing each beat of the bar) | yes | `set_beat(n, MetronomeBeat.ACCENT)` and `set_beats([...])`; `pyquadcortex.protocol.beats(preset)` reads them back. Tempo parameters 10-22 - the catalog's `STEPSTATE0` to `STEPSTATE12` - are beats 1 to 13, each a four-option list at `option / 3`: normal, off, accented, de-emphasized. Traced by touching cells on the unit; a cell cycles UP by 1/3 and wraps, so four touches return it to where it started. Set the time signature FIRST - changing it rewrites these | +| Tempo MODE (Global vs Preset) | yes | `tempo_mode()` reads it, `set_tempo_mode()` writes it - the device tempo block's parameter 1, `0.0` PRESET and `1.0` GLOBAL. **Global, not per preset**: it affects every preset and there is nothing to save. The unit never broadcasts the switch, so a READ is the only way to see it, and the reader must wait for a `GlobalTempo` reply carrying PARAMETERS - the type alternates that shape with a clock shape | | Per-scene tempo | n/a | `scene_tempo` is ignored and reads back empty, and the unit has no per-scene tempo - its Tempo MODE is global or per preset, nothing finer | | Modes: read or set PRESET/SCENE/STOMP/HYBRID | yes | `mode()` / `set_mode(slot)`, plus `mode_cycle()` to read the cycle - `mode()` accepts partial pushes and can report an empty one. `FootswitchMode` names the three base modes and `describe_mode()` names any value | | Modes: reorder, merge into HYBRID, remove | yes | `set_mode_cycle([...])`. All six HYBRID pairings are mapped and built with `hybrid_mode(top, bottom)`: a hybrid gives footswitches A-D one mode and E-H another, so 3-8 are the six ORDERED pairs (4 and 7 being the same pair swapped). A cycle holds at most one hybrid and a hybrid cannot be the only slot; value 9 is ACCEPTED by the device but leaves the footswitches dead, so it is refused here | diff --git a/docs/protocol.md b/docs/protocol.md index 15952ab..796ea29 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -44,6 +44,7 @@ confirming each finding live against hardware. - [7.4 Scenes](#74-scenes) - [7.5 Grid edits and the edit path](#75-grid-edits-and-the-edit-path) - [Per-preset tempo, LED and metronome](#per-preset-tempo-led-and-metronome) + - [MODE is the DEVICE tempo block's parameter 1](#mode-is-the-device-tempo-blocks-parameter-1) - [Grid block move](#grid-block-move) - [7.6 Per-preset MIDI Out](#76-per-preset-midi-out) - [7.6b Moving blocks, and creating a branch](#76b-moving-blocks-and-creating-a-branch) @@ -975,7 +976,7 @@ named order: | index | control on screen | catalog name | |---|---|---| | 0 | TEMPO | TEMPO | -| 1 | - | TYPE. NOT written by any control in the menu | +| 1 | - (in the preset) | TYPE. NOT written by any control in the menu, in the PRESET copy - it stayed 0.0 through every measured flip. In the DEVICE copy carried by `GlobalTempo.params` the same index is **MODE**: `0.0` PRESET, `1.0` GLOBAL. See below | | 2 | Tempo LED | LED LIGHT | | 3 | Volume | VOLUME | | 4 | **The unit's MUTE** (`1.0` = AUDIBLE, `0.0` = muted) | START in the catalog, PLAYBACK in the manual, MUTE on the unit's own Tempo page - one control, three names. TRACED: pressing the unit's MUTE button writes `0.0`, pressing it again writes `1.0`. Note it is INVERTED against the label a player sees. This table said the opposite for two releases, having inferred polarity from the Looper X mirror's NAME (METRONOME MUTE) - and that mirror is inverted too | @@ -1071,8 +1072,11 @@ so position is the index - the same convention as `models[]`. A host WRITE does `index`; it is only the device's stored form that omits it. `pyquadcortex.protocol.tempo_params()` reads them positionally. -**The menu's MODE control (global or per-preset tempo) is NEVER BROADCAST**, established -three times, the last two with instruments worth trusting. +**The menu's MODE control (global or per-preset tempo) is NEVER BROADCAST, and is +READABLE AND WRITABLE ANYWAY.** The silence is real, established three times, the last +two with instruments worth trusting - and it says nothing about whether the switch +answers a question, which it does. Keep this pair together: it is the clearest example +this project has of a trustworthy negative being over-read. 1. **The first attempt.** Toggling to GLOBAL, pressing OK, toggling back to PRESET, pressing OK again and then saving the preset produced no `Grid`, `GlobalTempo` or @@ -1090,31 +1094,88 @@ three times, the last two with instruments worth trusting. `BANK UP` that ends the section, so it repeats the toggle rather than the commit. The commit case rests on the two runs above. -**Read that for exactly what it says.** For eight releases - 0.33.0 through 0.40.0 - this -section said MODE "is NOT on the wire", and the coverage audit and the model design said -"not on the wire at all". Both are more than the measurement supports: every one of those -tests LISTENED, and none of them ASKED. A control the device never announces may still -answer a READ, or ride in one of the two message types the re-test did not decode. So the -state of knowledge is "we have not found the wire path", which is an open investigation, -not a closed door. It is still a good illustration of why a negative result needs a -trustworthy instrument - and now also of the second half of that rule, which is to state -only what the instrument measured. - -Untried, for whoever picks this up: a targeted READ of the candidate message types, and -decoding the two the re-test missed. Cortex Control offers the same switch, so a route -exists. Until it is found, the model shows the control and refuses it rather than guessing -which scope a tempo write landed in (ADR-0007); `domain-model.md` §13 carries the same -entry. - -**Ask `GlobalTempo` first.** A single READ of it returned only a running clock -(`current_beat`, `current_bar`, `current_tick`) with no parameters, and that was written up -here as a dead end. It should not be read as one. Section 8's broadcast notes record that -`GlobalTempo` **alternates two shapes** - one push carrying `metronome_status`, one -carrying the 25 params - and a captured Cortex Control session decodes a `GlobalTempo` -UPDATE carrying a `params` list in the same `index` / `param_values` shape as -`tempoProgramData`. One READ reply that happened to come back in the clock shape says -nothing about the other. It is the only message type this project has seen carrying global -tempo parameters, which makes it the first place to ask rather than a closed one. +All three are sound, and all three answer a narrower question than the one that was +asked of them. For eight releases - 0.33.0 through 0.40.0 - this section said MODE "is +NOT on the wire", and the coverage audit and the model design said "not on the wire at +all". Every one of those tests LISTENED, and none of them ASKED. **The switch was on the +wire the whole time.** + +### MODE is the DEVICE tempo block's parameter 1 + +Found 2026-08-12. `GlobalTempo.params[1]` is the Tempo menu's MODE switch: + +| wire | the menu shows | +|---|---| +| `0.0` | PRESET | +| `1.0` | GLOBAL | + +Readable by `GlobalTempo{READ}`, and **writable** by a `GlobalTempo{UPDATE, +params{index: 1, param_values}}` - the same `index` / `param_values` shape as every other +tempo write. `QuadCortex.tempo_mode()` and `set_tempo_mode()` are the operations; +`TempoMode` is the enum. + +**The unit keeps two tempo blocks and MODE picks which one plays.** The preset's is +`BinaryPreset.tempoProgramData` with 24 parameters; the device's rides in +`GlobalTempo.params` with 25. Neither is touched by the switch. On the unit measured they +disagreed at indices 2 (`LED LIGHT`), 3 (`VOLUME`) and 9 (`ROUTING`) as well as the +tempo itself, so which block is in effect is plainly audible. + +How it was established, because a negative that stood for eight releases deserves a +method rather than an assertion: **every field of every message the device answers was +captured in each switch position and diffed**, rather than any field being looked for. +Twelve state types were READ - `GlobalTempo`, `GeneralSettings`, `Mode`, `IOSettings`, +`MasterVolume`, `GlobalEQ`, `ShowGigView`, `Tuner`, `Looper`, `SetlistPosition`, `Scene`, +`PresetDirty` - the RX path was tapped for a 14-second window so everything else the +device pushed was captured too, and both the named fields and any field number the +recovered schema does not know were recorded. Across a PRESET -> GLOBAL -> PRESET cycle +exactly one field moved, and it moved back; the return capture differed from the baseline +in nothing at all. + +**What did NOT differ, since a negative is a result too.** `GeneralSettings` was +identical in both positions, and no message anywhere carried a field number the schema +does not know - so the mode is not hiding in the settings bag, which was the leading +hypothesis given that message's 39 gapless field numbers. `BinaryPreset.tempo` (field 10) +was ABSENT in both positions, and the preset's whole `tempoProgramData` block was +identical across the flip, so presence in the preset is not the discriminator either. Of +the device tempo block's 25 parameters, index 24 - which exists there, does not exist in +the preset's 24, and is described nowhere - held `0.0` in both positions and is still +unattributed. + +The harness is `tests/hardware/state_snapshot.py`, and +`tests/test_state_snapshot.py` proves offline that it can SEE an unknown field number, a +presence-tracked field set to zero, and a value appearing in only one of two message +shapes - the three ways this particular question could have come back falsely negative +again. + +Three independent confirmations, since one flip in one direction is what produced this +project's last false result: + +1. **The wire.** `0.0` -> `1.0` -> `0.0`, nothing else moving either way. +2. **The unit's own screen.** A host write moved the menu's switch, and the restore + moved it back - watched by the owner at the unit. +3. **The tempo in effect.** The unit displayed 111 bpm in PRESET and 120 in GLOBAL, with + the preset block holding `0.355` and the device block `0.400`. Both are exact on a + 40-240 bpm range, from a direction nobody was looking. + +A host write to parameter 1 left the preset's own copy of parameter 1 at `0.0`, measured +before and after, so the scope of the write is known rather than assumed. That matters +more than it looks: ADR-0007 rejected letting a tempo write land in whichever scope the +unit happened to be in, precisely because the device accepts a write it does not +understand and says nothing, which makes a guess and a success indistinguishable. + +**It is the catalog's `TYPE`,** which the table above records as written by no control in +the menu. That is true of the PRESET copy, which sat at `0.0` throughout. The DEVICE copy +is the switch. + +**Asking `GlobalTempo` was the lead that paid off, and it had been written off.** A single +READ of it once returned only a running clock (`current_beat`, `current_bar`, +`current_tick`) with no parameters, and that was recorded here as a dead end. It was not +one: `GlobalTempo` **alternates two shapes** - one push carrying `metronome_status`, one +carrying the 25 params - and that READ happened to land on the clock. A reader of this +message must match on CONTENT, waiting for a reply that actually carries parameters; +one that takes the first `GlobalTempo` to arrive will usually get the clock and read +nothing, which is exactly how the dead end was manufactured. The params shape was +measured arriving about once every seven seconds, so the wait needs to be generous. One genuine related dead end, for the record: `MetronomeStatusUpdate` carries only `is_enabled` and `preroll_enabled`, with no mute or level field at all - @@ -1707,7 +1768,11 @@ its cache; merge only what is present. (This is the documented reason `settings( **`GlobalTempo` arrives in pairs because it alternates two shapes** - one push carrying `metronome_status`, one carrying the 25 params. Anyone counting messages or diffing -consecutive pushes should expect the alternation. +consecutive pushes should expect the alternation. **A reader of this type must match on +CONTENT**, waiting for the shape it needs: taking the first `GlobalTempo` to arrive is +how a READ for the tempo parameters once came back holding only the clock and was +recorded as a dead end for eight releases. In a 14-second window the params shape +arrived twice, so allow roughly seven seconds per attempt. ## Two parameters called MUTE, with opposite polarities @@ -2312,7 +2377,8 @@ the partial-push warning at the top of this document. WITHOUT pressing OK, and the merge broadcast nothing - which had been recorded here as the pairing not being on the wire at all. It is; the state simply is not published until commit. Note this does NOT generalise: the Tempo menu's MODE control stayed silent through -an OK press and a save. +an OK press and a save, and yet answers a READ - "publishes on commit" and "readable" are +independent properties, and neither predicts the other. What the composite value encodes is unknown. 7 was Preset+Stomp, by elimination since Scene was the slot left standing. Other pairings have not been observed, so read the value @@ -2494,6 +2560,7 @@ visually on the device's own screen. | `set_splitter_param` | `Grid{UPDATE, preset{chains{row, combined_splitter{params{index, param_values}}}}}` | read-back | writes `combined_splitter`, NOT `splitter[]`, which is a read-only view; indices follow the unified model 10004 | | `splits` | reads `Chain.split_control_points` | read-back | branch and rejoin columns. `split == -1` means serial; `mix == -1` with `split >= 0` is a branch that never rejoins (`Split.rejoins`). Only rows 0 and 2 can carry one | | `set_tempo_param` | `Grid{UPDATE, preset{tempoProgramData{params{index, param_values}}}}` | read-back | per-preset tempo, LED and metronome level; NOT row-keyed yet applied | +| `tempo_mode` / `set_tempo_mode` | `GlobalTempo{READ}`, and `GlobalTempo{UPDATE, params{index: 1, param_values}}` | read-back + on-unit | the Tempo menu's MODE switch: `0.0` PRESET, `1.0` GLOBAL. GLOBAL, not per preset - nothing to save, and every preset is affected. The device never broadcasts this, so a READ is the only way to see it; the reader must match on a reply carrying PARAMETERS, since `GlobalTempo` alternates a clock shape with a params shape | | `set_lane_output` | `Grid{UPDATE, preset{chains{row, output_control{hash: 23000, params{index, param_values}}}}}` | read-back | VOLUME/PAN/MUTE/SOLO per row; PAN 0.5 -> 0.0 survived save and read-back | | `move_block` | `GridMove{move{from_row, from_col, to_row, to_col, is_drop}}` | read-back | drivable host-to-device; a cross-row move makes the device create a branch | | `set_split` / `clear_split` | `Grid{UPDATE, preset{chains{row, split_control_points{split, mix}}}}` | read-back | activates or clears a row's branch; the splitter itself always exists | @@ -2637,6 +2704,18 @@ Converting against such a range yields a number that means something else, so (`Parameter.range_is_placeholder` is the test) and `real=` is refused. Pass `value=` with the normalized 0..1 instead. +**Two of these spans have since been measured, so the placeholder no longer blocks +them:** the level parameters below, and `TEMPO`, which is **40 to 240 bpm** - +`bpm = 40 + 200 * value`. Three screen readings against simultaneous wire reads, each +landing on the displayed integer exactly: 59 bpm at `0.095`, 111 at `0.355`, 120 at +`0.400`. `tempo_bpm()` / `bpm_to_tempo()` convert, and `set_tempo_param("TEMPO", +real=...)` takes bpm - the one index where `real=` comes from a measurement rather +than from the catalog. The 59 is what makes the fit worth trusting: 111 and 120 are 9 +bpm apart, and the lane-level story below is what happens when a span is fitted from +points too close together. The endpoints are the fit's rather than separate +measurements - neither extreme was driven - and they land on the 40-240 range the +unit's manual documents, so the two agree. + **Unity for the level parameters is `0.76923077`** - 10/13, i.e. 0 dB on a -40..+12 dB span. Measured: `MIXER LEVEL` and `LEVEL TO A`/`LEVEL TO B` read exactly that on every one of the 34 rows carrying them across 17 factory presets, and lane `VOLUME` on @@ -2799,9 +2878,11 @@ Stated explicitly so nobody builds on a guess: - **DSP cost per model** is not published anywhere reachable, and `CPULoad` never arrives (see [above](#a-placement-can-be-refused-for-want-of-dsp-capacity)), so whether a block will fit can only be discovered by placing it. -- **The true spans behind the placeholder 0..1 ranges** (mixer/splitter/lane levels, - `TEMPO`) are not recoverable from the catalog. Unity for the levels is measured; - the endpoints are not. +- **The true spans behind the placeholder 0..1 ranges** are not recoverable from the + catalog, and two of the four have been measured off the screen instead: the + mixer/splitter/lane levels at -40..+12 dB, and `TEMPO` at 40..240 bpm (2026-08-12, + three points). In both cases the endpoints are the fit's rather than driven, so a + caller who needs an extreme exactly should drive it. - **Whether a capture id denotes different content on a different unit** is untested here, needing a second unit. - ~~**Whether a preset's descriptive `tags` can be set at all**~~ - ANSWERED: no. The diff --git a/pyquadcortex/protocol/__init__.py b/pyquadcortex/protocol/__init__.py index 37b804d..e22df66 100644 --- a/pyquadcortex/protocol/__init__.py +++ b/pyquadcortex/protocol/__init__.py @@ -20,6 +20,7 @@ from pyquadcortex.protocol.client import (SCENE_UNLABELLED, UNITY_LEVEL, db_to_input_level, db_to_lane_level, input_level_db, lane_level_db, + tempo_bpm, bpm_to_tempo, USER_SETLIST_ROOT, Block, BlockRefused, Folder, MidiOut, QuadCortex, Split, StompAssignment, blocks, field_present, @@ -39,7 +40,8 @@ MetronomeBeat, MetronomeRouting, MetronomeSound, MidiSource, Output, Scene, - SceneBypassBehavior, Setlist, TempoSubdivision, + SceneBypassBehavior, Setlist, TempoMode, + TempoSubdivision, TimeSignature) from pyquadcortex.protocol.session import DeviceNotFoundError, connect, open_device from pyquadcortex.protocol import models # generated factory-block constants @@ -78,6 +80,7 @@ "ExpressionBypassMode", "LooperState", "GlobalEQFilter", + "TempoMode", "TempoSubdivision", "MetronomeSound", "MetronomeBeat", @@ -102,6 +105,8 @@ "db_to_input_level", "lane_level_db", "db_to_lane_level", + "tempo_bpm", + "bpm_to_tempo", "SCENE_UNLABELLED", "field_present", "Input", diff --git a/pyquadcortex/protocol/client.py b/pyquadcortex/protocol/client.py index d5192dd..7fc0fa3 100644 --- a/pyquadcortex/protocol/client.py +++ b/pyquadcortex/protocol/client.py @@ -38,7 +38,7 @@ MetronomeBeat, MetronomeRouting, MetronomeSound, MidiOutType, MidiSource, Output, SceneBypassBehavior, Setlist, - TempoSubdivision, TimeSignature) + TempoMode, TempoSubdivision, TimeSignature) from pyquadcortex.protocol.proto import ProductionAutomation_pb2 as pa from pyquadcortex.protocol.proto import Preset_pb2 as preset @@ -122,6 +122,40 @@ def db_to_lane_level(db: float) -> float: ) return (db + 40.0) / 52.0 +def tempo_bpm(value: float) -> float: + """Convert a ``TEMPO`` wire value (0..1) to the bpm the unit displays. + + Tempo spans **40 to 240 bpm**, so ``bpm = 40 + 200 * value``. Solved from three + screen readings taken against simultaneous wire reads, each landing on the + displayed integer exactly: 59 bpm at 0.095, 111 bpm at 0.355, 120 bpm at 0.400. + The 59 is what makes the fit worth trusting - a span needs a point away from the + others, which is the lesson the lane levels taught (``protocol.md``, "Some + catalog ranges are placeholders"). + + The ENDPOINTS are the fit's, not separate measurements: neither extreme was + driven. They land on 40 and 240, which is the tempo range the unit's manual + documents, so the two agree - but if you need the extremes exactly, drive them. + + The catalog publishes ``TEMPO`` as 0..1 with a real-world unit - a placeholder, + which is why this helper exists. + """ + return 40.0 + 200.0 * value + + +def bpm_to_tempo(bpm: float) -> float: + """Convert a bpm to the wire value ``TEMPO`` takes. + + Inverse of :func:`tempo_bpm`; see it for how the scale was measured. A bpm + outside 40..240 does not exist on the unit and is refused rather than silently + clamped. + """ + if not 40.0 <= bpm <= 240.0: + raise ValueError( + f"the unit's tempo runs 40..240 bpm; {bpm} bpm does not exist" + ) + return (bpm - 40.0) / 200.0 + + #: Where user setlists live. They sit SIDE BY SIDE here rather than nested inside #: "My Presets" - a folder created under My Presets is not a setlist and the device #: ignores it. :meth:`QuadCortex.create_setlist` builds a key from this. @@ -1038,8 +1072,13 @@ def set_tempo_param(self, param, value: float = None, real=None): ``BinaryPreset.tempoProgramData`` - a REPEATED field with one entry, so read it as ``preset.tempoProgramData[0]`` - with 24 parameters, among them ``TEMPO``, ``LED LIGHT``, ``VOLUME``, ``TYPE``, ``TIME SIGNATURE`` and ``SOUND``. - These are per PRESET, unlike ``GlobalTempo``, which is global and only ever - reported a running clock. + These are per PRESET. ``GlobalTempo`` carries the DEVICE's copy of the same + block - the unit keeps both at once and the Tempo menu's MODE switch decides + which one plays (measured: 111 bpm under PRESET, 120 under GLOBAL, on the same + unit minutes apart). This method addresses the preset's block by construction, + so writing one while MODE is GLOBAL should store a value you will not hear + until you switch back. That last step is INFERRED from those two facts rather + than measured. See :meth:`tempo_mode`. Confirmed on hardware: although ``tempoProgramData`` is NOT row or column keyed - it sits outside ``chains[]`` - a ``Grid`` UPDATE carrying it is @@ -1063,9 +1102,15 @@ def set_tempo_param(self, param, value: float = None, real=None): list-valued ones prefer :meth:`set_tempo_option`, which range-checks an option number instead of taking a raw float. - The menu's MODE control - global or per-preset tempo - broadcasts NOTHING when - changed, so it is not reachable here. Index 1, the catalog's TYPE, was not - touched by any control in the menu. + The menu's MODE control - global or per-preset tempo - is NOT here: it is the + DEVICE block's index 1, so :meth:`set_tempo_mode` drives it. The PRESET copy of + index 1, the catalog's TYPE, is touched by no control in the menu and held 0.0 + through every flip and edit measured. + + ``real=`` on index 0 means BPM, over the measured 40..240 span + (:func:`tempo_bpm`). The catalog cannot convert it - the range it publishes for + ``TEMPO`` is a placeholder - so this is the one index where ``real=`` comes from + a measurement rather than from the catalog. Convenience wrappers: :meth:`set_tempo_led`, :meth:`set_metronome_volume`, and :meth:`set_tempo_option` for the lists. @@ -1088,14 +1133,21 @@ def set_tempo_param(self, param, value: float = None, real=None): else: index = self.catalog[self.TEMPO_CONTROL].parameter(param).index if real is not None: - model = self.catalog[self.TEMPO_CONTROL] - if index >= len(model.parameters): - raise ValueError( - f"the catalog does not describe tempo parameter {index} (it " - f"describes 0 to {len(model.parameters) - 1}), so real= cannot be " - f"converted - pass value= with the normalized 0..1 instead" - ) - value = model.parameters[index].to_normalized(real) + if index == 0: + # TEMPO's catalog range is a placeholder (0..1 with a real unit), so + # the catalog cannot convert it. The span was measured instead - + # 40..240 bpm, three screen-vs-wire points - so real= means bpm here + # rather than raising. Same shape as lane VOLUME and its dB helper. + value = bpm_to_tempo(real) + else: + model = self.catalog[self.TEMPO_CONTROL] + if index >= len(model.parameters): + raise ValueError( + f"the catalog does not describe tempo parameter {index} (it " + f"describes 0 to {len(model.parameters) - 1}), so real= cannot " + f"be converted - pass value= with the normalized 0..1 instead" + ) + value = model.parameters[index].to_normalized(real) if value is None: raise TypeError("set_tempo_param needs value= (0..1) or real= (own units)") msg = pa.GridMessage(action=pa.MessageAction.UPDATE) @@ -1265,6 +1317,70 @@ def set_metronome_volume(self, value: float = None, real: float = None): """ return self.set_tempo_param("VOLUME", value=value, real=real) + #: Index of MODE inside the DEVICE tempo block. It is the catalog's ``TYPE``, + #: which no control in the preset's own Tempo page writes - the preset copy of + #: parameter 1 sat at 0.0 through every flip measured. + TEMPO_MODE_PARAM = 1 + + def tempo_mode(self, timeout: float = 30.0) -> "TempoMode": + """Whether the unit is running on the PRESET's tempo or the DEVICE's. + + The Tempo and Metronome menu's MODE switch. Returns a + :class:`~pyquadcortex.protocol.enums.TempoMode`. + + Confirmed on hardware (2026-08-12) by capturing every field of every + message the device answers in each switch position and diffing the two: + exactly one field moved, this one. The tempo actually in effect + corroborates it from a second direction - the unit displayed 111 bpm in + PRESET with the preset block holding 0.355, and 120 bpm in GLOBAL with the + device block holding 0.400. + + **This is a READ, and the device never volunteers it.** Three earlier + investigations watched for a broadcast when the switch moves and correctly + found none; the mistake was concluding from that that the switch was not on + the wire. It is, and only asking finds it. + + The timeout is generous because ``GlobalTempo`` alternates two shapes, one + push each, and only one of them carries parameters - measured at roughly + one every seven seconds. This waits for that shape specifically rather than + taking the first ``GlobalTempo`` to arrive, which is how a single earlier + READ came back holding only the running clock and got written up as a dead + end. + """ + index = self.TEMPO_MODE_PARAM + reply = self._t.await_broadcast( + pa.GlobalTempoMessage, + lambda: self._t.send(pa.GlobalTempoMessage(action=pa.MessageAction.READ)), + timeout=timeout, + match=lambda m: (len(m.params) > index + and len(m.params[index].param_values) > 0)) + return TempoMode(round(reply.params[index].param_values[0].float_value)) + + def set_tempo_mode(self, mode: "TempoMode"): + """Move the MODE switch: run on the preset's tempo, or the device's. + + Takes a :class:`~pyquadcortex.protocol.enums.TempoMode`. Confirmed on + hardware and ON THE UNIT'S OWN SCREEN (2026-08-12): writing GLOBAL moved + the menu's switch and changed the tempo in effect from 111 to 120 bpm, + writing PRESET moved both back. + + **Global, not per preset**, despite riding a tempo message: there is + nothing to save afterwards, and every preset is affected. Read + :meth:`tempo_mode` first if you intend to put it back. + + This does NOT move either tempo block. The preset's + ``tempoProgramData`` parameter 1 was measured before and after the write + and did not move, which is what makes the scope of this write knowable + rather than assumed - the device accepts a write it does not understand + and says nothing, so a write whose target is guessed is indistinguishable + from one that worked. + """ + message = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + param = message.params.add() + param.index = self.TEMPO_MODE_PARAM + param.param_values.add().float_value = float(int(TempoMode(mode))) + return self._t.send(message) + def set_chain_output(self, row: int, out_portid: int): """Point one grid ``row``'s output at ``out_portid`` (row-keyed update). diff --git a/pyquadcortex/protocol/enums.py b/pyquadcortex/protocol/enums.py index 7cc99a9..edb0e55 100644 --- a/pyquadcortex/protocol/enums.py +++ b/pyquadcortex/protocol/enums.py @@ -387,6 +387,25 @@ class MetronomeBeat(IntEnum): QUIET = 3 #: De-emphasized - softer than NORMAL, but still audible. +class TempoMode(IntEnum): + """The Tempo and Metronome menu's MODE switch - which tempo block is in effect. + + The unit keeps TWO tempo blocks. The preset's lives in + ``BinaryPreset.tempoProgramData``; the device's rides in ``GlobalTempo.params``. + MODE picks which one the unit plays, and neither block is touched by the + switch itself. + + Confirmed on hardware 2026-08-12, by reading the device's whole answerable + state in each position and diffing: exactly one field differed, the device + tempo block's parameter 1. The bpm the unit displayed corroborates it + independently - PRESET showed 111 with the preset block holding 0.355, GLOBAL + showed 120 with the device block holding 0.400, both exact on a 40-240 range. + """ + + PRESET = 0 #: The loaded preset's own tempo and metronome settings. + GLOBAL = 1 #: The device's, shared by every preset. + + class TempoSubdivision(IntEnum): """Metronome SUBDIVISIONS - the rhythmic pulses per beat. diff --git a/tests/hardware/state_snapshot.py b/tests/hardware/state_snapshot.py new file mode 100644 index 0000000..8525a10 --- /dev/null +++ b/tests/hardware/state_snapshot.py @@ -0,0 +1,275 @@ +"""Snapshot the unit's readable state, so two snapshots can be diffed. + +Written for the TEMPO MODE investigation, and deliberately general: the method +is *diff, do not hunt*. Every earlier attempt at MODE looked for a field it +expected and concluded "not on the wire" when that field did not appear. This +records EVERY set field of every message the device answers with - names it has +a schema for and field numbers it does not - so the question becomes "what +differs between the two menu positions" rather than "is it the field I guessed". + +Three things here exist because of specific past mistakes: + +* **Unknown field numbers are recorded.** The schema is recovered from Cortex + Control, so a field the firmware sends and that build never had would decode + as nothing at all. ``GeneralSettingsMessage`` uses field numbers 1-39 with no + gaps, so if MODE rides there it rides in a number the schema does not know. +* **Values are collected as a SET per field path, over a window**, not sampled + once. ``GlobalTempo`` alternates two shapes (``protocol.md`` section 8), so a + single reply proves nothing about the other shape. +* **Nothing is filtered out.** Fields known to move on their own - the running + clock, meters, request ids - are LABELLED as noise in the diff, not dropped. + A filter that hides the answer is exactly how this question got its previous + wrong answer. +""" +import json +import time + +from google.protobuf.unknown_fields import UnknownFieldSet + +#: State types to READ. Each is a subscription the device answers with its +#: current value. ``Updater`` is deliberately absent (CLAUDE.md: never send +#: anything to the firmware surface), as are the cloud types. +READ_TYPES = ( + "GlobalTempo", # H3: the only type seen carrying global tempo params + "GeneralSettings", # H2: the global settings bag + "Mode", "IOSettings", "MasterVolume", "GlobalEQ", "ShowGigView", + "Tuner", "Looper", "SetlistPosition", "Scene", "PresetDirty", +) + +#: Arrives constantly and carries per-sample values. Recorded as a field-path +#: census - has a field appeared or gone? - rather than as values, which would +#: bury the snapshot in numbers that differ every time by design. +NOISY_TYPES = frozenset({ + "GridModelMeterMessage", "IOMeterMessage", "CPULoadMessage", + "ModuleStatsMessage", "KeepAliveMessage", "SystemTimeSyncMessage", + "ModelRepoMessage", +}) + +#: Handled separately by :func:`preset_fields` - a full grid dump is enormous +#: and the interesting part of it is small. +PRESET_TYPES = frozenset({"RecallPresetMessage", "GridMessage"}) + +#: Substrings marking a path that moves on its own. NOT a filter: the diff +#: prints these under their own heading, below everything else. +NOISE_PATHS = ( + "request_id", "current_beat", "current_bar", "current_tick", + "available_disk_space", "cpu", "meter", "timestamp", "session_id", + "elapsed", "position", +) + + +def _scalar(field, value): + """One leaf value, rendered so a diff of two snapshots reads plainly.""" + if field.type == field.TYPE_ENUM: + entry = field.enum_type.values_by_number.get(value) + return f"{value}:{entry.name}" if entry is not None else value + if field.type == field.TYPE_BYTES: + return value.hex() + return value + + +def _is_map(field): + return (field.message_type is not None + and field.message_type.GetOptions().map_entry) + + +def describe(message, prefix="", skip=()): + """Every SET field of ``message``, flattened to ``path -> value``. + + ``ListFields`` is the presence-correct reading of this schema: a field in a + synthetic ``oneof`` appears only when the device actually sent it, which is + the distinction the whole model rests on (CLAUDE.md). A field that is absent + is absent from the result, so a diff shows it as a key appearing rather than + as a zero that could mean either thing. + + ``skip`` names top-level fields to leave out - ``chains`` on a preset, which + is most of the payload and none of the question. + """ + out = {} + for field, value in message.ListFields(): + if not prefix and field.name in skip: + out[f"{field.name}."] = True + continue + name = f"{prefix}{field.name}" + if _is_map(field): + for key in sorted(value): + item = value[key] + if hasattr(item, "ListFields"): + out.update(describe(item, f"{name}[{key!r}].")) + else: + out[f"{name}[{key!r}]"] = item + elif field.is_repeated: + if field.message_type is not None: + out[f"{name}."] = len(value) + for index, item in enumerate(value): + out.update(describe(item, f"{name}[{index}].")) + else: + out[name] = [_scalar(field, v) for v in value] + elif field.message_type is not None: + # Recorded even when empty: a present-but-empty submessage is a + # real answer, and without this it would vanish from the diff. + out[f"{name}."] = True + out.update(describe(value, f"{name}.")) + else: + out[name] = _scalar(field, value) + for unknown in UnknownFieldSet(message): + data = unknown.data + out[f"{prefix}"] = ( + data.hex() if isinstance(data, bytes) else data) + return out + + +def preset_fields(binary_preset): + """The preset's non-grid fields, which is where H1 lives. + + ``chains`` is skipped: it is nearly the whole payload and the tempo question + is not in it. What is kept is ``tempo`` (field 10, presence-tracked), + ``tempoProgramData`` (field 19, the ``TempoControl`` block), and every other + preset-level field, so a difference anywhere outside the grid shows up + whether or not it was the one being looked for. + """ + return describe(binary_preset, skip=("chains",)) + + +class _Tap: + """Records every decoded inbound message for the life of the capture.""" + + def __init__(self, transport): + self._transport = transport + self._inner = transport._dispatch + self.shapes = {} # type name -> {fingerprint: {"count", "fields"}} + self.census = {} # type name -> {"count", "paths"} + self.errors = [] + transport._dispatch = self._tap + + def _tap(self, message, *args, **kwargs): + try: + self._record(message) + except Exception as exc: # noqa: BLE001 - the RX thread never dies + # Counted rather than swallowed. A describe() that raises on one + # message type would otherwise read as that type never arriving, + # which is the exact failure mode this whole investigation exists + # to undo. + self.errors.append(f"{type(message).__name__}: {type(exc).__name__}: {exc}") + return self._inner(message, *args, **kwargs) + + def _record(self, message): + name = type(message).__name__ + if name in PRESET_TYPES: + return # captured separately, not here + fields = describe(message) + if name in NOISY_TYPES: + entry = self.census.setdefault(name, {"count": 0, "paths": set()}) + entry["count"] += 1 + entry["paths"].update(fields) + return + entry = self.shapes.setdefault(name, {}) + fingerprint = json.dumps(fields, sort_keys=True, default=repr) + shape = entry.setdefault(fingerprint, {"count": 0, "fields": fields}) + shape["count"] += 1 + + def stop(self): + self._transport._dispatch = self._inner + + +def capture(qc, label, window=14.0, spacing=0.15): + """READ everything readable, watch for ``window`` seconds, return a snapshot. + + Read-only: every message sent is a ``READ``. Nothing here writes to the + unit, so the run needs no restore (ADR-0005 is satisfied trivially). + + ``window`` has to span several beats, because ``GlobalTempo`` alternates its + two shapes one per push and only one of them has ever been seen carrying + parameters. At 40 bpm a pair arrives every 1.5 s, so 14 s is roughly nine + pairs at the slowest tempo the unit offers. + """ + from pyquadcortex.protocol import registry + from pyquadcortex.protocol.proto import ProductionAutomation_pb2 as pa + + tap = _Tap(qc._t) + try: + for name in READ_TYPES: + cls = registry.class_for(pa.CortexMessageType.Enum.Value(name)) + qc._t.send(cls(action=pa.MessageAction.READ)) + time.sleep(spacing) + time.sleep(window) + preset = qc.read_current_preset() + fields = preset_fields(preset) + finally: + tap.stop() + + return { + "label": label, + "window_seconds": window, + "preset": fields, + "shapes": {name: sorted(shapes.values(), key=lambda s: -s["count"]) + for name, shapes in sorted(tap.shapes.items())}, + "census": {name: {"count": entry["count"], "paths": sorted(entry["paths"])} + for name, entry in sorted(tap.census.items())}, + "tap_errors": tap.errors, + } + + +def _values_by_path(snapshot): + """``type -> path -> sorted set of every value seen for it in the window``. + + Per-path value SETS, not one sample: the answer may be a field that takes + one value in one shape of a message and another in the other shape. + """ + out = {} + for name, shapes in snapshot["shapes"].items(): + paths = out.setdefault(name, {}) + for shape in shapes: + for path, value in shape["fields"].items(): + paths.setdefault(path, set()).add(json.dumps(value, default=repr)) + return out + + +def _is_noise(path): + lowered = path.lower() + return any(marker in lowered for marker in NOISE_PATHS) + + +def _compare(name, before, after, signal, noise): + for path in sorted(set(before) | set(after)): + left = sorted(before.get(path, ())) + right = sorted(after.get(path, ())) + if left == right: + continue + line = f"{name}.{path}: {_render(left)} -> {_render(right)}" + (noise if _is_noise(path) else signal).append(line) + + +def _render(values): + if not values: + return "" + return values[0] if len(values) == 1 else "{" + ", ".join(values) + "}" + + +def diff(before, after): + """What moved between two snapshots, signal first and noise named. + + Returns ``(signal, noise)``: two lists of lines. ``noise`` holds paths whose + names say they move on their own - the running clock, meters, request ids. + They are reported, not discarded, because a filter is how a previous answer + to this question went wrong. + """ + signal, noise = [], [] + + left, right = _values_by_path(before), _values_by_path(after) + for name in sorted(set(left) | set(right)): + _compare(name, left.get(name, {}), right.get(name, {}), signal, noise) + + _compare("preset", + {p: {json.dumps(v, default=repr)} for p, v in before["preset"].items()}, + {p: {json.dumps(v, default=repr)} for p, v in after["preset"].items()}, + signal, noise) + + for name in sorted(set(before["census"]) | set(after["census"])): + was = set(before["census"].get(name, {}).get("paths", ())) + now = set(after["census"].get(name, {}).get("paths", ())) + for path in sorted(was ^ now): + line = f"census {name}.{path}: {'gone' if path in was else 'appeared'}" + (noise if _is_noise(path) else signal).append(line) + + return signal, noise diff --git a/tests/hardware/test_tempo_mode.py b/tests/hardware/test_tempo_mode.py new file mode 100644 index 0000000..f6e1307 --- /dev/null +++ b/tests/hardware/test_tempo_mode.py @@ -0,0 +1,195 @@ +"""Where the unit keeps TEMPO MODE (GLOBAL or PRESET). + +The unit's Tempo and Metronome menu has a MODE switch, and in PRESET mode the +tempo and all seven metronome settings belong to the preset. Cortex Control has +the same switch, so a route to it exists. Three earlier tests watched for a +broadcast when the switch moves and saw nothing, and that was written up as "not +on the wire at all" - which is more than those tests measured. **They listened; +none of them asked.** See ADR-0007 and ``protocol.md`` "Per-preset tempo, LED +and metronome". + +This asks. It is read-only - every message it sends is a ``READ`` - so it writes +nothing to the unit and needs no restore. + +Run it once per MODE position, with the switch moved on the touchscreen in +between:: + + QC_SNAPSHOT_LABEL=global pytest tests/hardware --hardware -s -k tempo_mode + # flip MODE on the unit, then: + QC_SNAPSHOT_LABEL=preset pytest tests/hardware --hardware -s -k tempo_mode + +The second run diffs the two. ``-s`` matters: the finding is what it prints. + +The three hypotheses it covers at once, cheapest first: + +1. ``BinaryPreset.tempo`` (field 10) and ``tempoProgramData`` (field 19) are + presence-tracked, and presence may itself be the discriminator - a preset + saved under PRESET mode carries them, one saved under GLOBAL does not. +2. ``GeneralSettings`` carries the mode and never broadcasts it. A READ would + show it. Its schema uses field numbers 1-39 with no gaps, so if it is there + it is in a number the recovered schema does not know - which is why unknown + field numbers are recorded rather than dropped. +3. ``GlobalTempo.params`` holds an unmapped index. + +Nothing here looks for a field it expects. It records every set field of every +message the device answers with and diffs the two positions, so a difference is +found wherever it is rather than only where it was predicted. +""" +import importlib.util +import json +import os +import time +from pathlib import Path + +import pytest + +from pyquadcortex.protocol.client import QuadCortex +from pyquadcortex.protocol.enums import TempoMode + +_MODULE = Path(__file__).parent / "state_snapshot.py" +_spec = importlib.util.spec_from_file_location("qc_state_snapshot", _MODULE) +state_snapshot = importlib.util.module_from_spec(_spec) +_spec.loader.exec_module(state_snapshot) + +#: Working artifacts, not evidence to commit - the findings go in the docs. +CAPTURES = Path(__file__).parent / "captures" + + +def test_capture_tempo_mode_state(qc): + """READ everything readable and save it under ``QC_SNAPSHOT_LABEL``.""" + label = os.environ.get("QC_SNAPSHOT_LABEL") + if not label: + pytest.fail( + "set QC_SNAPSHOT_LABEL to the MODE position shown on the unit RIGHT " + "NOW, e.g. QC_SNAPSHOT_LABEL=global. The label is what the diff is " + "reported against, so a wrong one makes the answer unreadable.") + + snapshot = state_snapshot.capture(qc, label) + CAPTURES.mkdir(exist_ok=True) + path = CAPTURES / f"{label}.json" + path.write_text(json.dumps(snapshot, indent=2, sort_keys=True, default=repr)) + + preset = snapshot["preset"] + tempo_params = sorted(p for p in preset if p.startswith("tempoProgramData")) + global_tempo = snapshot["shapes"].get("GlobalTempoMessage", []) + with_params = [s for s in global_tempo + if any(p.startswith("params") for p in s["fields"])] + unknown = sorted( + f"{name}: {path_}" + for name, shapes in snapshot["shapes"].items() + for shape in shapes for path_ in shape["fields"] if "UNKNOWN" in path_) + unknown += sorted(f"preset: {p}" for p in preset if "UNKNOWN" in p) + + print(f"\n=== snapshot '{label}' -> {path} ===") + print(f"message types answered : {len(snapshot['shapes'])}") + print(f"preset name : {preset.get('name', '')}") + print(f"preset.tempo (field 10): {preset.get('tempo', '')}") + print(f"tempoProgramData count : {preset.get('tempoProgramData.', 0)}" + f" block(s), {len(tempo_params)} field path(s)") + print(f"GlobalTempo shapes : {len(global_tempo)} distinct, " + f"{len(with_params)} carrying params") + print(f"UNKNOWN field numbers : {unknown if unknown else 'none'}") + if snapshot["tap_errors"]: + # Not decoration. A describe() that raises on one type would otherwise + # look exactly like that type never arriving, which is the failure this + # whole investigation exists to undo. + print(f"TAP ERRORS (the snapshot is incomplete): {snapshot['tap_errors']}") + + assert snapshot["shapes"], "no device traffic at all - is the link up?" + assert not snapshot["tap_errors"], snapshot["tap_errors"] + + +#: How long the written value is left in place before the restore puts it back. +#: Sized for a PERSON: script output does not reach the operator until the run +#: exits, so the only way they can confirm the unit's own menu moved is to be +#: watching it while this window is open. +HOLD_SECONDS = 8.0 + +#: A read straight after a write returns the PREVIOUS value - three settings have +#: already looked like they refused a write that had in fact landed (client.py). +SETTLE_SECONDS = 3.0 + + +def test_tempo_mode_is_writable(qc, restores): + """Drive MODE from the host, and prove which scope the write landed in. + + Exercises the SHIPPED methods - ``tempo_mode`` and ``set_tempo_mode`` - rather + than building the messages here, so what the coverage table claims is verified + is the code a caller actually runs. + + Sets MODE to whichever value the unit is NOT currently showing, holds it long + enough to be seen on the unit's own screen, reads it back, and restores. + ADR-0005: the restore is registered BEFORE the write, so the unit is put back + whether this passes or fails. + + The second assertion is the one ADR-0007 actually cares about. Option (c) - + let a tempo write through and work out the scope afterwards - was rejected + because a guess and a success look identical to the caller. So this checks + that the write moved the DEVICE block and left the preset's + ``tempoProgramData`` alone, rather than trusting that it went where it was + aimed. + """ + before = qc.tempo_mode() + target = TempoMode.GLOBAL if before is TempoMode.PRESET else TempoMode.PRESET + preset_before = _preset_mode_param(qc) + + restores(f"tempo MODE -> {before.name}", lambda: qc.set_tempo_mode(before)) + qc.set_tempo_mode(target) + + time.sleep(SETTLE_SECONDS) + after = qc.tempo_mode() + preset_after = _preset_mode_param(qc) + time.sleep(HOLD_SECONDS) + + print("\n=== TEMPO MODE write ===") + print(f"was : {before.name}") + print(f"wrote : {target.name}") + print(f"read back : {after.name}") + print(f"preset tempoProgramData: param {QuadCortex.TEMPO_MODE_PARAM} " + f"{preset_before} -> {preset_after}") + print(f"restoring to : {before.name}") + + assert after is target, ( + f"set_tempo_mode({target.name}) and tempo_mode() returned {after.name}. " + f"The device accepts a write it does not understand and says nothing, so " + f"this is exactly what an unsupported write looks like.") + assert preset_after == preset_before, ( + f"the write was aimed at the DEVICE tempo block but the preset's " + f"tempoProgramData param moved too ({preset_before} -> {preset_after}). " + f"Scope is not what it appears - do not model this as a device setting.") + + +def _preset_mode_param(qc): + """The PRESET copy of the same parameter, read positionally. + + The stored preset carries all 24 with ``index`` absent, so position is the + index (``protocol.md``). Returns ``None`` if the block is not there at all, + which is a different answer from zero and has to stay so. + """ + index = QuadCortex.TEMPO_MODE_PARAM + params = qc.read_current_preset().tempoProgramData + if not params or len(params[0].params) <= index: + return None + values = params[0].params[index].param_values + return values[0].float_value if values else None + + +def test_diff_captured_snapshots(): + """Diff every pair of snapshots on disk. Needs no unit; needs two files.""" + files = sorted(CAPTURES.glob("*.json")) if CAPTURES.exists() else [] + if len(files) < 2: + pytest.skip(f"{len(files)} snapshot(s) in {CAPTURES} - need two to diff") + + snapshots = [json.loads(f.read_text()) for f in files] + for index, before in enumerate(snapshots): + for after in snapshots[index + 1:]: + signal, noise = state_snapshot.diff(before, after) + print(f"\n=== {before['label']} -> {after['label']} ===") + print(f"--- {len(signal)} field(s) moved ---") + for line in signal: + print(f" {line}") + if not signal: + print(" nothing outside the known-noisy paths differed") + print(f"--- {len(noise)} known-noisy path(s), shown for completeness ---") + for line in noise: + print(f" {line}") diff --git a/tests/test_client.py b/tests/test_client.py index 35baaf3..0508cbe 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -12,7 +12,7 @@ from pyquadcortex.protocol import catalog, client from pyquadcortex.protocol.enums import (Footswitch, Input, Instrument, MidiSource, - Output, SceneBypassBehavior, Setlist) + Output, SceneBypassBehavior, Setlist, TempoMode) from pyquadcortex.protocol.proto import ProductionAutomation_pb2 as pa from pyquadcortex.protocol.proto import Preset_pb2 as preset @@ -2836,6 +2836,122 @@ def test_set_tempo_option_range_checks_against_the_catalogs_step_count(): qc.set_tempo_option("ROUTING", 5) +# -- the TEMPO span ------------------------------------------------------------ +# The catalog publishes TEMPO as 0..1 with a real-world unit - a placeholder - so +# the span was measured instead: three screen readings taken against simultaneous +# wire reads on 2026-08-12, each landing on the displayed integer exactly. + + +#: (wire value, bpm on the unit's screen). The 59 is the one that earns the fit: +#: 111 and 120 sit 9 bpm apart, and two close points cannot distinguish spans - +#: the lesson the lane levels taught after two releases of a wrong one. +MEASURED_TEMPO_POINTS = ((0.095, 59.0), (0.355, 111.0), (0.400, 120.0)) + + +@pytest.mark.parametrize("value,bpm", MEASURED_TEMPO_POINTS) +def test_tempo_bpm_matches_every_measured_point(value, bpm): + assert client.tempo_bpm(value) == pytest.approx(bpm, abs=0.01) + assert client.bpm_to_tempo(bpm) == pytest.approx(value, abs=1e-6) + + +def test_tempo_bpm_refuses_a_tempo_the_unit_does_not_have(): + with pytest.raises(ValueError, match="40..240"): + client.bpm_to_tempo(300.0) + with pytest.raises(ValueError, match="40..240"): + client.bpm_to_tempo(39.0) + + +def test_set_tempo_param_takes_real_as_bpm_for_index_zero(): + """The one index where ``real=`` comes from a measurement, not the catalog. + + Every other tempo parameter converts through the catalog, which refuses TEMPO + because its published range is a placeholder. No catalog is loaded here, which + is the point: if this path went through the catalog it would raise. + """ + qc = client.QuadCortex(FakeTransport()) + qc.set_tempo_param("TEMPO", real=111.0) + sent = qc._t.sent[-1].preset.tempoProgramData[0].params[0] + assert sent.index == 0 + assert sent.param_values[0].float_value == pytest.approx(0.355, abs=1e-6) + + +# -- the Tempo menu's MODE switch ---------------------------------------------- +# Found 2026-08-12 by capturing every field of every message the device answers in +# each switch position and diffing: exactly one moved, the DEVICE tempo block's +# parameter 1. Three earlier investigations watched for a broadcast on commit, +# correctly found none, and concluded the switch was not on the wire - it is, and +# only a READ finds it. + + +def _global_tempo_with_mode(value, count=25): + """A ``GlobalTempo`` push in the shape that carries parameters.""" + message = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + for index in range(count): + message.params.add().param_values.add( + float_value=value if index == client.QuadCortex.TEMPO_MODE_PARAM else 0.0) + return message + + +def test_tempo_mode_reads_parameter_one_of_the_device_block(): + fake = FakeTransport() + fake.broadcast = _global_tempo_with_mode(1.0) + qc = client.QuadCortex(fake) + + assert qc.tempo_mode() is TempoMode.GLOBAL + assert isinstance(fake.sent[-1], pa.GlobalTempoMessage) + assert fake.sent[-1].action == pa.MessageAction.READ + + +def test_tempo_mode_skips_the_clock_shaped_push(): + """The predicate is the whole instrument, so it is pinned here. + + ``GlobalTempo`` alternates two shapes, one push per beat, and only one carries + parameters. A single earlier READ of this type happened to land on the clock + shape and was written up as a dead end - "returned only a running clock" - which + set the investigation back by two releases. A waiter that accepts any + ``GlobalTempo`` would reproduce that exactly, and read nothing while the device + was answering. + """ + fake = FakeTransport() + fake.broadcast = _global_tempo_with_mode(0.0) + qc = client.QuadCortex(fake) + qc.tempo_mode() + match = fake.last_match + + clock = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + clock.metronome_status.current_beat = 2 + assert not match(clock), "the clock shape carries no parameters and must be skipped" + assert not match(pa.GlobalTempoMessage()), "an empty push is not an answer" + assert match(_global_tempo_with_mode(0.0)), "the params shape IS the answer" + + +def test_set_tempo_mode_writes_the_device_block_and_not_the_preset(): + """Scope is the point. ADR-0007 rejected letting a tempo write land in + whichever scope the unit happened to be in, because a guess and a success look + identical to the caller. Measured on hardware: the preset's own parameter 1 did + not move across this write.""" + fake = FakeTransport() + qc = client.QuadCortex(fake) + + qc.set_tempo_mode(TempoMode.GLOBAL) + sent = fake.sent[-1] + assert isinstance(sent, pa.GlobalTempoMessage), "not a Grid/preset edit" + assert sent.action == pa.MessageAction.UPDATE + assert len(sent.params) == 1 + assert sent.params[0].index == 1 + assert sent.params[0].param_values[0].float_value == 1.0 + + qc.set_tempo_mode(TempoMode.PRESET) + assert fake.sent[-1].params[0].param_values[0].float_value == 0.0 + + +def test_set_tempo_mode_refuses_a_value_that_is_not_a_mode(): + qc = client.QuadCortex(FakeTransport()) + with pytest.raises(ValueError): + qc.set_tempo_mode(2) + assert qc._t.sent == [] + + # -- per-beat metronome states (STEPSTATE) ------------------------------------- # Traced on hardware: from a 4/4 preset reading ENNN, one touch on beat 3 wrote # index 12 = 0.3333, three touches on beat 4 walked index 13 through 0.3333, diff --git a/tests/test_state_snapshot.py b/tests/test_state_snapshot.py new file mode 100644 index 0000000..d5f1d55 --- /dev/null +++ b/tests/test_state_snapshot.py @@ -0,0 +1,195 @@ +"""The state-snapshot instrument, checked offline. + +``tests/hardware/test_tempo_mode.py`` only runs with a unit attached, and the +question it asks - where the unit keeps TEMPO MODE - has already been answered +wrongly once by an instrument nobody had checked. Three earlier tests reported +"MODE is not on the wire"; what they had measured was that it is never +broadcast, and one of them silently dropped 27 of the device's 72 message types +while doing it. + +So the failure to guard against here is not "the device says nothing". It is +**the snapshot cannot see it even when the device does say it**, which reads +identically from the transcript. Each test below feeds the describer a message +carrying the thing it must not miss, and fails if the snapshot comes back empty: + +* a field the recovered schema does not know at all - the likeliest hiding place + in ``GeneralSettings``, whose 39 field numbers have no gaps; +* a presence-tracked field that is set to its zero value, which is the whole + ``oneof`` distinction the model rests on (CLAUDE.md); +* a value that appears in only one of ``GlobalTempo``'s two alternating shapes. + +What this file cannot do: prove the device sends any of it. Only a unit can. +""" +import importlib.util +from pathlib import Path + +import pytest + +from pyquadcortex.protocol.proto import ProductionAutomation_pb2 as pa +from pyquadcortex.protocol.proto import Preset_pb2 as preset + + +@pytest.fixture(scope="module") +def snapshot(): + """The hardware suite's helper, imported as a plain module, not collected. + + ``tests/hardware/conftest.py`` refuses to COLLECT that directory without + ``--hardware``; importing one module out of it is a different thing, and safe + because nothing here touches a device. + """ + path = Path(__file__).parent / "hardware" / "state_snapshot.py" + spec = importlib.util.spec_from_file_location("qc_state_snapshot", path) + module = importlib.util.module_from_spec(spec) + spec.loader.exec_module(module) + return module + + +# -- describe: what the snapshot must not miss -------------------------------- + + +def test_a_field_number_the_schema_does_not_know_is_recorded(snapshot): + """The one that decides H2. + + ``GeneralSettingsMessage`` uses field numbers 1-39 with no gaps, so a MODE + field in it is a number this schema - recovered from one Cortex Control + build - has never seen. protobuf keeps such a field but decodes it to + nothing, so a describer reading only named fields would report an empty + difference and it would read as "the unit does not answer". + """ + message = pa.GeneralSettingsMessage() + message.MergeFromString(bytes([0xF8, 0x06, 0x07])) # field 111, varint, 7 + + described = snapshot.describe(message) + + assert described == {"": 7} + + +def test_a_presence_tracked_field_set_to_zero_is_recorded(snapshot): + """Absent and present-but-zero are different answers, and must stay so. + + Most of this schema sits in synthetic ``oneof``s precisely so the device can + say "false" as distinct from saying nothing. A GLOBAL/PRESET switch is a + two-valued thing, so one of its two values is very likely the zero - and a + describer that dropped zeros would see the switch move and report silence. + """ + with_zero = pa.GeneralSettingsMessage(swap_tempo_tuner_access=False) + without = pa.GeneralSettingsMessage() + + assert snapshot.describe(with_zero) == {"swap_tempo_tuner_access": False} + assert snapshot.describe(without) == {} + + +def test_repeated_params_are_recorded_by_position(snapshot): + """``tempoProgramData`` and ``GlobalTempo.params`` are both lists of ``Param``. + + In the stored preset all 24 arrive with ``index`` ABSENT, so position is the + index (``protocol.md``). Flattening them positionally is what makes "param 7 + differs" a readable line in the diff. + """ + message = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + for value in (120.0, 4.0): + message.params.add().param_values.add(float_value=value) + + described = snapshot.describe(message) + + assert described["params."] == 2 + assert described["params[0].param_values[0].float_value"] == 120.0 + assert described["params[1].param_values[0].float_value"] == 4.0 + assert described["action"] == "1:UPDATE" # enums read as number:NAME + + +def test_the_grid_is_skipped_but_says_so(snapshot): + """A preset dump minus ``chains`` is readable; minus a note it is a lie.""" + binary = preset.BinaryPreset(name="test", tempo=120) + binary.chains.add() + + described = snapshot.preset_fields(binary) + + assert described["tempo"] == 120 + assert described["chains."] is True + assert not any(path.startswith("chains.models") for path in described) + + +def test_an_absent_preset_tempo_is_absent_not_zero(snapshot): + """H1 rests on this. ``BinaryPreset.tempo`` is field 10, presence-tracked.""" + assert "tempo" not in snapshot.preset_fields(preset.BinaryPreset(name="x")) + + +# -- diff: what the comparison must not miss ---------------------------------- + + +def _snap(label, shapes, preset_fields=None, census=None): + """A snapshot in the on-disk shape, without needing a device.""" + return { + "label": label, + "window_seconds": 0, + "preset": preset_fields or {}, + "shapes": {name: [{"count": 1, "fields": fields} for fields in shape_list] + for name, shape_list in shapes.items()}, + "census": census or {}, + "tap_errors": [], + } + + +def test_a_field_appearing_in_one_mode_only_is_signal(snapshot): + before = _snap("global", {"GeneralSettingsMessage": [{}]}) + after = _snap("preset", {"GeneralSettingsMessage": [{"": 1}]}) + + signal, noise = snapshot.diff(before, after) + + assert signal == [ + "GeneralSettingsMessage.: -> 1"] + assert noise == [] + + +def test_a_value_present_in_only_one_of_two_shapes_still_diffs(snapshot): + """``GlobalTempo`` alternates two shapes, one push each. + + A comparison that sampled one message per type would compare a clock reply + against a params reply and call the difference real. Values are collected as + a SET per path across the whole window, so the clock-only shape contributes + nothing to the params path and vice versa. + """ + clock = {"metronome_status.": True, "metronome_status.current_beat": 1} + before = _snap("global", {"GlobalTempoMessage": [ + clock, {"params.": 25, "params[7].param_values[0].float_value": 0.0}]}) + after = _snap("preset", {"GlobalTempoMessage": [ + clock, {"params.": 25, "params[7].param_values[0].float_value": 1.0}]}) + + signal, _ = snapshot.diff(before, after) + + assert signal == [ + "GlobalTempoMessage.params[7].param_values[0].float_value: 0.0 -> 1.0"] + + +def test_the_running_clock_is_named_noise_not_dropped(snapshot): + """It moves every beat by design, and it is still reported. + + Filtering is how the previous answer went wrong, so a path that looks noisy + is printed under its own heading rather than discarded. + """ + before = _snap("global", {"GlobalTempoMessage": [{"metronome_status.current_beat": 1}]}) + after = _snap("preset", {"GlobalTempoMessage": [{"metronome_status.current_beat": 3}]}) + + signal, noise = snapshot.diff(before, after) + + assert signal == [] + assert noise == ["GlobalTempoMessage.metronome_status.current_beat: 1 -> 3"] + + +def test_a_preset_field_that_moves_is_signal(snapshot): + """H1: presence itself may be the discriminator.""" + before = _snap("global", {}, preset_fields={"name": "x"}) + after = _snap("preset", {}, preset_fields={"name": "x", "tempo": 120}) + + signal, _ = snapshot.diff(before, after) + + assert signal == ["preset.tempo: -> 120"] + + +def test_two_identical_snapshots_diff_to_nothing(snapshot): + """The negative result has to be readable as one, or it is worthless.""" + fields = {"GeneralSettingsMessage": [{"midi_channel": 1}]} + signal, noise = snapshot.diff(_snap("a", fields), _snap("b", fields)) + + assert (signal, noise) == ([], []) From d03c0b4c71b7db0e52078c6b2b85da6b2d90a1f7 Mon Sep 17 00:00:00 2001 From: Jonathan Stokes Date: Wed, 12 Aug 2026 23:21:07 -0500 Subject: [PATCH 2/3] review: close the findings from the PR #22 triage The two that changed technical content: MODE is not "never broadcast". The device emits no CHANGE EVENT when the switch moves - which is all three earlier tests ever measured - but the value itself rides the ambient GlobalTempo params push, twice per 14-second window against 63 clock pushes. So a state tracker CAN follow it, and docs/api.md said the opposite, which is the sentence M3's cache design would have been built on. Worse and sharper: that push is the first thing capture.md's own listener recipe filters out by TYPE, so the answer was very probably discarded by the instrument three times. The recipe now filters the clock SHAPE instead, and carries the lesson that a noise filter is an unchecked claim that a type cannot hold the answer. tempo_mode() read positionally while the device keys by index. Checked against the captures: the unit sets index on all 25 params, and there it equals position - so the read was right by luck and would return a neighbouring tempo parameter from a sparse push. Neighbours are 0.0/1.0 floats too, so the wrong answer would have rounded cleanly. Now prefers the explicit index, same fallback as set_block.echoes_cell. It also read .float_value off a REAL oneof without checking the member, and rounded anything to an enum - 0.4 answered PRESET. Both closed; out-of-range now raises quoting the value, matching beats()'s policy. The test that was supposed to prevent all this did not: gutting the predicate to "any params at all" left it green. Mutation-checked three ways now, and the index case was rebuilt after the first version passed under mutation too. _Tap and capture() had no tests at all despite being the producer the offline diff tests assume; they have two now. Harness: _is_noise matched substrings, so "meter" inside "parameters" buried every GlobalEQ param and "position" buried SetlistPosition - the field that reveals the operator changed preset and invalidated the comparison. Matched on whole segments now. The capture asserts the params shape actually arrived BEFORE writing the file, since a blind capture would diff to "nothing differed" - verbatim the wrong answer this exists to overturn. The diff asserts, refuses stale cross-session pairs, and the operator-driven capture skips rather than failing, so the readme's own invocation can be green again. The restore reads back, because MODE is global and survives a recall. Docs: the placeholder-span accounting named a denominator matching nothing (8 parameters, 2 spans measured, splitter FREQUENCY still open); "none of them is a tap" over-claimed against 2 unattributed indices; the 40..240 endpoints are the fit's and three surfaces had dropped that caveat; "every preset is affected" and the audibility claim are labelled as inferences. Verified: 538 offline. The reworked reader was replayed against all three real captured pushes and returns PRESET/GLOBAL/PRESET correctly. The live hardware suite could NOT be re-run - the unit dropped off USB - so the wire shapes remain hardware-verified from the original run (set_tempo_mode's bytes are unchanged and pinned offline) but the reworked hardware test itself is unrun. --- changelog.md | 15 ++- docs/STEERING.md | 2 +- docs/api.md | 15 ++- docs/capture.md | 36 +++++-- docs/domain-model.md | 2 +- docs/manual-coverage.md | 6 +- docs/protocol.md | 48 ++++++---- pyquadcortex/protocol/client.py | 131 +++++++++++++++++++++----- tests/hardware/state_snapshot.py | 48 ++++++++-- tests/hardware/test_tempo_mode.py | 151 +++++++++++++++++++++++------- tests/test_client.py | 114 +++++++++++++++++++--- tests/test_state_snapshot.py | 87 +++++++++++++++++ 12 files changed, 540 insertions(+), 115 deletions(-) diff --git a/changelog.md b/changelog.md index 94d05b4..287539e 100644 --- a/changelog.md +++ b/changelog.md @@ -114,11 +114,16 @@ put it back. It does not move either tempo block: the unit keeps the preset's settings and the device's at the same time, and MODE only picks which one plays. The entry above withdrew the claim that this control was not on the wire. It was -on the wire the whole time, and one READ shows it. The three tests that found -nothing were sound - the unit genuinely never broadcasts the switch - and the -mistake was reading "does not announce" as "cannot be asked". Confirmed on the -wire, on the unit's own screen, and by the tempo actually in effect, which -switched between the two blocks' stored values. +on the wire the whole time - though not via a naive READ, see the caveat below. The three tests that found nothing were measuring +something real and narrower - the unit emits no CHANGE EVENT when the switch moves - +and the mistake was reading that as "cannot be asked". The current value in fact +rides the tempo stream the unit sends anyway. Confirmed on the wire, on the unit's +own screen, and by the tempo actually in effect, which switched between the two +blocks' stored values. + +The method that found it - capture the whole readable state in each position +and diff, rather than looking for the field you expect - is now what ADR-0008 +requires before any control is written down as having no wire path. Watch out for one thing if you read `GlobalTempo` yourself: it alternates two message shapes, one carrying the running clock and one carrying the 25 diff --git a/docs/STEERING.md b/docs/STEERING.md index 552318a..b97b8ae 100644 --- a/docs/STEERING.md +++ b/docs/STEERING.md @@ -127,7 +127,7 @@ Single-device, single-connection USB HID at interactive rates (129-byte reports) - ADR.md: ADR-0008 - a control with no known wire path gets a differential state capture before it is recorded as having none. ADR-0007's rule is unchanged and now has no instance, which is the healthy state for it - `docs/domain-model.md`: `Tempo.mode` stops being refused and becomes an ordinary property; §13's *Genuinely open* loses its first entry and the *Closed* table records where the answer lives; both appendix tempo rows updated. `manual-coverage.md` gains a MODE row and its tally moves to 104 / 65 yes - `docs/capture.md` gains "Diff the whole state, do not hunt for a field" - the method that found it, and the four things in the harness that are load-bearing. Its listener chapter, which used this claim as its exemplar, now carries the ending -- **`TEMPO`'s span is 40..240 bpm**, measured at three screen-vs-wire points during the same session and exact to the displayed integer at each. `real=` on that parameter now takes bpm, via `tempo_bpm()` / `bpm_to_tempo()`; `protocol.md`'s list of unrecoverable placeholder spans is down to two of four +- **`TEMPO`'s span fits 40..240 bpm**, from three INTERIOR screen-vs-wire points measured during the same session, exact to the displayed integer at each. The endpoints are the fit's, not driven. `real=` on that parameter now takes bpm, via `tempo_bpm()` / `bpm_to_tempo()`; `protocol.md`'s placeholder-span list now has two of its eight parameters' spans measured and seven covered; splitter `FREQUENCY` is the one still unrecovered - `tests/hardware/state_snapshot.py` is the harness, reusable for the next control of this kind; `tests/test_state_snapshot.py` proves offline that it can see an unknown field number, a presence-tracked zero, and a value in only one of two message shapes **Why:** diff --git a/docs/api.md b/docs/api.md index 35ebd79..0e33253 100644 --- a/docs/api.md +++ b/docs/api.md @@ -276,14 +276,21 @@ setters below address the preset's block by construction, since they write hear until you switch back. That last step is inferred from those two facts rather than measured, so treat it as a caution and not as a verified behaviour. -The device never broadcasts this switch, so a state tracker cannot learn it from -pushes - `tempo_mode()` has to ask, and it waits for a `GlobalTempo` reply carrying -parameters rather than the running clock, which can take a few seconds. +The device emits no CHANGE EVENT when the switch moves, but the current value +rides the ambient `GlobalTempo` params push (measured: twice per 14-second window), +so a state tracker CAN follow it - it just cannot be told the moment it moves. +`tempo_mode()` waits for a reply carrying parameters rather than the running clock, +which can take a few seconds. + +**A read straight after a write can return the previous value.** This message type +does not echo `request_id` - zero of 64 captured pushes carried one - so there is no +way to tell your READ's reply from an ambient push already in flight. Allow a second +or two to settle after `set_tempo_mode` before believing `tempo_mode()`. The per-preset controls: ```python -qc.set_tempo_param("TEMPO", real=120) # bpm - the span is 40..240, measured +qc.set_tempo_param("TEMPO", real=120) # bpm - three points fit a 40..240 span qc.set_tempo_led(False) # this preset's TEMPO LED off qc.set_metronome_muted(True) # silence the click - the unit's own MUTE qc.set_tempo_param("TIME SIGNATURE", value=0.1) diff --git a/docs/capture.md b/docs/capture.md index c0b4235..6c3efa5 100644 --- a/docs/capture.md +++ b/docs/capture.md @@ -27,8 +27,14 @@ from pyquadcortex.protocol.proto import ProductionAutomation_pb2 as pa # rather than from this list. (CPULoadMessage, for instance, never arrives at # all, subscribed or not, so filtering it is harmless but pointless.) # +# WARNING, learned the expensive way: GlobalTempoMessage is heavy because it +# alternates a running clock with a 25-parameter shape, and the PARAMETERS are +# real device state - parameter 1 is the Tempo menu's MODE switch. Filtering the +# whole type discards them. Filter the clock shape, not the type: +# m.HasField("metronome_status") and not m.params +# # Note this is a NOISE list, not an allow-list, and pair it with a heartbeat - -# see "Two ways a listener lies about silence" above. +# see "Three ways your instrument lies about silence" below. NOISE = {"GlobalTempoMessage", "IOMeterMessage", "GridModelMeterMessage", "KeepAliveMessage", "ModuleStatsMessage"} @@ -91,7 +97,7 @@ LOG.write(f"-- heartbeat: {suppressed} chatter msgs, " ``` A silent log with a beating heart is a finding. A silent log without one is nothing at all. -The finding that the Tempo menu's MODE control is never broadcast was reached three times: +The finding that the Tempo menu's MODE control emits no change event was reached three times: the first time with neither safeguard, and twice more with both, and only the later two were worth anything. @@ -101,12 +107,21 @@ eight releases - 0.33.0 through 0.40.0 - that measurement was written up as "MOD the wire at all", which is a claim about readability that no amount of listening can support. Say what the instrument measured, not what it implies. -And the ending, which is the point: **MODE was on the wire the whole time.** It is the -device tempo block's parameter 1, and one READ shows it. The three listener runs were -correct and correctly reported; the eight releases were lost to the gap between the -question they answered and the question everyone read them as answering. When a listener -comes back silent, the next move is to ASK - see "Diff the whole state, do not hunt for a -field" below. +And the ending, which is the point: **MODE was on the wire the whole time** - and worse, +it was in the traffic those very runs were recording. It is the device tempo block's +parameter 1, carried in the params-shaped `GlobalTempo` push, which arrives about twice +per 14 seconds whether or not anyone touches the switch. + +Look at the NOISE list above. `GlobalTempoMessage` is first in it, because it is the +heaviest chatter on the link. So the most likely account of all three runs is not that +the device stayed silent, but that **the listener threw the answer away before it reached +the log** - three times, using a filter this very document recommends. + +So this one example teaches the whole section, and the sharpest part of it last: **a +noise filter is a claim that a message type cannot carry the answer**, and nobody had +checked it. Filter a SHAPE, never a whole type. And when a listener comes back silent, +do not reach for a longer window - ASK, and diff what comes back. See "Diff the whole +state, do not hunt for a field" below. **3. A match predicate that tests a field the reply never sets rejects every valid answer.** Reading the unit's Favorites list needs `RecentsFavorites{READ, is_favorites: true}`, and @@ -154,7 +169,10 @@ each is there because of a specific way this kind of capture lies: answer, and the answer here turned out to sit in a message the noise list would have been a natural home for. -Two practical notes. Prove the instrument offline first - `tests/test_state_snapshot.py` +Two practical notes. This is not merely a suggestion: ADR-0008 makes it the step that has to happen +before a control is recorded as having no wire path. + +Prove the instrument offline first - `tests/test_state_snapshot.py` feeds it a message carrying each thing it must not miss and fails if the snapshot comes back empty, which is the only cheap way to tell "the device said nothing" from "the capture cannot see it". And expect a large, boring diff: the connect burst's `File` diff --git a/docs/domain-model.md b/docs/domain-model.md index 3850650..6d057fb 100644 --- a/docs/domain-model.md +++ b/docs/domain-model.md @@ -1113,7 +1113,7 @@ the n/a rows below where they intersect the API at all. | Live Tuner (streaming needle) | **omitted** | no | the device refuses `enable_meter` from a host; unsupported by decision | | Tempo (BPM) | `device.tempo.bpm` | yes | the tempo in effect. Which scope it comes from is the unit's business, and depends on the MODE row below. Both blocks exist at once: measured 111 bpm from the preset's and 120 from the device's on the same unit, minutes apart | | Tempo MODE (Global vs Preset) | `device.tempo.mode` | yes | `GlobalTempo.params[1]`, `0.0` PRESET and `1.0` GLOBAL, readable and writable (`tempo_mode()` / `set_tempo_mode()`). A **device** setting despite riding a tempo message: it affects every preset and there is nothing to save. Never broadcast, which is why three earlier tests found nothing and why only a READ finds it. M3 with the rest of `Tempo` | -| Tap tempo | **omitted** | no | a `GlobalTempo` READ carries the 25 tempo parameters, none of which is a tap; MIDI CC#44 is the documented route | +| Tap tempo | **omitted** | no | a `GlobalTempo` READ carries the 25 tempo parameters, and none of the 23 attributed ones is a tap; indices 23 and 24 are unattributed, so this is not quite a closed door. MIDI CC#44 is the documented route | | Tempo LED | `device.tempo.led` | yes | | | Metronome volume/playback/pan/T-sig/subdivisions/sound/routing | `device.tempo.metronome.*` | yes | full enums for all four option lists | | Per-scene tempo (Cortex Control's bottom bar claims it) | **omitted** | n/a | the unit has no per-scene tempo; `scene_tempo` is inert on the wire. On-unit presentation wins | diff --git a/docs/manual-coverage.md b/docs/manual-coverage.md index 77a4782..00521fc 100644 --- a/docs/manual-coverage.md +++ b/docs/manual-coverage.md @@ -64,11 +64,11 @@ which this document had over-read as unreachable, and it answers a READ perfectl | Tuner: open/close | partly | `show_tuner()` is accepted; that it opens on screen has not been eyeballed | | Tuner: reference pitch, input source, mute | yes | `set_tuner_input()`, `set_tuner_reference()` and `set_tuner_mute()` all confirmed. Reference is an OFFSET in Hz from 440. Input accepts both inputs, both returns, INPUT_1_2 and USB 5/6; `RETURN_1_2` is refused by the DEVICE, so combined-returns tuning does not exist | | Tuner: Live Tuner (the needle) | no | UNSUPPORTED by decision. `enable_meter` refuses a host write - it stays false and `meter` stays 0.0 - so the needle never streams. Not worth chasing for an instrument you have to be holding; see `docs/roadmap.md` | -| Tap tempo | no | a `GlobalTempo` READ carries the 25 device tempo parameters, and none of them is a tap. MIDI CC#44 is the documented route | -| Tempo value (per preset) | yes | `set_tempo_param("TEMPO", real=120)` in bpm, or `value=` for the raw 0..1. The catalog range is a placeholder, so the 40..240 bpm span was measured off the screen instead - three points, exact to the displayed integer | +| Tap tempo | no | a `GlobalTempo` READ carries the 25 device tempo parameters, and none of the 23 ATTRIBUTED ones is a tap (indices 23 and 24 are unattributed). MIDI CC#44 is the documented route | +| Tempo value (per preset) | yes | `set_tempo_param("TEMPO", real=120)` in bpm, or `value=` for the raw 0..1. The catalog range is a placeholder, so the span was measured off the screen instead: three INTERIOR points (59, 111, 120 bpm), each exact to the displayed integer, fitting 40..240. The endpoints are the fit's - neither extreme was driven | | Metronome level, LED, time signature, note length | yes | `set_tempo_param()` by screen name, `set_tempo_option()` by option number, and typed setters with full enums: `set_tempo_subdivision()`, `set_metronome_sound()`, `set_metronome_routing()`, `set_time_signature()`. The menu's MODE is `tempo_mode()` / `set_tempo_mode()` - see the row below | | Per-beat accents (customizing each beat of the bar) | yes | `set_beat(n, MetronomeBeat.ACCENT)` and `set_beats([...])`; `pyquadcortex.protocol.beats(preset)` reads them back. Tempo parameters 10-22 - the catalog's `STEPSTATE0` to `STEPSTATE12` - are beats 1 to 13, each a four-option list at `option / 3`: normal, off, accented, de-emphasized. Traced by touching cells on the unit; a cell cycles UP by 1/3 and wraps, so four touches return it to where it started. Set the time signature FIRST - changing it rewrites these | -| Tempo MODE (Global vs Preset) | yes | `tempo_mode()` reads it, `set_tempo_mode()` writes it - the device tempo block's parameter 1, `0.0` PRESET and `1.0` GLOBAL. **Global, not per preset**: it affects every preset and there is nothing to save. The unit never broadcasts the switch, so a READ is the only way to see it, and the reader must wait for a `GlobalTempo` reply carrying PARAMETERS - the type alternates that shape with a clock shape | +| Tempo MODE (Global vs Preset) | yes | `tempo_mode()` reads it, `set_tempo_mode()` writes it - the device tempo block's parameter 1, `0.0` PRESET and `1.0` GLOBAL. **Global, not per preset**: it affects every preset and there is nothing to save. The unit emits no change event when the switch moves - which is what three earlier tests measured - but the current value rides the ambient `GlobalTempo` params push. The reader must wait for a reply carrying PARAMETERS, since the type alternates that shape with a clock shape | | Per-scene tempo | n/a | `scene_tempo` is ignored and reads back empty, and the unit has no per-scene tempo - its Tempo MODE is global or per preset, nothing finer | | Modes: read or set PRESET/SCENE/STOMP/HYBRID | yes | `mode()` / `set_mode(slot)`, plus `mode_cycle()` to read the cycle - `mode()` accepts partial pushes and can report an empty one. `FootswitchMode` names the three base modes and `describe_mode()` names any value | | Modes: reorder, merge into HYBRID, remove | yes | `set_mode_cycle([...])`. All six HYBRID pairings are mapped and built with `hybrid_mode(top, bottom)`: a hybrid gives footswitches A-D one mode and E-H another, so 3-8 are the six ORDERED pairs (4 and 7 being the same pair swapped). A cycle holds at most one hybrid and a hybrid cannot be the only slot; value 9 is ACCEPTED by the device but leaves the footswitches dead, so it is refused here | diff --git a/docs/protocol.md b/docs/protocol.md index 796ea29..083b90a 100644 --- a/docs/protocol.md +++ b/docs/protocol.md @@ -1072,11 +1072,11 @@ so position is the index - the same convention as `models[]`. A host WRITE does `index`; it is only the device's stored form that omits it. `pyquadcortex.protocol.tempo_params()` reads them positionally. -**The menu's MODE control (global or per-preset tempo) is NEVER BROADCAST, and is -READABLE AND WRITABLE ANYWAY.** The silence is real, established three times, the last -two with instruments worth trusting - and it says nothing about whether the switch -answers a question, which it does. Keep this pair together: it is the clearest example -this project has of a trustworthy negative being over-read. +**The menu's MODE control emits NO CHANGE EVENT, and is READABLE AND WRITABLE +ANYWAY.** The silence is real, established three times, the last two with instruments +worth trusting - and it is a fact about change notification, not about readability. +The current value rides the ambient tempo stream. Keep this pair together: it is the +clearest example this project has of a trustworthy negative being over-read. 1. **The first attempt.** Toggling to GLOBAL, pressing OK, toggling back to PRESET, pressing OK again and then saving the preset produced no `Grid`, `GlobalTempo` or @@ -1094,11 +1094,20 @@ this project has of a trustworthy negative being over-read. `BANK UP` that ends the section, so it repeats the toggle rather than the commit. The commit case rests on the two runs above. -All three are sound, and all three answer a narrower question than the one that was -asked of them. For eight releases - 0.33.0 through 0.40.0 - this section said MODE "is -NOT on the wire", and the coverage audit and the model design said "not on the wire at -all". Every one of those tests LISTENED, and none of them ASKED. **The switch was on the -wire the whole time.** +All three are sound as far as they go, and all three answer a narrower question than +the one that was asked of them. For eight releases - 0.33.0 through 0.40.0 - this +section said MODE "is NOT on the wire", and the coverage audit and the model design +said "not on the wire at all". Every one of those tests LISTENED, and none of them +ASKED. **The switch was on the wire the whole time.** + +Worse than that, and worth stating because it is the sharper lesson: **the value was +in the ambient stream those tests were recording.** The params-shaped `GlobalTempo` +push carries `params[1]`, and it arrives about twice per 14 seconds regardless of +whether anyone touches the switch. A 420-second run should have caught it flipping. +The likeliest reason none did is the noise list - `capture.md`'s own listener recipe +names `GlobalTempoMessage` first among the types to filter out, because it is the +heaviest chatter on the link. So the answer was very probably discarded by the +instrument before it reached the log, three times. ### MODE is the DEVICE tempo block's parameter 1 @@ -1118,7 +1127,9 @@ tempo write. `QuadCortex.tempo_mode()` and `set_tempo_mode()` are the operations `BinaryPreset.tempoProgramData` with 24 parameters; the device's rides in `GlobalTempo.params` with 25. Neither is touched by the switch. On the unit measured they disagreed at indices 2 (`LED LIGHT`), 3 (`VOLUME`) and 9 (`ROUTING`) as well as the -tempo itself, so which block is in effect is plainly audible. +tempo itself. The tempo difference was heard and seen - 111 bpm against 120 - so +which block is in effect is not a subtle distinction; the other three were read off +the wire rather than listened to. How it was established, because a negative that stood for eight releases deserves a method rather than an assertion: **every field of every message the device answers was @@ -1141,7 +1152,9 @@ the device tempo block's 25 parameters, index 24 - which exists there, does not the preset's 24, and is described nowhere - held `0.0` in both positions and is still unattributed. -The harness is `tests/hardware/state_snapshot.py`, and +This method is now required rather than optional: ADR-0008 makes a differential +state capture the thing that happens before a control is recorded as having no wire +path. The harness is `tests/hardware/state_snapshot.py`, and `tests/test_state_snapshot.py` proves offline that it can SEE an unknown field number, a presence-tracked field set to zero, and a value appearing in only one of two message shapes - the three ways this particular question could have come back falsely negative @@ -2560,7 +2573,7 @@ visually on the device's own screen. | `set_splitter_param` | `Grid{UPDATE, preset{chains{row, combined_splitter{params{index, param_values}}}}}` | read-back | writes `combined_splitter`, NOT `splitter[]`, which is a read-only view; indices follow the unified model 10004 | | `splits` | reads `Chain.split_control_points` | read-back | branch and rejoin columns. `split == -1` means serial; `mix == -1` with `split >= 0` is a branch that never rejoins (`Split.rejoins`). Only rows 0 and 2 can carry one | | `set_tempo_param` | `Grid{UPDATE, preset{tempoProgramData{params{index, param_values}}}}` | read-back | per-preset tempo, LED and metronome level; NOT row-keyed yet applied | -| `tempo_mode` / `set_tempo_mode` | `GlobalTempo{READ}`, and `GlobalTempo{UPDATE, params{index: 1, param_values}}` | read-back + on-unit | the Tempo menu's MODE switch: `0.0` PRESET, `1.0` GLOBAL. GLOBAL, not per preset - nothing to save, and every preset is affected. The device never broadcasts this, so a READ is the only way to see it; the reader must match on a reply carrying PARAMETERS, since `GlobalTempo` alternates a clock shape with a params shape | +| `tempo_mode` / `set_tempo_mode` | `GlobalTempo{READ}`, and `GlobalTempo{UPDATE, params{index: 1, param_values}}` | read-back + on-unit | the Tempo menu's MODE switch: `0.0` PRESET, `1.0` GLOBAL. A DEVICE setting, not a preset one - nothing to save. No change event is emitted when the switch moves, but the value rides the ambient params push; the reader must match on a reply carrying PARAMETERS, since `GlobalTempo` alternates a clock shape with a params shape | | `set_lane_output` | `Grid{UPDATE, preset{chains{row, output_control{hash: 23000, params{index, param_values}}}}}` | read-back | VOLUME/PAN/MUTE/SOLO per row; PAN 0.5 -> 0.0 survived save and read-back | | `move_block` | `GridMove{move{from_row, from_col, to_row, to_col, is_drop}}` | read-back | drivable host-to-device; a cross-row move makes the device create a branch | | `set_split` / `clear_split` | `Grid{UPDATE, preset{chains{row, split_control_points{split, mix}}}}` | read-back | activates or clears a row's branch; the splitter itself always exists | @@ -2879,10 +2892,11 @@ Stated explicitly so nobody builds on a guess: arrives (see [above](#a-placement-can-be-refused-for-want-of-dsp-capacity)), so whether a block will fit can only be discovered by placing it. - **The true spans behind the placeholder 0..1 ranges** are not recoverable from the - catalog, and two of the four have been measured off the screen instead: the - mixer/splitter/lane levels at -40..+12 dB, and `TEMPO` at 40..240 bpm (2026-08-12, - three points). In both cases the endpoints are the fit's rather than driven, so a - caller who needs an extreme exactly should drive it. + catalog. Eight parameters are affected; two spans covering seven of them have since + been measured off the screen: the mixer/splitter/lane levels at -40..+12 dB, and + `TEMPO` at 40..240 bpm (2026-08-12, three points). **Splitter `FREQUENCY` is the one + still unrecovered.** In both measured cases the endpoints are the fit's rather than + driven, so a caller who needs an extreme exactly should drive it. - **Whether a capture id denotes different content on a different unit** is untested here, needing a second unit. - ~~**Whether a preset's descriptive `tags` can be set at all**~~ - ANSWERED: no. The diff --git a/pyquadcortex/protocol/client.py b/pyquadcortex/protocol/client.py index 7fc0fa3..1263899 100644 --- a/pyquadcortex/protocol/client.py +++ b/pyquadcortex/protocol/client.py @@ -122,6 +122,47 @@ def db_to_lane_level(db: float) -> float: ) return (db + 40.0) / 52.0 + +def _mode_param(message, index: int): + """The float at tempo parameter ``index`` in a ``GlobalTempo``, or ``None``. + + ``None`` means "this message does not answer the question" - the clock shape, + a params list too short, a param carrying no value, or a value stored as + something other than a float. It is deliberately not an exception: this runs + as a match predicate, where the right response to a message that cannot + answer is to keep waiting for one that can. + + Two things here are not decoration. + + **The explicit ``index`` wins over position.** ``Param.index`` is + presence-tracked, the device sets it on every param of a captured push + (checked: all 25, and there it equals position), and this library's own writes + set it. Reading positionally while the device keys by index is the + ``ColBypass.column`` mistake in a new place - it works until the device sends + a sparse or reordered list, and then it returns a neighbouring tempo + parameter as the answer. The neighbours are 0.0/1.0 floats too, so the wrong + answer would round cleanly and look right. Same fallback as + ``set_block.echoes_cell``: trust the index when present, use position when not. + + **``ParamValue.value`` is a REAL oneof**, not a synthetic one - ``int_value``, + ``float_value``, ``string_value``. Reading ``.float_value`` off a param that + holds an int yields 0.0 with no error, which would report PRESET with total + confidence. ``tempo_params()`` already guards this; so does this. + """ + by_index = None + for position, param in enumerate(message.params): + at = param.index if field_present(param, "index") else position + if at == index: + by_index = param + break + if by_index is None or not by_index.param_values: + return None + first = by_index.param_values[0] + if not field_present(first, "float_value"): + return None + return first.float_value + + def tempo_bpm(value: float) -> float: """Convert a ``TEMPO`` wire value (0..1) to the bpm the unit displays. @@ -138,7 +179,15 @@ def tempo_bpm(value: float) -> float: The catalog publishes ``TEMPO`` as 0..1 with a real-world unit - a placeholder, which is why this helper exists. + + A wire value outside 0..1 is refused rather than converted, for the same reason + :func:`bpm_to_tempo` refuses a bpm outside the span: the tempo the caller would + read back does not exist on the unit. """ + if not 0.0 <= value <= 1.0: + raise ValueError( + f"a tempo wire value runs 0..1; {value} is outside it" + ) return 40.0 + 200.0 * value @@ -1335,26 +1384,62 @@ def tempo_mode(self, timeout: float = 30.0) -> "TempoMode": PRESET with the preset block holding 0.355, and 120 bpm in GLOBAL with the device block holding 0.400. - **This is a READ, and the device never volunteers it.** Three earlier - investigations watched for a broadcast when the switch moves and correctly - found none; the mistake was concluding from that that the switch was not on - the wire. It is, and only asking finds it. - - The timeout is generous because ``GlobalTempo`` alternates two shapes, one - push each, and only one of them carries parameters - measured at roughly - one every seven seconds. This waits for that shape specifically rather than - taking the first ``GlobalTempo`` to arrive, which is how a single earlier - READ came back holding only the running clock and got written up as a dead - end. + **The device emits no CHANGE EVENT when the switch moves** - three earlier + investigations watched for one and correctly found none. The current VALUE + is a different matter: it rides the ambient ``GlobalTempo`` params push, + which arrived twice per 14-second window in each of the three captures + (against 63 clock-shaped pushes in the same window). So a state tracker CAN + follow this field from pushes; what it cannot do is be told the moment it + moves. + + That is also why the timeout is generous. ``GlobalTempo`` alternates two + shapes and only one carries parameters, so this waits for that shape + specifically rather than taking the first ``GlobalTempo`` to arrive - which + is how a single earlier READ came back holding only the running clock and + got written up as a dead end. + + **A read straight after a write can return the PREVIOUS value.** This type + does not echo ``request_id`` - zero of 64 captured pushes carried one - so + there is no way to tell the reply to your READ from an ambient push already + in flight. Allow a settle of a second or two after + :meth:`set_tempo_mode` before believing the answer. """ index = self.TEMPO_MODE_PARAM - reply = self._t.await_broadcast( - pa.GlobalTempoMessage, - lambda: self._t.send(pa.GlobalTempoMessage(action=pa.MessageAction.READ)), - timeout=timeout, - match=lambda m: (len(m.params) > index - and len(m.params[index].param_values) > 0)) - return TempoMode(round(reply.params[index].param_values[0].float_value)) + seen = [] + + def carries_mode(message): + seen.append(message) + return _mode_param(message, index) is not None + + try: + reply = self._t.await_broadcast( + pa.GlobalTempoMessage, + lambda: self._t.send( + pa.GlobalTempoMessage(action=pa.MessageAction.READ)), + timeout=timeout, match=carries_mode) + except TimeoutError: + # "No broadcast arrived" and "none of them carried what I asked for" + # are different facts, and this project has already spent eight + # releases on the difference. Say which one happened. + raise TimeoutError( + f"no GlobalTempo carrying tempo parameter {index} within " + f"{timeout}s. {len(seen)} GlobalTempo push(es) DID arrive - this " + f"type alternates a clock shape with a params shape and only the " + f"params shape answers, so a longer timeout may be all that is " + f"needed. Do not read this as the device being silent." + ) from None + + value = _mode_param(reply, index) + if value not in (float(TempoMode.PRESET), float(TempoMode.GLOBAL)): + # Not rounded into an enum. The same policy as beats(): a value + # outside the states we know means the assumption is wrong, and + # rounding would convert that signal into a confident answer. + raise ValueError( + f"tempo parameter {index} holds {value!r}, which is neither " + f"{float(TempoMode.PRESET)} (PRESET) nor {float(TempoMode.GLOBAL)} " + f"(GLOBAL). The MODE mapping may not hold on this firmware." + ) + return TempoMode(int(value)) def set_tempo_mode(self, mode: "TempoMode"): """Move the MODE switch: run on the preset's tempo, or the device's. @@ -1364,9 +1449,13 @@ def set_tempo_mode(self, mode: "TempoMode"): the menu's switch and changed the tempo in effect from 111 to 120 bpm, writing PRESET moved both back. - **Global, not per preset**, despite riding a tempo message: there is - nothing to save afterwards, and every preset is affected. Read - :meth:`tempo_mode` first if you intend to put it back. + **Global, not per preset**, despite riding a tempo message: there is nothing to + save afterwards. Read :meth:`tempo_mode` first if you intend to put it back. + + What was MEASURED is that the write moves the device block and leaves the + loaded preset's own copy alone. That it therefore affects EVERY preset + follows from the menu being a device setting, and was not tested with a + second preset loaded. This does NOT move either tempo block. The preset's ``tempoProgramData`` parameter 1 was measured before and after the write diff --git a/tests/hardware/state_snapshot.py b/tests/hardware/state_snapshot.py index 8525a10..7202569 100644 --- a/tests/hardware/state_snapshot.py +++ b/tests/hardware/state_snapshot.py @@ -7,8 +7,12 @@ a schema for and field numbers it does not - so the question becomes "what differs between the two menu positions" rather than "is it the field I guessed". -Three things here exist because of specific past mistakes: +Four things here exist because of specific past mistakes: +* **Only fields the device actually SET are recorded** (``ListFields``), so an + absent field shows up in a diff as a key appearing rather than as a zero that + could mean either thing. Most of this schema sits in synthetic ``oneof``s + precisely so absent and zero stay distinguishable (CLAUDE.md). * **Unknown field numbers are recorded.** The schema is recovered from Cortex Control, so a field the firmware sends and that build never had would decode as nothing at all. ``GeneralSettingsMessage`` uses field numbers 1-39 with no @@ -49,13 +53,23 @@ #: and the interesting part of it is small. PRESET_TYPES = frozenset({"RecallPresetMessage", "GridMessage"}) -#: Substrings marking a path that moves on its own. NOT a filter: the diff -#: prints these under their own heading, below everything else. -NOISE_PATHS = ( - "request_id", "current_beat", "current_bar", "current_tick", - "available_disk_space", "cpu", "meter", "timestamp", "session_id", - "elapsed", "position", -) +#: Leaf field NAMES that move on their own. NOT a filter: the diff prints these +#: under their own heading, below everything else. +#: +#: Matched against whole path SEGMENTS, never as substrings. The first version +#: matched substrings and it was quietly wrong in the way that matters most here: +#: ``"meter"`` is inside ``"parameters"``, so every ``GlobalEQ.parameters`` path - +#: and ``GlobalEQ`` is in :data:`READ_TYPES` - was labelled noise, and +#: ``"position"`` swallowed ``SetlistPosition.position``, which is the one field +#: that would reveal the operator changed preset between two captures and +#: invalidated the whole comparison. An instrument whose thesis is that the label +#: makes the answer findable cannot bury a device-settings surface under +#: "known-noisy". +NOISE_FIELDS = frozenset({ + "request_id", "current_beat", "current_bar", "current_tick", "current_time", + "available_disk_space", "cpu_percent", "timestamp", "session_id", + "elapsed", "count_in_beats_remaining", "count_in_bars_remaining", +}) def _scalar(field, value): @@ -169,7 +183,13 @@ def _record(self, message): shape["count"] += 1 def stop(self): + # Restored by assignment, then the instance attribute is DELETED - leaving + # it in place shadows nothing useful and makes a second tap stopped out + # of order unhook the wrong one. Then a beat for any message already + # inside _tap to finish, so the caller can serialize `shapes` without + # racing the RX thread mutating it. self._transport._dispatch = self._inner + time.sleep(0.2) def capture(qc, label, window=14.0, spacing=0.15): @@ -226,8 +246,16 @@ def _values_by_path(snapshot): def _is_noise(path): - lowered = path.lower() - return any(marker in lowered for marker in NOISE_PATHS) + """Whether every leaf of ``path`` is a field known to move on its own. + + Splits on ``.`` and strips any ``[...]`` subscript, so ``parameters`` and + ``position`` are compared whole and cannot be swallowed by a substring. + """ + for segment in path.split("."): + name = segment.split("[", 1)[0] + if name in NOISE_FIELDS: + return True + return False def _compare(name, before, after, signal, noise): diff --git a/tests/hardware/test_tempo_mode.py b/tests/hardware/test_tempo_mode.py index f6e1307..644efbd 100644 --- a/tests/hardware/test_tempo_mode.py +++ b/tests/hardware/test_tempo_mode.py @@ -5,14 +5,32 @@ the same switch, so a route to it exists. Three earlier tests watched for a broadcast when the switch moves and saw nothing, and that was written up as "not on the wire at all" - which is more than those tests measured. **They listened; -none of them asked.** See ADR-0007 and ``protocol.md`` "Per-preset tempo, LED -and metronome". +none of them asked.** See ADR-0008 and ``protocol.md`` "MODE is the DEVICE tempo +block's parameter 1". -This asks. It is read-only - every message it sends is a ``READ`` - so it writes -nothing to the unit and needs no restore. +**It was asked, and it answered.** MODE is the DEVICE tempo block's parameter 1, +carried in ``GlobalTempo.params``: ``0.0`` PRESET, ``1.0`` GLOBAL. The winning +hypothesis was the third of three - the other two were killed by the same +capture, and both negatives are recorded in ``protocol.md``: -Run it once per MODE position, with the switch moved on the touchscreen in -between:: +1. ``BinaryPreset.tempo`` (field 10) as the discriminator. **Dead**: absent in + both positions, and ``tempoProgramData`` was identical across the flip. +2. ``GeneralSettings`` carrying it in a field number the recovered schema does + not know - its schema uses 1-39 with no gaps. **Dead**: identical in both + positions, and no message anywhere carried an unknown field number. +3. ``GlobalTempo.params`` holding an unmapped index. **This one.** + +This module keeps two tests, and they do different jobs: + +* :func:`test_tempo_mode_is_writable` is the REGRESSION test. It drives the + shipped ``tempo_mode`` / ``set_tempo_mode`` and always runs. +* :func:`test_capture_tempo_mode_state` is the INSTRUMENT that found the answer, + kept because ADR-0008 makes a differential state capture the thing you do + before recording a control as having no wire path. It is read-only, needs an + operator, and skips unless one asks for a capture. + +To use the instrument on some other control, run it once per position with the +control moved on the touchscreen in between:: QC_SNAPSHOT_LABEL=global pytest tests/hardware --hardware -s -k tempo_mode # flip MODE on the unit, then: @@ -20,17 +38,6 @@ The second run diffs the two. ``-s`` matters: the finding is what it prints. -The three hypotheses it covers at once, cheapest first: - -1. ``BinaryPreset.tempo`` (field 10) and ``tempoProgramData`` (field 19) are - presence-tracked, and presence may itself be the discriminator - a preset - saved under PRESET mode carries them, one saved under GLOBAL does not. -2. ``GeneralSettings`` carries the mode and never broadcasts it. A READ would - show it. Its schema uses field numbers 1-39 with no gaps, so if it is there - it is in a number the recovered schema does not know - which is why unknown - field numbers are recorded rather than dropped. -3. ``GlobalTempo.params`` holds an unmapped index. - Nothing here looks for a field it expects. It records every set field of every message the device answers with and diffs the two positions, so a difference is found wherever it is rather than only where it was predicted. @@ -56,38 +63,48 @@ def test_capture_tempo_mode_state(qc): - """READ everything readable and save it under ``QC_SNAPSHOT_LABEL``.""" + """READ everything readable and save it under ``QC_SNAPSHOT_LABEL``. + + An operator-driven CAPTURE INSTRUMENT, not a regression test - it needs a + person to have set the control to a known position and to say which. So it + skips rather than fails when unlabelled, and the skip names what to do. + + That is the one skip this directory's no-silent-skips rule tolerates, and + only because of what it is: a test that has stopped exercising the device is + invisible as a skip, but this one has nothing to exercise until somebody asks + for a capture. ``test_tempo_mode_is_writable`` below is the regression test, + and it always runs. + """ label = os.environ.get("QC_SNAPSHOT_LABEL") if not label: - pytest.fail( - "set QC_SNAPSHOT_LABEL to the MODE position shown on the unit RIGHT " - "NOW, e.g. QC_SNAPSHOT_LABEL=global. The label is what the diff is " - "reported against, so a wrong one makes the answer unreadable.") + pytest.skip( + "no QC_SNAPSHOT_LABEL - this is an operator-driven capture, not a " + "regression test. To take one, set the label to the MODE position " + "the unit is showing RIGHT NOW: QC_SNAPSHOT_LABEL=global pytest " + "tests/hardware --hardware -s -k capture_tempo") snapshot = state_snapshot.capture(qc, label) - CAPTURES.mkdir(exist_ok=True) - path = CAPTURES / f"{label}.json" - path.write_text(json.dumps(snapshot, indent=2, sort_keys=True, default=repr)) preset = snapshot["preset"] tempo_params = sorted(p for p in preset if p.startswith("tempoProgramData")) global_tempo = snapshot["shapes"].get("GlobalTempoMessage", []) with_params = [s for s in global_tempo if any(p.startswith("params") for p in s["fields"])] + arrivals = sum(s["count"] for s in with_params) unknown = sorted( f"{name}: {path_}" for name, shapes in snapshot["shapes"].items() for shape in shapes for path_ in shape["fields"] if "UNKNOWN" in path_) unknown += sorted(f"preset: {p}" for p in preset if "UNKNOWN" in p) - print(f"\n=== snapshot '{label}' -> {path} ===") + print(f"\n=== snapshot '{label}' ===") print(f"message types answered : {len(snapshot['shapes'])}") print(f"preset name : {preset.get('name', '')}") print(f"preset.tempo (field 10): {preset.get('tempo', '')}") print(f"tempoProgramData count : {preset.get('tempoProgramData.', 0)}" f" block(s), {len(tempo_params)} field path(s)") - print(f"GlobalTempo shapes : {len(global_tempo)} distinct, " - f"{len(with_params)} carrying params") + print(f"GlobalTempo : {len(global_tempo)} distinct shape(s); the " + f"params shape arrived {arrivals}x") print(f"UNKNOWN field numbers : {unknown if unknown else 'none'}") if snapshot["tap_errors"]: # Not decoration. A describe() that raises on one type would otherwise @@ -95,8 +112,25 @@ def test_capture_tempo_mode_state(qc): # whole investigation exists to undo. print(f"TAP ERRORS (the snapshot is incomplete): {snapshot['tap_errors']}") + # Asserted BEFORE the file is written. A snapshot that fails any of these is + # not evidence, and writing it anyway is worse than not capturing: the diff + # step globs the directory, so a bad file gets compared and reported as a + # result. The tap_errors check is the same argument - a type that raised in + # describe() is missing from `shapes`, and the diff renders that as + # " -> ...", which reads exactly like a discovery. assert snapshot["shapes"], "no device traffic at all - is the link up?" assert not snapshot["tap_errors"], snapshot["tap_errors"] + assert with_params, ( + "no GlobalTempo push carrying tempo PARAMETERS arrived in the window. " + "That shape is the only one that answers, so this capture cannot see " + "MODE at all - and a diff of it would report 'nothing differed', which " + "is verbatim the wrong answer this harness exists to overturn. Re-run, " + "or lengthen the window.") + + CAPTURES.mkdir(exist_ok=True) + path = CAPTURES / f"{label}.json" + path.write_text(json.dumps(snapshot, indent=2, sort_keys=True, default=repr)) + print(f"written : {path}") #: How long the written value is left in place before the restore puts it back. @@ -133,7 +167,19 @@ def test_tempo_mode_is_writable(qc, restores): target = TempoMode.GLOBAL if before is TempoMode.PRESET else TempoMode.PRESET preset_before = _preset_mode_param(qc) - restores(f"tempo MODE -> {before.name}", lambda: qc.set_tempo_mode(before)) + # The restore READS BACK. Everywhere else in this suite a restore is a blind + # write, which is tolerable for preset state - unsaved and discarded by any + # recall. MODE is GLOBAL: it survives a recall, so an unnoticed failed restore + # leaves the unit changed for good. And this test's own thesis is that a + # guess and a success are indistinguishable on this device. + def put_back(): + qc.set_tempo_mode(before) + time.sleep(SETTLE_SECONDS) + landed = qc.tempo_mode() + assert landed is before, ( + f"MODE left on {landed.name}, should be {before.name} - set it by hand") + + restores(f"tempo MODE -> {before.name}", put_back) qc.set_tempo_mode(target) time.sleep(SETTLE_SECONDS) @@ -174,22 +220,61 @@ def _preset_mode_param(qc): return values[0].float_value if values else None +#: How far apart two captures may be and still be treated as one experiment. +#: The point of the pair is that ONE thing changed between them - the operator +#: moving the control. Two files hours apart differ in whatever else happened in +#: between (a preset recall, an edit, a reboot), and the diff cannot tell those +#: from the answer. `captures/` is gitignored and nothing prunes it, so without +#: this an old file silently becomes half of a new comparison. +PAIR_WINDOW_SECONDS = 3600.0 + + def test_diff_captured_snapshots(): """Diff every pair of snapshots on disk. Needs no unit; needs two files.""" files = sorted(CAPTURES.glob("*.json")) if CAPTURES.exists() else [] if len(files) < 2: pytest.skip(f"{len(files)} snapshot(s) in {CAPTURES} - need two to diff") - snapshots = [json.loads(f.read_text()) for f in files] - for index, before in enumerate(snapshots): - for after in snapshots[index + 1:]: + loaded = [(f, json.loads(f.read_text())) for f in files] + compared = 0 + for index, (left_file, before) in enumerate(loaded): + for right_file, after in loaded[index + 1:]: + if before["label"] == after["label"]: + continue # same position; nothing to learn + gap = abs(left_file.stat().st_mtime - right_file.stat().st_mtime) + if gap > PAIR_WINDOW_SECONDS: + print(f"\n=== SKIPPED {before['label']} -> {after['label']}: " + f"{gap / 60:.0f} min apart, outside the pairing window. " + f"Delete the stale one and re-capture. ===") + continue + + compared += 1 signal, noise = state_snapshot.diff(before, after) print(f"\n=== {before['label']} -> {after['label']} ===") print(f"--- {len(signal)} field(s) moved ---") for line in signal: print(f" {line}") if not signal: - print(" nothing outside the known-noisy paths differed") + print(" nothing outside the known-noisy fields differed") print(f"--- {len(noise)} known-noisy path(s), shown for completeness ---") for line in noise: print(f" {line}") + + # The pair has to be diffable at all. Both snapshots must carry the + # shape that answers, or "nothing differed" means "the instrument was + # blind", not "the device did not move" - the confusion that cost + # this project eight releases. + for snapshot in (before, after): + assert any( + p.startswith("params") + for shape in snapshot["shapes"].get("GlobalTempoMessage", []) + for p in shape["fields"]), ( + f"snapshot {snapshot['label']!r} carries no GlobalTempo params " + f"shape, so this diff cannot see MODE") + assert not snapshot["tap_errors"], ( + f"snapshot {snapshot['label']!r} was captured with tap errors " + f"and is not evidence: {snapshot['tap_errors']}") + + assert compared, ( + f"{len(files)} snapshot(s) present but no valid pair to diff - they share " + f"a label, or are further than {PAIR_WINDOW_SECONDS / 60:.0f} min apart") diff --git a/tests/test_client.py b/tests/test_client.py index 0508cbe..fa2dfe9 100644 --- a/tests/test_client.py +++ b/tests/test_client.py @@ -2883,11 +2883,22 @@ def test_set_tempo_param_takes_real_as_bpm_for_index_zero(): # only a READ finds it. -def _global_tempo_with_mode(value, count=25): - """A ``GlobalTempo`` push in the shape that carries parameters.""" +def _global_tempo_with_mode(value, count=25, keyed=True): + """A ``GlobalTempo`` push in the shape that carries parameters. + + ``keyed`` reflects what the DEVICE actually sends, which was checked against + the 2026-08-12 captures rather than assumed: every one of the 25 params + carries an explicit ``index``, and there it equals position. The first version + of this helper built them with ``index`` absent - a shape the unit has never + been observed sending - which made the reader look correct for the wrong + reason. ``keyed=False`` is kept to prove the positional fallback still works. + """ message = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) for index in range(count): - message.params.add().param_values.add( + param = message.params.add() + if keyed: + param.index = index + param.param_values.add( float_value=value if index == client.QuadCortex.TEMPO_MODE_PARAM else 0.0) return message @@ -2902,15 +2913,20 @@ def test_tempo_mode_reads_parameter_one_of_the_device_block(): assert fake.sent[-1].action == pa.MessageAction.READ -def test_tempo_mode_skips_the_clock_shaped_push(): +def test_tempo_mode_predicate_rejects_everything_that_cannot_answer(): """The predicate is the whole instrument, so it is pinned here. - ``GlobalTempo`` alternates two shapes, one push per beat, and only one carries - parameters. A single earlier READ of this type happened to land on the clock - shape and was written up as a dead end - "returned only a running clock" - which - set the investigation back by two releases. A waiter that accepts any - ``GlobalTempo`` would reproduce that exactly, and read nothing while the device - was answering. + ``GlobalTempo`` alternates two shapes and only one carries parameters. A + single earlier READ landed on the clock shape and was written up as a dead + end - "returned only a running clock" - and that stood for eight releases + (0.33.0 through 0.40.0). A + waiter that accepts any ``GlobalTempo`` reproduces it exactly. + + Every case below was chosen because it DISTINGUISHES the real predicate from + a weaker one. An earlier version of this test used only the clock shape, an + empty push and a full params push - all three separated by "params non-empty" + alone - so replacing the predicate with ``len(m.params) > 0`` kept it green. + Mutation-checked: each assertion here fails under that weakening. """ fake = FakeTransport() fake.broadcast = _global_tempo_with_mode(0.0) @@ -2920,11 +2936,87 @@ def test_tempo_mode_skips_the_clock_shaped_push(): clock = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) clock.metronome_status.current_beat = 2 - assert not match(clock), "the clock shape carries no parameters and must be skipped" + assert not match(clock), "the clock shape carries no parameters" assert not match(pa.GlobalTempoMessage()), "an empty push is not an answer" + + short = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + short.params.add().param_values.add(float_value=0.4) # only index 0 + assert not match(short), "params present, but none of them is the mode" + + no_values = _global_tempo_with_mode(0.0) + no_values.params[1].ClearField("param_values") + assert not match(no_values), "the mode param carries no value" + + as_int = _global_tempo_with_mode(0.0) + as_int.params[1].ClearField("param_values") + as_int.params[1].param_values.add(int_value=1) + assert not match(as_int), ( + "ParamValue.value is a REAL oneof - reading .float_value off an " + "int-valued param yields 0.0 silently, which would report PRESET") + assert match(_global_tempo_with_mode(0.0)), "the params shape IS the answer" +def test_tempo_mode_reads_by_index_not_by_position(): + """The device keys these params, so the reader must not count them. + + Checked against the 2026-08-12 captures: every param of the pushed shape + carries an explicit ``index``. It happens to equal position on this firmware, + so a positional read is right by luck - and a sparse or reordered push would + silently return a NEIGHBOURING tempo parameter. The neighbours are 0.0/1.0 + floats too (LED LIGHT, START), so the wrong answer would look valid. + """ + # Built so the two readings DISAGREE, which is the only way this test can + # fail when the fix is removed. Position 1 holds index 2 (LED LIGHT = 1.0); + # index 1 - the mode - is last and holds 0.0. A positional read answers + # GLOBAL, the correct read answers PRESET. + sparse = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + for index, value in ((0, 0.4), (2, 1.0), (1, 0.0)): + param = sparse.params.add() + param.index = index + param.param_values.add(float_value=value) + + fake = FakeTransport() + fake.broadcast = sparse + assert client.QuadCortex(fake).tempo_mode() is TempoMode.PRESET, ( + "read position 1 (LED LIGHT) instead of the param keyed index 1") + + # And the positional fallback still applies when the device omits index. + fake = FakeTransport() + fake.broadcast = _global_tempo_with_mode(1.0, keyed=False) + assert client.QuadCortex(fake).tempo_mode() is TempoMode.GLOBAL + + +def test_tempo_mode_refuses_a_value_that_is_not_a_mode(): + """Rounding would turn "the mapping is wrong" into a confident answer. + + Same policy as ``beats()``, which returns an unrecognised quantized value as + a raw float rather than rounding it into an enum. 0.4 is not PRESET. + """ + fake = FakeTransport() + fake.broadcast = _global_tempo_with_mode(0.4) + with pytest.raises(ValueError, match="0.4"): + client.QuadCortex(fake).tempo_mode() + + +def test_tempo_mode_timeout_says_which_silence_it_was(): + """"No push arrived" and "none of them answered" are different facts. + + Conflating them is what cost this project eight releases, so the error must + not assert device silence when it observed predicate silence. + """ + class Silent(FakeTransport): + def await_broadcast(self, expected_class, trigger, timeout=40.0, match=None): + self.last_match = match + trigger() + for _ in range(3): + match(pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE)) + raise TimeoutError("no GlobalTempoMessage broadcast within 30.0s") + + with pytest.raises(TimeoutError, match="3 GlobalTempo push"): + client.QuadCortex(Silent()).tempo_mode() + + def test_set_tempo_mode_writes_the_device_block_and_not_the_preset(): """Scope is the point. ADR-0007 rejected letting a tempo write land in whichever scope the unit happened to be in, because a guess and a success look diff --git a/tests/test_state_snapshot.py b/tests/test_state_snapshot.py index d5f1d55..9e1538c 100644 --- a/tests/test_state_snapshot.py +++ b/tests/test_state_snapshot.py @@ -115,6 +115,75 @@ def test_an_absent_preset_tempo_is_absent_not_zero(snapshot): assert "tempo" not in snapshot.preset_fields(preset.BinaryPreset(name="x")) +# -- the recorder: the producer behind every snapshot on disk ------------------ + + +def test_the_tap_keeps_two_shapes_apart_and_counts_arrivals(snapshot): + """``_Tap`` is the producer, and nothing else in the suite touched it. + + The diff tests below hand-build the snapshot shape - ``shapes[name] -> + [{count, fields}]`` - so they encode an assumption about this class and could + all stay green while it emitted something else entirely. That is the same + "instrument nobody had checked" failure the rest of this file guards against, + one layer down. + + What must hold: the two ``GlobalTempo`` shapes stay DISTINCT (they are keyed + by a fingerprint of their fields), repeat arrivals are COUNTED rather than + collapsed, noisy types are censused instead of valued, and the real dispatch + still runs so the transport keeps working while the tap is installed. + """ + import types + + delivered = [] + transport = types.SimpleNamespace(_dispatch=lambda m: delivered.append(m)) + tap = snapshot._Tap(transport) + + clock = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + clock.metronome_status.current_beat = 3 + params = pa.GlobalTempoMessage(action=pa.MessageAction.UPDATE) + params.params.add().param_values.add(float_value=1.0) + + transport._dispatch(clock) + transport._dispatch(params) + transport._dispatch(clock) + transport._dispatch(pa.CPULoadMessage()) + tap.stop() + + shapes = tap.shapes["GlobalTempoMessage"] + assert len(shapes) == 2, "the clock and params shapes must not merge" + assert sorted(s["count"] for s in shapes.values()) == [1, 2], "arrivals counted" + assert "CPULoadMessage" in tap.census, "a noisy type is censused, not valued" + assert "CPULoadMessage" not in tap.shapes + assert len(delivered) == 4, "the real dispatch still ran for every message" + assert transport._dispatch is not tap._tap, "the tap was removed" + assert not tap.errors + + +def test_the_tap_survives_a_message_it_cannot_describe(snapshot): + """CLAUDE.md: the RX thread never dies. A counted error, not a lost link. + + The class comment says a ``describe()`` that raises would otherwise look + exactly like that type never arriving - which is the failure this whole + investigation exists to undo - so the swallow-and-count has to be real, and + the message has to keep flowing. + """ + import types + + delivered = [] + transport = types.SimpleNamespace(_dispatch=lambda m: delivered.append(m)) + tap = snapshot._Tap(transport) + original = snapshot.describe + snapshot.describe = lambda *a, **k: (_ for _ in ()).throw(RuntimeError("boom")) + try: + transport._dispatch(pa.GlobalTempoMessage()) + finally: + snapshot.describe = original + tap.stop() + + assert delivered == [pa.GlobalTempoMessage()], "the message still got through" + assert len(tap.errors) == 1 and "boom" in tap.errors[0], "and it was RECORDED" + + # -- diff: what the comparison must not miss ---------------------------------- @@ -177,6 +246,24 @@ def test_the_running_clock_is_named_noise_not_dropped(snapshot): assert noise == ["GlobalTempoMessage.metronome_status.current_beat: 1 -> 3"] +@pytest.mark.parametrize("path", [ + "GlobalEQMessage.parameters[0].gain", # "meter" is inside "parameters" + "SetlistPositionMessage.position", # the preset-changed confound + "TunerMessage.enable_meter", # a setting, not a moving value +]) +def test_a_real_field_is_not_swallowed_by_a_noise_substring(snapshot, path): + """Noise is matched on whole path SEGMENTS, never as a substring. + + The first version matched substrings, and the collisions were exactly the + wrong ones: ``GlobalEQ`` is a type this harness READs and its params field is + literally ``parameters``, and ``SetlistPosition.position`` is the field that + would reveal the operator changed preset between two captures - the one + confound that invalidates the whole comparison. Nothing was dropped, but + "known-noisy, shown for completeness" is an invitation to skip. + """ + assert not snapshot._is_noise(path) + + def test_a_preset_field_that_moves_is_signal(snapshot): """H1: presence itself may be the discriminator.""" before = _snap("global", {}, preset_fields={"name": "x"}) From f0f7551a3a320cb70691e12e64eb4cabdf7a1632 Mon Sep 17 00:00:00 2001 From: Jonathan Stokes Date: Thu, 13 Aug 2026 08:18:47 -0500 Subject: [PATCH 3/3] fix: the settle after a MODE write has to clear the push interval Caught on hardware. A restore wrote PRESET, waited 3 s, read back GLOBAL - and the write had in fact landed: four reads two seconds apart afterwards all said PRESET. tempo_mode() cannot correlate its reply, because this message type never echoes request_id, so it returns the next AMBIENT params push. That shape arrives about every seven seconds, so a 3 s settle can hand back a push generated before the write. SETTLE_SECONDS was 3.0 and the hardware test had been passing on luck. Now 10.0, and the docstring and api.md say the requirement in terms of the interval rather than "a moment". Also from the same run: free_storage_size_kb drifts between captures 30 s apart with nothing touching storage, so it joins NOISE_FIELDS - named from the wire this time. The earlier list guessed "available_disk_space", which is a real field on GeneralSettings but not the one that moves. Hardware, on the reworked code: the regression test passes, a self-driven capture pair (set_tempo_mode between them, no operator at the touchscreen) diffs to exactly params[1] 1.0 -> 0.0, and the unit is left in PRESET where it started. The params shape arrived 2x per 14 s window again on a fresh session, which is the third independent measurement of that cadence. --- docs/api.md | 11 +++++++---- pyquadcortex/protocol/client.py | 16 +++++++++++----- tests/hardware/state_snapshot.py | 5 +++++ tests/hardware/test_tempo_mode.py | 11 ++++++++++- 4 files changed, 33 insertions(+), 10 deletions(-) diff --git a/docs/api.md b/docs/api.md index 0e33253..985e147 100644 --- a/docs/api.md +++ b/docs/api.md @@ -282,10 +282,13 @@ so a state tracker CAN follow it - it just cannot be told the moment it moves. `tempo_mode()` waits for a reply carrying parameters rather than the running clock, which can take a few seconds. -**A read straight after a write can return the previous value.** This message type -does not echo `request_id` - zero of 64 captured pushes carried one - so there is no -way to tell your READ's reply from an ambient push already in flight. Allow a second -or two to settle after `set_tempo_mode` before believing `tempo_mode()`. +**A read straight after a write returns the previous value, and "a moment" is not +enough.** This message type does not echo `request_id` - zero of 64 captured pushes +carried one - so `tempo_mode()` returns the next ambient params push, which may have +been generated before your write. That shape arrives only about every seven seconds, +so wait longer than that: **ten seconds** is what the hardware suite uses. Measured +the hard way - a write followed by a 3-second settle read back the old value, while +the write had in fact landed. The per-preset controls: diff --git a/pyquadcortex/protocol/client.py b/pyquadcortex/protocol/client.py index 1263899..a511dab 100644 --- a/pyquadcortex/protocol/client.py +++ b/pyquadcortex/protocol/client.py @@ -1398,11 +1398,17 @@ def tempo_mode(self, timeout: float = 30.0) -> "TempoMode": is how a single earlier READ came back holding only the running clock and got written up as a dead end. - **A read straight after a write can return the PREVIOUS value.** This type - does not echo ``request_id`` - zero of 64 captured pushes carried one - so - there is no way to tell the reply to your READ from an ambient push already - in flight. Allow a settle of a second or two after - :meth:`set_tempo_mode` before believing the answer. + **A read straight after a write returns the PREVIOUS value, and "a moment" + is not long enough.** This type does not echo ``request_id`` - zero of 64 + captured pushes carried one - so this returns the next AMBIENT params push, + which may have been generated before your write. That shape arrives only + about every seven seconds, so **wait longer than that interval** - ten + seconds is the figure the hardware suite uses. Observed directly: a write + followed by a 3-second settle read back the old value, while the write had + landed and every read afterwards agreed. + + A caller who needs certainty rather than a settle should discard the first + matching push and take the second, which cannot predate the call. """ index = self.TEMPO_MODE_PARAM seen = [] diff --git a/tests/hardware/state_snapshot.py b/tests/hardware/state_snapshot.py index 7202569..4c8149f 100644 --- a/tests/hardware/state_snapshot.py +++ b/tests/hardware/state_snapshot.py @@ -69,6 +69,11 @@ "request_id", "current_beat", "current_bar", "current_tick", "current_time", "available_disk_space", "cpu_percent", "timestamp", "session_id", "elapsed", "count_in_beats_remaining", "count_in_bars_remaining", + # Observed drifting between two captures 30 seconds apart, with nothing + # touching storage. Named from the wire rather than guessed: an earlier + # version of this list said "available_disk_space", which is a field on + # GeneralSettings itself and not the one the device actually moves. + "free_storage_size_kb", }) diff --git a/tests/hardware/test_tempo_mode.py b/tests/hardware/test_tempo_mode.py index 644efbd..ab1e2d8 100644 --- a/tests/hardware/test_tempo_mode.py +++ b/tests/hardware/test_tempo_mode.py @@ -141,7 +141,16 @@ def test_capture_tempo_mode_state(qc): #: A read straight after a write returns the PREVIOUS value - three settings have #: already looked like they refused a write that had in fact landed (client.py). -SETTLE_SECONDS = 3.0 +#: +#: This one has to clear a specific, measured bar rather than be merely generous. +#: ``tempo_mode()`` cannot correlate its reply (this type never echoes +#: ``request_id``), so it returns the next AMBIENT params push - and that shape +#: arrives only about every seven seconds. A settle shorter than that interval can +#: hand back a push generated BEFORE the write. Caught in the act: a restore with a +#: 3.0 s settle read back the old value while the write had in fact landed, and +#: four reads two seconds apart afterwards all agreed it had. Ten seconds clears +#: the interval with room to spare. +SETTLE_SECONDS = 10.0 def test_tempo_mode_is_writable(qc, restores):