Created attachment 1584 [details]
sort time options.

in trunk at change 10039
in 6.5 at change 10040

sleep options are now in chronological order, placing remaining time into the right location each time the sleep button is pressed. This only works when the player is playing songs. 

If sleep time is set to end of song at 22 minutes, the next option would be 30. Eight minutes later, if sleep is pressed, then next option becomes 15 minutes.

This will appear in the Spet 26 nightly build, but I will leave this report open for evaluation

Thanks, KDF.  However, it's not strictly necessary to leave a bug open for me to review.  The system I have set up makes sure I look at every bug whether it's already been resolved or not :)

I don't think sorting the options will completely fix this. The problem is that you can't set the Squeezebox to sleep in a number of minutes that is smaller than the number of minutes remaining on the currently playing song. Say I start playing a file that is 120 minutes long. The first time I press sleep it says it'll sleep in 120 minutes time (which is the end of the current song). The second time I press sleep the sleep function is canceled.

In this example it should give the options to sleep in 15 minutes, 30 minutes, 45 minutes, 60 minutes, 90 minutes and then 120 minutes (end of song). All these sleep durations are shorter than the time remaining on the current song. The way sleep works in 6.5.0 at the moment is that it only offers sleep time options are greater than or equal to the time remaining on the current song. Which seem wrong to me. The Squeezebox should be able to go to sleep before the current song has finished.

I hope that makes more sense.

Emlyn.

Please don't do that without testing.  The sort does EXACTLY what you asked for.

Say you play your 120 minutes song.  At 110 minues left, sleep options will be (in order):

15, 30, 45, 60, 90, 110, cancel.

thanks.

try the nightly build from here:
http://www.slimdevices.com/downloads/nightly/latest/6.5/

if you do have problems, then reopen.
thanks.

I have tested this and I think you're wrong.

I now have the following line in my /Slim/Buttons/Common.pm file:

my @sleepChoices = $remaining ? sort(0, $remaining, 15,30,45,60,90) : (0,15,30,45,60,90);

And I'm running slimserver.pl using perl.exe. This is the only line that's been changed for this fix. If the remaining time on the song that's playing is under 90 minutes then the sleep function behaves as you describe. But if the time remaining is over 90 minutes the only sleep options are the time remaining and sleep canceled. So if there is 110 minutes remaining, the only options I get are sleep in 110 minutes and cancel.

Did this behave differently in your testing?

I can't set a sleep out time that is less than the remaining time on the current song, if the remaining time on the current song is greater than 90 minutes.

Thanks.

what about under 90 minues but more than 15?  does THAT at least work?

patches also welcome.

also, if you wish to provide a track longer than 90 minutes, I'll be happy to test.  I'll take a look at the sleep time display, and see if that is confusing the issue.  Otherwise, there should be nothing wrong with an option above 90 minutes.

I'm afraid I'll have to take your word for it.  I see nothing in the code blocking a value over 90.  I'll do some faking on the songtime and see if I can reproduce.

Created attachment 1595 [details]
120 min mp3 file containing 440Hz sine

Subject: Re:  sleep function with SlimServer 6.5.0

Hm, I don't think that this file is right.  It sounds like a left- 
right test.

this covers the sleep options:
my @sleepChoices = $remaining ? sort({$a <=> $b} (0, $remaining, 15,30,45,60,90)) : (0,15,30,45,60,90);

but the ui feedback will need some rework as it doesn't keep track of the original sleep time, thus can't sort out what the 'end of song' options should be.  

sample file appears to be only 5 seconds long.

sorting works with my hardcoded tests.  problem is that sort works on alpha sorting, not numeric.

for UI, change Player/Client.pm so that sub prettySleepTime() becomes:

sub prettySleepTime {
	my $client = shift;
	
	
	my $sleeptime = $client->sleepTime() - Time::HiRes::time();
	my $sleepstring = '';
	
	my $dur = Slim::Player::Source::playingSongDuration($client);
	my $remaining = $dur - Slim::Player::Source::songTime($client);
	
	if ($client->sleepTime) {
		
		# check against remaining time to see if sleep time matches within 1 second.
		if (int($sleeptime + 0.5) == int($remaining)) {
			$sleepstring = join(' ',$client->string('SLEEPING_AT'),$client->string('END_OF_SONG'));
		} else {
			$sleepstring = join(" " ,$client->string('SLEEPING_IN'),int($sleeptime/60)+1,$client->string('MINUTES'));
		}
	}
	
	return $sleepstring;
}


This allows any skin or player to show the current sleep time, subbing in "End of song" string when the remaining sleep time matches the remaining song time.  This works most of the time, but I'm seeing odd cases where the remaining song time appears to jump much higher suddenly.  

Comment on attachment 1595 [details]
120 min mp3 file containing 440Hz sine

How embarrassing.

Created attachment 1596 [details]
120 minute silent mp3 file

Great Chris, thanks.  I can now confirm with a real track that this now sorts numerically.  In this case, end of song comes in as the last option.  It is a bit harder to track the setting as 'end of song'.  The 1s range I've given in the code does appear to slip a little bit.   This only matters with the fishbone skin since that has a sleep time link that is updated every 10s via ajax.  What this does highlight is the inaccuracy of the songtime values vs the timer set for sleep.

new changes in trunk at change 10055, in 6.5.1 at change 10056.  please test and confirm

From a functional point of view this is now working fine.

But, the UI on the Squeezebox more often than not displays the remaining time of the song in minutes, rather than the text 'Sleeping at end of song'. I guess this is because of the problem you have identifying the time out as 'end of song'. This doesn't particularly bother me, but I do think it makes the UI less intuitive. Especially if you have the end of song time that's the same as one of the hard coded values. You can get the sequence on the display that goes 15, 30, 45, 60, 90, 90, canceled. Rather than 15, 30, 45, 60, Sleeping at end of song, 90, canceled.

Subject: Re:  sleep function with SlimServer 6.5.0

Here's how I think it should work:

If the player is already in sleep mode and you press the SLEEP  
button, you should continue to rotate through the settings, resetting  
to the next higher value.  (i.e. if we're going to sleep before the  
end of the current song, reset to end of song.  if we're going to  
sleep in 14 minutes, then make it 15, etc...)

That is how it functions now.

The fishbone skin, however, displays the status at all times if sleep mode is active, so the ui needs to know when to say "end of song" even tho the $client->sleepTime is just an absolute unix time stamp. 

ok, this is just a fishbone issue now.  renaming.

I'm going to mark this fixed as of change 10076.  Instead of having the possible redunancy of 'end of song' and one of the standard sleep times, I've changed it to automatically reset the sleep time to the 'end of song' of the match is within a minute.  This also means the fishbone skin only has to worry about matching within a minute to call it "end of song".

Of course, I may not have been able to test every possible case.  If any problem does crop up, please feel free to reopen and describe the issue.

OK, we're getting closer. Here are the tests I've done and the results. These tests were done with the versions of Slim/Buttons/Common.pm and Slim/Player/Client.pm from change 10076. I used the sleep button on the Squeezebox remote rather than the sleep link from the SlimServer web UI.

120 mins remaining: Sleep options in order: 15,30,45,60,90,End of Song,Cancel - Correct, fixes the initial problem.
 80 mins remaining: Sleep options in order: 15,30,45,60,End of Song,Cancel - Incorrect, 90 is missing.
 50 mins remaining: Sleep options in order: 15,30,45,End of Song,90,Cancel - Incorrect, 60 is missing.
 40 mins remaining: Sleep options in order: 15,30,End of Song,60,90,Cancel - Incorrect, 45 is missing.
 20 mins remaining: Sleep options in order: 15,End of Song,45,60,90,Cancel - Incorrect, 30 is missing.
 10 mins remaining: Sleep options in order: End of Song,30,45,60,90,Cancel - Incorrect, 15 is missing.

If I understood your last update correctly the 'End of Song' option should only replace a hard coded option (15,30,45,60,90) if the 'End of Song' value is within a minute of the nearest hard coded value. All of the remining times I've tested above have been more than a minute away from their nearest hard coded sleep values, but in all but the first case one of the hard coded options is not show.

Created attachment 1609 [details]
rework

harder to implement that one might think.  I leave this for you to test.  I won't put this into any builds becuase I don't have time right now to test this thoroughly.  Let me know if this does work for you. will need to run using slimserver.pl to test.

thanks.

right, committed for nightly after running a few tests.  it seems to work for me.
let me know what you think.

This looks good to me. I did the same tests as before and they were all OK. I also tested when the remaining time was close to one of the hard coded sleep values, and the hard coded value was correctly replace with 'End of Song'.

Thanks.

thanks.  and now we wait for the request to have the feature removed :)