[tony-xmelon]

Summary

• expose inhibited_modules(), set_device_name(), undo(), and redo()
• add offline wire tests and guarded, reversible hardware coverage
• make hardware-gate paths and diagnostics portable on Windows
• make the historical namespace hash tolerant of Git CRLF conversion
• write generated catalog files explicitly as UTF-8
• baseline watchdog threads so combined offline/hardware runs do not interfere
• update api.md, manual-coverage.md, the hardware guide, and the changelog
• preserve the measured profile-specific Version announce behavior after syncing fix: the seam's first hardware run - correct the burst record, and skips are not regressions #53

This is PR A from the split requested in review. The CorOS 4.1 catalog and profile evidence remain isolated in #44.

Related issue

Split and revised in response to the review on this PR.

Testing

• final head: cb4d96e
• python -m pytest -q: 2702 passed, 3 skipped
• focused post-sync hardware-gate/report tests: 51 passed
• python -m mypy: passed
• git diff --check: passed

Hardware

No compatible unit is available for a full final-head run: the attached standard Quad Cortex runs CorOS 4.1.0, while this PR deliberately excludes the CorOS 4.1 snapshot now isolated in #44 at the maintainer's request. Consequently the full suite correctly refuses its three missing 4.1 generated snapshots. A maintainer with the baseline CorOS 4.0.1 profile will need to run pytest tests/hardware --hardware before merge.

The four operations added here were nevertheless run individually on final commit cb4d96e against the standard Quad Cortex on CorOS 4.1.0, with read-back/device confirmation and restoration:

--verifies set_device_name
tests: 1 passed, 0 failed, 0 errored, 0 skipped
passed: ['set_device_name'] (1)
===================== 1 passed, 101 deselected in 16.07s ======================

--verifies undo
tests: 1 passed, 0 failed, 0 errored, 0 skipped
passed: ['redo', 'undo'] (2)
===================== 1 passed, 101 deselected in 52.77s ======================

--verifies inhibited_modules
tests: 1 passed, 0 failed, 0 errored, 0 skipped
passed: ['inhibited_modules'] (1)
===================== 1 passed, 101 deselected in 12.40s ======================

set_device_name was read back and restored. Undo and redo reversed and reapplied a disposable scratch-preset edit, after which the original preset state was restored and the scratch preset removed. inhibited_modules returned both optional module flags from the unit.

Checklist

• Opened as a draft; Hardware limitation and targeted runs are recorded above
• Tests pass locally
• Added or updated tests for the change
• Updated documentation for the public API changes
• No CorOS 4.1 catalog changes bundled in

Upstream sync — 2026-09-12

Merged current upstream main at 640a8e9 (including merged PRs #63–#65) into this branch. Current head: 5b9bca4

Full offline Windows suite: 2727 passed, 3 skipped; mypy: clean across 46 source files; git diff --check: clean.

The Windows path-separator and CRLF-fixture portability checks pass on this branch because their fixes are part of #42.

Upstream sync - 2026-09-19

Merged upstream main at cfe39cc6592148d4196fa3028c2776f6618c41b6 (including maintainer PRs #70-#75) and resolved overlaps in favor of the current upstream API and evidence model.

Current head: f0ac2e4039d058ca4fa96efea920f641c9ac9194

Focused validation: 4 focused protocol tests passed; the updated manual-coverage count check passed.

GitHub CI on this exact head is green: build + twine check, mypy, and tests on Python 3.11, 3.12, and 3.13 all pass.

No new maintainer comments were posted during this sync. This PR is ready for maintainer re-review; the displayed CHANGES_REQUESTED decision is the earlier review and has not yet been refreshed.

[jonathanstokes]

Thanks @tony-xmelon. The four new operations are exactly the kind of thing I want in here, and the Windows fixes are overdue. I'd like to get those in quickly.

The catalog refresh is where I have to slow down. My unit is on 4.0.1 and I'm keeping it there for a while, so I can't verify the 4.1 constants, and tests/hardware/test_generated_constants.py would fail on my unit the day this merges. The doc changes about PresetDirty and the connect burst were also observed on 4.1, so I'd rather record them as 4.1 findings next to the 4.0.1 ones than replace what I measured.

Where I want this library to go is support for both firmware versions at once. Connect, detect what we're talking to, and hand back the right client and device object for it. That's the same shape I asked for on PR #31 for the Mini. Once that exists, a 4.1 catalog has a home that doesn't break 4.0.1 users.

So, if you don't mind, could you split this in two?

PR A, mergeable once I verify it on 4.0.1:

• inhibited_modules(), set_device_name(), undo(), redo() and their offline wire tests
• the Windows fixes: as_posix() in the gate, UTF-8 on the generator writes, the CRLF-tolerant fixture hash
• the watchdog thread baselining in test_state.py, with a comment saying it's for the combined run
• the api.md, manual-coverage.md and changelog entries for the four operations
• the new hardware test for name and undo/redo

PR B, after we agree on the two-version design:

• the regenerated models.py, options.py, params.py and the 412 to 420 counts
• the renamed constants and the eleven removed option enums, declared as breaking with an old-to-new map
• the PresetDirty, connect-burst Version count and dropped-read observations, recorded as 4.1.0 findings with the capture behind them
• the RecallPreset retry and the _retry_read helper, since the drops were seen on 4.1
• the loosened hardware assertions that came with those observations
• the STEERING and protocol.md baseline change, which I'll write as an ADR

I have other smaller feedback I'll save for the split PRs. If splitting is more than you want to take on, say so and I'll carve PR A out myself and credit you on it.

Stokes

[tony-xmelon]

Split as requested. This PR is now PR A: the four operations, offline/hardware coverage, and the Windows/combined-run fixes only. The CorOS 4.1 catalog, retry, and observation-driven assertions are isolated in draft PR #44, with the breaking-name migration map and the version-design/capture/ADR work called out as remaining. I also rebased PR A onto current main.

[tony-xmelon]

Thank you, Stokes, for the swift and detailed feedback — it was very helpful.

I have split the work as requested:

• Expose protocol operations and make the test suite portable #42 is now PR A only: inhibited_modules(), set_device_name(), undo(), redo(), their offline and guarded hardware tests, and the Windows/combined-run portability fixes.
• Add profile-owned CorOS 4.1 catalog and observations #44 is a draft PR B containing the CorOS 4.1 catalog and firmware-specific observations. It is explicitly held for the two-version design discussion, capture evidence, and the ADR/baseline work.

We have more potentially useful work locally, particularly around ModelRepo clone/parameter inheritance and metadata, native local backups, and additional remote-control surfaces. We will keep these as small, focused contributions and test them thoroughly — offline and on hardware where applicable — before submitting anything further.

Two adjacent areas are deliberately not being claimed as finished pyquadcortex features yet: our tap-tempo control currently uses the documented MIDI CC #44 rather than a verified HID operation, and although the protobuf exposes ScreenshotMessage, we have not yet established and hardware-tested its complete request/reply flow. If MIDI-backed helpers fit the project's scope, or if you have pointers on the intended Screenshot flow, those would be especially welcome.

Any other feedback on scope, API shape, testing expectations, or preferred priorities is very welcome. Thanks again.

[tony-xmelon]

As promised, the additional work has now been split into small, independent PRs:

• Resolve cloned ModelRepo parameter layouts #45 — ModelRepo clone/parameter inheritance
• Expose ModelRepo editor metadata #46 — ModelRepo editor metadata
• Add native local backup creation #47 — native local backup creation
• Allow deferring the initial file listing #48 — optional initial file listing
• Add preset screenshot support #49 — preset screenshot PNGs

Each was branched directly from main, reviewed independently, and run through focused tests, the applicable full offline suite, mypy, build/package checks, and live hardware checks where the behavior depends on the device.

The screenshot route is now established: a bare Screenshot{READ} is ignored, but a request carrying the preset display-folder name, factory flag, and slot returns an 800 x 384 PNG. #49 deliberately exposes this as preset_screenshot() rather than claiming arbitrary live-screen capture.

Tap tempo remains unsubmitted because the only verified remote route is the documented MIDI CC #44, not a HID/protobuf operation in this client. I did not want to submit a speculative API. Any guidance on whether MIDI-backed helpers belong in this project—or any other priorities, API-shape feedback, or testing pointers—is very welcome. Thanks again.

[jonathanstokes]

Thank you for splitting this so quickly. This half is clean, and I want to get it in.

What I tested on my unit (CorOS 4.0.1, firmware d14e). All three hardware tests pass here: the device-name round trip, the inhibited-modules read, and the undo/redo test against a scratch preset. So none of these four operations is 4.1-only, and the docstrings can say they are confirmed on both versions. For the undo test I made a copy of my loaded preset in an empty slot, pointed the env var at that copy, and deleted it afterwards. That worked well and I think it is the shape the test itself should use (see the inline notes).

While testing I also found one thing about the unit that matters for this PR. A Version READ is answered twice: the full reply, and about 1 ms later an echo of the READ that carries only action. version() correlates by type, so two calls close together alternate between the full reply and the empty echo. I saw this directly: five calls in a row returned full, empty, full, empty, full. The name test reads version().custom_name and writes it back, so on a bad turn it would write an empty name. The 1 s sleeps happen to hide this. Details inline.

Two things I could not attach to a line, because the files are not in this diff:

• docs/protocol.md has no coverage-table rows yet for set_device_name, undo/redo and inhibited_modules. The rows were in the original PR and got lost in the split. The table header also says every operation was exercised on 4.0.1, so the rows can now say "4.0.1 and 4.1.0" honestly.
• docs/domain-model.md still says (around line 1145) that there is no grid-level undo a host can trigger, and rows 1274 and 1300 still give "never driven" as the reason for omitting it. This PR proves otherwise, so those lines need a small update.

Everything else is inline. Items marked Nit: are optional. Thanks again, and please tell me if any of this is unclear.

Stokes

[tony-xmelon]

One correction to my previous note: by remote tap I meant clicking the physical Quad Cortex touchscreen from the app, not tap tempo. We have now recovered and hardware-verified that Cortex Control flow and submitted it separately as #50.

#50 adds live 800 x 480 screen capture plus raw-coordinate touchscreen taps. It is distinct from #49, which renders a preset screenshot rather than capturing the live framebuffer. The touch path was tested twice on CorOS 4.1: it opened the intended block's Parameter Editor, returned cleanly to the Grid, and left the preset hash, slot, scene, and dirty state unchanged.

Thank you again for the swift feedback and for prompting the split. We will continue submitting further additions only as small, independently reviewed and thoroughly tested PRs. Any further pointers on scope, API shape, priorities, or protocol expectations are very welcome.

[tony-xmelon]

Thank you for the swift and detailed feedback, Stokes. I pushed f64984b addressing the review: Version replies are filtered to the full device response; device-name presence/type handling is tightened; the undo/redo hardware test now creates and cleans up its own scratch preset and restores the original state; the controlled evidence wording and coverage tables are corrected; watchdog baselines are inclusive; and the optional fixture/readability nits are included. The complete offline suite is green (2,543 passed, 3 skipped), mypy is clean, and git diff --check is clean.\n\nI have also split the broader work into focused PRs and will submit further additions in similarly small, tested pieces. If you have any further feedback or pointers on scope, protocol evidence, or preferred API shape, they are very welcome.

[tony-xmelon]

@jonathanstokes All requested changes are now pushed, each inline comment has a specific response, and all 15 review conversations are resolved. When convenient, could you please re-review this PR? Thank you again.

[tony-xmelon]

Updated onto current main in b3cd54c and resolved the overlap with ADR-0020/#51 by keeping the maintainer's await_broadcast-based version() implementation and partial-identity semantics. The four contributed operations remain isolated here. Validation: 2,544 passed, 3 skipped; mypy clean; git diff --check clean. GitHub would not allow me to submit a formal review request through the API, so @jonathanstokes, could you please re-review when convenient?

[tony-xmelon]

Updated onto the merged ADR-0020/device-profile seam from #52 in 4dcafd8. The PR remains limited to the four operations and portability fixes; the new hardware-operation inventory now marks set_device_name, undo, redo, and inhibited_modules explicitly. Validation on the updated branch: 2,698 passed, 3 skipped; mypy clean; git diff --check clean. @jonathanstokes, could you please re-review when convenient? Thank you.

[tony-xmelon]

Synced with upstream �2331cc (including #53 and the new #54 contribution policy) at head 50238a1. I moved this back to draft so the required hardware suite can be rerun on this exact final commit before requesting review. Offline validation after the sync: 2702 passed, 3 skipped; mypy clean; git diff --check clean.

[tony-xmelon]

@jonathanstokes, thank you again for the detailed review. The requested split and follow-up changes are complete, the final head is cb4d96e, and the PR is marked ready with fresh offline and targeted hardware evidence in the description. GitHub does not permit this contributor account to invoke the re-request-review API here, so please treat this as the re-review request when convenient.

[tony-xmelon]

Synced with upstream main at 640a8e9 (including #63–#65). New head: 5b9bca4. Full offline Windows suite: 2727 passed, 3 skipped; mypy: clean across 46 source files; git diff --check: clean. The three shared Windows-only baseline checks remain isolated in #42.

[jonathanstokes]

Thanks @tony-xmelon. I re-reviewed this on f0ac2e40.

I did not run the hardware suite. Four things need changing first, and three of them came in with the September 19 sync from main rather than from your earlier work. I checked all fifteen threads from the September 3 review against the code instead of reading the replies, and those are genuinely addressed. This is new ground.

One thing to say up front, because most of what follows is about my unit. You do not have a 4.0.1 unit, and the full suite refuses on yours while the 4.1 snapshots are held back in #44. So 4.0.1 behavior was never something you could check, and I am not treating any of it as something you missed. Catching it is what the maintainer run is for. I am writing it down so the next push has it.

1. The connect-burst test cannot pass on my unit.

tests/hardware/test_model_state.py picks what to expect by counting factory models:

expected_announces = {412: 1, 420: 0}
assert factory_count in expected_announces

412 stopped being the 4.0.1 count on September 15. Model.hidden was read by presence, and that wrongly dropped two Bogna amps. tests/test_models.py now pins the count at 414 and explains why in its comment. The generator builds models.ALL from the same factory_models() call this assertion makes. So my unit reports 414, and the run fails on that line.

There was no way for you to see this. The number only bites on a 4.0.1 unit, your runs were on cb4d96e which is before the sync, and the offline suite cannot reach it either, since tests/hardware/ modules only have to import.

2. That assertion keys firmware behavior on a model count.

CLAUDE.md says anything that differs by firmware or model lives on the profile class, and nothing else tests a version string. A factory-model count is standing in for the CorOS version here. profiles.py is untouched, and the QuadCortex41 docstring already says what to do instead. 420 is the 4.1 count too, so this is 4.1 material that did not get split out. Easy to lose track of in a split this size.

3. A 4.1.0 observation is recorded only in a test docstring.

The note about four 4.1.0 connections carrying no announce answer should sit in docs/protocol.md beside the 4.0.1 record, dated and named. This is an easy one to miss, and it matters, because 4.0.1 and 4.1.0 disagree here, one announce against none. Right now that disagreement is not written anywhere a reader would look. Your 4.1.0 half of it is the part I cannot produce myself, so I do want it kept.

The simplest fix for all three is to put tests/hardware/test_model_state.py back to its base state here, and move the connect-burst observation to #44 where the profile and the 420-model catalog both live. None of the four new operations needs it.

4. The undo test can discard my unsaved edits.

test_undo_and_redo_reverse_and_reapply_a_scratch_edit builds its own scratch preset instead of using scratch_preset, which arrived in the same sync. The fixture opens with a guard the hand-rolled version does not have:

assert qc.preset_dirty() is False, "the loaded preset has unsaved edits; save or reload it first"

If I run the suite with unsaved edits on the loaded preset, the test saves those edits into the copy, then teardown recalls the original and discards them. ADR-0005 asks a successful run to leave the unit as it found it.

Your run passed, and mine would pass too on any day the loaded preset happens to be clean. The guard is for the day it is not. Switching to scratch_preset covers it, and it drops the second scratch name at the same time.

Smaller things, none of them blocking:

• tests/hardware/readme.md still says "Nothing here saves a preset" at line 78, and the undo test now saves one. The new paragraph went in at the top and the old sentence stayed. Your reply on that thread said the readme now covers the instrument tag and the default scene, and I could not find that part.
• Lines 27 and 32 of the name test still read custom_name with no HasField. I asked for all three reads and only the first got one. version() returns a partial reply on purpose, and a partial without the name makes that assert fail even when the restore worked.
• Transport.request(match=) has no caller. Main solved the double answer through await_broadcast(match=) while this was in flight, so the merge left your version behind. Drop it, or tell me why it stays.
• Six entries in UNMARKED_OPERATIONS have reasons that are now out of date. The new test drives list_presets, save_current_preset, delete_preset, read_preset, wait_for_listing and loaded_position, and their excuses still say no hardware test uses them.
• QuadCortex41.VERIFIED is still only create_local_backup. A 4.1 user calling set_device_name() gets refused by a method whose docstring says it is confirmed on 4.1.0. That one is on your side of the line, so you can settle it without me.

The four operations are good and the docstrings read well. I want this in.

This is waiting on you now. Once the hardware test file is back to base and the dirty-preset guard is there, I will run the suite on my 4.0.1 unit and we can finish it.

Stokes

[tony-xmelon]

Thank you for the careful re-review and for distinguishing the 4.0.1 findings from things we could have observed on our 4.1 unit.

All requested changes are now on 68be881:

• restored tests/hardware/test_model_state.py to the 4.0.1/base connect-burst behavior, removing both the stale 412 count and the count-as-firmware proxy;
• moved the 4.1 connect-burst observation into Add profile-owned CorOS 4.1 catalog and observations #44's protocol documentation, where the profile-owned 4.1 evidence belongs;
• rewrote the undo/redo hardware test around the shared scratch_preset fixture and its dirty-preset guard;
• removed the contradictory save claim and documented the scratch preset's instrument reset/default scene behavior;
• applied HasField("custom_name") handling to all three device-name reads;
• removed the now-unused Transport.request(match=) path;
• refreshed the six stale hardware-inventory reasons; and
• added set_device_name to QuadCortex41.VERIFIED.

The upstream September 21 changes are merged as well. Exact-head validation on Windows is 2968 passed, 7 skipped; mypy is clean; build and twine checks pass; and git diff --check is clean. I also fixed the new writing-test path normalization on Windows in this PR's portability scope.

The branch is ready for your 4.0.1 hardware run when convenient. Thanks again—the profile guidance and the new shared fixture made the end state materially clearer.

[jonathanstokes]

Re-reviewed on 68be881f, and I ran the hardware suite on my 4.0.1 unit. This is an approval.

The hardware run

Full suite, on this exact head, CorOS 4.0.1 / d14e:

============== operations on QuadCortex (CorOS 4.0.1, MAINTAINER) ==============
tests: 120 passed, 0 failed, 0 errored, 3 skipped; operations measured: 21 of 21 claimed
passed:                                 ['active_scene', 'clear_expression', 'create_local_backup', 'inhibited_modules', 'read_current_preset', 'redo', 'set_bypass', 'set_chain_input', 'set_device_name', 'set_expression', 'set_global_eq', 'set_hold_timing', 'set_input_port', 'set_param', 'set_scene_color', 'set_scene_label', 'set_tempo_mode', 'switch_scene', 'tempo_mode', 'undo', 'update_settings'] (21)
failed or skipped:                      [] (0)
passed, not VERIFIED:                   [] (0)   <- candidates to add
VERIFIED and claimed by a test, failed: [] (0)   <- regressions by name
120 passed, 3 skipped in 285.29s (0:04:45)

All four new operations passed and are named by a test: set_device_name, undo, redo, inhibited_modules. Nothing failed, nothing errored, and the run left the unit as it found it.

The four points from my September 21 re-review

I checked each against the code rather than the reply. All four are genuinely fixed.

1. tests/hardware/test_model_state.py is byte-for-byte what is on main. The 412 count is gone, and so is the assertion that carried it.
2. The model count no longer stands in for a firmware version. The revert took it with it.
3. The 4.1.0 connect-burst observation moved to #44. Nothing left here depends on it.
4. The undo test takes scratch_preset and gets the dirty-preset guard with it. It is the post-#76 fixture, keyed on the start-of-session flag, not a shape that only still imports.

Of the five smaller ones, three are fully fixed: the readme sentence, HasField on all three name reads, and Transport.request(match=) on the production side.

What I am not blocking on

None of the below can fail CI or the hardware run, and none of it changes what a 4.0.1 user gets. I am happy for this to merge as is and for these to come later, but they are all real and four of them are the shape this repo cares most about: a true statement replaced by a false one.

Two of the six refreshed reasons in UNMARKED_OPERATIONS are not true. This is my point (d), answered a little too eagerly. Suggestions inline.

docs/protocol.md now has two undo/redo rows. Line 2159 is mine, already on main, and it says the operations "land in PR #42". This PR added a second row at 2095 instead of updating that one. After merge a reader gets two different answers, and one of them says the work has not landed yet. The same stale forward reference sits in the prose at line 1786.

undo, redo and inhibited_modules are not in QuadCortex41.VERIFIED. You added set_device_name, which is what I named, but the sentence I wrote about it is now true of the other three word for word.

Three test doubles kept the match= you dropped from the real transport. Harmless today, and a trap later.

Thanks for this. The four operations are clean, the wire bytes are pinned both ways, and the evidence is in the docstring and the coverage table where it belongs.

[jonathanstokes]

wait_for_listing appears nowhere under tests/hardware/. _release_scratch calls delete_preset and nothing else, so the teardown does not use it and confirms no deletion. The reason it replaced was correct. The marker test only checks a reason is longer than 20 characters, so CI cannot catch this.

Suggested change

	"wait_for_listing": "scratch_preset teardown uses it to confirm deletion; no test independently verifies its polling and stability contract",
	"wait_for_listing": "no hardware test yet; it is the polling wrapper round list_presets and says nothing until a write changes a listing",

[jonathanstokes]

The teardown does not confirm absence. _release_scratch deletes unconditionally, and the fixture's own comment explains that a listing guard was tried there and removed on purpose, because list_presets is unreliable on a single READ. delete_preset also returns None rather than raising when the unit does not answer.

Suggested change

	"delete_preset": "the undo/redo test's scratch_preset teardown deletes its copy and confirms absence; no test isolates the delete wire shape itself",
	"delete_preset": "the undo/redo test's scratch_preset teardown deletes its copy, but the delete is deliberately unconfirmed and a teardown asserts nothing",

[jonathanstokes]

Splitting the semicolon left this sentence starting lowercase.

Suggested change

	other inherited operations refuse under ``Support.VERIFIED`` and run with
	Other inherited operations refuse under ``Support.VERIFIED`` and run with

[jonathanstokes]

This is the one thing I would most like settled, and it is yours to settle. What I wrote about set_device_name is now true of three more: on a 4.1.0 unit with the default Support.VERIFIED, qc.undo(), qc.redo() and qc.inhibited_modules() raise ControlNotDrivable, while their docstrings say they are confirmed on 4.1.0. Your evidence for those three looks the same as for set_device_name. If it is, promote them. If you are holding them back on purpose, say why here and I will take that.

[jonathanstokes]

This table already has an undo/redo row at line 2159, which I wrote: host undo / redo (undo() / redo() land in PR #42). Two rows for one operation, and they disagree on the verification column. The fix is to update 2159 and drop this one, rather than add beside it. Line 1786 needs the same treatment: its (PR #42, where undo() and redo() land) becomes false on merge.

[jonathanstokes]

inhibited_modules() goes through _read_state to await_broadcast, which raises TimeoutError when no matching broadcast arrives. There is no retry. So on the unit this comment describes, a dropped first READ fails the test after the timeout rather than being covered by the second pass. The loop repeats the assertion, which is still worth doing; it just is not what the comment claims.

Suggested change

	# Read twice because a first state READ can be dropped; both successful
	# Read twice so a second reply must also carry explicit fields rather
	# than protobuf defaults. A dropped first READ raises; there is no retry.

[jonathanstokes]

You dropped match= from the real Transport.request, but the doubles here, in tests/test_device.py and in tests/test_state.py kept it, and the last two implement the match semantics fully. The doubles are now wider than the thing they stand for: a later self._t.request(msg, match=...) in client.py would pass the whole offline suite and raise TypeError against a real unit. Worth removing from all three while it is still free. self.last_match on the next line has no reader either.