Bug 3105 - Pressing ADD in Live Music Archive plugin does wrong thing
: Pressing ADD in Live Music Archive plugin does wrong thing
Status: CLOSED FIXED
Product: MySqueezebox.com
Classification: Unclassified
Component: Internet Radio
: unspecified
: Macintosh Other
: P2 normal (vote)
: ---
Assigned To: Andy Grundman
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2006-03-03 10:12 UTC by Blackketter Dean
Modified: 2008-12-15 12:15 UTC (History)
2 users (show)

See Also:
Category: ---


Attachments
add a missing 'onAdd' to gotOPML (8.15 KB, patch)
2006-03-03 14:57 UTC, KDF
Details | Diff
add feedback for audio files (8.89 KB, patch)
2006-03-03 19:20 UTC, KDF
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Blackketter Dean 2006-03-03 10:12:53 UTC
It should add the tracks to the end of the playlist, rather than removing the currently playing song.  (Should be back-ported to trunk plugin.)
Comment 1 Andy Grundman 2006-03-03 10:32:27 UTC
XMLBrowser is not customized for SN, so I'm calling this a SlimServer bug.
Comment 2 KDF 2006-03-03 11:12:42 UTC
sideline note on this, looks like only play and add are supported, while insert (via add.hold) seems to be missing.
the code from XMLBRowser.pm does look to be calling the right command ($client->execute([ 'playlist', $action, $url, $title ]);), so I wonder if this is perhaps pointing to a problem in the legacy command handler.  obviously this is using "play" or "add" as the $action, since the newer "loadtracks" and "addtracks" requires a datastore object.

If you have yet to test this in slimserver (watch with d_command debug enabled).  It does seem to call the right button function, but as you say deletes the current song.  This would seem to indicate that the server is acting on the "add" button as if it were in the "playlist" mode, which does delete the current track.  Possible that the mode change is happening too fast, or the command dispatcher is mapping to the wrong mode somehow.
Comment 3 KDF 2006-03-03 14:35:40 UTC
Looks like the params are getting lost or are somehow not properly provided when into the sublevels of LMA, onAdd is not used. Instead INPUT.Choice seems to be using 'passback'
Comment 4 KDF 2006-03-03 14:57:26 UTC
Created attachment 1154 [details]
add a missing 'onAdd' to gotOPML

however, what is then missing is a showBriefly feedback to say the item is added. It looks like nothing happens, but the item does appear to be getting added properly.  I can continue to look at that, and I noticed that the XMLBrowser should have some work on the params hashes, wrapping the keys in '' is more slimserver style is it not?

Attached patch fixes the style and the function, still need to track down the missing showBriefly.
Comment 5 Andy Grundman 2006-03-03 16:13:48 UTC
Nice work, thanks for looking into this.

IMO putting quotes around hash keys is unnecessary unless there's a space in the key for some reason.  But to each their own. :) 
Comment 6 KDF 2006-03-03 17:36:32 UTC
I just follow it becuase it has been what I've seen so far in slimserver code (also stands out nicely when syntax highlighting).  I also seem to remember something about a problem with reserved words getting interpreted unless eclosed in single quotes.  I can't for the life of me recall which module was involved, or it might have been a plugin. Up to you guys to decide on style.  

The key part of the patch is the addition of onAdd in the gotOPML. still haven't chased down the missing feedback
Comment 7 KDF 2006-03-03 19:20:07 UTC
Created attachment 1155 [details]
add feedback for audio files

adds the showBriefly feedback when adding or playing audio.  I've noticed that INPUT.Choice doesn't support add.hold, or insert.  I'm not sure if that is deliberate or not.
Comment 8 Andy Grundman 2006-03-03 19:33:12 UTC
That patch works for me, I'd say go ahead and commit it.
Comment 9 KDF 2006-03-03 19:55:34 UTC
done at trunk change 6481.  Marking for SN until it is ported over.
Comment 10 Andy Grundman 2006-03-03 19:56:21 UTC
Marking fixed as SN uses the trunk version.  Thanks!
Comment 11 James Richardson 2008-12-15 12:15:33 UTC
This bug has been fixed in the latest release of SqueezeNetwork!

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