feat: TEMPO MODE is the device tempo block's parameter 1 - #22
Conversation
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.
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.
Self-review before opening this upRan four reviewers over the diff (correctness, silent failures, tests, docs/comments) and fixed 24 findings in 1. "The device never broadcasts this switch" was wrongWhat the three historical tests measured is that the unit emits no change event when the switch moves. The value is a different matter: it rides the ambient So a state tracker can follow this field from pushes. Sharper still: that params push is the first thing 2. The reader was right by luck
Two more in the same read path: it took 3. The test meant to prevent exactly this didn't
Worth admitting: the replacement index test also passed under mutation on the first attempt, because the sparse message I built gave the same answer either way. Rebuilt so the two readings disagree.
Other fixes
One reviewer finding I rejectedThe "~7 seconds" cadence was called a misreading of fingerprint-deduped shape variety. It isn't — the harness records arrival Verification status — one gap538 offline. The reworked reader was replayed against all three real captured device pushes and returns PRESET / GLOBAL / PRESET correctly, with the shipped predicate accepting each. The live hardware suite could not be re-run: the unit dropped off USB mid-session. |
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.
Hardware verification gap closed — and it found one more bugThe unit came back. Everything in the review-fix commit is now verified on The bug: the settle was too short, and the test was passing on luckA restore wrote PRESET, waited 3 seconds, read back GLOBAL — and the write had in fact landed. Four reads two seconds apart afterwards all said PRESET.
This is the exact hazard the previous commit added to the docs. Writing it down did not stop me from getting it wrong two hours later in the restore path — which is a decent argument for the read-back assertion in the restore, since that is what caught it. Verified on hardware
The answer, found again, this time by the shipped API rather than by hand.
Two smaller things the run produced
And the params shape arrived 2x per 14-second window again on a fresh session — a third independent measurement of the cadence a reviewer had challenged as an artifact of shape deduplication. 538 offline, hardware suite green. No known verification gaps left. |
Two text conflicts and one substantive one. ADR numbering: main took 0008 (grpcio-tools floor) and 0009 (persistent listeners), so this branch's record renumbers to ADR-0010. The lifecycle is append-only, and theirs landed first. Every reference that meant this branch's ADR moved with it; the grpcio ones are left alone. The substantive conflict is that PR #20 shipped exactly the hook this branch's harness needed. state_snapshot._Tap monkey-patched transport._dispatch, reaching past the public surface and unhookable out of order; it now subscribes through Transport.add_listener (ADR-0009). Verified the two are equivalent rather than assuming it: run side by side against the same 14-second window they saw the identical 478 messages, the same 23 types, in the same order. docs/capture.md's listener recipe monkey-patched _dispatch too, and it is the document that teaches this. Rewritten onto add_listener, with the two rules that come with a listener - do not block, do not read from the device - both of which the transport now enforces. That comparison turned up a real defect. A capture's type coverage varies run to run, because the handshake's reply burst lands lazily (the File enumeration takes 10-25 s) and a fixed window catches a different tail each time: two runs here saw 23 types and 12. diff() was rendering a type present in one snapshot and absent from the other field by field as "<absent> -> value", which reads exactly like a discovery. It now reports one line in the noise bucket naming what actually happened. Hardware, after the merge: the capture and the write regression both pass through the listener-based harness, and the unit is left in PRESET. 557 offline.
Merged
|
Three change logs conflicted, all in the same place: main's Unreleased section and this branch's both grew an entry after the `Device` one. Kept both, in the order they landed. - changelog.md: the broadcast listener (#20) then the translation groundwork - docs/STEERING.md: the boundary entry sits above TEMPO MODE (#22) and the listener (#20), which is where the newest entry goes in that file - docs/domain-model.md: TEMPO MODE closing then principle 5 being built, which is the order that file reads in
PR #22 added `tempo_bpm()` / `bpm_to_tempo()` next to the level helpers in protocol/client.py. They belong at the model boundary the same way the level scales do, so `device/translate.py` wraps them. They cannot MOVE. `set_tempo_param(real=)` calls `bpm_to_tempo` from inside protocol/client.py, so relocating the helper would make the protocol layer import the model. Checked by patching that call site to import from `pyquadcortex.device.translate` and watching `test_the_protocol_layer_never_imports_the_model` name it. So the pair delegates, exactly as `input_level_db` and `lane_level_db` do: one copy of the measured span, with the measurement attributed to the protocol layer rather than restated here, plus the type guard this seam adds because the protocol helpers are arithmetic and will happily multiply a bool. `tempo_bpm` and `bpm_to_tempo` also join the protocol-conversion allowlist in tests/test_translation.py, which is the half that does the work: without it, a model module reaching for `protocol.tempo_bpm` passes the check. Verified by dropping a file into the package that does both a `row - 1` and a `protocol.tempo_bpm(...)`, watching both structural checks name it, and removing it again. New in the tests: a guard that every name on that allowlist resolves in the protocol layer. A typo there reads like a rule and protects nothing.
The blocker first. `translate.slot_to_position("٢٨C")` returned 218 - a real
preset, from a name no screen shows, through a public function of the module
whose whole job is to stop that. Only `PresetAddress.parse` carried the
ASCII-digit pattern, while a comment and a test both read as though the module
was covered. Both doors share one pattern now, and one list of malformed names
runs through both. The protocol helper still accepts those digits by design
(`str.isdigit()` is true for them), so there is a test pinning that too - if it
ever tightens, the comment saying the boundary holds the line stops being true.
Then the guard cluster, which was the same finding as the last two reviews, one
layer down: the checks were narrower than they read.
- the arithmetic check now sees a letter table as a tuple, list or dict, not
only as a string; `string.ascii_uppercase`; the literal 65; and
`ROWS.index(row)`, which converts a coordinate using the boundary's own
exported table with no arithmetic in it anywhere
- the allowlist gained the protocol readers that hand back raw wire coordinates,
`stomp_assignments` among them - it returns the footswitch index whose
confusion with a column is why `FootswitchLetter` exists. Verified: a file
doing all of that at once passed both checks before and fails both now
- `test_the_boundary_itself_does_the_arithmetic` was anchored to the file, and
was satisfied by an error-message formatter in `_screen_number`. It names the
four converters now, so they cannot quietly stop converting
- a new derived check: everything the boundary delegates to must be on the
allowlist. That is the direction the list actually rots, and it is what
missed #22's tempo helpers
Both checks now pin their KNOWN blind spots as blind spots. A sample table where
every "should be caught" case is caught reads like a completeness proof; these
fail if a listed gap closes, which is the edit where the prose gets fixed too.
Also:
- the layering check could not see `from pyquadcortex import PresetAddress`, a
hole this story opened by re-exporting the value types. It reads
`device.__all__` now, so it follows the code
- `tests/test_docs.py` still exempted the dead `model` path and flagged the live
`device` one - the rename missed it, and it would have fired at story #12
- the hold-timing and tuner tests said more than they prove. Both are delegation
checks; they say so now, and say where the numbers are actually pinned
- `check_artifacts.py` did not require the two `__init__.py` files that decide
what `import pyquadcortex` hands back
- the prose in CLAUDE.md, STEERING, architecture.md and domain-model.md scoped
the rule to the model directory while the test scans the whole package
- roadmap.md's illustrative snippet still showed `preset.rows[0]`
- architecture.md said compile_protos.sh is the only script in the repo
1 participant
What this is
The Tempo menu's GLOBAL/PRESET switch is readable and writable. It is the device tempo block's parameter 1, carried in
GlobalTempo.params:0.0PRESET,1.0GLOBAL.This closes the first bullet of
domain-model.md§13 and the TEMPO MODE dependency on #8. It was a prerequisite of M3's device-settings work; it never gated M1, and nothing user-visible ships at M1 as a result.Why the old answer was wrong
Three tests had watched for a broadcast when the switch moves and seen nothing. That negative holds - the unit genuinely never announces the switch - and for eight releases, 0.33.0 through 0.40.0, the docs carried it as "not on the wire". Every one of those tests listened, and none of them asked.
A second, subtler failure kept it hidden after the first was spotted.
GlobalTempohad been the leading candidate and had been READ once; the reply carried only a running clock, and that went down as a dead end.GlobalTempoalternates two shapes, one push each - clock, params, clock, params - and that READ landed on the clock. The candidate was right; the instrument sampled once, and one sample of an alternating source is a coin flip.How it was found
By diffing, not hunting. Earlier attempts looked for the field they expected (
GeneralSettings,BinaryPreset.tempo) in the messages they expected. MODE was one index away inside a message shape already written off, so looking harder in the chosen places would never have found it.tests/hardware/state_snapshot.pyREADs twelve state types, taps the RX path for a window, and flattens every SET field of everything that arrives topath -> value- including field numbers the recovered schema does not know, sinceGeneralSettingsMessageuses 1-39 with no gaps and anything new there would decode to nothing. Across PRESET -> GLOBAL -> PRESET exactly one field moved, and moved back; the return capture differed from the baseline in nothing at all.Evidence
Three confirmations, because one flip in one direction is what produced this project's last false result:
0.0->1.0->0.0, nothing else in the device's readable state moving either way.0.355, 120 under GLOBAL with the device block holding0.400.A host write left the preset's own parameter 1 at
0.0, measured before and after, so the write's scope is known rather than assumed. That is the failure ADR-0007 named: the device accepts a write it does not understand and says nothing, so a guess and a success are indistinguishable.Negatives recorded too, since they are results:
GeneralSettingsidentical in both positions, no unknown field number in any message,BinaryPreset.tempoabsent in both.Second finding:
TEMPO's span is 40 to 240 bpmprotocol.mdlisted this span as not recoverable, because the catalog publishes a placeholder range for it. Measured off the screen instead, each point exact to the displayed integer: 59 bpm at0.095, 111 at0.355, 120 at0.400. Sobpm = 40 + 200 * value.set_tempo_param("TEMPO", real=120)now takes bpm, via newtempo_bpm()/bpm_to_tempo()helpers - the same shape aslane_level_db(). The 59 is what earns the fit: 111 and 120 sit 9 bpm apart, and two close points cannot distinguish spans, which is exactly how the lane levels carried a wrong span for two releases. The endpoints are the fit's rather than driven, and they agree with the unit's documented range.ADR-0008
ADR-0007's rule is unchanged: a control we understand but cannot drive is modelled and refuses. What changes is what has to happen first - a differential state capture before a control is recorded as having no wire path. "No broadcast was observed" is not a finding about a wire path.
ADR-0007 now has no instance, which is the healthy state for it.
Tests
tests/test_state_snapshot.py(new) proves offline that the harness can see the three things it must not miss: an unknown field number, a presence-tracked field set to zero, and a value appearing in only one of two message shapes. Each is a way this question could have come back falsely negative again.tests/test_client.pypins the exact wire shape of both new operations. One test feeds the reader's predicate a clock-shapedGlobalTempoand fails if it accepts it - that acceptance is what manufactured the original dead end.tests/hardware/test_tempo_mode.py(new) drives the shipped methods, holds the written value long enough to be seen at the unit, and asserts the preset's block did not move. State-neutral per ADR-0005.530 passed offline; hardware suite run against
d14e/ CorOS 4.0.1.Left open, on purpose
The device block carries 25 parameters to the preset's 24, and the catalog describes 23. Index 24 is device-only and described nowhere. It held
0.0through a MODE flip in both directions and through a preset tempo edit that moved seven other parameters, so it is not MODE and it is not preset-tempo-related. Recorded as unattributed rather than quietly dropped.