Bug 7149 - Suggest adding a Factory Default button the Players Settings tab
: Suggest adding a Factory Default button the Players Settings tab
Status: CLOSED FIXED
Product: Logitech Media Server
Classification: Unclassified
Component: Player UI
: unspecified
: PC Windows XP
: P1 normal (vote)
: ---
Assigned To: Michael Herger
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-02-14 14:35 UTC by James Richardson
Modified: 2009-09-08 09:23 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
full patch, including web UI (2.86 KB, patch)
2008-06-13 05:03 UTC, Michael Herger
Details | Diff
updated patch (2.84 KB, patch)
2008-06-13 07:36 UTC, Michael Herger
Details | Diff
Reset player prefs patch (3.01 KB, patch)
2008-06-15 09:36 UTC, Adrian Smith
Details | Diff
adding $client->initPrefs() (5.96 KB, patch)
2008-06-24 04:31 UTC, Michael Herger
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description James Richardson 2008-02-14 14:35:58 UTC
There does not appear to be a way for customers to reset their SB Players back to a "factory default" settings state.  Some times customers like to "reset" their players to what ever the manufacture suggests as a default.

From a customer point of view, it may be good to add one for SN and SC.
Comment 1 KDF 2008-02-14 16:34:25 UTC
I'd be scared that this was "too easy".  One thing perhaps to have a "set to default prefs", but to reset the player setup, network settings and all would cause an instant rant every time it was done by mistake.  The hardware access to this feature already exists, much the same as other devices have those buttons that are only accessible via paperclip.
Comment 2 KDF 2008-02-14 16:35:18 UTC
oh yeah, not to mention that the web ui is inherently insecure, which means if someone isn't careful an outside action could wipe everything.
Comment 3 James Richardson 2008-02-15 07:56:21 UTC
(In reply to comment #1)
> I'd be scared that this was "too easy".  One thing perhaps to have a "set to
> default prefs", but to reset the player setup, network settings and all would
> cause an instant rant every time it was done by mistake.  The hardware access
> to this feature already exists, much the same as other devices have those
> buttons that are only accessible via paperclip.
> 

KDF -- you are correct, I think my original text was a bit too broad.  I meant just the Player Prfs...not any other settings.  Just the individual player prefs as found on the Settings Page > Display & Audio & Menu

Thank you for clarifying this point.
Comment 4 Blackketter Dean 2008-04-01 11:05:16 UTC
Consider fro Beck
Comment 5 Andy Grundman 2008-05-21 15:00:52 UTC
Is this request only for SN or both SC and SN?
Comment 6 James Richardson 2008-05-21 15:47:43 UTC
If we could do it for both SN & SC that would be a good user experience.
Comment 7 Blackketter Dean 2008-06-09 22:19:46 UTC
Consider for 7.2
Comment 8 Michael Herger 2008-06-13 05:01:02 UTC
Why would the following patch not work? At first it seems to be fine: I get the default values in the web UI, on the player etc. But the changes are never written to disc, even if I add a $prefs->write() to it. Thus after a restart the player has its previous settings again.

Index: /Users/mh/Documents/workspace/Boom/server/Slim/Player/Client.pm
===================================================================
--- /Users/mh/Documents/workspace/Boom/server/Slim/Player/Client.pm	(revision 20723)
+++ /Users/mh/Documents/workspace/Boom/server/Slim/Player/Client.pm	(working copy)
@@ -287,6 +287,8 @@
 Accessors for the list of known clients. These are functions, and do not need
 clients passed. They should be moved to be class methods.
 
+=cut
+
 =head2 clients()
 
 Returns an array of all client objects.
@@ -297,6 +299,19 @@
 	return values %clientHash;
 }
 
+=head2 resetPrefs()
+
+Resets a client's preference object.
+
+=cut
+
+sub resetPrefs {
+	my $client = shift;
+
+	delete $prefs->client($client)->{'prefs'};
+	$client->init();
+}
+
 =head2 clientCount()
 
 Returns the number of known clients.
Comment 9 Michael Herger 2008-06-13 05:03:00 UTC
Created attachment 3426 [details]
full patch, including web UI
Comment 10 Michael Herger 2008-06-13 07:36:28 UTC
Created attachment 3427 [details]
updated patch
Comment 11 Adrian Smith 2008-06-13 08:46:33 UTC
does the new patch work Michael or are you still looking for a reason why it doesn't work?
Comment 12 Michael Herger 2008-06-13 09:04:44 UTC
no, it's only updated not to break with latest changes in SVN. It still doesn't work.
Comment 13 Adrian Smith 2008-06-13 09:17:54 UTC
Ok will look at it....  We only want to do for client prefs in the server namespace?
Comment 14 Michael Herger 2008-06-13 09:27:50 UTC
I thought about this. And I think it's fine. As long as no pref is changed, no plugin will be enabled for this player?

But if you know a simple solution, don't hesitate. Cycling through all namespaces might work, but some plugins probably won't correctly initialize. Eg. MusicInfoSCR only initializes on new player events.
Comment 15 Adrian Smith 2008-06-15 03:51:07 UTC
Michael - don't quite understand the last comment - are you saying just the prefs in the server namespaces is ok (its probably safest)

There are two complexities with this we need to handle - how to reset prefs synced to SN and also the prefs stored on the player itself.  I assume we want to zero these too?

We need to be careful just calling $client->init to reset the prefs as this may call more code than we want.  Possibly we just need to avoid calling the timer routines again without killing the old ones (screensaver and display refresh for old players).
Comment 16 Michael Herger 2008-06-15 04:18:08 UTC
> Michael - don't quite understand the last comment - are you saying just the
> prefs in the server namespaces is ok (its probably safest)

Yes. Many plugins need to call some initialization - which is done in many different ways. My MIS wouldn't like the prefs being wiped.

In a longer term we might want to define a plugin API call to do the prefs init.
Comment 17 Adrian Smith 2008-06-15 09:36:10 UTC
Created attachment 3435 [details]
Reset player prefs patch

Try the attached - I think it should reset the prefs and push them to the player/sn.

Remaining issue is that calling $client->init is not really safe to just reset default prefs.  It will do things we don't want to do.  We could solve this by passing a param to init which specifies whether to only set the prefs or also do the other tasks carried out by init.
Comment 18 Michael Herger 2008-06-16 01:31:16 UTC
Patch applied at change 20776 - thanks! I wasn't aware of the fact that some settings (except name) are stored on the player.
Comment 19 Adrian Smith 2008-06-16 14:55:46 UTC
Michael - I think we need to resolve the init issued before closing this.  

It will definately start a second screensaver timer, and I think we should probably avoid some of the other init code such as startup.
Comment 20 Michael Herger 2008-06-24 04:31:13 UTC
Created attachment 3480 [details]
adding $client->initPrefs()

Adrian - would something along the lines of this patch be the solution (might need to do the same for Display classes): add a initPrefs() method to clients, which in turn is called during init(). resetPrefs() would then call initPrefs() instead of init().

Most of the init methods don't do more than initialize a few player specific prefs. In these cases renaming is ok.
Comment 21 Michael Herger 2008-06-24 04:43:59 UTC
Oops... the previous changes (adding the button etc.) are already checked in to 7.1. So we need to fix this in 7.1.
Comment 22 Adrian Smith 2008-06-24 11:39:24 UTC
Yes agreed - this would allow us to differentiate between init and initPrefs which is what we need to do here.
Comment 23 Michael Herger 2008-06-24 23:47:55 UTC
change 21126 - move prefs initialization from player's/display's init() to its own initPrefs() method. This allows for savely resetting preferences without triggering the remaining initialization (screen refresh timers etc.)
Comment 24 Michael Herger 2008-06-24 23:57:36 UTC
Need to change Boom classes, too. Awaiting merge from 7.1 to 7.2.
Comment 25 Michael Herger 2008-06-25 06:31:30 UTC
change 21136 - add initPrefs to Boom classes
Comment 26 James Richardson 2008-06-26 08:34:51 UTC
Doing the factory reset on my boom, I can now no longer set the Player Name.  Each time I set it, via SC or SN, the name is alwasy reset with in 1 minute to SqueezeBox 88

Tested with 7.2-21169
Comment 27 Michael Herger 2008-06-26 09:44:59 UTC
confusion alert: the functionality we're talking about here has nothing to do with the hardware factory reset. Are you doing the factory reset on the hardware or in SC?
Comment 28 James Richardson 2008-06-26 10:01:36 UTC
My testing was done via SC

Settings > Player > Basic Settings > Reset Player Preferences

using SC 7.2 with firmware r17

The player name was reset to a factory name of Squeezebox##, every time I attempted to set it another name, SC UI shows it would change but then 30-45 seconds later it would be reset back to Squeezebox##

Had to revert player to FW r12 to get the name to stick
Comment 29 Adrian Smith 2008-06-26 11:41:08 UTC
I presume you mean revert the SC version?  Do you have sync prefs to SN enabled on this player?
Comment 30 James Richardson 2008-07-30 14:33:31 UTC
Tested with 7.2-22185, it appears to cause SC to get into an update loop.  My browser(s) will show the Player page updating even after it has applied the settings.

I need to shutdown the server and bring it back to get the browser to stop "updating" the player page.



Comment 31 James Richardson 2008-07-31 09:28:59 UTC
Michael: Can you have a look at this feature, I seem to be having issues with it working properly.
Comment 32 Michael Herger 2008-07-31 09:40:56 UTC
Can you please explain more what issue you're seeing?
Comment 33 Michael Herger 2008-08-04 02:11:52 UTC
change 22337 - we didn't initialize the display prefs, which caused the preference list not to be complete. Page wouldn't load any more.
Comment 34 James Richardson 2008-08-05 15:05:12 UTC
Working as expected SqueezeCenter 7.2-22374
Comment 35 James Richardson 2008-12-15 11:58:21 UTC
This bug has been fixed in the latest release of SqueezeCenter!

Please download the new version from http://www.slimdevices.com/su_downloads.html if you haven't already.  

If you are still experiencing this problem, feel free to reopen the bug with your new comments and we'll have another look.