Bug 6528 - SS/SC do not use CLI for sync, preventing overriding behavior
: SS/SC do not use CLI for sync, preventing overriding behavior
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Sync
: unspecified
: PC Other
: P2 enhancement (vote)
: ---
Assigned To: Unassigned bug - please assign me!
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-02 18:44 UTC by Peter Watkins
Modified: 2008-12-18 11:12 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
diff agains SC7 that implements the desired behavior (1.19 KB, text/plain)
2008-01-03 17:28 UTC, Peter Watkins
Details
fixing the "sync join" bug (1.06 KB, patch)
2008-01-06 05:54 UTC, Peter Watkins
Details | Diff
patch for Slim::Player::Sync::restoreSync() (426 bytes, patch)
2008-01-08 20:01 UTC, Peter Watkins
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Peter Watkins 2008-01-02 18:44:42 UTC
Slim/Buttons/Synchronize.pm directly calls the Slim::Player::Sync::sync and Slim::Player::Sync::unsync methods rather than using the 'sync' CLI. This prevents third party code from using addDispatch to wrap/override the behavior.

Background: when syncing Player B to Player A and Player A is playing a number of local files, I would prefer that Player B default to waiting until Player A starts the next track so that the sync operation doesn't force the track to start over again at the start. If the core code used the CLI instead of direct methods, I could override the behavior and even offer options like "Waiting for next song, press Play to force restarting playlist"
http://forums.slimdevices.com/showthread.php?t=40981
Comment 1 Peter Watkins 2008-01-03 17:28:43 UTC
Created attachment 2606 [details]
diff agains SC7 that implements the desired behavior

This patch switches to $client->execute() so that the 'sync' events triggered by the web UI or the player UI can be trapped via the normal addDispatch() mechanism.
Comment 2 Andy Grundman 2008-01-04 07:40:44 UTC
Thanks, patch applied as change 15872.
Comment 3 Peter Watkins 2008-01-04 20:22:04 UTC
Andy, there's a very good chance that patch is wrong. I'm mainly using 6.5.x, and there in Slim/Buttons/Synchronize the line
  Slim::Player::Sync::sync($client, $selectedClient);
apparently should be replaced NOT with
  $client->execute( ['sync', $selectedClient->id()] );
but rather with
  $selectedClient->execute( ['sync', $client->id()] );
I need to test more with at least 3 players connected, hopefully tomorrow.
Comment 4 Andy Grundman 2008-01-05 07:52:51 UTC
Oh, hmm, maybe this would explain some of the weirdness I am seeing when syncing now.

Please use SC7 though.
Comment 5 Peter Watkins 2008-01-05 10:17:18 UTC
Will do. I'm pretty sure about that reversal -- it's certainly the case with 6.5.x, which I've just spent the morning working with for the plugin that needs this change. I've just fired up the latest SC7 SVN to use with real players & test my reversal theory. More info later...
Comment 6 Peter Watkins 2008-01-06 05:54:45 UTC
Created attachment 2619 [details]
fixing the "sync join" bug

I've tested with 3 physical players, both web and remote control UI. I just got the symantics reversed on the CLI for joining. The attached patch makes things right. 

I'm very sorry for sending the previous patch without proper testing.

-Peter
Comment 7 Andy Grundman 2008-01-06 11:01:31 UTC
Thanks, applied as change 15940.
Comment 8 Peter Watkins 2008-01-08 20:01:31 UTC
Created attachment 2637 [details]
patch for Slim::Player::Sync::restoreSync()

Here's one I missed. This is needed so the restoreSync() method that's called when a synced (but powered separately) player is turned on will also use the CLI. Patch tested with two players (SB3, Softsqueeze).
Comment 9 Andy Grundman 2008-01-08 21:16:17 UTC
Thanks, applied.
Comment 10 Chris Owens 2008-03-07 09:05:18 UTC
This bug is being closed since it was resolved for a version which is now released!  Please download the new version of SqueezeCenter (formerly SlimServer) at http://www.slimdevices.com/su_downloads.html

If you are still seeing this bug, please re-open it and we will consider it for a future release.