Skip to content

perf(audeze): answer every info capability from one status read - #550

Open
AdrianKuriata wants to merge 1 commit into
Sapd:masterfrom
AdrianKuriata:reuse-status-read
Open

perf(audeze): answer every info capability from one status read#550
AdrianKuriata wants to merge 1 commit into
Sapd:masterfrom
AdrianKuriata:reuse-status-read

Conversation

@AdrianKuriata

@AdrianKuriata AdrianKuriata commented Jul 29, 2026

Copy link
Copy Markdown
Contributor

getDeviceStatus() sends 15 init packets plus 6 status requests, each after a 60 ms
delay, and returns battery, chatmix, sidetone, equalizer and noise filter
together. But getBattery() and getChatmix() each call it, so asking for both pays
twice for one set of numbers.

Audeze Maxwell 2 (0x3329:0x4b28), release build:

headsetcontrol -d 0x3329:0x4b28 -b -o json      1.48 s
headsetcontrol -d 0x3329:0x4b28 -b -m -o json   2.83 s

With the status reused for 500 ms the second one drops to 1.41 s. The window is
much shorter than the 1.4 s read it saves, so nothing gets a value staler than its
own read would have produced. Reading both from one pass also makes them a
coherent snapshot instead of two readings 1.4 s apart.

Keyed on the device handle since DeviceRegistry keeps one instance per device
class, and mutex-guarded since that instance is shared - it is the only mutable
state in the class. Happy to drop the lock if you consider concurrent calls out of
scope.

headsetcontrol_tests passes.

Unrelated but visible if you test this: status reads on this device fail
intermittently on master too - five consecutive -b -m -o json runs gave
battery = -1, 74, -1, -1, 74 - so a flaky reading is not a regression from this
patch.

Independent of #549, different file and different cause; either can go in without
the other.

getDeviceStatus() returns battery, chatmix, sidetone and eq together, but
getBattery() and getChatmix() each called it. On a Maxwell 2 that made
-b -m -o json take 2.83 s instead of 1.41 s for one set of values.

Cached for 500 ms, keyed on the handle since one instance serves every attached
device, and mutex-guarded since it is shared.
@Sapd

Sapd commented Jul 30, 2026

Copy link
Copy Markdown
Owner

This one is clearly worth it, thanks. Setting read_at_ after the read instead of before is the right call.

Please drop the mutex. HeadsetControl is single threaded, there is no other lock in the whole codebase and I would rather not start here.

One thing I want handled though: the cache only expires by time, never when something is written. Right now only getBattery and getChatmix go through it and neither is affected by the setters, so nothing is broken today. But MaxwellStatus also carries sidetone, equalizer and noise filter, and setSidetone, setEqualizerPreset and setNoiseFilter all change exactly those. The moment someone adds a getter for one of them on top of statusFor, a set followed by a get within 500ms silently returns the old value and it will look like a firmware quirk, not like a cache. Either clear last_status_ in those setters or put a clear warning on statusFor.

Small thing, the doc comment sits above STATUS_REUSE_WINDOW so it reads as if it documents the constant. Move it down to statusFor.

I have no Maxwell 2 here, so I only verified that it builds and the tests pass and took your measurements as given.

For the intermittent status reads please open a separate issue, otherwise it gets lost in here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants