Fix TWS battery, rfcomm crash, quality/speed selector, equalizer fetch and macOS icon - #6
Open
zezic wants to merge 6 commits into
Open
Fix TWS battery, rfcomm crash, quality/speed selector, equalizer fetch and macOS icon#6zezic wants to merge 6 commits into
zezic wants to merge 6 commits into
Conversation
requestBattery() probed the single-battery sub-type first and returned on any reply, so TWS earbuds never reached the dual probe and L/R stayed unset. Probe order now follows the model name (WF-*/LinkBuds try dual first), with a fallback to the other layout either way. - add DUAL2 (0x01) for the WF-C500/C510/C700N line - keep the per-earbud charging flags instead of discarding them - treat a level of 0 as "not reporting" (docked), not a flat battery - read the charging case (0x0a) - re-read battery on the dynamic poll so it stops going stale (v2 only: 0x22 is POWER_OFF on v1) Sub-types and byte offsets verified against a WF-1000XM5 by docking one earbud and watching byte [4] track it; captured frames are in Constants.h.
rfcommchannel was a non-owning __bridge pointer that was never cleared when the link dropped, so a poll already in flight would writeSync: to a released channel and crash (EXC_BAD_ACCESS / pointer auth failure). Docking an earbud was enough to trigger it. Retain the channel and clear it under a mutex, so a concurrent send sees either a live channel or nil; send() now raises a recoverable error instead. Also fixes a self-join: the RFCOMM delegate runs on uthread's own run loop, so rfcommChannelClosed -> disconnect() was joining that thread from itself. recv() now wakes on disconnect rather than waiting out its full timeout.
The icon was a full-bleed square, which looks wrong in the Dock. Reshape to Apple's grid: 824x824 body centred in 1024, continuous (not circular) corner curve, subtle drop shadow. Artwork unchanged.
A short audio dropout was enough to desync the response stream for the rest of the session: changing NC/Ambient or an EQ preset reverted a second or two later, and DSEE wouldn't toggle at all. _readMessage() left the parser stranded whenever a chunk arrived garbled or truncated - bytes before the first START_MARKER were concatenated into the message, and a second start marker threw. 60/61/62 are escaped inside a frame, so a start marker unambiguously begins one; resync on it in both cases, and cap the buffer so a stream that never delivers an END_MARKER can't grow it forever. _waitForAck() read exactly one frame and assumed it was the ACK. The device pushes unsolicited notifications on the same stream (a state change, a press of the headset's own button), so it could take _seqNumber from the wrong frame - and the device dedupes on that sequence number, silently discarding the next command as a duplicate. Wait for an actual ACK frame instead; skipped notifications are still acked back by _readMessage(). Also: - requestAmbientState() matches sub-type 0x17 rather than any 0x67, so a notification with a different layout is no longer parsed as the reply - the read-back no longer overwrites a change that hasn't been sent yet. It cleared the pending change too (isChanged() went false), so the command was never sent at all. discardAmbientChanges() releases that state when a write genuinely fails, so polling can't stay suppressed - the 2s poll stopped republishing dsee/eqPreset/eqBands, which it never re-read - they came straight back from the C++ cache, which only updates once a write succeeds. Failed writes roll back explicitly with the error instead, and the EQ sliders no longer fight the timer mid-drag
Selecting Manual showed the previous preset's slider positions, not the custom curve stored on the device. The bands were only ever read once, on connect, where the device reports the curve of whatever preset is active - setEqualizerPreset() then sent 58 00 <preset> 00 and updated the cached preset byte alone, so the cached bands stayed behind. Touching a slider from there wrote those stale values back, overwriting the stored curve. Re-read with requestEqualizer() after the write, so the cache holds the bands that belong to the preset the device just switched to. The read-back is guarded: it runs after the property lock is released (requestEqualizer() takes the same non-recursive mutex), and a failed read leaves the bands stale rather than reporting the write that already landed as failed. The macOS model publishes eqPreset/clearBass/eqBands on a successful preset write, and the ImGui GUI pulls them into its sliders once the command future completes - neither re-read them on any other path.
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
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.
Sound quality mode
Prioritize sound quality vs. stable connection
New control on all three platforms, capability-probed like the rest. It's the AUDIO parameter family this repo already speaks for DSEE (e6/e7/e8), where the sub-type byte picks the feature: 0x01 is upscaling, 0x00 and 0x02 are the connection mode (plain, and the variant LDAC-capable models answer on). GET e6 sub → RET e7 sub mode, SET e8 sub mode, with mode (Sony's PriorMode) 0x00 = sound quality, 0x01 = stable connection.
GadgetBridge doesn't implement this one, so the layout comes from mos9527/SonyHeadphonesClient's libmdr, generated from the decompiled Sony MDR tables. Its Command enum matches our v2 opcodes byte-for-byte (0x12 common, 0x22 power, 0x56 EQ, 0xe6–0xe8 audio), and 0xe6 01 is already hardware-confirmed here as DSEE.
probeCapabilities() tries the LDAC sub-type then the plain one and keeps whichever replies, since SET has to go back out on the same sub-type. Two guards: 0x05 (CONNECTION_MODE_CLASSIC_AUDIO_LE_AUDIO) is never sent — its SET carries a fourth byte that also switches the device between Classic and LE Audio — and the probe only accepts a value byte of 0x00/0x01, so a misread layout leaves the feature unsupported instead of exposing a control that writes garbage.
Writing it makes the device re-negotiate A2DP, which takes the RFCOMM channel down with it, so the SET is fire-and-forget and the disconnect watcher explains the drop rather than reporting a bare failure. The macOS card shows the negotiated codec next to the toggle, which is the only way to see the setting do anything on a host without LDAC.
Was tested on my macOS machine and it let A2DP to negotiate 192kbps instead of default 144kbps which I'm very happy with (because no LDAC on Mac, you know).
Battery
Per-earbud levels on TWS
requestBattery() probed the single-battery sub-type first and returned on any reply, so earbuds never reached the dual probe and L/R stayed empty. Probe order now follows the model name, with a fallback either way.
Also: DUAL2 (0x01) for the WF-C5xx/C700N line, per-earbud charging flags, level 0 treated as docked rather than flat, case battery (0x0a), and a periodic re-poll so it stops going stale (v2 only — 0x22 is POWER_OFF on v1).
Verified on a WF-1000XM5: docking one earbud moves byte [4] and nothing else. Its replies are longer than the documented layout (8/5 bytes vs 6/4), so size checks are lower bounds; captured frames are in Constants.h.
Crash on disconnect
rfcommchannel was a non-owning __bridge pointer, never cleared when the link dropped — an in-flight poll would writeSync: to a freed channel (EXC_BAD_ACCESS). Docking an earbud was enough to trigger it. Now retained and cleared under a mutex; send() raises a recoverable error instead.
Also fixes a self-join (rfcommChannelClosed runs on uthread, so disconnect() joined that thread from itself) and makes recv() wake on disconnect rather than waiting out its timeout.
Settings snapping back a second later
A brief link glitch left the frame parser stranded mid-stream for the rest of the session: bytes before the first START_MARKER were concatenated into the message, and a second one threw. 60/61/62 are escaped inside a frame, so a start marker unambiguously begins one — both cases now resync on it instead.
_waitForAck() read exactly one frame and assumed it was the ACK, but the device pushes unsolicited notifications on the same stream (a state change, a press of the headset's own button). It took _seqNumber from the wrong frame, and the device dedupes on that, so the next command was silently discarded as a duplicate — DSEE wouldn't toggle at all, while ASC and the EQ snapped back on the next poll.
Also: requestAmbientState() matches sub-type 0x17 rather than any 0x67, so a notification with a different layout is no longer parsed as the reply; the read-back no longer overwrites a change that hasn't been sent yet (isChanged() went false, so the command was never sent at all); and the 2s poll stopped republishing dsee/eqPreset/eqBands, which it never re-read — failed writes roll back explicitly now, and the EQ sliders no longer fight the timer mid-drag.
Equalizer
Manual showed the wrong bands
The band values were read once, on connect, where the device reports the curve of whatever preset happens to be active. setEqualizerPreset() then sent 58 00 preset 00 and updated the cached preset byte alone, so selecting Manual left the sliders on the previous preset's curve instead of the custom one the device restores — and dragging a slider from there wrote those stale values back over the stored curve.
Preset writes now re-read with requestEqualizer(), after the property lock is released (it takes the same non-recursive mutex) and guarded so a failed read-back doesn't report a write that already landed as failed. Both UIs republish the bands when the write completes; neither re-read them on any other path.
Icon
Apple's grid: 824 body centered in 1024, rounded corners, subtle shadow.
P.S. Let me know if it's better to split this PR into few smaller PRs