Bugzilla – Bug 3992
PLAY button shouldn't pass through when waking up idle screen saver
Last modified: 2009-09-08 09:18:54 UTC
When using Now Playing - Jump back on Wake or MusicInfo SCR Screen savers (and others no doubt), it is confusing behaviour for the screen/menu system to 'jump back' once the screensaver's kicked in when you use these controls to control the current playlist. So the suggestion is to treat Fwd, Rew, Play, and Pause/Stop commands in a similar manner to the Vol controls - ie they don't cancel the screensaver. see this thread for more background info:- http://forums.slimdevices.com/showthread.php?t=26651
cc'ed Dean for his comments.
That's a reasonable thing to try, though we need to make sure that the new song title is displayed when appropriate and make sure that the screensavers don't mash up with the showBrieflies that get displayed. Also, let's consider no waking up on Volume changing, possibly even shuffle, repeat, sleep too. Essentially anything that doesn't cause a navigation. Let's try this out for 7.0 as part of the UI update.
The complex part here is that you need to consider player state. Curently, ONLY brightness or dead buttons technically do not wake the screensaver. Volume simply looks like it since it's function returns to the previous mode once it's task is complete. There are actually TWO things that happen on a button press while in screesaver modes: 1) Screensaver::Wakup is called, resetting the ir timer, adjusting brightness back to active level (if set on auto) and jumping back to the current song if it was previously showing that current song (for cases where screensaver IS NOT one that shows current song info) 2) Execute the 'done' function from the screensaver where described by the button mapping. Assuming we don't really want to NOT WAKE, then we can leave the wakeup() routine the same, letting brightness do what it's been doing and changing when it senses active ir input. Now for the problem: each screensaver defines it's own functions. This means, every screensaver mode that no longer wants these buttons to exit the screensaver, either a new function or a rework of the 'done' function must be made for EACH case. *also note* These commands: FWD, REW, PLAY and PAUSE/STOP can, of course, cause a state change (idle->playing, off->playing) which will of course, result in a change of mode to the correct screensaver for that mode.
It seems that I spoke too soon. The functions fall back to the generic 'screensaver' mappings, so all that's needed is to take the requested functions off the 'done' mapping so that the button uses the 'common' mode mapping. FWD, REW, PAUSE, PLAY all cause proper showBriefly result. STOP however is currently set up to force to playlist mode. This of course, can be changed by checking for the screensaverTimeout. It is also possible to suppress the showBriefly by calling $client->suppressStatus(1) from the given screensavers. mapping change is as follows: Index: IR/Default.map =================================================================== --- IR/Default.map (revision 14224) +++ IR/Default.map (working copy) @@ -292,12 +292,7 @@ arrow_left = done arrow_right = done arrow_up = done -fwd = done_passback -rew = done_passback -pause.* = done_passback -play = done add = done -stop = done_passback knob = done [INPUT.Time]
Kevin - you've lost me. Do you mean all we'd need to change is the IR mapping?
basically, yes. There are a couple of minor tweaks depending on what you want to happen on stop and with showBriefly triggers.
Punting enhancements post 7.0.x series
I'm not sure taking play/stop off that list, as they change mode anyway: once you've pressed any of them, you'll no longer be in playing, but idle mode. This pretty often is a different screensaver. Thus it's imho reasonable to have them "wake" up the now playing screensaver. change 19426 - don't wake up the playing screensaver when hitting fwd/rwd/pause
Marking as fixed. Please re-open if needed.
This bug has now been fixed in the 7.1 release version of SqueezeCenter! Please download the new version from http://www.slimdevices.com 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.
This appears broken in the latest 7.2 builds. The default screen saver is changed to jump back on wake which confuses things, but behavior now seems to be that the transport keys (play rwd ffwwd pause/stop) cancel the screen saver. So for example, if you power the unit up and press play, it goes to a now playing screen that says stopped, and you have to press play again to get the track to play. If you are playing a track and the screen saver has kicked in and you press play, it jumps back to the previous menu position and if you then press play again something else (other than playing the song) happens depending on where you are in the menus. This is really non-obvious behaviour for the user!
As stated in my comment #8, rwd/fwd/pause don't cancel the screensaver. Play does. And as you noticed yourself it does not immediately trigger the play action (which would be really confusing). In this regard I think it's working as requested in this bug. Being different from how it has been for a while doesn't mean it's broken. it's different, might need some getting used for the existing user. But imho it's well behaving. Pressing fwd/rwd/pause usually should be applied to the current track - no need to wake up. But you wouldn't want to play something which is already playing - thus wake up and offer the user whatever he wanted to play.
Well no. The bug is Don't wake up Player / cancel screensaver on FWD, REW, PLAY, PAUSE/STOP and it's waking up the player on PLAY (and not acting on it) - so it's not as requested by this bug. You do want it to play when you switch on the player and want to restart playing the current song - it's really odd behaviour to press play and for the player to tell you it's stopped .... yes I know you're stopped and that's why I just pressed play! IMV the transport functions should always act on the current playlist, and I think that's what most users would expect. Having these dedicated button functions do different things whilst the player is in a (hidden) state, is really bad ergonomics. I don't think it has any bearing on me getting used to new behaviour, I think it's what a new user would expect to happen and you're forcing them to adapt to something non-intuitive and that requires them to understand the (hidden) software state of the player and to predict from that what action their command will have. Pressing play when in now playing will have a different effect to pressing play when the now playing screen saver has kicked in, even though the player may seem to the user to be in exactly the same state!
Ok, waking up when hitting Play in "power off" mode makes sense. But it doesn't in active (now playing) screensaver. We'll probably have to handle those differently.
Sure I agree with you (I think Jim C wanted Play to be Play/Pause), but the most important thing is that its behaviour is consistent so that it does the same on the now playing screen and on the now playing screen saver (which looks the same).
OK there are some more issues related to current behaviour. Play appears to both cancel the screen saver and do the play action. This means for example that if you've browsed to an album and press play, it commences playing at the first track and loads the rest of the album into the playlist. Then after 30 seconds the now playing screen saver kicks in. You listen to a few tracks and the phone rings, so you press stop or pause and answer the phone. When you return the display is the now playing screen saver with the current track stopped or paused, but if you press play it jumps back to the previous browse album position and starts playing the album from the start again! This is clearly a bug IMV.
Could you please define how you'd expect those buttons to work when pressed in the three states: off, idle, playing? Thanks in advance!
I'm now getting slight confused because this bug seems to have changed its name ... or is that me? I thought it was Don't wake up Player / cancel screensaver on FWD, REW, PLAY, PAUSE/STOP Anyway ... Off Play - Switch on player and play current song from start Fwd - No action Rwd - No action Pause/Stop - No action Idle (Paused or Stopped) Play - Play current song from start Fwd - Play next song in Playlist Rwd - Play previous song in Playlist Pause/Stop - Unpause/No action Playing Play - No action (Play current song from start would be acceptable) Fwd - Play next song in Playlist Rwd - Play previous song in Playlist Pause/Stop - Pause/Stop I don't think any of these should change the screen saver state.
In general, the PAUSE button is the button to use to unpause. The play button can be used to unpause when in Now Playing, scrolled to the current (paused) song, including when the NP screensaver is visible. If there's anything else on the screen, (rss feed, date, etc...) the PLAY button should just wake up the player, just as the arrow buttons do. (The idea here is that PLAY operates on the thing on the display.)
> In general, the PAUSE button is the button to use to unpause. Does this mean it's working as expected? No bug then? I don't want to play the fool here. Just want to be sure we really know what we want, or we're just going on closing/reopening this bug, fixing/breaking expectations etc.
"The play button can be used to unpause when in Now Playing, scrolled to the current (paused) song, including when the NP screensaver is visible." So if you press play when it's paused it unpauses and if you press play when it's stopped or playing it does something completely different? That seems confusing to me. "If there's anything else on the screen, (rss feed, date, etc...) the PLAY button should just wake up the player, just as the arrow buttons do." What if the screen is showing the NP screen saver? What should play do then? "Does this mean it's working as expected? No bug then?" No - pressing play is canceling the NP screen saver AND doing the play action at the same time. At the very least it should ONLY cancel the NP screen saver, although I'm not sure whether Dean agrees it should do that on the NP screen saver, since it ought to act on what's on the display ie the current song/playlist I also think not playing when you press play and the player is stopped is a bug, but I'm not sure whether Dean does or not? IMV the NP screen saver should definitely have the transport buttons behave exactly like the NP screen, because the user can't tell one from the other. BTW this is exactly why when I tried the NP jump back screensaver mode, I reverted back - it's just too confusing. IMV it would be MUCH better to ditch the jump back altogether and use the NP button to jump back (see bug 9164).
> the NP screen saver should definitely have the transport buttons behave exactly > like the NP screen, because the user can't tell one from the other. The NP area is at the heart of the player UI and has to be simple and intuitive. However, in my experience with new users the current implementation can (almost immediately) be confusing and frustrating. The fact there are two virtually indistinguishable screens with almost the same name but with different functionality tied to them is a large part of the problem (but certainly not all of it by any means). I've got a bit lost with what Patrick is proposing here (but from experience know he'll probably be correct, as he always seems to have simplicity and intuitive use at heart), but have to agree with the Now Playing button bit. One press to bring up the Now Playing screen and another press to get you back where you were, dumping the rather unnecessary scrolling through Now Playing screens functionality can onlyu be a good thing.
It sounds like it's working as designed now, but we should discuss how it could be improved.
No it isn't! When you press play it doesn't just cancel the screen saver, it does the action too. Try it for yourself.
Which screensaver?
Ok, I see. If the idle screensaver is set to Date/Time, it wakes up and passes through play to whatever screen it's jumping back to. Definitely a bug. Other buttons, like LEFT just wake it up. Retargetting this bug (and retitling it so it's clearer as to how we should get back to "as designed".) Let's have a separate discussion on how to improve the UI.
Yes, but it does it with the NP screen saver too - which is really horrid. You have what appears to be a NP screen on the display, you press play and something totally unexpected happens (if I have the X10 plugin up, it switches the lights on!). It never used to do this - this is a new bug. I think you should look at the UI completely from fresh. I don't know what your new product roadmap is, but if you get the UI right you can out-Apple Apple.
How do you have the NP screensaver on when paused? Can you outline the specific steps to reproduce? I'm getting more confused. Re: Improving the UI. Absolutely.
Start playing a playlist/album. Browse somewhere. Wait 30 seconds for NP screen saver to kick in Allow a couple of tracks to play. Press pause (phone rings or something, maybe you meant to stop but in your hurry to answer it you didn't hold the button long enough ;-) ). Return and press play to restart track. SOMETHING TOTALLY UNEXPECTED HAPPENS! (Lights turn on, something totally different overwrites your playlist and starts playing etc, etc.)
Kevin - I'm lost here. Do you know where to best hook in here? I've been looking in Buttons::Screensaver, or the screensaver's 'done' handler, Hardware::IR. But even in the backtraces I didn't find out where the actuall play command was triggered :-/
sorry Michael. I've tried to read through this three times and my brain shuts off half way through every time. I can only vent...but I won't. Let the paid folks lose the hours tracing over feature creep. I gave up being an abused grunt a long time ago (largely unnoticed, so clearly good time to do so) so I really can't remember how it all snaked through. sorry.
This one isn't feature creep, it's a bug that's crept in over the past few weeks.
The complexity of what button does what, in what mode, and what state...is a long history of feature creep and request/response. I was speaking of the CODE not of the report. Sensitive much? (yes, I can use that line too) If you truly believe it's something "new" in the last few weeks, you are welcome to do the svn reversion test and pinpoint the changes that caused the problem. I've done that routine many times for the benefit of others. Feel free to take that on yourself now. I won't.
Unfortunately I'm not in a position to do the svn revision thing as my server isn't set-up for that at all and I have limited scope to break it before I get in trouble with the more important member of the family. For me the change is related to the promotion of the jump back on wake screensaver since running with the previous default ss doesn't show it. Therefore it isn't easy for me to say exactly when this bug was introduced. Sorry about that. When I coded stuff many years ago .... I'd have had a table of button presses and machine states with each entry pointing to a routine to execute. But I guess computer science has moved on apace since then.
Woud the following change do what you're looking for? Index: /Users/mh/Documents/workspace/7.3/server/IR/Default.map =================================================================== --- /Users/mh/Documents/workspace/7.3/server/IR/Default.map (revision 22964) +++ /Users/mh/Documents/workspace/7.3/server/IR/Default.map (working copy) @@ -323,8 +323,10 @@ arrow_right = done_passbackplaylist knob_push = done_passbackplaylist arrow_up = done_passbackplaylist -play = done -add = done +play.single = done +play.hold = done +add.single = done +add.hold = done stop = done_passback knob = done knob_left = volume @@ -346,8 +348,10 @@ arrow_right = done_passbackplaylist knob_push = done_passbackplaylist arrow_up = done_passbackplaylist -play = done -add = done +play.single = done +play.hold = done +add.single = done +add.hold = done stop = done_passback knob = done knob_left = done_passback
That seems good to me Michael - thank you very much!
Patrick - as this patch has not been applied yet, I'm a bit confused whether this is now working as you wanted _with_ or _without_ the patch?!?
I downloaded the 7.3 deb and applied your patch and it works correctly and consistently AFAICS. The keys work on the screen currently displayed, and non-transport keys + play cancel the NP ss, but don't action anything else until pressed again.
applied in change 23000
Verified fix in 7.2.1-23472
This bug has been fixed in the 7.3.0 release version 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.
Reduce number of active targets for SC