Volume wheel bounce + mini-player-over-Spotify fixes - #1
Open
edward-rosado wants to merge 11 commits into
Open
Conversation
Adds a small badge to the connection overlay naming the link that is actually carrying data. The transport is probed, not assumed: /__bt is answered locally by the Bluetooth mux when it owns the client port, so a JSON reply means Bluetooth while a 404 means the request travelled to the server over USB — no server support required, the badge simply reads USB without it. Also prefers Spotify's 300x300 artwork variant over the 640x640 one on the proxied thumbnail path. The panel is 800x480, so the larger asset is wasted bytes — about 0.7s of a saturated ~155KB/s Bluetooth link on every track change. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
When the computer initiates pairing, the on-device agent exposes the 6-digit numeric-comparison code at 127.0.0.1:8892/pairing; this overlay polls it and draws the code fullscreen, then flashes the outcome — the original Car Thing pairing experience. On anything that is not a provisioned Car Thing the endpoint does not exist, the first two polls fail, and the overlay permanently stands down. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
A pairing exchange can resolve in a couple of seconds; at a 2s poll the overlay could miss the whole window (observed on a real pairing — the device agent captured the code but the screen never showed it). Poll every 500ms so the code is on screen the entire time the person is comparing it. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
closeExisting only closed a socket in the OPEN state, so a socket stuck in CONNECTING — which is what happens when the server is unreachable at dial time, the norm on a Bluetooth link that comes up ~80s after the device boots — was abandoned without closing. Those half-open sockets lingered, held a tunnel stream, and kept the client from cleanly reconnecting once the link was finally up. Over USB the server is reachable instantly so this never surfaced. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The size preference was only applied in the startsWith('http') branch,
but that branch is not the one Spotify artwork takes. The DeskThing
server rewrites thumbnails to a relative /proxy/v1?url=... before
sending them (songCache.ts:124), so the client lands in the
startsWith('/') branch, which only prefixed the host. The optimization
was dead code and the device kept fetching 640x640.
Caught by screenshotting the device and reading the actual <img> src:
ab67616d0000b273 (640px) where 00001e02 (300px) was expected.
Two changes: match the bare Spotify id prefix instead of '/image/<id>',
because by the time we see the URL the slashes are usually percent-
encoded inside the proxy query while the id is not; and apply it in the
relative branch too.
Verified on hardware over Bluetooth: the element now loads
ab67616d00001e02... at 300x276 instead of 640x588, and the art renders.
That is ~110KB down to ~36KB per track change on a link that saturates
at 155KB/s.
Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
Measured on hardware: 11.9s from page load until the track and artwork were on screen. Two independent causes, both fixed. The client never asked for song state. requestMusicData was only called from inside the handler for an incoming song message, so the only path to data was an unsolicited push — after any (re)connect the screen sat on "Waiting For Track…" until the track happened to change or the server's refresh interval came round. It now requests the current track as soon as the socket reports connected. Reconnect took a flat 15s before even trying. onclose waited 5s before calling reconnect(), which waited a further 10s before dialling, with no fast first attempt — so a link that dropped and returned immediately still cost 15 seconds. Now 250ms to the first attempt and exponential backoff from 500ms to a 10s ceiling, reset on a successful open. Both are general client behaviour, not Bluetooth-specific, but Bluetooth makes them constantly visible: the link legitimately drops when the device is power-cycled, and it has no battery, so every power change is a reconnect. Co-Authored-By: Claude Fable 5 <noreply@anthropic.com>
The hardware wheel's VolUp/VolDown compute each notch from song.volume, and every server push merges its (lagging) volume back over it — so the wheel re-seeds from a stale base and fights the user: turn up it jumps down, turn down it jumps up. Seen live in the server log as requests ping-ponging 45<->100 before settling. setVolume already stamps _volumeTouchedAt; setSong now holds the local volume for VOLUME_HOLD_MS (4s) after a touch so the echo can't clobber it. Extracted the rule to stores/volumeHold.ts (pure) with volumeHold.test.ts — its 'THE REVERSAL' case models the exact 45<->100 loop and fails if the hold is removed. Confirmed fixed on the device. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…nsport An app that IS the audio source (Spotify) draws its own transport, so the shell's mini-player was a second set of the same controls and a second scrub bar — and it swallowed the volume wheel. Gate it on the focused app's isAudioSource manifest flag, read from the app LIST (currentView is persisted prefs and carries no manifest after a view change). Fails CLOSED: while the app list is still loading after a connect, assume the focused app owns its transport, so the bar never flashes back over Spotify (and never eats the wheel) on a fresh attach. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
…the hold state - musicStore now calls resolveVolumeOnPush (the TESTED helper) instead of re-implementing its one-line decision inline, and _volumeTouchedAt is declared in MusicState — the 'as any' smuggling meant a typo in either string compiled clean and silently disabled the anti-bounce hold. - The focused-app-owns-transport rule extracted to overlay/useFocusedAppOwnsTransport (one home for the fail-closed logic and the app-list-not-currentView lookup; Miniplayer's internal stale currentView.manifest read is documented there as a follow-up). - npm test self-contained: tsm as a devDependency instead of loading the loader out of a sibling repo's node_modules. - VOLUME_HOLD_MS documented as DERIVED from the server's cache TTL + confirm margin, cross-referenced both ways — the two constants encode one physical quantity with no import to enforce it. - Drop a pre-existing unused useEffect import surfaced by the build. Co-Authored-By: Claude Opus 4.8 <noreply@anthropic.com>
Two classes of error made `tsc --noEmit` fail, and the first would have broken CI outright on Linux: - TS1261: App.tsx imported './components/ButtonListener' while the file was buttonListener.tsx. macOS resolves this by accident; a case-sensitive filesystem does not. Renamed the file to match its exported component and the convention every sibling already follows (ConfigComponent, SwipeListener, TimeUpdater). - 7x TS6133: unused imports across ConnectingScreen, WelcomeScreen, timeStore and serverWebsocketHandler. Each name appeared exactly once -- the import itself -- so all were dead. Adds a `typecheck` script so this is one command in CI instead of a convention. tsc 0 errors, tests 6/6, vite build unaffected. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Runs typecheck, tests and the vite build on ubuntu. The case guard exists because the bug it catches shipped: App.tsx imported './components/ButtonListener' while the file was buttonListener.tsx -- macOS resolved it, a case-sensitive runner does not. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
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.
Two device-shell fixes, both confirmed on the Car Thing.
What's new here (the top 2 commits)
server echo, so it fought the user (turn up → jumps down, and vice-versa; seen
live as requests ping-ponging 45↔100).
setSongnow holds the local volume forVOLUME_HOLD_MSafter a wheel touch. Rule extracted tostores/volumeHold.tswith a test whose "THE REVERSAL" case models the exact loop and fails if the
hold is removed.
its own transport, so the shell bar was a duplicate that also swallowed the
volume wheel on reconnect. Gated on the app's
isAudioSourceflag, failingclosed so it never flashes back over Spotify while the app list is still loading.
Notes
main. The 6 commits below the top 2 (Show the current track…etc.) are the Bluetooth-transport work already under upstream PR Bluetooth pairing overlay, transport badge, smaller artwork ItsRiprod/DeskThing-Client#31(
feature/bluetooth-transport-client → ItsRiprod:main); they show up onlybecause fork
maindoesn't carry them yet. The actual review surface is the toptwo commits. This branch was deliberately cut off Bluetooth pairing overlay, transport badge, smaller artwork ItsRiprod/DeskThing-Client#31's head without pushing
to it.
npm testruns the new unit tests via the sibling spotify app'stsmloader(repo-layout dependent) — a self-contained runner is a reasonable follow-up.
🤖 Generated with Claude Code