From 6d6cfcedfcb8311ff0354b5a88c7abe61672b0c8 Mon Sep 17 00:00:00 2001 From: ccross Date: Mon, 24 Aug 2026 16:52:47 -0400 Subject: [PATCH] docs: the quirk contribution workflow was stale, and one shipped quirk was undocumented MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit Closes #84. Closes #85. @rampa3 reported that docs/hardware-compatibility.md still documents the OLD quirk workflow. Confirmed: it listed three steps, omitting both the registration in crates/visage-hw/src/quirks.rs and the tests. Following it produces a quirk that is embedded nowhere and read never. That is the same drift this repository has now hit five times — one process, two documents, one updated. The registration step and its guards were added to contrib/hw/README.md on 2026-08-17 and the compatibility doc was never touched. A contributor reading the wrong one ships an inert quirk, and the failure is silent by design: quirk_db() skips an unregistered entry rather than panicking, because the daemon authenticates logins and one bad contribution must not stop it starting. At runtime an unregistered quirk is indistinguishable from no quirk. #85 asked for the disable-external-tools and power-cycle test to be recommended before submission. Added to both documents as an explicit step. It is a real negative control — it is what made #76 trustworthy, since @rampa3 ran it on their own contribution before submitting. FOUND WHILE HERE, unreported: the quirk table was missing an entry. contrib/hw/ ships five quirks and quirks.rs registers five; the table listed four. The missing one is 04f2:b6d0, the Lenovo ThinkPad P14s Gen 2a — @rampa3's own contribution from #76, and the project's first external hardware quirk. An owner of that laptop reading the compatibility page would have concluded their camera was unsupported when it has shipped since v0.4.0-rc.1. Both documents now describe the same six steps, verified equal, with contrib/hw/README.md named as authoritative. An earlier revision of this change left them at six and five, which would have been the very defect being fixed. Signed-off-by: ccross Co-Authored-By: Claude Opus 5 (1M context) --- contrib/hw/README.md | 11 ++++++++++- docs/hardware-compatibility.md | 32 +++++++++++++++++++++++++++++++- 2 files changed, 41 insertions(+), 2 deletions(-) diff --git a/contrib/hw/README.md b/contrib/hw/README.md index 8de3466..c139218 100644 --- a/contrib/hw/README.md +++ b/contrib/hw/README.md @@ -47,7 +47,16 @@ The `control_bytes` values are found via `linux-enable-ir-emitter configure` or ("04f2-b6d0.toml", QUIRK_04F2_B6D0), ``` -5. Run `cargo test -p visage-hw quirks::` and submit a PR +5. **Verify the quirk is doing the work, not another tool.** Disable any external emitter + activation tool (`linux-enable-ir-emitter` and similar) and **power-cycle the laptop or the + camera** to clear residual control bytes, then test. A camera left illuminated by something + else will make a non-working quirk look correct, and the contribution ships inert. + + This is a real negative control, and it is what made [#76](https://github.com/sovren-software/visage/pull/76) + trustworthy — @rampa3 did exactly this before submitting, then suggested documenting it + ([#85](https://github.com/sovren-software/visage/issues/85)). + +6. Run `cargo test -p visage-hw quirks::` and submit a PR Quirk files are embedded at compile time via `include_str!` — there is no runtime file loading, so **dropping a `.toml` into this directory does nothing on its own.** A file that is not diff --git a/docs/hardware-compatibility.md b/docs/hardware-compatibility.md index dfe57d2..98d4df3 100644 --- a/docs/hardware-compatibility.md +++ b/docs/hardware-compatibility.md @@ -103,6 +103,7 @@ VID:PID to the correct control bytes for each known device. | Device | VID:PID | Status | |--------|---------|--------| | ASUS Zenbook 14 UM3406HA | `04f2:b6d9` | ✅ Verified on hardware | +| Lenovo ThinkPad P14s Gen 2a 21A0000RMX | `04f2:b6d0` | ✅ Verified on hardware (community, [#76](https://github.com/sovren-software/visage/pull/76)) | | Lenovo ThinkPad X1 Carbon Gen 9 20XW00FPUS | `174f:2454` | ✅ Verified on hardware | | Lenovo ThinkBook 14 MP2PQAZG | `30c9:00c2` | ✅ Verified on hardware | | HP OmniBook X Flip | `30c9:0120` | ✅ Verified on hardware | @@ -124,7 +125,36 @@ lit/unlit every frame. A dark-frame *count* cannot distinguish that from an expo 1. Run `visage discover` to find your camera's VID:PID 2. Use `linux-enable-ir-emitter configure` or UVC descriptor analysis to find the control bytes (see [contrib/hw/README.md](../contrib/hw/README.md)) -3. Create `contrib/hw/{vid}-{pid}.toml` and submit a PR +3. Create `contrib/hw/{vid}-{pid}.toml` +4. **Register it in `crates/visage-hw/src/quirks.rs`.** Quirk files are embedded at + compile time with `include_str!` — there is no runtime file loading, so **dropping a + `.toml` into `contrib/hw/` does nothing on its own.** Two lines: + + ```rust + const QUIRK_04F2_B6D0: &str = include_str!("../../../contrib/hw/04f2-b6d0.toml"); + // …then add it to QUIRK_SOURCES: + ("04f2-b6d0.toml", QUIRK_04F2_B6D0), + ``` + +5. **Verify the quirk is doing the work, not another tool.** Disable any external emitter + activation tool (`linux-enable-ir-emitter` and similar) and **power-cycle the laptop or the + camera** to clear residual control bytes, then test. A camera left illuminated by something + else will make a non-working quirk look correct, and the contribution ships inert. This is a + real negative control — it is what made [#76](https://github.com/sovren-software/visage/pull/76) + trustworthy, and @rampa3 suggested documenting it in + [#85](https://github.com/sovren-software/visage/issues/85). +6. Run `cargo test -p visage-hw quirks::` and submit a PR + +⚠️ **Step 4 is the one that is easy to miss, and its failure is silent.** `quirk_db()` +skips a malformed or unregistered quirk rather than panicking — the daemon authenticates +logins, so one bad contribution must not stop it starting. The cost is that at runtime a +camera with an unregistered quirk is **indistinguishable from a camera with no quirk**: no +error, no crash, the emitter simply never fires. The tests in step 5 exist precisely to +catch that before it ships. + +The full field reference, the registration step and what each test catches are in +[contrib/hw/README.md](../contrib/hw/README.md), which is the authoritative version of this +workflow. ---