Bug 6519 - Music Library disapears from Jive interface if plugin register menu
: Music Library disapears from Jive interface if plugin register menu
Status: CLOSED FIXED
Product: SB Controller
Classification: Unclassified
Component: Browser
: unspecified
: PC Ubuntu Linux
: P2 major with 2 votes (vote)
: 7.0
Assigned To: Ben Klaas
http://forums.slimdevices.com/showthr...
:
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2008-01-01 08:35 UTC by Erland Isaksson
Modified: 2008-05-15 13:01 UTC (History)
1 user (show)

See Also:
Category: ---


Attachments
old version (1.88 KB, text/plain)
2008-01-10 10:10 UTC, KDF
Details
new version (1.88 KB, text/plain)
2008-01-10 10:11 UTC, KDF
Details

Note You need to log in before you can comment on or make changes to this bug.
Description Erland Isaksson 2008-01-01 08:35:10 UTC
There seems to be a problem introduced in SqueezeCenter r15477 which makes it impossible for third party plugins to use the registerPluginNode or registerPluginMenu functions in SqueezeCenter to register Jive menu. I'm trying to register a menu under "Music Library" and this works fine as long as there is only one plugin that tries to do this. However, as soon as a second plugin also calls these registration functions, the Jive interface no longer works. Basically the "Music Library" entry is removed from the Jive interface.

Since "Random Mix" already registers its Jive menus under "Music Library" this makes it impossible to use these functions from a third party plugin.

The following thread contains more information regarding the problem:
http://forums.slimdevices.com/showthread.php?p=250375#post250375

If I revert to SqueezeCenter r15476 everything works correctly, so the problem seems to be introduced in r15477.
Comment 1 Hugh Pyle 2008-01-07 19:18:00 UTC
Ping - just checking whether there's an ETA for this one yet.
Comment 2 KDF 2008-01-09 23:59:06 UTC
Index: Slim/Control/Jive.pm
===================================================================
--- Slim/Control/Jive.pm	(revision 16085)
+++ Slim/Control/Jive.pm	(working copy)
@@ -309,7 +309,7 @@
 	# we also do not allow any hash without an id into the array, and will log an error if that happens
 
 	my %seen; my @new;
-	for my $href (@$menuArray, @pluginMenus) {
+	for my $href (@pluginMenus, @$menuArray) {
 		my $id = $href->{'id'};
 		if ($id) {
 			if (!$seen{$id}) {
Comment 3 Ben Klaas 2008-01-10 06:38:39 UTC
KDF-- that doesn't strike me as a fix... by reversing the order of that for() loop, you say that ids from already registered plugin menus should be taken over those trying to register. But that's not the behavior that I want-- I want a plugin to be able to take over an existing menu item (by id), be it from a plugin or elsewhere.

Note that if you don't want a menu to disappear, it's important to not use the same id of an existing menu item. Maybe that's what's going on here? And if so, perhaps I need to make a better convention for ids so that doesn't happen by accident.

Erland, can you attach a sample plugin that has this issue? I think if I saw your code I could figure out what's going on pretty quickly.

as for ETA for a fix, this is a 7.0/P2 bug. My guess is that I'll have it fixed this week or early next.
Comment 4 Hugh Pyle 2008-01-10 06:55:45 UTC
Here's an example: http://inguzaudio.com/EQ/20080108_InguzEQ_test.zip.  (May be flaky.  Call me if you have problems getting it to run).  I'm not attempting to override any system IDs; all my IDs are unique to the plugin. But it is incompatible with RandomPlay (unless I comment out the RandomPlay calls to registerPluginXXX).
Comment 5 Erland Isaksson 2008-01-10 07:51:37 UTC
Version 2.0.beta06 of Dynamic Playlists plugin has the problem, you'll find it here:
http://erland.homeip.net/download/do/downloadapplication?name=slimserver-dynamicplaylist&filename=DynamicPlayList-2.0.beta06.zip

(I've disabled the Jive support in 2.0.beta07, so you can't use the latest version).

Comment 6 Ben Klaas 2008-01-10 08:13:15 UTC
-------------------------
Hugh, the Inguz plugin wouldn't load, which blocked seeing the issue.

[10:12:39.5652] Slim::bootstrap::tryModuleLoad (266) Warning: Module [Plugins::InguzEQ::Plugin] failed to load:
Global symbol "$client" requires explicit package name at /home/bklaas/slimserver/Plugins/InguzEQ/Plugin.pm line 2972.
Global symbol "$client" requires explicit package name at /home/bklaas/slimserver/Plugins/InguzEQ/Plugin.pm line 2975.
Global symbol "$client" requires explicit package name at /home/bklaas/slimserver/Plugins/InguzEQ/Plugin.pm line 2984.
Global symbol "$client" requires explicit package name at /home/bklaas/slimserver/Plugins/InguzEQ/Plugin.pm line 2987.
Compilation failed in require at (eval 545) line 2.
BEGIN failed--compilation aborted at (eval 545) line 2.

[10:12:39.5712] Slim::Utils::PluginManager::enablePlugins (436) Warning: Couldn't load Plugins::InguzEQ::Plugin

-------------------------
Erland, 

am I missing something in your DynamicPlaylist plugin? I'm not seeing a Plugin.pm or any code that has the string registerPlugin in it...

bklaas@mediumspicy:~/slimserver/Plugins/DynamicPlayList$ ls
HTML  install.xml  lib  LICENSE.txt  Mixes  README.txt  SQL  strings.txt

bklaas@mediumspicy:~/slimserver/Plugins/DynamicPlayList$ grep -R registerPlugin *
bklaas@mediumspicy:~/slimserver/Plugins/DynamicPlayList$ 
Comment 7 Ben Klaas 2008-01-10 08:15:58 UTC
nevermind Erland, I see that your plugin has recreated the issue. I just need to poke around a bit to understand what you're doing in your code.
Comment 8 KDF 2008-01-10 08:59:57 UTC
I knew it wasn't a fix, but it was late and I wanted to make a note.  The reversal hints that something gets mixed up.  The resulting hashes seem to contain the same info, just different order (the patched order being the one that matches the order of the pre-15477 version. 
Comment 9 Ben Klaas 2008-01-10 09:12:32 UTC
in that spirit, here's some SC debug output of the two "competing" items...this looks fine, which makes me think that the problem might be jive-side in HomeMenu.lua...

[11:09:29.2892] Slim::Control::Jive::registerPluginMenu (317) TRYING TO REGISTER ID randomplay
[11:09:29.2896] Slim::Control::Jive::registerPluginMenu (320) adding to myMusic menu structure randomplay
[11:09:29.2905] Data::Dump::dump (100) Warning: {
  displayWhenOff => 0,
  id => "randomplay",
  isANode => 1,
  node => "myMusic",
  text => "Random Mix",
  weight => 60,
  window => { titleStyle => "random" },
}


[11:09:29.3025] Slim::Control::Jive::registerPluginMenu (317) TRYING TO REGISTER ID dynamicplaylist
[11:09:29.3028] Slim::Control::Jive::registerPluginMenu (320) adding to myMusic menu structure dynamicplaylist
[11:09:29.3037] Data::Dump::dump (100) Warning: {
  displayWhenOff => 0,
  id => "dynamicplaylist",
  isANode => 1,
  node => "myMusic",
  text => "Dynamic Playlists",
  weight => 85,
  window => { titleStyle => "mymusic" },
}
Comment 10 KDF 2008-01-10 10:04:59 UTC
hrm.  I'm confused.  If @$menuArray is first, then it takes precedence over anything in @pluginMenus.  However, @pluginMenus seems to be the returned array, while @$menuArray is the new menu items to be registered.  This would seem to allow plugins to call registerJiveMenu and override the existing @pluginMenus.

However, all id's are present and unique, so that isn't the main issue.

I'll attach two results.  First is the old (@pluginMenus, @$menuArray), second will be the reverse @new.
Comment 11 KDF 2008-01-10 10:10:08 UTC
Created attachment 2643 [details]
old version
Comment 12 KDF 2008-01-10 10:11:51 UTC
Created attachment 2644 [details]
new version

note that the only difference is the position of the dynamic playlist hash in the list (last in old, first in new). presumably this means that if it is jive side, then something in far too dependent on order of items.
Comment 13 KDF 2008-01-10 10:20:46 UTC
This also seems to make things work, and sticks with the original precedence (which I'm still confused about). I suspect that the jive side issue could be a failure to handle the order if a submenu refers to a node that hasn't been handled yet because it's later in the array. 

Index: Slim/Control/Jive.pm
===================================================================
--- Slim/Control/Jive.pm        (revision 16085)
+++ Slim/Control/Jive.pm        (working copy)
@@ -309,7 +309,7 @@
        # we also do not allow any hash without an id into the array, and will
log an error if that happens

        my %seen; my @new;
-       for my $href (@$menuArray, @pluginMenus) {
+       for my $href (@$menuArray, reverse @pluginMenus) {
                my $id = $href->{'id'};
                if ($id) {
                        if (!$seen{$id}) {
Comment 14 KDF 2008-01-10 10:39:22 UTC
this seems to support my theory, when the order is wrong, random song mix is first it gives this error:
103453:1546 DEBUG (C:\slim\src\pkg\debug\lua\jive\ui\HomeMenu.lua:131) - JiveMain.addItem: Adding Random Song Mix to randomplay
error in network pump:
        C:\slim\src\pkg\debug\lua\jive\ui\HomeMenu.lua:146: attempt to index local 'nodeEntry' (a nil value)stack traceback:
        C:\slim\src\pkg\debug\lua\jive\ui\HomeMenu.lua:146: in function 'addItem'
        ...\debug\lua\applets/SlimBrowser/SlimBrowserApplet.lua:766: in function 'func'
        C:\slim\src\pkg\debug\lua\jive\net\Comet.lua:397: in function 'sink'
        C:\slim\src\pkg\debug\lua\jive\net\Socket.lua:128: in function 'msg'
        C:\slim\src\pkg\debug\lua\jive\net\NetworkThread.lua:320: in function <C:\slim\src\pkg\debug\lua\jive\net\NetworkThread.lua:305>
        [C 0041AEA0]: in function 'processEvents'
        C:\slim\src\pkg\debug\lua\jive\JiveMain.lua:191: in function <C:\slim\src\pkg\debug\lua\jive\JiveMain.lua:120>
        (tail call): ?
        C:\slim\src\pkg\debug\lua\jive\JiveMain.lua:269: in main chunk
        [C 1003ECC0]: ?
        [C 004168A0]: ?
Comment 15 Ben Klaas 2008-01-10 10:51:36 UTC
KDF's patch in comment#13 is the winner.

checked in with SC change 16114

moving to RESOLVED
Comment 16 snarlydwarf 2008-01-11 08:40:36 UTC
(In reply to comment #15)
> KDF's patch in comment#13 is the winner.
> 
> checked in with SC change 16114
> 
> moving to RESOLVED
> 

Are we sure this is resolved?  I see even stranger things now: Bedroom player has full menu, livingroom player only shows Now Playing, Settings, Extras and Choose Player.

If I switch back to Bedroom, it goes back to normal...

If I power-cycle while on Livingroom.... the roles are reversed: Livingroom has the full menu, Bedroom has the useless menu.
Comment 17 Ben Klaas 2008-01-11 08:52:32 UTC
I think that's unrelated. Jive r1396, just checked in, should fix that I hope.
Comment 18 snarlydwarf 2008-01-11 16:49:03 UTC
(In reply to comment #17)
> I think that's unrelated. Jive r1396, just checked in, should fix that I hope.
> 

1404 has the same problem.   Livingroom shows: "Now Playing, Settings, Extras, Choose Player" as choices, and despite music playing, Now playing doesn't show anything except a title bar.
Comment 19 James Richardson 2008-05-15 13:01:17 UTC
This bug has recently been fixed in the latest release of SqueezeCenter 7.0.1

Please try that version, if you still see the error, then reopen this bug.

To download this version, please navigate to: http://www.slimdevices.com/su_downloads.html