Apex keyboard support, and two rounds of audit fixes - #15
Merged
Conversation
A base station could sit in the UI as "connected" while showing nothing,
and only an unplug-replug plus an app restart brought it back. Three
things combined to produce that, all of them in the discovery path:
- Presence was emitted as soon as the hidraw node could be OPENED.
Opening proves a file exists, not that the device on the other end
speaks the protocol - so a node that never answered was announced as
a working headset and the UI waited forever.
- Only one candidate was ever tried. device::scan returned the single
best node and the supervisor retried that same node every three
seconds, so a silent one was a dead end rather than a reason to look
at the next.
- Which node won was decided by read_dir order, which is arbitrary and
can differ between boots. That is the part that makes the symptom
look intermittent and superstitious: replugging changes enumeration,
so sometimes the good node happened to come first.
Now discovery returns every candidate in a total order (priority, then
node number), and the supervisor rotates through them: open, send the
safe handshake, then wait up to three seconds for a frame the
generation's decoder actually accepts. Only then is anyone told the
headset is connected. A node that stays silent is logged and skipped.
Adds `inari doctor` for the case where it still goes wrong. It runs in
the caller's process - unlike every other verb, which is forwarded to the
running instance and prints into its log - so it works on a machine where
Inari will not start properly, which is when it is needed. It lists every
SteelSeries HID node with its product id, interface, descriptor prefix,
whether the table claims it and whether it opens, then per class the
candidates in try order and whether each answers. It writes nothing to
any device.
Also fixes two things the vendor's own specification exposed, both
verified against the hardware here:
- Command 0x47 is the stream mix - [main][unused][aux][mic] - not
"line out volumes" [left][right][aux]. The second slider was writing
into the unused byte and the microphone share could not be set at
all. Reading the audio-settings frame back confirms the layout, and
the station floors these to steps of five (42 reads back as 40, 99 as
95), so the values are snapped where they are built rather than
letting the UI show a number the device is not on. The sliders are
now Main / Aux / Microphone and are seeded from the device.
- The keyboard battery byte is the same encoding as every other
SteelSeries device (bit 7 charging, low bits in 5% steps), not two
BCD digits. BCD agrees at 95% and disagrees everywhere else, and
cannot express 100% at all. One decoder now serves all families.
Queries on the keyboard now match replies to the command that asked. The
device answers one behind - a live probe found 24 stale replies queued,
and a read after 0xBC returned the firmware string from an earlier 0x90 -
so a battery read could decode a byte of an ASCII version string into a
plausible-looking percentage. Acceptance is per command because the
device is not consistent about it: most replies echo the command, the
firmware query answers with bare ASCII and no echo.
449 Rust tests, 161 frontend tests, clippy -D warnings clean. Verified on
the attached base station, keyboard and mouse.
The switches tab was a placeholder built on a third-party write-up: it sent
0x2D, which the hardware accepts and ignores, and the page said Rapid
Trigger, Rapid Tap and Protection Mode "have never been captured by
anyone". That was true of the public reverse-engineering sources and false
of the hardware. The real commands are ordinary, and now implemented:
- 0x2F per-key actuation, a table of [hid, press, release]
- 0x37 Rapid Trigger sensitivity per key
- 0x36 release mode — what actually switches Rapid Trigger on
- 0x14 Protection Mode per key
- 0x17 Rapid Tap / SOCD
- 0x20 lighting config: brightness, idle brightness, idle timeout
- 0x29 power saving: sleep timeout, high-efficiency mode
- 0xA0/0xA9 read both configs back
Measured rather than assumed, on the attached board: writing 40 to a single
key made it trigger visibly later and 15 restored it, so the unit is tenths
of a millimetre and the switch's range is its full travel. num_keys is
honest — one key works, though the vendor driver always sends all seventy.
Four traps, each of which produces a feature that looks implemented and is
not:
- Rapid Trigger needs TWO commands. The sensitivity table alone changes
nothing; 0x36 is the switch, and its "on" value is 1 on wireless boards
and 2 on wired ones.
- The wired Gen 3 boards do not take millimetres at all — they take raw
sensor counts through a forty-entry table that is nowhere near linear
(2.0 mm is 50, not the ~100 a straight line between its ends gives) and
whose last rows are not even monotonic. It is transcribed verbatim;
interpolating would have put keys at twice the intended depth.
- The keyboard saves to flash with 0x11, not 0x09. 0x09 is the Arctis
base station's save command, which the write-up had copied across.
- Only lighting config takes fields one at a time, each behind its own
apply flag. That is what lets the idle timer change without disturbing
brightness — and it is why "the screensaver setting does nothing" was a
real bug: Inari's slider governed its own OLED blanking, while the
timer the user meant lives in the keyboard and was never touched.
The idle and power timers are read back from the board on connect and the
sliders show those, so they reflect what the keyboard is actually set to
even when something else changed it. The measured board reported 60 s to
dim and 5 min to sleep.
Docs corrected, including an explicit note that the earlier page was wrong.
459 Rust tests, 161 frontend tests, clippy -D warnings clean.
The four that were live on the connected board: * `lighting_config` (0x20), `power_saving` (0x29) and `rapid_tap_enable` (0x17) went out as feature reports. They are 65-byte output reports. The ioctl succeeds either way — the length happens to be valid — so the idle timer, sleep timeout, high-efficiency mode and the rapid-tap switch were set in the UI and not on the board. * Those same sliders then snapped back: the status is a connect-time snapshot the UI treats as the truth, so the next event handed the slider its pre-write value and the UI wrote *that* to the board. Successful writes are now mirrored into the status. * The EQ faders started at ten zeros and flattened the station's stored curve on first touch, with a flash save behind it. The 0x20 frame carries all ten bands at offsets 7..16 and was already being parsed for the stream mix; they now seed the faders. * Actuation, Rapid Trigger, protection mode and rapid tap were never re-sent after the board lost power. These writes deliberately skip the flash-save, so a power cycle silently returned every switch to its onboard value while the UI kept showing the user's numbers. And the rest: * The OLED was never handed back. `0x0B` (`0x69` on the wired and older boards) on the mode-off transition and on exit. * `brightness()` sent 0x22, which on these boards is `clear_direct_write` — a byte-identical valid command that wipes the direct-write buffer, with the percentage landing in the reply's error byte. Gen 2+ boards now set brightness through `lighting_config`; `zone_color` is dropped for them, since 0x21 there is the per-key direct write. * Painting one key set every other key to 1.5 mm, because a per-key value was thought to need a global baseline. 0x2F addresses keys individually. * The headset gain toggle was hardcoded to 0 and always read "Low", so clicking "Low" to toggle actually undid the change. The frame carries gain at offset 4. * One failure counter served two independent write paths: an animated effect zeroed it every 33 ms and hid a permanently failing panel, while an onboard effect never zeroed it and let transients spread over hours add up to a dropped connection. * `alt_envelope` and `oled_choice` latched for the process lifetime and survived a device swap. The envelope latch would have sent the next board the fallback shape, which the verified 0x1632 answers with EPIPE. * `release()` re-derived the model from the table and lost the firmware generation correction; the corrected model is now kept. * "Fill all"/"Clear" bypassed the 150 ms debounce, so a pending key colour landed afterwards and repainted that key with nothing left to tell the UI. The debouncer grows a cancel-by-prefix. * Swapping the mouse cable for the dongle leaves no gap where the device is absent, so the transport badge kept describing the connection that ended. The switches tab is now gated on boards whose firmware dialect Inari actually knows. The vendor specification gives the Gen 3 wired boards an entirely different set of opcodes, a different per-key struct order and a driver-mode command around the whole exchange — the existing implementation would have produced a complete-looking tab that moves nothing. Same gate on the 0xA0/0xA9/0xF5 connect reads, which cost a 700 ms timeout each on boards that do not implement them. Also corrects the keyboard battery decode in the docs: 0x95 is 100 % on the cable, not 95 % in BCD.
Outright defects first — these are not taste: * Three CSS variables were never declared. `--fs-small` (4 rules) made the scene names fall back to 15px body text while the hint directly under them sat at 10px, which is where the broken hierarchy in the scene tiles came from; `--warn` landed on a hardcoded orange although `--warning` exists; `--fs-title-lg` did the same to the dialog title. * `className="rtext"` is used five times in the keyboard screen and no such rule exists. Without `flex: 1; min-width: 0` the toggle sat against the text instead of the card's right edge. The real class is `rmain`. * `--font-mono` resolved through Tailwind's default theme to a foreign stack, so the keyboard's inputs were the only ones in the app not in Fira Code. * The effect chip said "Twelve themed scenes"; there are 24. * The per-key actuation panel still warned that the command "changed nothing on the one board this could be tested against". It was verified on that board. Tokyo Night's `--fg-muted` was `#565f89` — that palette's colour for comments and rules, used here as a text colour at 2.76:1 on `--bg-surface`. It carries roughly every second line of text in the app. Lightened to the same hue at 5.78:1; the original stays as `--line-muted` for the ticks and rules it was meant for. Keyboard screen, which had grown to eight cards in one column — two of them visible at the default window size, with the key map starting around 800px down: * Sub-tabs in the head (Lighting / Keys / Display / System), the last two gated on what the board actually has, falling back if the open tab disappears with the device. * The key map is its own card at the top of the tabs that can change it. It is the only picture of what is lit, and it used to be four cards below the chips that change it. * The "Scenes" effect chip is gone. It set `kind: "scene"` without choosing one, silently restarting whichever scene was picked last; the card below is where a scene gets chosen. The EFFECTS entry stays — the colour picker and speed slider read `colors` and `speed` from it. * With the master switch off the cards the backend ignores are now inert and say so, instead of showing active states for controls that reach nothing. * Origin badges replace three prose hints: live from Inari, stored in the board, or sent to the board and volatile. * The transport comparison and the raw command probe fold into `<details>`. They are diagnostics, and open by default they cost ~350px of the column. * Scene tiles carry their own palette as a tint and a 12px band instead of a 4px strip over neutral grey, with the running scene marked on the tile. Both equalizers: * The hardware EQ's ten faders showed no value at all. They now carry their dB reading, a 0dB line with ±5 references, and an axis — the EQ that actually lives in the headset was neither readable nor reproducible. * The software curve spanned ±24dB although the loudest bundled preset peaks below 9.2, so a +6dB shelf drew 25px tall. The axis starts at ±12 and opens up only for curves that need it. * Each band draws its own response under the sum, so scrolling a dot to change Q is visible, and a label follows the pointer while dragging. And the rest: * `.hs-body` never matched `.screen-head`'s centring, so on a 1280px window the title started 242px in and the first card at 20px. Device screens get their own column width — 720px is a form measure, and these screens carry pictures of hardware. * Battery rows render a placeholder instead of nothing, so the card no longer grows after the first status frame. "Charging" gets its own tag and a striped bar rather than the same grey as "Offline". * Status values are tinted by state instead of setting "Unpaired" and "Muted" in the same weight as "Paired" and "Live". * The mixer's drag grip keeps a resting visibility. That strips can be reordered is something you have to be told; deleting one is not. * Channel/mix colour coding and the danger and online tints were raw hex in the rules, so the theme picker moved the accent while the strips stayed violet. All of it is tokens in both theme blocks now. * Preset lists get a filter that also matches descriptions, and the descriptions show as a second line rather than only in a tooltip. The hardware EQ's categories become a visible row of switches with grouped headings instead of a dropdown in the card header. * Type scale: `--fs-h2` to 24, `--fs-title` folded into `--fs-body`, and `--fs-micro`/`--fs-nano` for the fifteen rules that had been picking their own 9-11px.
Three sets of near-duplicates that had drifted: * `.card` and `.hs-card` both wrap the same `.row` but set different padding and a different radius, so the same row began 22px from the card edge in Settings and 30px on a device screen, with corners 2px apart. The shared look is declared once; `.hs-card` keeps only what genuinely differs, which is that it lays its children out with a gap instead of relying on `.row + .row`. Six inline `padding: var(--sp-2)` styles that repeated what `.card` already sets are gone with it — they would have overridden the shared rule from the outside. * `.media-seek-fill` and `.media-seek-cap` were byte-for-byte copies of `.hs-fill` and `.hs-cap`, and `.media-seek-track` differed from `.hs-track` in one declaration. Merged, with `flex: 1` kept as the one real difference: the headset's tracks share a row with a label and a value, the seek bar owns its line.
Setting a shelf band to a steep slope put both poles exactly on the unit circle. The band then never stopped ringing: a measured 77 Hz tone for a 100 Hz corner, at an amplitude above full scale, continuing after the audio stopped and lasting until the setting was changed. The cookbook's `alpha` carries `sqrt((A + 1/A)(1/S - 1) + 2)`. That radicand goes negative once `A + 1/A > S/(S - 1)`, and the code clamped it at zero to avoid a NaN. That avoids the NaN and produces `alpha == 0`, at which point `a0 = ap1 + am1*cos - 0` and `a2 = ap1 + am1*cos + 0` are the same expression: after normalisation a2 is exactly 1.0, so the poles multiply to exactly 1. The zeros do not cancel them. This is not an extreme corner. At slope 10 it starts at ±8.1 dB of shelf gain, at slope 5 at ±12 — an ordinary steep bass boost, and cuts as well as boosts, since A and 1/A are symmetric. The UI offers slope up to 10 and gain up to ±24. Clamp the slope to what the gain can carry instead, leaving the radicand alone, and mirror the same limit in the curve the UI draws so what is shown stays what is heard. Two tests, both of which fail on the old code: * Every (kind, gain, Q, rate, frequency) the UI permits must keep its poles inside the unit circle. The bar is `|pole| < 1`, not a comfortable margin: a narrow peaking band legitimately reaches 0.99999 and that is what such a filter is. On the old code this reports a radius of exactly 1. * A shelf on the settings that were measured to ring must fall silent after its input does. On the old code the output still sits at 7.08 — seven times full scale — five seconds into the silence.
The strips were a cache that was filled once, at startup, and never again. `adopt_live_channel_state` exists precisely for this and its own doc comment describes the symptom — "a channel restored to 0% used to show 100% in the UI and be silent with no visible reason" — but it was only ever called from `init_virtual_devices`. Anything that changed a volume outside Inari left the fader showing a number the sink had stopped using, and the same one-shot lost the reading entirely when the first Props event had not arrived yet at init, with no second try. Measured on this machine while writing this: PipeWire had sink_system at 0.0 and sink_music at 0.6585 (87%), while the stored profile carried 100 and unmuted for both. One channel was silent behind a fader reading 100%. `get_virtual_devices` now refreshes from the sinks before answering — an in-memory read of what the loop thread already mirrors, no server traffic — and the Props listener announces volume and mute changes the way the info handler beside it already announces its own fields, so the UI is pushed as well as pulled. Three variants of the same mistake in the frontend, each with a regression test that fails without its fix: * `set_mic_config` writes the whole struct, and the payload was frozen when the call was made rather than when the debounce fired. Pressing the mic-mute hotkey during a gain drag reopened the microphone while the UI, the tray and the status label all still said "Muted". Built at send time from the current store now. * `state-changed` fires for a hotkey, the tray, the CLI, a tablet, even showing the window, and each one refetches the channels. During the 90 ms debounce the backend still holds the old volume, so the refetch handed the fader its old position back while the write went out with the new one — sink and strip diverged permanently, because nothing refetches again. Channels with a write in flight now keep the user's value. * A `media_status` read already in flight when the user taps pause carries the state from before the tap, and the backend shells out to playerctl so it is easily the slower of the two. The button flipped straight back. Optimistic edits bump a counter that stale reads check. And the headset screen's mix and EQ seeds are one-shot by design but were never re-armed, so the next base station kept the previous one's values on screen while its own sat unread in the status.
`drain` is a `swap(0)`, so reading a peak destroys it. Three loops read the same store — the keyboard's audio-reactive lighting every 33 ms, the OLED's VU mode every 40, the window's level emitter every 100. Whichever ticked first took the peak; the others got whatever had accumulated since. With a lit keyboard and the OLED in VU mode, the meters in the window were showing the leftovers of the other two. Each consumer now has its own peak array and asks for it by name. `raise` writes all of them, registered or not: taking only the registered ones would make the result depend on whether a consumer happened to tick before the audio started. It stays lock-free and fit for the realtime callback — a handful of relaxed compare-exchanges on cache lines the thread just touched. Two more from the same corner: * `MAX_METERS` was 12, but the UI allows 10 channels plus four buses plus the master, and the channel and bus sinks claim their slots before the microphone chain is built. A full configuration made the *microphone* fail with "meter budget exhausted", which is nowhere near where the limit is. It now derives from `MAX_CHANNELS + MAX_BUSES`, with a const assertion so raising either limit breaks the build instead of the microphone. * Turning the microphone chain off never released its meter slot, leaving a permanently silent "MIC" bar in the OLED's VU mode — which divides the panel by the number of registered names, so it also made every real channel shorter. And `DestroySink` only cleared five of the nine maps keyed by node name. Node names are derived from the label and deterministic, so deleting "Game" and creating it again produces `sink_game` a second time and inherits whatever was left: the failover-off flag survived that way, so a fresh channel came up with automatic fallback disabled while the UI, which had correctly forgotten the setting, showed it as on. All nine now go through one `forget_channel`, so the next one added has a single place to be added to.
Global ids are recycled hard — measured on this machine, the highest live id is 242 while `object.serial` is at 183684, so the id space has wrapped hundreds of times this session. Anything that holds an id across a delay can act on whatever inherited the number. Two places did: * `set_app_volume` is debounced by 90 ms, and browsers create and destroy a stream per media element. Drag an app's fader while its stream ends and the write lands on the new owner of that id. The UI now sends the serial it was looking at, and the command resolves it to the current index immediately before acting; a stream that has genuinely ended is an error rather than a write to a stranger. `route_app_to_channel` the same. * The auto-routing ledger was keyed by index, and its "forget streams that have gone away" pass cannot tell a recycled id from a surviving one. Discord closing and Spotify opening onto the same id between two polls made Spotify look already-handled, and it never reached its channel for the rest of its life. The window is not small: with the window minimised the poll stops entirely. `StreamKey` carries the serial where there is one and falls back to the index for the pactl backend, which does not expose serials. Underneath both, the loop thread now refuses `SetNodeVolumeById` for a node that is not an application stream. If an id has been recycled onto a sink or a device, writing an app's fader value onto the headset's output node is exactly the failure worth refusing.
**The restart still died, by a different route.** 1.0.11 fixed the case where we run as our own unit and systemd killed the helper in the cgroup sweep. Launched from the application menu, KDE wraps us in a transient `app-<id>@<hash>.service` with `KillMode=control-group` — verified here for Dolphin, kitty and Chrome — and `running_as_unit()` deliberately returns false for a launcher scope, so the `setsid` branch ran. `setsid` leaves the session but stays in the cgroup, so the relauncher was swept a second after being spawned and the app never came back. It now goes through `systemd-run --user --collect`, which puts it in a transient unit of its own; `setsid` and a plain shell remain as fallbacks for systems without systemd, where nothing sweeps a cgroup in the first place. **The download directory was predictable and shared.** `/tmp/inari-update` is a fixed name, `/tmp` is world-writable with the sticky bit, and the service runs without `PrivateTmp`. Another local user creating that directory first turns `create_dir_all` into a no-op and the `set_permissions` that follows into a swallowed `let _ =` — leaving the `.deb` somewhere they control between the SHA256 check and `pkexec apt-get`, where swapping it installs their package as root. The directory is now created exclusively with `create_dir`, under a name carrying the pid and a clock reading, and failing to make the package readable is an error rather than something to shrug at. **The tray could not un-minimise the window.** `show()` is a no-op for a window that is visible but minimised, and `set_focus()` does not restore one under KWin. The single-instance handler had `unminimize()` and the tray's "Show Window" did not, so clicking it did nothing at all. Both go through one `reveal_window` now.
Three variants of the same omission — the listener, the permit, the
connection — plus the contract test that should have caught the fourth.
* `stop()` signalled and returned; the listener closed on another thread.
Every caller — regenerating the token, changing the bind address, the
enable toggle — binds the same port again immediately afterwards, so
whether that worked was a thread race. It almost always won on an idle
machine and lost 25-70% of the time under load, which is exactly when
someone reaches for the tablet. Losing meant "Address already in use", a
dead server, and the remote switched off behind the user's back. `stop`
now joins the server thread with a 2 s ceiling.
* The in-flight permit was held across `replies.send()`. It is meant to
bound how many handlers *run*; holding it over the send made it bound how
many wait for one client's TCP back-pressure. There are eight for all
clients together, so a tablet that keeps sending and stops reading filled
its 32-slot reply channel, parked all eight handlers, and every other
client stopped getting answers. The permit is released before the send,
and a client that will not drain its replies is now dropped rather than
waited on.
* The idle deadline was only checked in `poll_read`. A connection stuck in a
write is precisely one where `poll_read` is never polled, so it outlived
the 90 s timeout indefinitely against a peer that acknowledges without
reading — holding one of the 64 slots, keeping `Clients::leave` from ever
running, and keeping the level and graph emitters pushing at 10 Hz into a
window nobody was looking at.
`is_read` guesses from the name prefix, and the media reads do not fit the
pattern, so a tablet sitting on the media screen — polling once a second —
made the desktop and every other tablet re-read channels and microphone
every second for nothing. They are named explicitly now, along with the
three `headset_get_*` reads. `headset_oled_status` stays a write on purpose.
And the contract test only saw `call("literal")`, so everything routed
through a helper was invisible to it — which is the normal shape in the
mixer store, where all five write commands go through `debouncedInvoke`.
The proof that the net had holes is in the test itself: three of the names
in its exemption list were not real commands, and nothing noticed, because
the scanner never produced them either. It now reads the `Command` union,
which `ipc_contract` already pins to `generate_handler!`, so indirection
cannot hide anything from it — and it immediately found a real gap,
`headset_oled_media`, which is correctly hidden behind `isTauri` in the UI
but had never been written down as deliberate.
…rine
**Deleting a channel committed before it could fail.** `remove_channel` took
the definition out of `channel_defs`, then evacuated the streams, then
destroyed the sink — and returned on failure without putting anything back.
The two halves of the mixer then disagreed with immediate consequences and
no file written: the strip stayed on screen while `teardown_virtual_sinks`,
which iterates `channel_defs`, never destroyed the sink at exit; the output
and failover rows vanished from that strip; monitoring refused it. The next
unrelated save made it permanent while assignments.json and the WirePlumber
fragment still pointed apps at a channel that no longer existed. Validation
is now a read (`can_remove`), and both halves are committed together once
the parts that can fail have succeeded — the same shape `add_channel` has
had all along.
**A new channel could come up silent.** Channel names are derived from the
label, so recreating one you deleted produces the same node name, and
WirePlumber remembers levels per node name and never forgets them — this
machine still has `sink_system={"channelVolumes":[0.0]}` stored for a
channel that no longer exists. Its restore lands after our write, so the
strip claimed 100% over a silent sink. `add_channel` now reads back what the
sink ended up at, insists once if something moved it, and shows what it
actually got.
**Two existence checks asked the parser instead of the filesystem.**
`create_blank_profile` used `load(..).is_ok()`, which answers "is there a
*readable* profile" — so a profile that had become unparseable read as
absent and was silently overwritten, while remaining visible in the list the
whole time because `list` works off filenames. Saving an EQ preset had the
same shape and now refuses to replace an existing one, which is what "save a
copy as a new preset" meant anyway.
**Three stores ignored the project's own schema doctrine.** json.rs requires
a `version` and a `#[serde(flatten)] extra` on every store so an older Inari
can load a newer file and save it back without eating what it did not
understand. Eight stores comply; `Channels`, `ChannelDef` and `Profile` did
not — and those hold the channel set itself.
**Autosave writes could land out of order.** The snapshot is taken under the
mixer guard and written outside it, which is deliberate (an fsync under the
guard stalls every other command, TD-004) but removed the only thing making
writes happen in snapshot order. With a second thread in play — the remote's
runtime, the hotkey callback, the CLI's D-Bus thread — the slower writer
wins the `rename` and puts an older whole-profile snapshot on top of a newer
one. Snapshots now carry a sequence handed out under the guard, and the
writer holds a lock of its own and drops anything older than what is already
on disk.
**And the stream-mix migration was dead code.** It wrote the old
per-channel `stream_mix` flags into the *master* mix, which carries every
channel by definition and gets its membership rewritten by `sync_master` at
init. A user who had deliberately kept music out of the recorded stream
silently got it back on the first start after upgrading. It becomes a mix of
its own now — which is what the flag meant, and which `sync_master` leaves
alone.
Every claim here was checked against the code as it now stands, and the entries say plainly what the old behaviour was — the point of a troubleshooting page is to be findable by someone still running the version that has the bug. * **eq** — shelf slope is capped by the gain, and why. The dB axis is now adaptive and each band draws its own curve. * **mixer** — volume and mute follow the sinks continuously rather than being read once at startup, with the deliberate exception that a newly created channel is put back to 100% rather than inheriting the level WirePlumber remembers for a channel of the same name that used to exist. * **mixes** — the pre-mixes `stream_mix` flag now migrates into a mix of its own instead of into the Master Mix, which was overwritten immediately. * **headset** — the hardware EQ faders start from the station's own curve and show their values; the gain control shows the real state; the line-out section is the stream mix, including the microphone share. * **keyboard** — the screen's four sections and why the key map sits above the controls that change it. * **configuration** — every store now carries a schema version and a catch-all, and atomic writes are not ordered writes: the autosave sequence is explained. * **remote** — regenerating a token no longer races itself for the port, and one wedged client can no longer starve the others. * **troubleshooting** — two new entries (a shelf band that rings for ever; meters under-reading while the keyboard or OLED is lit), and the "didn't restart after update" entry now covers all three causes, since it has been fixed three times for three different reasons. * **protocols** — the `0x20` audio-settings frame documents the fields Inari was ignoring.
…cons **The channel refresh was in the wrong place.** The previous commit taught `get_virtual_devices` to read the sinks before answering, which fixed the window and nothing else: `mixer.channels` is also read by the tray's mute rows and by `inari status`, and neither goes through that command. Whether the tray told the truth depended on whether the window happened to be open. Caught by installing the build and comparing `inari status` against `pw-dump` — the CLI still reported 100% for a sink sitting at 0%. The read now lives in one `AppState::refresh_channel_state` that all three call. **Symlinked icons never loaded.** `/usr/share/pixmaps` is mostly links into each application's own directory, and the asset protocol matches its allowed scope against the *canonical* path — so the scope was checked against a path the caller never handed it. Chromium's icon worked because its link points back into `/usr/share/icons`, which was in scope; Discord's points at `/usr/share/discord/discord.png`, which was not, and the icon silently failed with one line in the log. Two halves to that: icon lookup now canonicalises before handing the path over, so the path we pass and the path that gets checked are the same thing, and the scope covers where those links actually point. The scope is correspondingly narrower in the dimension that matters — it was any file under a handful of icon directories, and is now only `.png`, `.svg` and `.xpm` under the share trees. `**` matching direct children was checked against the glob crate rather than assumed; that was my first theory for this bug and it was wrong.
`inari status` showed "Microphone sink_mic 94%", but 94% is the gain of Inari's microphone chain, not that node's volume — `sink_mic` was sitting at 8% at the same moment. Printing the node name beside it invited exactly the comparison against `pw-dump` that it fails, which is how I noticed. The number was always right; the column next to it was not.
The shelf-stability test swept gain per dB and Q per 0.1, which is 9,800 configurations and six seconds of curve evaluation. That fit inside vitest's 5 s default on this machine and did not on a CI runner, so it timed out. The unstable region is an area, not a knife edge — every combination past the boundary fails, not just the ones next to it — so a 3 dB / 0.5 Q grid still crosses it many times over. The combinations that were actually measured to ring are now asserted explicitly rather than left to the grid to happen to land on. Verified the faster test still fails on the unfixed code: reverting the slope clamp puts it at 69.8 dB where the bound is 60. 6025 ms to 47 ms.
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
SteelSeries Apex keyboard support, and the results of two adversarial audit
passes over the parts of the codebase that had never been systematically
reviewed.
Keyboard
Per-key RGB, the 128×40 OLED, 24 themed scenes, and the switch settings the
firmware actually exposes — verified against an Apex Pro TKL Wireless (2023)
over
/dev/hidraw, not inferred from third-party write-ups.The protocol notes in
website/reference/protocols.mdrecord what wasmeasured, including the parts where the existing public documentation is
wrong: the apply command is
0x11and not0x09, the third-party0x2D"actuation" command is accepted by the hardware and does nothing, and the
real per-key table is
0x2Fin tenths of a millimetre.The switches tab is deliberately limited to boards whose firmware dialect
was measured. The vendor specification gives the Gen 3 wired boards an
entirely different opcode set, a different per-key struct order and a
driver-mode command wrapping the exchange; guessing one dialect from the
other would have produced a complete-looking tab that moves nothing.
Audit results
Two rounds, each finding candidates and then trying to disprove them. The
first covered the device layer, the second the mixer, the PipeWire core,
persistence, the shell and updater, the remote and the frontend stores.
Roughly half the candidates did not survive the disproof step, which is what
makes the rest worth reading.
The ones that mattered most:
A shelf EQ band could become an oscillator. Clamping a negative radicand
to zero avoided a NaN and produced
alpha == 0, at which point the biquad'sa0anda2are the same expression — both poles exactly on the unitcircle. The band then rang for ever: measured at 77 Hz for a 100 Hz corner,
above full scale, continuing after the audio stopped. At slope 10 it started
at ±8.1 dB of shelf gain, so an ordinary steep bass boost reached it. The
guard against one failure was the other failure.
Channel volumes were read once and never again. Measured while writing
this: PipeWire had
sink_systemat 0.0 while the stored profile said 100%and unmuted — a silent channel behind a fader reading 100%, with nothing on
screen to explain it. Three separate readers shared that stale cache.
Application streams were addressed by a recycled id. On a running
desktop the PipeWire global id space wraps hundreds of times per session
(measured: highest live id 242,
object.serialat 183684). A debouncedvolume write could land on whatever inherited the number, and the
auto-routing ledger could mark a new stream as already-handled for the rest
of its life.
Four reads that overtook optimistic writes, in the mixer, the microphone
config, the media store and the channel list — one mistake made in four
places. The regression tests for them live in one file, with the pattern
named at the top.
The post-update restart died again, for the third time and the third
reason: launched from the application menu, KDE wraps the app in a transient
unit with
KillMode=control-group, andsetsidleaves the session but notthe cgroup.
The remote never waited for a resource to actually be free — the
listener, the in-flight permit, the connection. Regenerating a token raced
itself for its own port; on an idle machine the teardown almost always won,
under load it lost between a quarter and two thirds of the time.
Full list with file references in the commit messages.
Interface
The design pass found three CSS variables that were never declared and a
class name used five times that does not exist. It also found that Tokyo
Night's
--fg-mutedwas the palette's colour for comments and rules beingused as a text colour, at 2.76:1 against the surface it sits on — carrying
roughly every second line of text in the application.
The keyboard screen had grown to eight cards in one column, of which two
were visible at the default window size; it is now four sections with the
key map above the controls that change it. Both equalizers gained the thing
they were missing: the hardware one showed no values at all, and the
software one spanned ±24 dB when the loudest bundled preset peaks below 9.
Verification
474 Rust tests, 168 frontend tests,
clippy -D warningsandtscclean.Every fix that could be tested has a test that fails without it — several
were checked by reverting the fix and watching the test go red, and one
regression test caught a mistake in its own fix within a minute of being
written.
Where a claim could be checked against the running system it was: the volume
divergence against
pw-dump, the restart path againstsystemd-run, theglob semantics against the
globcrate rather than assumed — that last onedisproved my first theory for the icon bug.