Fix Spotify Connect: playlist advance, seek support, log noise#210
Open
hansherlighed wants to merge 1 commit intomichaelherger:masterfrom
Open
Fix Spotify Connect: playlist advance, seek support, log noise#210hansherlighed wants to merge 1 commit intomichaelherger:masterfrom
hansherlighed wants to merge 1 commit intomichaelherger:masterfrom
Conversation
📝 WalkthroughWalkthroughThis pull request introduces comprehensive Spotify Connect support for multi-device playback control, adding daemon orchestration, per-client playback context tracking, device control APIs, and configuration UI integration with LMS's event and playback systems. Changes
Sequence Diagram(s)sequenceDiagram
participant LMS as LMS Player
participant Connect as Spotify Connect Module
participant Context as Playback Context
participant Daemon as Connect Daemon
participant API as Spotify API
LMS->>Connect: onNewSong event
Connect->>Context: update(context)
Context->>API: fetch tracks for context
API-->>Context: track URIs
Context->>Context: build known-tracks map
LMS->>Connect: onVolume event
Connect->>Connect: _bufferedSetVolume (with grace period)
Connect->>Daemon: volume control
Daemon->>Daemon: spotifyId TTL check
LMS->>Connect: getNextTrack request
Connect->>Context: check isLastTrack
Context-->>Connect: hasPlay/known-tracks status
Connect->>Daemon: nextTrack via API
API-->>Daemon: next track data
Daemon-->>Connect: track info
Connect-->>LMS: track callback
sequenceDiagram
participant Manager as DaemonManager
participant Client as LMS Client
participant Prefs as Preferences
participant Daemon as Connect Daemon
participant API as Spotify API
Manager->>Prefs: listen to preference changes
Prefs->>Manager: enableSpotifyConnect changed
Manager->>Manager: initHelpers evaluation
loop For each client
alt Spotify Connect enabled
Manager->>Daemon: startHelper(clientId)
Daemon->>API: get devices list
API-->>Daemon: available devices
Daemon->>Daemon: spotifyId management
Daemon-->>Manager: daemon started/verified
else Spotify Connect disabled
Manager->>Daemon: stopHelper
end
end
Manager->>Manager: schedule watchdog timer
loop Watchdog loop
Manager->>Daemon: check alive status
Daemon-->>Manager: pid/uptime
alt Connectivity issue
Manager->>API: checkAPIConnectPlayer
API-->>Manager: updated device list
end
end
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 3✅ Passed checks (3 passed)
✏️ Tip: You can configure your own custom pre-merge checks in the settings. ✨ Finishing Touches🧪 Generate unit tests (beta)
📝 Coding Plan
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment Tip Migrating from UI to YAML configuration.Use the |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 11
Caution
Some comments are outside the diff and can’t be posted inline due to platform limitations.
⚠️ Outside diff range comments (2)
strings.txt (2)
340-346:⚠️ Potential issue | 🟡 MinorOrphaned translation lines without key header.
Lines 340-346 contain translations (CS, DA, DE, EN, FR, HU, NL) but appear to be missing their translation key header. These look like they should belong to
PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESCbased on the content ("Only show my library's albums for my library's artists").🐛 Proposed fix to add missing key header
NL Optimaliseer pre-buffering - schakel deze optie alleen in als de Spotify App nummers voorloopt op de Spotty weergave in de Connect modus. + +PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESC CS Zobrazit pouze alba mé knihovny pro interprety mé knihovny DA Vis kun albums i mit bibliotek for kunstnere i mit bibliotek🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@strings.txt` around lines 340 - 346, These lines are orphaned translations missing their key header; add the key header PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESC above the CS/DA/DE/EN/FR/HU/NL translations so each translation line is associated with that key, preserving the exact translation texts and formatting convention used elsewhere in the file (e.g., "PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESC\tOnly show my library's albums for my library's artists" and similarly for the other locales).
402-413:⚠️ Potential issue | 🟡 MinorUpdate
authentication.htmlto reference the correct translation key.The key
PLUGIN_SPOTTY_DOCKER_NETWORK_HINTwas removed fromstrings.txt(replaced byPLUGIN_SPOTTY_CHECK_DAEMON_IS_CONNECTEDat lines 402-413), but the template still references it atHTML/EN/plugins/Spotty/settings/authentication.html:60. Update the template to use the new key or restore the translation if the message is still required.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@strings.txt` around lines 402 - 413, The template HTML/EN/plugins/Spotty/settings/authentication.html still references the removed translation key PLUGIN_SPOTTY_DOCKER_NETWORK_HINT; update that reference to the new key PLUGIN_SPOTTY_CHECK_DAEMON_IS_CONNECTED (or restore the removed PLUGIN_SPOTTY_DOCKER_NETWORK_HINT entry in strings.txt if the original Docker hint is still needed), and verify the replacement key exists across languages in strings.txt (use the new PLUGIN_SPOTTY_CHECK_DAEMON_IS_CONNECTED symbol used in the diff).
🧹 Nitpick comments (4)
install.xml (1)
21-21: Reminder: Revert version suffix before merging.The version
4.60.7-localindicates a local development build. Ensure this is reverted to a proper release version (e.g.,4.60.8or similar) before merging to the main branch.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@install.xml` at line 21, The <version> element currently reads "4.60.7-local" which is a local-dev suffix; update the <version> value in install.xml (the <version> tag) to the intended release version (remove the "-local" suffix and set the correct release number, e.g., "4.60.8" or the proper target release) before merging.Connect.pm (2)
329-332: RedundanthasDigitalOutcheck after$client->masterassignment.Line 329 checks
$client->hasDigitalOutbefore reassigning$clientto$client->masteron line 331. Then line 332 checks the same condition on the master. If the original client and master differ, this could lead to inconsistent behavior.♻️ Suggested simplification
sub _onVolume { my $request = shift; return if $request->source && $request->source eq __PACKAGE__; my $client = $request->client(); - return if !defined $client || ($client->hasDigitalOut && $serverPrefs->client($client)->get('digitalVolumeControl')); + return if !defined $client; $client = $client->master; return if $client->hasDigitalOut && $serverPrefs->client($client)->get('digitalVolumeControl');🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Connect.pm` around lines 329 - 332, The code redundantly checks $client->hasDigitalOut before reassigning $client to $client->master; to fix, first ensure $client is defined (leave the !defined $client early return), then reassign $client = $client->master, and only after that perform the combined check using $client->hasDigitalOut and serverPrefs->client($client)->get('digitalVolumeControl') so the decision is consistently based on the master client; update the block around the $client variable (the two return lines and the $client->master assignment) to reflect this single post-assignment check.
539-549: Missing error logging when credentials parsing fails.If
from_json(read_file($credentialsFile))throws an exception or returns invalid data, the error is silently swallowed viaeval. Consider logging the error for debugging purposes.🔧 Proposed improvement
my $credentials = eval { from_json(read_file($credentialsFile)); }; - if ( !$@ && $credentials || ref $credentials && $credentials->{auth_data} ) { + if ($@) { + main::DEBUGLOG && $log->is_debug && $log->debug("Failed to read credentials from $credentialsFile: $@"); + } + + if ( !$@ && $credentials && ref $credentials && $credentials->{auth_data} ) {🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@Connect.pm` around lines 539 - 549, The eval that parses credentials with from_json(read_file($credentialsFile)) currently swallows errors; after that eval check $@ and also validate that $credentials and $credentials->{auth_data} are present, and if not log a clear error including $@ and $credentialsFile (or a warning if credentials are present but missing auth_data) before attempting to construct Plugins::Spotty::API->new; add the logging using your module’s logger (or warn) so failures parsing or invalid credential content are visible.ProtocolHandler.pm (1)
123-134: Consider adding explicitrequire Plugins::Spotty::Connectfor code clarity, though the current implementation is safe.
Plugins::Spotty::Connectis not explicitly imported in ProtocolHandler.pm, though the module is loaded conditionally in Plugin.pm before being used. SincegetNextTrack()callsisSpotifyConnect()as a guard, which triggers the conditional loading viaPlugin->canSpotifyConnect(), the code is functionally safe. However, making the dependency explicit at the module level would improve clarity and self-document the requirement.🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed. In `@ProtocolHandler.pm` around lines 123 - 134, Add an explicit require for Plugins::Spotty::Connect to ProtocolHandler by importing the module at the top of the file so the dependency is self-documented; specifically, in the same module that defines getNextTrack, add a require Plugins::Spotty::Connect (or use statement) so callers of getNextTrack and the branch that invokes Plugins::Spotty::Connect->getNextTrack are clearly pinned to that module, while leaving the existing guard via Plugins::Spotty::Plugin->isSpotifyConnect intact.
🤖 Prompt for all review comments with AI agents
Verify each finding against the current code and only fix it if needed.
Inline comments:
In `@API.pm`:
- Around line 254-267: The playerPause handler (and the other playback-targeted
methods in the same area such as playerPlay, playerNext, playerPrevious) must
abort when the MAC→Spotify device lookup fails; modify the anonymous callback
passed to withIdFromMac in playerPause (and the equivalent callbacks in
playerPlay/playerNext/playerPrevious) to check the resolved device id (e.g.,
$_[0] or $deviceId) and if it's falsey call $cb->() if $cb and then return
immediately, mirroring playerTransfer()'s guard so no request is sent without a
valid device_id.
In `@Connect.pm`:
- Around line 543-549: The conditional that decides whether to instantiate
Plugins::Spotty::API is mis-grouped; replace the expression with a parenthesized
check that ensures no exception and that credentials are present and, if
$credentials is a reference, it contains auth_data — i.e. change the condition
around the Plugins::Spotty::API new block to something like: !$@ && $credentials
&& ( !ref $credentials || $credentials->{auth_data} ) so $api is only created
when all required credential data exists.
- Around line 119-146: The callback passed to $spotty->player assumes
$state->{item} exists when calling $song->streamUrl($state->{item}->{uri}),
which can be undef; add a defensive check inside the anonymous sub (the
$spotty->player callback) to verify $state->{item} and $state->{item}->{uri}
before calling $song->streamUrl and $class->setSpotifyConnect; if missing,
handle gracefully by skipping the streamUrl call (and still clear newTrack and
invoke $successCb) or return early so you don't dereference undef. Target the
anonymous callback created after $class->getAPIHandler($client) and modify the
flow around the $song->streamUrl(...) and $class->setSpotifyConnect(...) calls
to avoid null-pointer dereference.
- Around line 246-255: The _delayedStop function computes $remaining by
subtracting Slim::Player::Source::songTime($client) from
$client->controller()->playingSongDuration() without guarding for undef or
non-positive values; update _delayedStop to validate both values
(playingSongDuration and songTime) are defined and numeric, compute $remaining =
max(0, duration - songTime) (or bail out early if duration is undef), log a
warning when inputs are invalid, and only call Slim::Utils::Timers::setTimer
when $remaining is a sane positive number (otherwise just call
Slim::Utils::Timers::killTimers and return) so the timer never gets scheduled
with an invalid time.
In `@Connect/Context.pm`:
- Around line 83-104: The async callback for trackURIsFromURI writes _lastURL
and KNOWN_TRACKS_KEY unconditionally, so its stale response can overwrite a
newer context; capture the current context identity before calling
trackURIsFromURI (e.g. my $expected_id = $self->_contextId and/or my
$expected_type = $self->_context->{type}) and inside the callback verify that
$self->_contextId (and/or $self->_context->{type}) still matches those captured
values before setting $self->_lastURL or calling
$self->_setCache(KNOWN_TRACKS_KEY, ...); if they don’t match, return early and
ignore the response. Ensure checks reference trackURIsFromURI, _contextId,
_context, _lastURL, _setCache, KNOWN_TRACKS_KEY, and uri2url so the guard is
unambiguous.
In `@Connect/Daemon.pm`:
- Around line 95-99: The encode_base64() call used when building the
'--lms-auth' arg is adding a trailing newline which breaks the Basic auth token;
update the call in the conditional that checks $serverPrefs->get('authorize')
and Plugins::Spotty::Helper->getCapability('lms-auth') so that
encode_base64(sprintf(...)) is invoked with the second argument as an empty
string (encode_base64(..., '')) to suppress the line break/newline before
pushing the result onto `@helperArgs`.
In `@Connect/DaemonManager.pm`:
- Around line 87-131: initHelpers currently starts helpers unconditionally for
clients that appear enabled, which can relaunch daemons against a helper binary
that no longer satisfies CONNECT_HELPER_VERSION; modify initHelpers so that
before calling startHelper($client) it verifies the helper binary compatibility
(the same check used in the helper-change path that compares against
CONNECT_HELPER_VERSION) and only calls startHelper when the helper is
compatible, otherwise call stopHelper($client). Ensure you reference and reuse
the existing compatibility check (or helper version validation code) rather than
duplicating logic, and apply this check in all branches where startHelper is
invoked.
- Around line 80-84: The watcher registered with
preferences('server')->setChange currently triggers on 'authorize' and
'username' only, so password rotations don't restart helpers; update the
setChange call to include 'password' (i.e., watch 'authorize', 'username',
'password') so that when the server password changes the callback runs and
executes $class->shutdown() and initHelpers() (keeping the existing logging via
main::INFOLOG && $log->is_info && $log->info).
- Around line 181-193: idFromMac currently triggers _getDevices when
spotifyIdIsRecent is false but still returns helper->spotifyId immediately,
allowing callers to use an expired ID; change idFromMac so that after calling
_getDevices(Plugins::Spotty::Connect->getAPIHandler($mac)) it only returns
$helper->spotifyId if spotifyIdIsRecent is true (or otherwise return undef),
i.e. re-check helper->spotifyIdIsRecent after the refresh and return undef when
the ID remains expired so API.pm::withIdFromMac will wait for a valid mapping.
In `@HTML/EN/plugins/Spotty/settings/basic.html`:
- Around line 149-154: The hidden and checkbox inputs both use the same HTML id
pref_optimizePreBuffer, causing duplicate-ID accessibility/JS issues; change the
hidden input (the one rendered when canConnect is false) to omit the id
attribute (keep its name and value as prefs.pref_optimizePreBuffer) so only the
visible checkbox input retains id="pref_optimizePreBuffer"; update any JS that
relied on selecting the hidden element by id to instead use the input's name or
a data- attribute if needed, and ensure this change is applied within the
conditional block around canConnect.
In `@Plugin.pm`:
- Around line 337-355: canSpotifyConnect currently mixes a capability predicate
with side-effects (initializing Plugins::Spotty::Connect and logging), causing
noisy/stateful probes and unwanted init calls (e.g., from shutdownPlugin());
change canSpotifyConnect to be a pure predicate that only checks helper version
and returns boolean (remove require Plugins::Spotty::Connect,
Plugins::Spotty::Connect->init() and the error log), add a separate explicit
initializer (e.g., initSpotifyConnect or callPluginsSpottyConnectInit) that
performs require Plugins::Spotty::Connect, logs failures, and calls
Plugins::Spotty::Connect->init() when actual initialization is required, and
update isSpotifyConnect and any new call sites to use the pure canSpotifyConnect
predicate for capability checks and call the new initializer only when they
intend to perform initialization.
---
Outside diff comments:
In `@strings.txt`:
- Around line 340-346: These lines are orphaned translations missing their key
header; add the key header PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESC above the
CS/DA/DE/EN/FR/HU/NL translations so each translation line is associated with
that key, preserving the exact translation texts and formatting convention used
elsewhere in the file (e.g., "PLUGIN_SPOTTY_SHOW_MY_ALBUMS_ONLY_DESC\tOnly show
my library's albums for my library's artists" and similarly for the other
locales).
- Around line 402-413: The template
HTML/EN/plugins/Spotty/settings/authentication.html still references the removed
translation key PLUGIN_SPOTTY_DOCKER_NETWORK_HINT; update that reference to the
new key PLUGIN_SPOTTY_CHECK_DAEMON_IS_CONNECTED (or restore the removed
PLUGIN_SPOTTY_DOCKER_NETWORK_HINT entry in strings.txt if the original Docker
hint is still needed), and verify the replacement key exists across languages in
strings.txt (use the new PLUGIN_SPOTTY_CHECK_DAEMON_IS_CONNECTED symbol used in
the diff).
---
Nitpick comments:
In `@Connect.pm`:
- Around line 329-332: The code redundantly checks $client->hasDigitalOut before
reassigning $client to $client->master; to fix, first ensure $client is defined
(leave the !defined $client early return), then reassign $client =
$client->master, and only after that perform the combined check using
$client->hasDigitalOut and
serverPrefs->client($client)->get('digitalVolumeControl') so the decision is
consistently based on the master client; update the block around the $client
variable (the two return lines and the $client->master assignment) to reflect
this single post-assignment check.
- Around line 539-549: The eval that parses credentials with
from_json(read_file($credentialsFile)) currently swallows errors; after that
eval check $@ and also validate that $credentials and $credentials->{auth_data}
are present, and if not log a clear error including $@ and $credentialsFile (or
a warning if credentials are present but missing auth_data) before attempting to
construct Plugins::Spotty::API->new; add the logging using your module’s logger
(or warn) so failures parsing or invalid credential content are visible.
In `@install.xml`:
- Line 21: The <version> element currently reads "4.60.7-local" which is a
local-dev suffix; update the <version> value in install.xml (the <version> tag)
to the intended release version (remove the "-local" suffix and set the correct
release number, e.g., "4.60.8" or the proper target release) before merging.
In `@ProtocolHandler.pm`:
- Around line 123-134: Add an explicit require for Plugins::Spotty::Connect to
ProtocolHandler by importing the module at the top of the file so the dependency
is self-documented; specifically, in the same module that defines getNextTrack,
add a require Plugins::Spotty::Connect (or use statement) so callers of
getNextTrack and the branch that invokes Plugins::Spotty::Connect->getNextTrack
are clearly pinned to that module, while leaving the existing guard via
Plugins::Spotty::Plugin->isSpotifyConnect intact.
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: d0602685-6225-4ca8-a79f-1632fb1cdfc0
📒 Files selected for processing (16)
API.pmAccountHelper.pmConnect.pmConnect/Context.pmConnect/Daemon.pmConnect/DaemonManager.pmHTML/EN/plugins/Spotty/settings/basic.htmlHTML/EN/plugins/Spotty/settings/player.htmlHelper.pmOPML.pmPlugin.pmProtocolHandler.pmSettings.pmSettings/Player.pminstall.xmlstrings.txt
Comment on lines
+254
to
+267
| sub playerPause { | ||
| my ( $self, $cb, $device ) = @_; | ||
|
|
||
| $self->withIdFromMac(sub { | ||
| my $args = {}; | ||
| $args->{device_id} = $_[0] if $_[0]; | ||
|
|
||
| $self->_call('me/player/pause', | ||
| sub { | ||
| $cb->() if $cb; | ||
| }, | ||
| PUT => $args | ||
| ); | ||
| }, $device); |
There was a problem hiding this comment.
🧩 Analysis chain
🌐 Web query:
For the Spotify Web API endpoints /me/player/pause, /me/player/next, /me/player/seek, and /me/player/volume, what device is targeted when device_id is omitted?
💡 Result:
When device_id is omitted, the command targets the user’s currently active Spotify Connect playback device (the device with is_active: true). This is explicitly stated in the docs for Pause, Next, Seek, and Set Volume. [1][2][3][4]
Sources:
[1] Pause Playback (/me/player/pause) (developer.spotify.com)
[2] Skip To Next (/me/player/next) (developer.spotify.com)
[3] Seek To Position (/me/player/seek) (developer.spotify.com)
[4] Set Playback Volume (/me/player/volume) (developer.spotify.com)
Citations:
- 1: https://developer.spotify.com/documentation/web-api/reference/pause-a-users-playback?utm_source=openai
- 2: https://developer.spotify.com/documentation/web-api/reference/skip-users-playback-to-next-track?utm_source=openai
- 3: https://developer.spotify.com/documentation/web-api/reference/seek-to-position-in-currently-playing-track?utm_source=openai
- 4: https://developer.spotify.com/documentation/web-api/reference/set-volume-for-users-playback?utm_source=openai
🏁 Script executed:
#!/bin/bash
# Search for the playerTransfer method to see its guard implementation
rg -A 15 'sub playerTransfer' API.pmRepository: michaelherger/Spotty-Plugin
Length of output: 309
🏁 Script executed:
#!/bin/bash
# Search for withIdFromMac to understand its behavior on failure
rg -B 2 -A 10 'sub withIdFromMac' API.pmRepository: michaelherger/Spotty-Plugin
Length of output: 334
🏁 Script executed:
#!/bin/bash
# Search for the four methods to confirm they lack the guard
rg -A 12 'sub playerPause|sub playerNext|sub playerSeek|sub playerVolume' API.pmRepository: michaelherger/Spotty-Plugin
Length of output: 1040
Abort targeted playback commands when MAC lookup fails.
playerTransfer() already short-circuits on an unresolved LMS MAC. These four methods don't, so a failed/stale MAC→Spotify ID lookup sends the request without device_id and targets whichever Spotify device is currently active instead of the intended player. Mirror playerTransfer()'s guard pattern:
if (!$deviceId) {
$cb->() if $cb;
return;
}Also applies to: 270-283, 286-303, 305-322
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@API.pm` around lines 254 - 267, The playerPause handler (and the other
playback-targeted methods in the same area such as playerPlay, playerNext,
playerPrevious) must abort when the MAC→Spotify device lookup fails; modify the
anonymous callback passed to withIdFromMac in playerPause (and the equivalent
callbacks in playerPlay/playerNext/playerPrevious) to check the resolved device
id (e.g., $_[0] or $deviceId) and if it's falsey call $cb->() if $cb and then
return immediately, mirroring playerTransfer()'s guard so no request is sent
without a valid device_id.
Comment on lines
+119
to
+146
| Slim::Utils::Timers::killTimers($client, \&_getNextTrack); | ||
|
|
||
| if ( $client->pluginData('newTrack') ) { | ||
| main::INFOLOG && $log->is_info && $log->info("Don't get next track as we got called by a play track event from spotty"); | ||
|
|
||
| my $spotty = $class->getAPIHandler($client); | ||
|
|
||
| $spotty->player(sub { | ||
| my $state = $_[0]; | ||
|
|
||
| if ( !$state->{item} && (my $uri = $client->pluginData('episodeUri')) ) { | ||
| $state->{item} = { | ||
| uri => $uri | ||
| }; | ||
| $client->pluginData( episodeUri => '' ); | ||
|
|
||
| $spotty->track(sub { | ||
| $state->{item} = $_[0]; | ||
| $class->setSpotifyConnect($client, $state); | ||
| }, $uri); | ||
| } | ||
|
|
||
| $song->streamUrl($state->{item}->{uri}); | ||
| $class->setSpotifyConnect($client, $state); | ||
| $client->pluginData( newTrack => 0 ); | ||
| $successCb->(); | ||
| }); | ||
| } |
There was a problem hiding this comment.
Potential null-pointer dereference on $state->{item}.
At line 141, $song->streamUrl($state->{item}->{uri}) is called unconditionally, but $state->{item} may still be undefined if the async track callback on line 135-138 hasn't completed or if the episodeUri branch wasn't taken and $state->{item} remained undefined.
🐛 Proposed fix to add defensive check
- $song->streamUrl($state->{item}->{uri});
+ $song->streamUrl($state->{item}->{uri}) if $state->{item} && $state->{item}->{uri};🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect.pm` around lines 119 - 146, The callback passed to $spotty->player
assumes $state->{item} exists when calling
$song->streamUrl($state->{item}->{uri}), which can be undef; add a defensive
check inside the anonymous sub (the $spotty->player callback) to verify
$state->{item} and $state->{item}->{uri} before calling $song->streamUrl and
$class->setSpotifyConnect; if missing, handle gracefully by skipping the
streamUrl call (and still clear newTrack and invoke $successCb) or return early
so you don't dereference undef. Target the anonymous callback created after
$class->getAPIHandler($client) and modify the flow around the
$song->streamUrl(...) and $class->setSpotifyConnect(...) calls to avoid
null-pointer dereference.
Comment on lines
+246
to
+255
| sub _delayedStop { | ||
| my ($class, $client) = @_; | ||
|
|
||
| # set a timer to stop playback at the end of the track | ||
| my $remaining = $client->controller()->playingSongDuration() - Slim::Player::Source::songTime($client); | ||
| main::INFOLOG && $log->is_info && $log->info("Stopping playback in ${remaining}s, as we have likely reached the end of our context (playlist, album, ...)"); | ||
|
|
||
| Slim::Utils::Timers::killTimers($client, \&_sendPause); | ||
| Slim::Utils::Timers::setTimer($client, Time::HiRes::time() + $remaining, \&_sendPause, $class); | ||
| } |
There was a problem hiding this comment.
Possible division by zero or undefined subtraction.
At line 250, $client->controller()->playingSongDuration() can return undef or 0, and Slim::Player::Source::songTime($client) may also be undefined. The subtraction could produce an invalid $remaining value, leading to timer issues.
🛡️ Proposed defensive fix
sub _delayedStop {
my ($class, $client) = `@_`;
# set a timer to stop playback at the end of the track
- my $remaining = $client->controller()->playingSongDuration() - Slim::Player::Source::songTime($client);
+ my $duration = $client->controller()->playingSongDuration() || 0;
+ my $songtime = Slim::Player::Source::songTime($client) || 0;
+ my $remaining = $duration - $songtime;
+ return if $remaining <= 0;
main::INFOLOG && $log->is_info && $log->info("Stopping playback in ${remaining}s, as we have likely reached the end of our context (playlist, album, ...)");🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect.pm` around lines 246 - 255, The _delayedStop function computes
$remaining by subtracting Slim::Player::Source::songTime($client) from
$client->controller()->playingSongDuration() without guarding for undef or
non-positive values; update _delayedStop to validate both values
(playingSongDuration and songTime) are defined and numeric, compute $remaining =
max(0, duration - songTime) (or bail out early if duration is undef), log a
warning when inputs are invalid, and only call Slim::Utils::Timers::setTimer
when $remaining is a sane positive number (otherwise just call
Slim::Utils::Timers::killTimers and return) so the timer never gets scheduled
with an invalid time.
Comment on lines
+543
to
+549
| if ( !$@ && $credentials || ref $credentials && $credentials->{auth_data} ) { | ||
| $api = Plugins::Spotty::API->new({ | ||
| client => $client, | ||
| cache => $cacheFolder, | ||
| username => $credentials->{username}, | ||
| }); | ||
| } |
There was a problem hiding this comment.
Logic error in credentials validation condition.
The condition !$@ && $credentials || ref $credentials && $credentials->{auth_data} has ambiguous operator precedence. Due to Perl's precedence rules, this evaluates as (!$@ && $credentials) || (ref $credentials && $credentials->{auth_data}), which may not be the intended behavior and could allow partial credential objects through.
🐛 Proposed fix with explicit parentheses
- if ( !$@ && $credentials || ref $credentials && $credentials->{auth_data} ) {
+ if ( !$@ && $credentials && ref $credentials && $credentials->{auth_data} ) {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| if ( !$@ && $credentials || ref $credentials && $credentials->{auth_data} ) { | |
| $api = Plugins::Spotty::API->new({ | |
| client => $client, | |
| cache => $cacheFolder, | |
| username => $credentials->{username}, | |
| }); | |
| } | |
| if ( !$@ && $credentials && ref $credentials && $credentials->{auth_data} ) { | |
| $api = Plugins::Spotty::API->new({ | |
| client => $client, | |
| cache => $cacheFolder, | |
| username => $credentials->{username}, | |
| }); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect.pm` around lines 543 - 549, The conditional that decides whether to
instantiate Plugins::Spotty::API is mis-grouped; replace the expression with a
parenthesized check that ensures no exception and that credentials are present
and, if $credentials is a reference, it contains auth_data — i.e. change the
condition around the Plugins::Spotty::API new block to something like: !$@ &&
$credentials && ( !ref $credentials || $credentials->{auth_data} ) so $api is
only created when all required credential data exists.
Comment on lines
+83
to
+104
| if ($self->_context->{type} =~ /album|playlist/) { | ||
| $self->_api->trackURIsFromURI( sub { | ||
| my ($tracks) = @_; | ||
|
|
||
| if ($tracks && ref $tracks) { | ||
| my $knownTracks; | ||
| my $lastTrack = $tracks->[-1]; | ||
| my @lastTrackOccurrences; | ||
|
|
||
| my $x = 0; | ||
| map { | ||
| push @lastTrackOccurrences, $x if $_ eq $lastTrack; | ||
| $knownTracks->{uri2url($_)}++; | ||
| $x++; | ||
| } @{ $tracks || [] }; | ||
|
|
||
| # TODO - use @lastTrackOccurrences to define a smarter filter, respecting previous track(s) or similar | ||
|
|
||
| $self->_lastURL(uri2url($lastTrack)) unless scalar @lastTrackOccurrences > 1; | ||
| $self->_setCache(KNOWN_TRACKS_KEY, $knownTracks); | ||
| } | ||
| }, $self->_contextId ); |
There was a problem hiding this comment.
Ignore stale trackURIsFromURI() responses.
This callback stores _lastURL and KNOWN_TRACKS_KEY unconditionally when the async lookup finishes. If the user changes context before it returns, the old album/playlist state overwrites the new one and end-of-context detection becomes wrong.
Minimal guard
- if ($self->_context->{type} =~ /album|playlist/) {
- $self->_api->trackURIsFromURI( sub {
+ if ($self->_context->{type} =~ /album|playlist/) {
+ my $context_id = $self->_contextId;
+ $self->_api->trackURIsFromURI( sub {
+ return unless $context_id eq $self->_contextId;
my ($tracks) = `@_`;
if ($tracks && ref $tracks) {
my $knownTracks;
my $lastTrack = $tracks->[-1];
@@
- }, $self->_contextId );
+ }, $context_id );
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect/Context.pm` around lines 83 - 104, The async callback for
trackURIsFromURI writes _lastURL and KNOWN_TRACKS_KEY unconditionally, so its
stale response can overwrite a newer context; capture the current context
identity before calling trackURIsFromURI (e.g. my $expected_id =
$self->_contextId and/or my $expected_type = $self->_context->{type}) and inside
the callback verify that $self->_contextId (and/or $self->_context->{type})
still matches those captured values before setting $self->_lastURL or calling
$self->_setCache(KNOWN_TRACKS_KEY, ...); if they don’t match, return early and
ignore the response. Ensure checks reference trackURIsFromURI, _contextId,
_context, _lastURL, _setCache, KNOWN_TRACKS_KEY, and uri2url so the guard is
unambiguous.
Comment on lines
+80
to
+84
| preferences('server')->setChange(sub { | ||
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | ||
| $class->shutdown(); | ||
| initHelpers(); | ||
| }, 'authorize', 'username'); |
There was a problem hiding this comment.
Also restart helpers on LMS password changes.
Connect::Daemon->start() includes both username and password in --lms-auth, but this watcher ignores password. Rotating the server password leaves running daemons with stale credentials until some other setting restarts them.
Suggested fix
- }, 'authorize', 'username');
+ }, 'authorize', 'username', 'password');📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| preferences('server')->setChange(sub { | |
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | |
| $class->shutdown(); | |
| initHelpers(); | |
| }, 'authorize', 'username'); | |
| preferences('server')->setChange(sub { | |
| main::INFOLOG && $log->is_info && $log->info("Authentication information for LMS has changed - re-initialize Connect helpers"); | |
| $class->shutdown(); | |
| initHelpers(); | |
| }, 'authorize', 'username', 'password'); |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect/DaemonManager.pm` around lines 80 - 84, The watcher registered with
preferences('server')->setChange currently triggers on 'authorize' and
'username' only, so password rotations don't restart helpers; update the
setChange call to include 'password' (i.e., watch 'authorize', 'username',
'password') so that when the server password changes the callback runs and
executes $class->shutdown() and initHelpers() (keeping the existing logging via
main::INFOLOG && $log->is_info && $log->info).
Comment on lines
+87
to
+131
| sub initHelpers { | ||
| my $class = __PACKAGE__; | ||
|
|
||
| Slim::Utils::Timers::killTimers( $class, \&initHelpers ); | ||
|
|
||
| main::INFOLOG && $log->is_info && $log->info("Checking Spotty Connect helper daemons..."); | ||
|
|
||
| # shut down orphaned instances | ||
| $class->shutdown('inactive-only'); | ||
|
|
||
| for my $client ( Slim::Player::Client::clients() ) { | ||
| my $syncMaster; | ||
|
|
||
| # if the player is part of the sync group, only start daemon for the group, not the individual players | ||
| if ( Slim::Player::Sync::isSlave($client) && (my $master = $client->master) ) { | ||
| if ( $prefs->client($master)->get('enableSpotifyConnect') ) { | ||
| $syncMaster = $master->id; | ||
| } | ||
| # if the master of the sync group doesn't have connect enabled, enable anyway if one of the slaves has | ||
| else { | ||
| ($syncMaster) = map { $_->id } grep { | ||
| $prefs->client($_)->get('enableSpotifyConnect') | ||
| } sort { $a->id cmp $b->id } Slim::Player::Sync::slaves($master); | ||
| } | ||
| } | ||
|
|
||
| if ( $syncMaster && $syncMaster eq $client->id ) { | ||
| main::INFOLOG && $log->is_info && $log->info("This is not the sync group's master itself, but the first slave with Connect enabled: $syncMaster"); | ||
| $class->startHelper($client); | ||
| } | ||
| elsif ( $syncMaster ) { | ||
| main::INFOLOG && $log->is_info && $log->info("This is not the sync group's master, and not the first slave with Connect either: $syncMaster"); | ||
| $class->stopHelper($client); | ||
| } | ||
| elsif ( !$syncMaster && $prefs->client($client)->get('enableSpotifyConnect') ) { | ||
| main::INFOLOG && $log->is_info && $log->info("This is the sync group's master, or a standalone player with Spotify Connect enabled: " . $client->id); | ||
| $class->startHelper($client); | ||
| } | ||
| else { | ||
| main::INFOLOG && $log->is_info && $log->info("This is a standalone player with Spotify Connect disabled: " . $client->id); | ||
| $class->stopHelper($client); | ||
| } | ||
| } | ||
|
|
||
| Slim::Utils::Timers::setTimer( $class, Time::HiRes::time() + DAEMON_WATCHDOG_INTERVAL, \&initHelpers ); |
There was a problem hiding this comment.
Re-check helper compatibility before starting daemons again.
Once this manager has been initialized, initHelpers() always walks clients and calls startHelper(). After the helper pref is changed to a binary that no longer satisfies CONNECT_HELPER_VERSION, the helper-change path at Lines 62-78 will still relaunch daemons against the unsupported helper instead of stopping cleanly.
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect/DaemonManager.pm` around lines 87 - 131, initHelpers currently starts
helpers unconditionally for clients that appear enabled, which can relaunch
daemons against a helper binary that no longer satisfies CONNECT_HELPER_VERSION;
modify initHelpers so that before calling startHelper($client) it verifies the
helper binary compatibility (the same check used in the helper-change path that
compares against CONNECT_HELPER_VERSION) and only calls startHelper when the
helper is compatible, otherwise call stopHelper($client). Ensure you reference
and reuse the existing compatibility check (or helper version validation code)
rather than duplicating logic, and apply this check in all branches where
startHelper is invoked.
Comment on lines
+181
to
+193
| sub idFromMac { | ||
| my ($class, $mac) = @_; | ||
|
|
||
| return unless $mac; | ||
|
|
||
| my $helper = $helperInstances{$mac} || return; | ||
|
|
||
| # refresh the Connect status every now and then | ||
| if (!$helper->spotifyIdIsRecent) { | ||
| _getDevices(Plugins::Spotty::Connect->getAPIHandler($mac)); | ||
| } | ||
|
|
||
| return $helper->spotifyId; |
There was a problem hiding this comment.
Do not return expired Spotify IDs.
When spotifyIdIsRecent() has expired, this method triggers a refresh but still returns spotifyId(). API.pm::withIdFromMac() treats any truthy value as resolved, so control requests can keep using an expired device ID instead of waiting for the refreshed mapping.
Suggested fix
sub idFromMac {
my ($class, $mac) = `@_`;
return unless $mac;
my $helper = $helperInstances{$mac} || return;
- # refresh the Connect status every now and then
- if (!$helper->spotifyIdIsRecent) {
+ my $spotifyId = $helper->spotifyIdIsRecent;
+ if (!$spotifyId) {
_getDevices(Plugins::Spotty::Connect->getAPIHandler($mac));
+ return;
}
- return $helper->spotifyId;
+ return $spotifyId;
}🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Connect/DaemonManager.pm` around lines 181 - 193, idFromMac currently
triggers _getDevices when spotifyIdIsRecent is false but still returns
helper->spotifyId immediately, allowing callers to use an expired ID; change
idFromMac so that after calling
_getDevices(Plugins::Spotty::Connect->getAPIHandler($mac)) it only returns
$helper->spotifyId if spotifyIdIsRecent is true (or otherwise return undef),
i.e. re-check helper->spotifyIdIsRecent after the refresh and return undef when
the ID remains expired so API.pm::withIdFromMac will wait for a valid mapping.
Comment on lines
+149
to
+154
| [% IF canConnect; WRAPPER setting title="" desc="" %] | ||
| <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="checkbox" [% IF prefs.pref_optimizePreBuffer %] checked [% END %]> | ||
| <label for="pref_optimizePreBuffer">[% "PLUGIN_SPOTTY_OPTIMIZE_PRE_BUFFERING" | string %]</label> | ||
| [% END; ELSE %] | ||
| <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="hidden" value="[% prefs.pref_optimizePreBuffer %]"> | ||
| [% END %] |
There was a problem hiding this comment.
Duplicate HTML ID pref_optimizePreBuffer causes accessibility and JavaScript issues.
Lines 150 and 153 both define elements with id="pref_optimizePreBuffer". Having duplicate IDs violates HTML specification and can break JavaScript selectors and accessibility tools.
🐛 Proposed fix to remove ID from hidden input
[% IF canConnect; WRAPPER setting title="" desc="" %]
<input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="checkbox" [% IF prefs.pref_optimizePreBuffer %] checked [% END %]>
<label for="pref_optimizePreBuffer">[% "PLUGIN_SPOTTY_OPTIMIZE_PRE_BUFFERING" | string %]</label>
[% END; ELSE %]
- <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="hidden" value="[% prefs.pref_optimizePreBuffer %]">
+ <input name="pref_optimizePreBuffer" type="hidden" value="[% prefs.pref_optimizePreBuffer %]">
[% END %]📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| [% IF canConnect; WRAPPER setting title="" desc="" %] | |
| <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="checkbox" [% IF prefs.pref_optimizePreBuffer %] checked [% END %]> | |
| <label for="pref_optimizePreBuffer">[% "PLUGIN_SPOTTY_OPTIMIZE_PRE_BUFFERING" | string %]</label> | |
| [% END; ELSE %] | |
| <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="hidden" value="[% prefs.pref_optimizePreBuffer %]"> | |
| [% END %] | |
| [% IF canConnect; WRAPPER setting title="" desc="" %] | |
| <input name="pref_optimizePreBuffer" id="pref_optimizePreBuffer" type="checkbox" [% IF prefs.pref_optimizePreBuffer %] checked [% END %]> | |
| <label for="pref_optimizePreBuffer">[% "PLUGIN_SPOTTY_OPTIMIZE_PRE_BUFFERING" | string %]</label> | |
| [% END; ELSE %] | |
| <input name="pref_optimizePreBuffer" type="hidden" value="[% prefs.pref_optimizePreBuffer %]"> | |
| [% END %] |
🧰 Tools
🪛 HTMLHint (1.9.2)
[error] 153-153: The id value [ pref_optimizePreBuffer ] must be unique.
(id-unique)
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@HTML/EN/plugins/Spotty/settings/basic.html` around lines 149 - 154, The
hidden and checkbox inputs both use the same HTML id pref_optimizePreBuffer,
causing duplicate-ID accessibility/JS issues; change the hidden input (the one
rendered when canConnect is false) to omit the id attribute (keep its name and
value as prefs.pref_optimizePreBuffer) so only the visible checkbox input
retains id="pref_optimizePreBuffer"; update any JS that relied on selecting the
hidden element by id to instead use the input's name or a data- attribute if
needed, and ensure this change is applied within the conditional block around
canConnect.
Comment on lines
+337
to
+355
| sub canSpotifyConnect { | ||
| my ($class, $dontInit) = @_; | ||
|
|
||
| # we need a minimum helper application version | ||
| if ( !Slim::Utils::Versions->checkVersion(Plugins::Spotty::Helper->getVersion(), CONNECT_HELPER_VERSION, 10) ) { | ||
| $log->error("Cannot support Spotty Connect, need at least helper version " . CONNECT_HELPER_VERSION); | ||
| return; | ||
| } | ||
|
|
||
| require Plugins::Spotty::Connect; | ||
|
|
||
| Plugins::Spotty::Connect->init() unless $dontInit; | ||
|
|
||
| return 1; | ||
| } | ||
|
|
||
| sub isSpotifyConnect { | ||
| my $class = shift; | ||
| return $class->canSpotifyConnect() && Plugins::Spotty::Connect->isSpotifyConnect(@_); |
There was a problem hiding this comment.
Split capability checks from Connect initialization.
canSpotifyConnect() is now used as a predicate in several new call sites, but it also initializes Plugins::Spotty::Connect and logs on failure. That makes harmless support probes noisy and stateful, defeats the delayed init in Helper.pm, and even lets shutdownPlugin() call init() just before shutdown.
Possible direction
+sub supportsSpotifyConnect {
+ my $class = shift;
+ return Slim::Utils::Versions->checkVersion(
+ Plugins::Spotty::Helper->getVersion(),
+ CONNECT_HELPER_VERSION,
+ 10,
+ );
+}
+
sub canSpotifyConnect {
my ($class, $dontInit) = `@_`;
- # we need a minimum helper application version
- if ( !Slim::Utils::Versions->checkVersion(Plugins::Spotty::Helper->getVersion(), CONNECT_HELPER_VERSION, 10) ) {
- $log->error("Cannot support Spotty Connect, need at least helper version " . CONNECT_HELPER_VERSION);
- return;
- }
+ return unless $class->supportsSpotifyConnect();
require Plugins::Spotty::Connect;
Plugins::Spotty::Connect->init() unless $dontInit;
return 1;
}📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
Suggested change
| sub canSpotifyConnect { | |
| my ($class, $dontInit) = @_; | |
| # we need a minimum helper application version | |
| if ( !Slim::Utils::Versions->checkVersion(Plugins::Spotty::Helper->getVersion(), CONNECT_HELPER_VERSION, 10) ) { | |
| $log->error("Cannot support Spotty Connect, need at least helper version " . CONNECT_HELPER_VERSION); | |
| return; | |
| } | |
| require Plugins::Spotty::Connect; | |
| Plugins::Spotty::Connect->init() unless $dontInit; | |
| return 1; | |
| } | |
| sub isSpotifyConnect { | |
| my $class = shift; | |
| return $class->canSpotifyConnect() && Plugins::Spotty::Connect->isSpotifyConnect(@_); | |
| sub supportsSpotifyConnect { | |
| my $class = shift; | |
| return Slim::Utils::Versions->checkVersion( | |
| Plugins::Spotty::Helper->getVersion(), | |
| CONNECT_HELPER_VERSION, | |
| 10, | |
| ); | |
| } | |
| sub canSpotifyConnect { | |
| my ($class, $dontInit) = `@_`; | |
| return unless $class->supportsSpotifyConnect(); | |
| require Plugins::Spotty::Connect; | |
| Plugins::Spotty::Connect->init() unless $dontInit; | |
| return 1; | |
| } | |
| sub isSpotifyConnect { | |
| my $class = shift; | |
| return $class->canSpotifyConnect() && Plugins::Spotty::Connect->isSpotifyConnect(`@_`); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against the current code and only fix it if needed.
In `@Plugin.pm` around lines 337 - 355, canSpotifyConnect currently mixes a
capability predicate with side-effects (initializing Plugins::Spotty::Connect
and logging), causing noisy/stateful probes and unwanted init calls (e.g., from
shutdownPlugin()); change canSpotifyConnect to be a pure predicate that only
checks helper version and returns boolean (remove require
Plugins::Spotty::Connect, Plugins::Spotty::Connect->init() and the error log),
add a separate explicit initializer (e.g., initSpotifyConnect or
callPluginsSpottyConnectInit) that performs require Plugins::Spotty::Connect,
logs failures, and calls Plugins::Spotty::Connect->init() when actual
initialization is required, and update isSpotifyConnect and any new call sites
to use the pure canSpotifyConnect predicate for capability checks and call the
new initializer only when they intend to perform initialization.
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
2 participants
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.
Used Github Copilot to re-introduce Spotfiy connect into your awsome plugin. The changes have been vefiried together with a compiled version of librespot from the fork https://github.com/hansherlighed/librespot
SessionConnected (pushes Spotify's stored device volume, overwriting
the LMS player's actual volume). Uses an AtomicBool flag that consumes
exactly one suppression on the next VolumeChanged.
dedicated "seek" notification to LMS via the JSON-RPC path, bypassing
the REST API which frequently lags 500ms+ behind Spirc's WebSocket state.
Summary by CodeRabbit
Release Notes
New Features
Documentation